Skip to content

Commit e480c74

Browse files
rmacnak-googleCommit Queue
authored andcommitted
[vm] Fix corruption of null and following objects during OOM.
Array::New[Uninitialized] don't expect to get null from Object::Allocate. Using longjmp seems more robust than adding checks everywhere. TEST=vm/dart/gc/scavenger_abort_test Bug: #60552 Change-Id: I2750427c41751f8306d5c8dc28afaf052b6e9d74 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422902 Commit-Queue: Ryan Macnak <[email protected]> Reviewed-by: Alexander Aprelev <[email protected]>
1 parent 779a196 commit e480c74

File tree

3 files changed

+28
-56
lines changed

3 files changed

+28
-56
lines changed

runtime/vm/exceptions.cc

Lines changed: 28 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -712,46 +712,44 @@ StackTracePtr Exceptions::CurrentStackTrace() {
712712
return GetStackTraceForException();
713713
}
714714

715-
static StackTracePtr CreateStackTrace(Zone* zone) {
716-
const Array& code_array = Array::Handle(
717-
zone, Array::New(StackTrace::kFixedOOMStackdepth, Heap::kOld));
718-
if (code_array.IsNull()) {
719-
return StackTrace::null();
720-
}
721-
const TypedData& pc_offset_array = TypedData::Handle(
722-
zone,
723-
TypedData::New(kUintPtrCid, StackTrace::kFixedOOMStackdepth, Heap::kOld));
724-
if (pc_offset_array.IsNull()) {
725-
return StackTrace::null();
726-
}
727-
const StackTrace& stack_trace =
728-
StackTrace::Handle(zone, StackTrace::New(code_array, pc_offset_array));
729-
// Expansion of inlined functions requires additional memory at run time,
730-
// avoid it.
731-
if (stack_trace.IsNull()) {
715+
static StackTracePtr TryCreateStackTrace(Thread* thread, Zone* zone) {
716+
LongJumpScope jump(thread);
717+
if (DART_SETJMP(*jump.Set()) == 0) {
718+
const Array& code_array = Array::Handle(
719+
zone, Array::New(StackTrace::kFixedOOMStackdepth, Heap::kOld));
720+
const TypedData& pc_offset_array = TypedData::Handle(
721+
zone, TypedData::New(kUintPtrCid, StackTrace::kFixedOOMStackdepth,
722+
Heap::kOld));
723+
const StackTrace& stack_trace =
724+
StackTrace::Handle(zone, StackTrace::New(code_array, pc_offset_array));
725+
// Expansion of inlined functions requires additional memory at run time,
726+
// avoid it.
727+
stack_trace.set_expand_inlined(false);
728+
return stack_trace.ptr();
729+
} else {
730+
RELEASE_ASSERT(thread->StealStickyError() ==
731+
Object::out_of_memory_error().ptr());
732732
return StackTrace::null();
733733
}
734-
stack_trace.set_expand_inlined(false);
735-
return stack_trace.ptr();
736734
}
737735

738736
static UnhandledExceptionPtr CreateUnhandledExceptionOrUsePrecanned(
739737
Thread* thread,
740738
const Instance& exception,
741739
const Instance& stacktrace) {
742-
UnhandledException& unhandled = UnhandledException::Handle(thread->zone());
743-
{
744-
NoThrowOOMScope no_throw_oom_scope(thread);
745-
unhandled ^= UnhandledException::New(Heap::kOld);
746-
}
747-
if (unhandled.IsNull()) {
748-
// If we failed to create new instance, use pre-canned one.
749-
unhandled ^= Object::unhandled_oom_exception().ptr();
750-
} else {
740+
LongJumpScope jump(thread);
741+
if (DART_SETJMP(*jump.Set()) == 0) {
742+
UnhandledException& unhandled =
743+
UnhandledException::Handle(UnhandledException::New(Heap::kOld));
751744
unhandled.set_exception(exception);
752745
unhandled.set_stacktrace(stacktrace);
746+
return unhandled.ptr();
747+
} else {
748+
RELEASE_ASSERT(thread->StealStickyError() ==
749+
Object::out_of_memory_error().ptr());
750+
// If we failed to create new instance, use pre-canned one.
751+
return Object::unhandled_oom_exception().ptr();
753752
}
754-
return unhandled.ptr();
755753
}
756754

757755
DART_NORETURN
@@ -802,12 +800,9 @@ static void ThrowExceptionHelper(Thread* thread,
802800
bool handler_needs_stacktrace = finder.needs_stacktrace;
803801
Instance& stacktrace = Instance::Handle(zone);
804802
if (create_stacktrace) {
805-
{
806-
NoThrowOOMScope no_throw_oom_scope(thread);
807-
stacktrace = CreateStackTrace(zone);
808-
}
809803
// Ensure we have enough memory to create stacktrace,
810804
// otherwise fallback to reporting OOM without stacktrace.
805+
stacktrace = TryCreateStackTrace(thread, zone);
811806
if (!stacktrace.IsNull()) {
812807
if (handler_pc == 0) {
813808
// No Dart frame.

runtime/vm/object.cc

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2891,9 +2891,6 @@ ObjectPtr Object::Allocate(intptr_t cls_id,
28912891
Report::LongJump(Object::out_of_memory_error());
28922892
UNREACHABLE();
28932893
} else if (thread->top_exit_frame_info() != 0) {
2894-
if (thread->IsInNoThrowOOMScope()) {
2895-
return Object::null();
2896-
}
28972894
Exceptions::ThrowOOM();
28982895
UNREACHABLE();
28992896
} else {

runtime/vm/thread.h

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -757,8 +757,6 @@ class Thread : public ThreadState {
757757
return stopped_mutators_scope_depth_ > 0;
758758
}
759759

760-
bool IsInNoThrowOOMScope() const { return no_throw_oom_scope_depth_ > 0; }
761-
762760
#define DEFINE_OFFSET_METHOD(type_name, member_name, expr, default_init_value) \
763761
static intptr_t member_name##offset() { \
764762
return OFFSET_OF(Thread, member_name); \
@@ -1458,7 +1456,6 @@ class Thread : public ThreadState {
14581456
VMHandles reusable_handles_;
14591457
int32_t stack_overflow_count_;
14601458
uint32_t runtime_call_count_ = 0;
1461-
intptr_t no_throw_oom_scope_depth_ = 0;
14621459

14631460
// Deoptimization of stack frames.
14641461
RuntimeCallDeoptAbility runtime_call_deopt_ability_ =
@@ -1632,7 +1629,6 @@ class Thread : public ThreadState {
16321629
friend class Simulator;
16331630
friend class StackZone;
16341631
friend class StoppedMutatorsScope;
1635-
friend class NoThrowOOMScope;
16361632
friend class ThreadRegistry;
16371633
friend class CompilerState;
16381634
friend class compiler::target::Thread;
@@ -1861,22 +1857,6 @@ class LeaveCompilerScope : public ValueObject {
18611857
};
18621858
#endif // defined(DEBUG)
18631859

1864-
class NoThrowOOMScope : public ThreadStackResource {
1865-
public:
1866-
explicit NoThrowOOMScope(Thread* thread) : ThreadStackResource(thread) {
1867-
thread->no_throw_oom_scope_depth_++;
1868-
ASSERT(thread->no_throw_oom_scope_depth_ >= 0);
1869-
}
1870-
1871-
~NoThrowOOMScope() {
1872-
thread()->no_throw_oom_scope_depth_ -= 1;
1873-
ASSERT(thread()->no_throw_oom_scope_depth_ >= 0);
1874-
}
1875-
1876-
private:
1877-
DISALLOW_COPY_AND_ASSIGN(NoThrowOOMScope);
1878-
};
1879-
18801860
} // namespace dart
18811861

18821862
#endif // RUNTIME_VM_THREAD_H_

0 commit comments

Comments
 (0)