[vm] Use the right stack maps between executing catch entry moves and jumping to the exception handler.

Adds a stack map to the catch block entry pc.

The alternative would be to prevent yielding to a safepoint between these two point. For throws coming from Dart or the embedding API, this is possible by unwinding the transitions before the first point and doing only a non-yielding VM->Generated between the two points. For FFI, this does not work because the final state is Native instead of Generated, and VM->Native is a blocking transition.

TEST=ci
Bug: https://github.com/dart-lang/sdk/issues/48668
Change-Id: I8164fddae018ed95d9147fafa65785b7638e5301
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/269746
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This commit is contained in:
Ryan Macnak 2022-11-17 17:20:18 +00:00 committed by Commit Queue
parent 942a7ff6d7
commit b0c6f00f39
14 changed files with 101 additions and 44 deletions

View file

@ -0,0 +1,23 @@
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// VMOptions=
// VMOptions=--deterministic
import "dart:io";
import "dart:isolate";
import "package:expect/expect.dart";
void main(List<String> args) async {
if (!args.contains("--child")) {
for (var i = 0; i < 4; i++) {
Isolate.spawn(main, ["--child"]);
}
}
for (var i = 0; i < 10000; i++) {
final d = Directory("does-not-exist");
Expect.throws(() => d.listSync(), (e) => e is FileSystemException);
}
}

View file

@ -0,0 +1,25 @@
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// @dart = 2.9
// VMOptions=
// VMOptions=--deterministic
import "dart:io";
import "dart:isolate";
import "package:expect/expect.dart";
void main(List<String> args) async {
if (!args.contains("--child")) {
for (var i = 0; i < 4; i++) {
Isolate.spawn(main, ["--child"]);
}
}
for (var i = 0; i < 10000; i++) {
final d = Directory("does-not-exist");
Expect.throws(() => d.listSync(), (e) => e is FileSystemException);
}
}

View file

@ -875,15 +875,14 @@ void FlowGraphCompiler::GenerateDeferredCode() {
}
}
void FlowGraphCompiler::AddExceptionHandler(intptr_t try_index,
intptr_t outer_try_index,
intptr_t pc_offset,
bool is_generated,
const Array& handler_types,
bool needs_stacktrace) {
exception_handlers_list_->AddHandler(try_index, outer_try_index, pc_offset,
is_generated, handler_types,
needs_stacktrace);
void FlowGraphCompiler::AddExceptionHandler(CatchBlockEntryInstr* entry) {
exception_handlers_list_->AddHandler(
entry->catch_try_index(), entry->try_index(), assembler()->CodeSize(),
entry->is_generated(), entry->catch_handler_types(),
entry->needs_stacktrace());
if (is_optimizing()) {
RecordSafepoint(entry->locs());
}
}
void FlowGraphCompiler::SetNeedsStackTrace(intptr_t try_index) {

View file

@ -870,12 +870,7 @@ class FlowGraphCompiler : public ValueObject {
// Return true-, false- and fall-through label for a branch instruction.
BranchLabels CreateBranchLabels(BranchInstr* branch) const;
void AddExceptionHandler(intptr_t try_index,
intptr_t outer_try_index,
intptr_t pc_offset,
bool is_generated,
const Array& handler_types,
bool needs_stacktrace);
void AddExceptionHandler(CatchBlockEntryInstr* entry);
void SetNeedsStackTrace(intptr_t try_index);
void AddCurrentDescriptor(UntaggedPcDescriptors::Kind kind,
intptr_t deopt_id,

View file

@ -2297,6 +2297,8 @@ class CatchBlockEntryInstr : public BlockEntryWithInitialDefs {
// corresponds.
intptr_t catch_try_index() const { return catch_try_index_; }
const Array& catch_handler_types() const { return catch_handler_types_; }
PRINT_TO_SUPPORT
DECLARE_CUSTOM_SERIALIZATION(CatchBlockEntryInstr)

View file

@ -3127,15 +3127,12 @@ void CloneContextInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
LocationSummary* CatchBlockEntryInstr::MakeLocationSummary(Zone* zone,
bool opt) const {
UNREACHABLE();
return NULL;
return new (zone) LocationSummary(zone, 0, 0, LocationSummary::kCall);
}
void CatchBlockEntryInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
__ Bind(compiler->GetJumpLabel(this));
compiler->AddExceptionHandler(
catch_try_index(), try_index(), compiler->assembler()->CodeSize(),
is_generated(), catch_handler_types_, needs_stacktrace());
compiler->AddExceptionHandler(this);
if (!FLAG_precompiled_mode) {
// On lazy deoptimization we patch the optimized code here to enter the
// deoptimization stub.

View file

@ -2774,15 +2774,12 @@ void CloneContextInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
LocationSummary* CatchBlockEntryInstr::MakeLocationSummary(Zone* zone,
bool opt) const {
UNREACHABLE();
return NULL;
return new (zone) LocationSummary(zone, 0, 0, LocationSummary::kCall);
}
void CatchBlockEntryInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
__ Bind(compiler->GetJumpLabel(this));
compiler->AddExceptionHandler(
catch_try_index(), try_index(), compiler->assembler()->CodeSize(),
is_generated(), catch_handler_types_, needs_stacktrace());
compiler->AddExceptionHandler(this);
if (!FLAG_precompiled_mode) {
// On lazy deoptimization we patch the optimized code here to enter the
// deoptimization stub.

View file

@ -2422,15 +2422,12 @@ void CloneContextInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
LocationSummary* CatchBlockEntryInstr::MakeLocationSummary(Zone* zone,
bool opt) const {
UNREACHABLE();
return NULL;
return new (zone) LocationSummary(zone, 0, 0, LocationSummary::kCall);
}
void CatchBlockEntryInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
__ Bind(compiler->GetJumpLabel(this));
compiler->AddExceptionHandler(
catch_try_index(), try_index(), compiler->assembler()->CodeSize(),
is_generated(), catch_handler_types_, needs_stacktrace());
compiler->AddExceptionHandler(this);
if (!FLAG_precompiled_mode) {
// On lazy deoptimization we patch the optimized code here to enter the
// deoptimization stub.

View file

@ -3122,15 +3122,12 @@ void CloneContextInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
LocationSummary* CatchBlockEntryInstr::MakeLocationSummary(Zone* zone,
bool opt) const {
UNREACHABLE();
return nullptr;
return new (zone) LocationSummary(zone, 0, 0, LocationSummary::kCall);
}
void CatchBlockEntryInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
__ Bind(compiler->GetJumpLabel(this));
compiler->AddExceptionHandler(
catch_try_index(), try_index(), compiler->assembler()->CodeSize(),
is_generated(), catch_handler_types_, needs_stacktrace());
compiler->AddExceptionHandler(this);
if (!FLAG_precompiled_mode) {
// On lazy deoptimization we patch the optimized code here to enter the
// deoptimization stub.

View file

@ -2805,15 +2805,12 @@ void CloneContextInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
LocationSummary* CatchBlockEntryInstr::MakeLocationSummary(Zone* zone,
bool opt) const {
UNREACHABLE();
return NULL;
return new (zone) LocationSummary(zone, 0, 0, LocationSummary::kCall);
}
void CatchBlockEntryInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
__ Bind(compiler->GetJumpLabel(this));
compiler->AddExceptionHandler(
catch_try_index(), try_index(), compiler->assembler()->CodeSize(),
is_generated(), catch_handler_types_, needs_stacktrace());
compiler->AddExceptionHandler(this);
if (!FLAG_precompiled_mode) {
// On lazy deoptimization we patch the optimized code here to enter the
// deoptimization stub.

View file

@ -257,6 +257,11 @@ void SSALivenessAnalysis::ComputeInitialSets() {
}
}
} else if (auto entry = block->AsBlockEntryWithInitialDefs()) {
// Initialize location summary for instruction if needed.
if (entry->IsCatchBlockEntry()) {
entry->InitializeLocationSummary(zone(), true); // opt
}
// Process initial definitions, i.e. parameters and special parameters.
for (intptr_t i = 0; i < entry->initial_definitions()->length(); i++) {
Definition* def = (*entry->initial_definitions())[i];
@ -625,6 +630,10 @@ void FlowGraphAllocator::BuildLiveRanges() {
if (auto join_entry = block->AsJoinEntry()) {
ConnectIncomingPhiMoves(join_entry);
} else if (auto catch_entry = block->AsCatchBlockEntry()) {
// Catch entries are briefly safepoints after catch entry moves execute
// and before execution jumps to the handler.
safepoints_.Add(catch_entry);
// Process initial definitions.
ProcessEnvironmentUses(catch_entry, catch_entry); // For lazy deopt
for (intptr_t i = 0; i < catch_entry->initial_definitions()->length();

View file

@ -279,12 +279,29 @@ class ExceptionHandlerFinder : public StackResource {
}
{
NoSafepointScope no_safepoint_scope;
Thread* thread = Thread::Current();
NoSafepointScope no_safepoint_scope(thread);
for (int j = 0; j < moves.count(); j++) {
const CatchEntryMove& move = moves.At(j);
*TaggedSlotAt(fp, move.dest_slot()) = dst_values[j]->ptr();
}
// Update the return address in the stack so the correct stack map is used
// for any stack walks that happen before we jump to the handler.
StackFrameIterator frames(ValidationPolicy::kDontValidateFrames, thread,
StackFrameIterator::kNoCrossThreadIteration);
bool found = false;
for (StackFrame* frame = frames.NextFrame(); frame != NULL;
frame = frames.NextFrame()) {
if (frame->fp() == handler_fp) {
ASSERT_EQUAL(frame->pc(), static_cast<uword>(pc_));
frame->set_pc(handler_pc);
found = true;
break;
}
}
ASSERT(found);
}
}
@ -597,6 +614,7 @@ void Exceptions::JumpToFrame(Thread* thread,
uword stack_pointer,
uword frame_pointer,
bool clear_deopt_at_target) {
ASSERT(thread->execution_state() == Thread::kThreadInVM);
const uword fp_for_clearing =
(clear_deopt_at_target ? frame_pointer + 1 : frame_pointer);
ClearLazyDeopts(thread, fp_for_clearing);

View file

@ -169,9 +169,10 @@ const char* StackFrame::ToCString() const {
ASSERT(thread_ == Thread::Current());
Zone* zone = Thread::Current()->zone();
const Code& code = Code::Handle(zone, GetCodeObject());
ASSERT(!code.IsNull());
const char* name =
code.QualifiedName(NameFormattingParams(Object::kInternalName));
code.IsNull()
? "Cannot find code object"
: code.QualifiedName(NameFormattingParams(Object::kInternalName));
return zone->PrintToString(" pc 0x%" Pp " fp 0x%" Pp " sp 0x%" Pp " %s",
pc(), fp(), sp(), name);
}

View file

@ -246,7 +246,7 @@ class StackFrameIterator {
// Checks if a next frame exists.
bool HasNextFrame() const { return frames_.fp_ != 0; }
// Get next frame.
// Get next frame. Becomes invalid after the next call to NextFrame.
StackFrame* NextFrame();
bool validate() const { return validate_; }