Commit f5c15f9c authored by Michael Lippautz's avatar Michael Lippautz Committed by Commit Bot

[oilpan] Fix stack limit check for recursive tracing

The limit was defined as ~0ul (unsigned long) which is unsigned 32bit on Windows
64bit and thus extended with 0 for uintptr_t. This results in bogus return of
StackFrameDepth::IsSafeToRecurse of true even though recursion might be disabled
due to stack pressure.

Bug: chromium:798479
Change-Id: I13710b3dca4d3785e35aedd8ba5f7ece167b20e4
Reviewed-on: https://chromium-review.googlesource.com/847580
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526666}
parent 8edd5f7d
......@@ -42,6 +42,7 @@
#include "platform/heap/HeapTestUtilities.h"
#include "platform/heap/SafePoint.h"
#include "platform/heap/SelfKeepAlive.h"
#include "platform/heap/StackFrameDepth.h"
#include "platform/heap/ThreadState.h"
#include "platform/heap/Visitor.h"
#include "platform/testing/UnitTestHelpers.h"
......@@ -6253,6 +6254,20 @@ TEST(HeapTest, StackGrowthDirection) {
EXPECT_EQ(kGrowsTowardsLower, StackGrowthDirection());
}
TEST(HeapTest, StackFrameDepthDisabledByDefault) {
StackFrameDepth depth;
// Only allow recursion after explicitly enabling the stack limit.
EXPECT_FALSE(depth.IsSafeToRecurse());
}
TEST(HeapTest, StackFrameDepthEnable) {
StackFrameDepth depth;
StackFrameDepthScope scope(&depth);
// The scope may fail to enable recursion when the stack is close to the
// limit. In all other cases we should be able to safely recurse.
EXPECT_TRUE(depth.IsSafeToRecurse() || !depth.IsEnabled());
}
class TestMixinAllocationA : public GarbageCollected<TestMixinAllocationA>,
public GarbageCollectedMixin {
USING_GARBAGE_COLLECTED_MIXIN(TestMixinAllocationA);
......
......@@ -5,6 +5,10 @@
#ifndef StackFrameDepth_h
#define StackFrameDepth_h
#if defined(COMPILER_MSVC)
#include <intrin.h>
#endif
#include <stdint.h>
#include <cstddef>
#include "base/macros.h"
......@@ -48,25 +52,16 @@ class PLATFORM_EXPORT StackFrameDepth final {
#endif
}
#if defined(COMPILER_MSVC)
// Ignore C4172: returning address of local variable or temporary: dummy. This
// warning suppression has to go outside of the function to take effect.
#pragma warning(push)
#pragma warning(disable : 4172)
#endif
static uintptr_t CurrentStackFrame(const char* dummy = nullptr) {
static uintptr_t CurrentStackFrame() {
#if defined(COMPILER_GCC)
return reinterpret_cast<uintptr_t>(__builtin_frame_address(0));
#elif defined(COMPILER_MSVC)
return reinterpret_cast<uintptr_t>(&dummy) - sizeof(void*);
return reinterpret_cast<uintptr_t>(_ReturnAddress());
#else
#error "Stack frame pointer estimation not supported on this platform."
return 0;
#endif
}
#if defined(COMPILER_MSVC)
#pragma warning(pop)
#endif
private:
// The maximum depth of eager, unrolled trace() calls that is
......@@ -77,7 +72,7 @@ class PLATFORM_EXPORT StackFrameDepth final {
// The stack pointer is assumed to grow towards lower addresses;
// |kMinimumStackLimit| then being the limit that a stack
// pointer will always exceed.
static const uintptr_t kMinimumStackLimit = ~0ul;
static const uintptr_t kMinimumStackLimit = ~uintptr_t{0};
static uintptr_t GetFallbackStackLimit();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment