From 133396cb94b48bdc58be3bb98b65c662707b74e0 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Wed, 14 Apr 2021 04:47:09 +0000 Subject: [PATCH] Bug 1515229 - Make MozStackWalk/MozWalkTheStack frame skipping more reliable. r=gerald,nika,bobowen,jld Differential Revision: https://phabricator.services.mozilla.com/D110899 --- memory/replace/dmd/DMD.cpp | 7 +- memory/replace/phc/PHC.cpp | 2 +- mfbt/Assertions.h | 8 +- mozglue/misc/StackWalk.cpp | 101 ++++--- mozglue/misc/StackWalk.h | 67 +++-- mozglue/tests/gtest/TestStackWalk.cpp | 271 ++++++++++++++++++ mozglue/tests/gtest/moz.build | 27 +- mozglue/tests/moz.build | 5 +- .../sandbox/win/loggingCallbacks.h | 4 +- .../chromium-shim/sandbox/win/loggingTypes.h | 2 +- .../sandbox/win/sandboxLogging.cpp | 29 +- .../sandbox/win/sandboxLogging.h | 9 +- security/sandbox/linux/Sandbox.cpp | 7 +- security/sandbox/linux/SandboxInternal.h | 2 +- security/sandbox/linux/glue/SandboxCrash.cpp | 12 +- toolkit/xre/nsSigHandlers.cpp | 13 +- xpcom/base/nsTraceRefcnt.cpp | 62 ++-- xpcom/build/LateWriteChecks.cpp | 3 +- xpcom/threads/BlockingResourceBase.cpp | 19 +- xpcom/threads/BlockingResourceBase.h | 3 +- 20 files changed, 496 insertions(+), 157 deletions(-) create mode 100644 mozglue/tests/gtest/TestStackWalk.cpp diff --git a/memory/replace/dmd/DMD.cpp b/memory/replace/dmd/DMD.cpp index bf64400ef4ae..ca1074bf20f2 100644 --- a/memory/replace/dmd/DMD.cpp +++ b/memory/replace/dmd/DMD.cpp @@ -663,12 +663,7 @@ static uint32_t gGCStackTraceTableWhenSizeExceeds = 4 * 1024; void* stackEnd = pthread_get_stackaddr_np(pthread_self()); FramePointerStackWalk(StackWalkCallback, MaxFrames, &tmp, fp, stackEnd); #else -# if defined(XP_WIN) && defined(_M_X64) - int skipFrames = 1; -# else - int skipFrames = 2; -# endif - MozStackWalk(StackWalkCallback, skipFrames, MaxFrames, &tmp); + MozStackWalk(StackWalkCallback, nullptr, MaxFrames, &tmp); #endif } diff --git a/memory/replace/phc/PHC.cpp b/memory/replace/phc/PHC.cpp index 2932f3615226..ec55ed8a4df8 100644 --- a/memory/replace/phc/PHC.cpp +++ b/memory/replace/phc/PHC.cpp @@ -232,7 +232,7 @@ void StackTrace::Fill() { void* stackEnd = pthread_get_stackaddr_np(pthread_self()); FramePointerStackWalk(StackWalkCallback, kMaxFrames, this, fp, stackEnd); #else - MozStackWalk(StackWalkCallback, /* aSkipFrames = */ 0, kMaxFrames, this); + MozStackWalk(StackWalkCallback, nullptr, kMaxFrames, this); #endif } diff --git a/mfbt/Assertions.h b/mfbt/Assertions.h index 6d414748f4f2..6c4e96372a0d 100644 --- a/mfbt/Assertions.h +++ b/mfbt/Assertions.h @@ -91,13 +91,13 @@ MOZ_ReportAssertionFailure(const char* aStr, const char* aFilename, "Assertion failure: %s, at %s:%d\n", aStr, aFilename, aLine); # if defined(MOZ_DUMP_ASSERTION_STACK) - MozWalkTheStackWithWriter(MOZ_ReportAssertionFailurePrintFrame, - /* aSkipFrames */ 1, /* aMaxFrames */ 0); + MozWalkTheStackWithWriter(MOZ_ReportAssertionFailurePrintFrame, CallerPC(), + /* aMaxFrames */ 0); # endif #else fprintf(stderr, "Assertion failure: %s, at %s:%d\n", aStr, aFilename, aLine); # if defined(MOZ_DUMP_ASSERTION_STACK) - MozWalkTheStack(stderr, /* aSkipFrames */ 1, /* aMaxFrames */ 0); + MozWalkTheStack(stderr, CallerPC(), /* aMaxFrames */ 0); # endif fflush(stderr); #endif @@ -112,7 +112,7 @@ MOZ_MAYBE_UNUSED static MOZ_COLD MOZ_NEVER_INLINE void MOZ_ReportCrash( #else fprintf(stderr, "Hit MOZ_CRASH(%s) at %s:%d\n", aStr, aFilename, aLine); # if defined(MOZ_DUMP_ASSERTION_STACK) - MozWalkTheStack(stderr, /* aSkipFrames */ 1, /* aMaxFrames */ 0); + MozWalkTheStack(stderr, CallerPC(), /* aMaxFrames */ 0); # endif fflush(stderr); #endif diff --git a/mozglue/misc/StackWalk.cpp b/mozglue/misc/StackWalk.cpp index cb840c1d174e..35901596fd8d 100644 --- a/mozglue/misc/StackWalk.cpp +++ b/mozglue/misc/StackWalk.cpp @@ -69,6 +69,26 @@ extern MOZ_EXPORT void* __libc_stack_end; // from ld-linux.so # include #endif +class FrameSkipper { + public: + constexpr FrameSkipper() : mPc(0) {} + bool ShouldSkipPC(void* aPC) { + // Skip frames until we encounter the one we were initialized with, + // and then never skip again. + if (mPc != 0) { + if (mPc != uintptr_t(aPC)) { + return true; + } + mPc = 0; + } + return false; + } + explicit FrameSkipper(const void* aPc) : mPc(uintptr_t(aPc)) {} + + private: + uintptr_t mPc; +}; + #ifdef XP_WIN # include @@ -92,7 +112,7 @@ struct WalkStackData { // Are we walking the stack of the calling thread? Note that we need to avoid // calling fprintf and friends if this is false, in order to avoid deadlocks. bool walkCallingThread; - uint32_t skipFrames; + const void* firstFramePC; HANDLE thread; HANDLE process; HANDLE eventStart; @@ -243,8 +263,7 @@ static void WalkStackMain64(struct WalkStackData* aData) { bool firstFrame = true; # endif - // Skip our own stack walking frames. - int skip = (aData->walkCallingThread ? 3 : 0) + aData->skipFrames; + FrameSkipper skipper(aData->firstFramePC); // Now walk the stack. while (true) { @@ -349,7 +368,7 @@ static void WalkStackMain64(struct WalkStackData* aData) { break; } - if (skip-- > 0) { + if (skipper.ShouldSkipPC((void*)addr)) { continue; } @@ -384,7 +403,7 @@ static void WalkStackMain64(struct WalkStackData* aData) { */ static void DoMozStackWalkThread(MozWalkStackCallback aCallback, - uint32_t aSkipFrames, uint32_t aMaxFrames, + const void* aFirstFramePC, uint32_t aMaxFrames, void* aClosure, HANDLE aThread, CONTEXT* aContext) { struct WalkStackData data; @@ -401,7 +420,7 @@ static void DoMozStackWalkThread(MozWalkStackCallback aCallback, data.walkCallingThread = (threadId == currentThreadId); } - data.skipFrames = aSkipFrames; + data.firstFramePC = aFirstFramePC; data.thread = targetThread; data.process = ::GetCurrentProcess(); void* local_pcs[1024]; @@ -435,14 +454,15 @@ static void DoMozStackWalkThread(MozWalkStackCallback aCallback, MFBT_API void MozStackWalkThread(MozWalkStackCallback aCallback, uint32_t aMaxFrames, void* aClosure, HANDLE aThread, CONTEXT* aContext) { - DoMozStackWalkThread(aCallback, /* aSkipFrames = */ 0, aMaxFrames, aClosure, - aThread, aContext); + DoMozStackWalkThread(aCallback, CallerPC(), aMaxFrames, aClosure, aThread, + aContext); } -MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames, - uint32_t aMaxFrames, void* aClosure) { - DoMozStackWalkThread(aCallback, aSkipFrames, aMaxFrames, aClosure, nullptr, - nullptr); +MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, + const void* aFirstFramePC, uint32_t aMaxFrames, + void* aClosure) { + DoMozStackWalkThread(aCallback, aFirstFramePC ? aFirstFramePC : CallerPC(), + aMaxFrames, aClosure, nullptr, nullptr); } static BOOL CALLBACK callbackEspecial64(PCSTR aModuleName, DWORD64 aModuleBase, @@ -691,12 +711,13 @@ void DemangleSymbol(const char* aSymbol, char* aBuffer, int aBufLen) { (MOZ_STACKWALK_SUPPORTS_MACOSX || MOZ_STACKWALK_SUPPORTS_LINUX)) static void DoFramePointerStackWalk(MozWalkStackCallback aCallback, - uint32_t aSkipFrames, uint32_t aMaxFrames, - void* aClosure, void** aBp, - void* aStackEnd); + const void* aFirstFramePC, + uint32_t aMaxFrames, void* aClosure, + void** aBp, void* aStackEnd); -MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames, - uint32_t aMaxFrames, void* aClosure) { +MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, + const void* aFirstFramePC, uint32_t aMaxFrames, + void* aClosure) { // Get the frame pointer void** bp = (void**)__builtin_frame_address(0); @@ -733,7 +754,7 @@ MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames, # else # error Unsupported configuration # endif - DoFramePointerStackWalk(aCallback, aSkipFrames, aMaxFrames, aClosure, bp, + DoFramePointerStackWalk(aCallback, aFirstFramePC, aMaxFrames, aClosure, bp, stackEnd); } @@ -744,7 +765,7 @@ MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames, struct unwind_info { MozWalkStackCallback callback; - int skip; + FrameSkipper skipper; int maxFrames; int numFrames; void* closure; @@ -755,7 +776,7 @@ static _Unwind_Reason_Code unwind_callback(struct _Unwind_Context* context, unwind_info* info = static_cast(closure); void* pc = reinterpret_cast(_Unwind_GetIP(context)); // TODO Use something like '_Unwind_GetGR()' to get the stack pointer. - if (--info->skip < 0) { + if (!info->skipper.ShouldSkipPC(pc)) { info->numFrames++; (*info->callback)(info->numFrames, pc, nullptr, info->closure); if (info->maxFrames != 0 && info->numFrames == info->maxFrames) { @@ -766,11 +787,12 @@ static _Unwind_Reason_Code unwind_callback(struct _Unwind_Context* context, return _URC_NO_REASON; } -MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames, - uint32_t aMaxFrames, void* aClosure) { +MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, + const void* aFirstFramePC, uint32_t aMaxFrames, + void* aClosure) { unwind_info info; info.callback = aCallback; - info.skip = aSkipFrames + 1; + info.skipper = FrameSkipper(aFirstFramePC ? aFirstFramePC : CallerPC()); info.maxFrames = aMaxFrames; info.numFrames = 0; info.closure = aClosure; @@ -840,8 +862,9 @@ bool MFBT_API MozDescribeCodeAddress(void* aPC, #else // unsupported platform. -MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames, - uint32_t aMaxFrames, void* aClosure) {} +MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, + const void* aFirstFramePC, uint32_t aMaxFrames, + void* aClosure) {} MFBT_API bool MozDescribeCodeAddress(void* aPC, MozCodeAddressDetails* aDetails) { @@ -859,13 +882,14 @@ MFBT_API bool MozDescribeCodeAddress(void* aPC, #if defined(XP_WIN) || defined(XP_MACOSX) || defined(XP_LINUX) MOZ_ASAN_BLACKLIST static void DoFramePointerStackWalk(MozWalkStackCallback aCallback, - uint32_t aSkipFrames, uint32_t aMaxFrames, - void* aClosure, void** aBp, - void* aStackEnd) { + const void* aFirstFramePC, + uint32_t aMaxFrames, void* aClosure, + void** aBp, void* aStackEnd) { // Stack walking code courtesy Kipp's "leaky". - int32_t skip = aSkipFrames; + FrameSkipper skipper(aFirstFramePC); uint32_t numFrames = 0; + while (aBp) { void** next = (void**)*aBp; // aBp may not be a frame pointer on i386 if code was compiled with @@ -885,7 +909,7 @@ static void DoFramePointerStackWalk(MozWalkStackCallback aCallback, void* pc = *(aBp + 1); aBp += 2; # endif - if (--skip < 0) { + if (!skipper.ShouldSkipPC(pc)) { // Assume that the SP points to the BP of the function // it called. We can't know the exact location of the SP // but this should be sufficient for our use the SP @@ -904,8 +928,10 @@ namespace mozilla { void FramePointerStackWalk(MozWalkStackCallback aCallback, uint32_t aMaxFrames, void* aClosure, void** aBp, void* aStackEnd) { - DoFramePointerStackWalk(aCallback, /* aSkipFrames = */ 0, aMaxFrames, - aClosure, aBp, aStackEnd); + // We don't pass a aFirstFramePC because we start walking the stack from the + // frame at aBp. + DoFramePointerStackWalk(aCallback, nullptr, aMaxFrames, aClosure, aBp, + aStackEnd); } } // namespace mozilla @@ -914,6 +940,7 @@ void FramePointerStackWalk(MozWalkStackCallback aCallback, uint32_t aMaxFrames, namespace mozilla { MFBT_API void FramePointerStackWalk(MozWalkStackCallback aCallback, + const void* aFirstFramePC, uint32_t aMaxFrames, void* aClosure, void** aBp, void* aStackEnd) {} } // namespace mozilla @@ -1006,10 +1033,11 @@ static bool WalkTheStackEnabled() { return result; } -MFBT_API void MozWalkTheStack(FILE* aStream, uint32_t aSkipFrames, +MFBT_API void MozWalkTheStack(FILE* aStream, const void* aFirstFramePC, uint32_t aMaxFrames) { if (WalkTheStackEnabled()) { - MozStackWalk(PrintStackFrame, aSkipFrames + 1, aMaxFrames, aStream); + MozStackWalk(PrintStackFrame, aFirstFramePC ? aFirstFramePC : CallerPC(), + aMaxFrames, aStream); } } @@ -1022,9 +1050,10 @@ static void WriteStackFrame(uint32_t aFrameNumber, void* aPC, void* aSP, } MFBT_API void MozWalkTheStackWithWriter(void (*aWriter)(const char*), - uint32_t aSkipFrames, + const void* aFirstFramePC, uint32_t aMaxFrames) { if (WalkTheStackEnabled()) { - MozStackWalk(WriteStackFrame, aSkipFrames + 1, aMaxFrames, (void*)aWriter); + MozStackWalk(WriteStackFrame, aFirstFramePC ? aFirstFramePC : CallerPC(), + aMaxFrames, (void*)aWriter); } } diff --git a/mozglue/misc/StackWalk.h b/mozglue/misc/StackWalk.h index 6c148e8b3983..250160934f95 100644 --- a/mozglue/misc/StackWalk.h +++ b/mozglue/misc/StackWalk.h @@ -15,6 +15,24 @@ MOZ_BEGIN_EXTERN_C +/** + * Returns the position of the Program Counter for the caller of the current + * function. This is meant to be used to feed the aFirstFramePC argument to + * MozStackWalk or MozWalkTheStack*, and should be used in the last function + * that should be skipped in the trace, and passed down to MozStackWalk or + * MozWalkTheStack*, through any intermediaries. + * + * THIS DOES NOT 100% RELIABLY GIVE THE CALLER PC, but marking functions + * calling this macro with MOZ_NEVER_INLINE gets us close. In cases it doesn't + * give the caller's PC, it may give the caller of the caller, or its caller, + * etc. depending on tail call optimization. + * + * Past versions of stackwalking relied on passing a constant number of frames + * to skip to MozStackWalk or MozWalkTheStack, which fell short in more cases + * (inlining of intermediaries, tail call optimization). + */ +#define CallerPC() __builtin_extract_return_addr(__builtin_return_address(0)) + /** * The callback for MozStackWalk and MozStackWalkThread. * @@ -34,18 +52,20 @@ typedef void (*MozWalkStackCallback)(uint32_t aFrameNumber, void* aPC, * Call aCallback for each stack frame on the current thread, from * the caller of MozStackWalk to main (or above). * - * @param aCallback Callback function, called once per frame. - * @param aSkipFrames Number of initial frames to skip. 0 means that - * the first callback will be for the caller of - * MozStackWalk. - * @param aMaxFrames Maximum number of frames to trace. 0 means no limit. - * @param aClosure Caller-supplied data passed through to aCallback. + * @param aCallback Callback function, called once per frame. + * @param aFirstFramePC Position of the Program Counter where the trace + * starts from. All frames seen before reaching that + * address are skipped. Nullptr means that the first + * callback will be for the caller of MozStackWalk. + * @param aMaxFrames Maximum number of frames to trace. 0 means no limit. + * @param aClosure Caller-supplied data passed through to aCallback. * * May skip some stack frames due to compiler optimizations or code * generation. */ -MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames, - uint32_t aMaxFrames, void* aClosure); +MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, + const void* aFirstFramePC, uint32_t aMaxFrames, + void* aClosure); typedef struct { /* @@ -147,29 +167,32 @@ MFBT_API int MozFormatCodeAddressDetails(char* aBuffer, uint32_t aBufferSize, /** * Walk the stack and print the stack trace to the given stream. * - * @param aStream A stdio stream. - * @param aSkipFrames Number of initial frames to skip. 0 means that - * the first callback will be for the caller of - * MozWalkTheStack. - * @param aMaxFrames Maximum number of frames to trace. 0 means no limit. + * @param aStream A stdio stream. + * @param aFirstFramePC Position of the Program Counter where the trace + * starts from. All frames seen before reaching that + * address are skipped. Nullptr means that the first + * callback will be for the caller of MozWalkTheStack. + * @param aMaxFrames Maximum number of frames to trace. 0 means no limit. */ MFBT_API void MozWalkTheStack(FILE* aStream, - uint32_t aSkipFrames FRAMES_DEFAULT, + const void* aFirstFramePC FRAMES_DEFAULT, uint32_t aMaxFrames FRAMES_DEFAULT); /** * Walk the stack and send each stack trace line to a callback writer. * Each line string is null terminated but doesn't contain a '\n' character. * - * @param aWriter The callback. - * @param aSkipFrames Number of initial frames to skip. 0 means that - * the first callback will be for the caller of - * MozWalkTheStack. - * @param aMaxFrames Maximum number of frames to trace. 0 means no limit. + * @param aWriter The callback. + * @param aFirstFramePC Position of the Program Counter where the trace + * starts from. All frames seen before reaching that + * address are skipped. Nullptr means that the first + * callback will be for the caller of + * MozWalkTheStackWithWriter. + * @param aMaxFrames Maximum number of frames to trace. 0 means no limit. */ -MFBT_API void MozWalkTheStackWithWriter(void (*aWriter)(const char*), - uint32_t aSkipFrames FRAMES_DEFAULT, - uint32_t aMaxFrames FRAMES_DEFAULT); +MFBT_API void MozWalkTheStackWithWriter( + void (*aWriter)(const char*), const void* aFirstFramePC FRAMES_DEFAULT, + uint32_t aMaxFrames FRAMES_DEFAULT); #undef FRAMES_DEFAULT diff --git a/mozglue/tests/gtest/TestStackWalk.cpp b/mozglue/tests/gtest/TestStackWalk.cpp new file mode 100644 index 000000000000..c27b9d526af6 --- /dev/null +++ b/mozglue/tests/gtest/TestStackWalk.cpp @@ -0,0 +1,271 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +// The header under test. +#include "mozilla/StackWalk.h" + +#include "mozilla/Assertions.h" +#include "mozilla/Attributes.h" + +#include "gtest/gtest.h" + +bool gDummy = true; + +struct StackWalkTester; + +// Descriptor of the recursive function calls wanted, and for each of them +// whether to perform tail call optimization or not. +struct CallInfo { + int (*mFunc)(int aDepth, int aLastSkipped, int aIgnored, + StackWalkTester& aTester); + bool mTailCall; + + bool TailCall() { +#ifdef __i386__ + // We can't make tail calls happen on i386 because all arguments to + // functions are on the stack, so the stack pointer needs to be updated + // before the call and restored after the call, so tail call optimization + // never happens. + return false; +#else + return mTailCall; +#endif + } +}; + +struct PCRange { + void* mStart; + void* mEnd; +}; + +// PCRange pretty printer for gtest assertions. +std::ostream& operator<<(std::ostream& aStream, const PCRange& aRange) { + aStream << aRange.mStart; + aStream << "-"; + aStream << aRange.mEnd; + return aStream; +} + +// Allow to use EXPECT_EQ with a vector of PCRanges and a vector of plain +// addresses, allowing a more useful output when the test fails, showing +// both lists. +bool operator==(const std::vector& aRanges, + const std::vector& aPtrs) { + if (aRanges.size() != aPtrs.size()) { + return false; + } + for (size_t i = 0; i < aRanges.size(); i++) { + auto range = aRanges[i]; + auto ptr = reinterpret_cast(aPtrs[i]); + if (ptr <= reinterpret_cast(range.mStart) || + ptr >= reinterpret_cast(range.mEnd)) { + return false; + } + } + return true; +} + +struct StackWalkTester { + // Description of the recursion of functions to perform for the testcase. + std::vector mFuncCalls; + // Collection of PCs reported by MozStackWalk. + std::vector mFramePCs; + // Collection of PCs expected per what was observed while recursing. + std::vector mExpectedFramePCs; + // The aFirstFramePC value that will be passed to MozStackWalk. + void* mFirstFramePC = nullptr; + + // Callback to be given to the stack walker. + // aClosure should point at an instance of this class. + static void StackWalkCallback(uint32_t aFrameNumber, void* aPC, void* aSP, + void* aClosure) { + ASSERT_NE(aClosure, nullptr); + StackWalkTester& tester = *reinterpret_cast(aClosure); + tester.mFramePCs.push_back(aPC); + EXPECT_EQ(tester.mFramePCs.size(), size_t(aFrameNumber)) + << "Frame number doesn't match"; + } + + // Callers of this function get a range of addresses with: + // ``` + // label: + // recursion(); + // AddExpectedPC(&&label); + // ``` + // This intends to record the range from label to the return of AddExpectedPC. + // The ideal code would be: + // ``` + // recursion(); + // label: + // AddExpectedPC(&&label); + // ``` + // and we wouldn't need to keep ranges. But while this works fine with Clang, + // GCC actually sometimes reorders code such the address received by + // AddExpectedPC is the address *before* the recursion. + // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99784 Using a label before the + // recursion and CallerPC() from a function call after the recursion makes it + // less likely for things to go wrong. + MOZ_NEVER_INLINE void AddExpectedPC(void* aPC) { + mExpectedFramePCs.push_back({aPC, CallerPC()}); + } + + // Function intended to be called in sequence for recursion. + // CallInfo lists are meant to contain a sequence of IntermediateCallback<1>, + // IntermediateCallback<2>, etc. + // aDepth is a counter of how deep the recursion has gone so far; + // aLastSkipped is the depth of the last frame we want skipped in the + // testcase; aIgnored is there to avoid the compiler merging both recursive + // function calls, which would prevent tail call optimization happening on one + // of them. aTester is the instance of this class for the testcase. + template + MOZ_NEVER_INLINE MOZ_EXPORT static int IntermediateCallback( + int aDepth, int aLastSkipped, int aIgnored, StackWalkTester& aTester) { + auto& callInfo = aTester.mFuncCalls.at(aDepth + 1); + if (aDepth == aLastSkipped) { + aTester.mFirstFramePC = CallerPC(); + } + if (aTester.mFuncCalls.at(aDepth).TailCall()) { + return callInfo.mFunc(aDepth + 1, aLastSkipped, Id, aTester); + // Since we're doing a tail call, we're not expecting this frame appearing + // in the trace. + } + here: + callInfo.mFunc(aDepth + 1, aLastSkipped, Id + 1, aTester); + aTester.AddExpectedPC(&&here); + return 0; + } + + MOZ_NEVER_INLINE MOZ_EXPORT static void LeafCallback( + int aDepth, int aLastSkipped, int aIgnored, StackWalkTester& aTester) { + if (aDepth == aLastSkipped) { + aTester.mFirstFramePC = CallerPC(); + } + if (aTester.mFuncCalls.at(aDepth).TailCall()) { + // For the same reason that we have the aIgnored argument on these + // callbacks, we need to avoid both MozStackWalk calls to be merged by the + // compiler, so we use different values of aMaxFrames for that. + return MozStackWalk(StackWalkTester::StackWalkCallback, + aTester.mFirstFramePC, + /*aMaxFrames*/ 19, &aTester); + // Since we're doing a tail call, we're not expecting this frame appearing + // in the trace. + } + here: + MozStackWalk(StackWalkTester::StackWalkCallback, aTester.mFirstFramePC, + /*aMaxFrames*/ 20, &aTester); + aTester.AddExpectedPC(&&here); + // Because we return nothing from this function, simply returning here would + // produce a tail-call optimization, which we explicitly don't want to + // happen. So we add a branch that depends on an extern value to prevent + // that from happening. + MOZ_RELEASE_ASSERT(gDummy); + } + + explicit StackWalkTester(std::initializer_list aFuncCalls) + : mFuncCalls(aFuncCalls) {} + + // Dump a vector of PCRanges as WalkTheStack would, for test failure output. + // Only the end of the range is shown. Not ideal, but + // MozFormatCodeAddressDetails only knows to deal with one address at a time. + // The full ranges would be printed by EXPECT_EQ anyways. + static std::string DumpFrames(std::vector& aFramePCRanges) { + std::vector framePCs; + framePCs.reserve(aFramePCRanges.size()); + for (auto range : aFramePCRanges) { + framePCs.push_back(range.mEnd); + } + return DumpFrames(framePCs); + } + + // Dump a vector of addresses as WalkTheStack would, for test failure output. + static std::string DumpFrames(std::vector& aFramePCs) { + size_t n = 0; + std::string result; + for (auto* framePC : aFramePCs) { + char buf[1024]; + MozCodeAddressDetails details; + result.append(" "); + n++; + if (MozDescribeCodeAddress(framePC, &details)) { + int length = + MozFormatCodeAddressDetails(buf, sizeof(buf), n, framePC, &details); + result.append(buf, std::min(length, (int)sizeof(buf) - 1)); + } else { + result.append("MozDescribeCodeAddress failed"); + } + result.append("\n"); + } + return result; + } + + // Dump a description of the given test case. + static std::string DumpFuncCalls(std::vector& aFuncCalls) { + std::string result; + for (auto funcCall : aFuncCalls) { + MozCodeAddressDetails details; + result.append(" "); + if (MozDescribeCodeAddress(reinterpret_cast(funcCall.mFunc), + &details)) { + result.append(details.function); + if (funcCall.TailCall()) { + result.append(" tail call"); + } + } else { + result.append("MozDescribeCodeAddress failed"); + } + result.append("\n"); + } + return result; + } + + MOZ_EXPORT MOZ_NEVER_INLINE void RunTest(int aLastSkipped) { + ASSERT_TRUE(aLastSkipped < (int)mFuncCalls.size()); + mFramePCs.clear(); + mExpectedFramePCs.clear(); + mFirstFramePC = nullptr; + auto& callInfo = mFuncCalls.at(0); + here: + callInfo.mFunc(0, aLastSkipped, 0, *this); + AddExpectedPC(&&here); + if (aLastSkipped < 0) { + aLastSkipped = mFuncCalls.size(); + } + for (int i = (int)mFuncCalls.size() - 1; i >= aLastSkipped; i--) { + if (!mFuncCalls.at(i).TailCall()) { + mExpectedFramePCs.erase(mExpectedFramePCs.begin()); + } + } + mFramePCs.resize(std::min(mExpectedFramePCs.size(), mFramePCs.size())); + EXPECT_EQ(mExpectedFramePCs, mFramePCs) + << "Expected frames:\n" + << DumpFrames(mExpectedFramePCs) << "Found frames:\n" + << DumpFrames(mFramePCs) + << "Function calls data (last skipped: " << aLastSkipped << "):\n" + << DumpFuncCalls(mFuncCalls); + } +}; + +TEST(TestStackWalk, StackWalk) +{ + const auto foo = StackWalkTester::IntermediateCallback<1>; + const auto bar = StackWalkTester::IntermediateCallback<2>; + const auto qux = StackWalkTester::IntermediateCallback<3>; + const auto leaf = reinterpret_cast( + StackWalkTester::LeafCallback); + + const std::initializer_list tests[] = { + {{foo, false}, {bar, false}, {qux, false}, {leaf, false}}, + {{foo, false}, {bar, true}, {qux, false}, {leaf, false}}, + {{foo, false}, {bar, false}, {qux, false}, {leaf, true}}, + {{foo, true}, {bar, false}, {qux, true}, {leaf, true}}, + }; + for (auto test : tests) { + StackWalkTester tester(test); + for (int i = -1; i < (int)test.size(); i++) { + tester.RunTest(i); + } + } +} diff --git a/mozglue/tests/gtest/moz.build b/mozglue/tests/gtest/moz.build index 5a5e2d8ac5c7..7d0a63bec1d7 100644 --- a/mozglue/tests/gtest/moz.build +++ b/mozglue/tests/gtest/moz.build @@ -2,16 +2,25 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +if CONFIG["OS_ARCH"] == "WINNT": + SOURCES += [ + "TestDLLBlocklist.cpp", + "TestNativeNtGTest.cpp", + ] + + TEST_DIRS += [ + "TestDllBlocklist_AllowByVersion", + "TestDllBlocklist_MatchByName", + "TestDllBlocklist_MatchByVersion", + "TestDllBlocklist_NoOpEntryPoint", + ] + SOURCES += [ - "TestDLLBlocklist.cpp", - "TestNativeNtGTest.cpp", + "TestStackWalk.cpp", ] +# The test relies on optimizations being on, so we can't let --disable-optimize +# getting in the way. See details in the source file. +SOURCES["TestStackWalk.cpp"].flags += ["-O2"] + FINAL_LIBRARY = "xul-gtest" - -TEST_DIRS += [ - "TestDllBlocklist_AllowByVersion", - "TestDllBlocklist_MatchByName", - "TestDllBlocklist_MatchByVersion", - "TestDllBlocklist_NoOpEntryPoint", -] diff --git a/mozglue/tests/moz.build b/mozglue/tests/moz.build index 3d7106ea978b..aa88eb5c8e93 100644 --- a/mozglue/tests/moz.build +++ b/mozglue/tests/moz.build @@ -38,7 +38,6 @@ if CONFIG["OS_ARCH"] == "WINNT": ) TEST_DIRS += [ "interceptor", - "gtest", ] OS_LIBS += [ "ntdll", @@ -50,3 +49,7 @@ if CONFIG["OS_TARGET"] == "WINNT" and CONFIG["CC_TYPE"] in ("gcc", "clang"): LDFLAGS += [ "-municode", ] + +TEST_DIRS += [ + "gtest", +] diff --git a/security/sandbox/chromium-shim/sandbox/win/loggingCallbacks.h b/security/sandbox/chromium-shim/sandbox/win/loggingCallbacks.h index 73f1ab7905b5..3c5f8eea8985 100644 --- a/security/sandbox/chromium-shim/sandbox/win/loggingCallbacks.h +++ b/security/sandbox/chromium-shim/sandbox/win/loggingCallbacks.h @@ -45,7 +45,7 @@ Log(const char* aMessageType, const char* aFunctionName, const char* aContext, const bool aShouldLogStackTrace = false, - uint32_t aFramesToSkip = 0) + const void* aFirstFramePC = nullptr) { std::ostringstream msgStream; msgStream << "Process Sandbox " << aMessageType << ": " << aFunctionName; @@ -61,7 +61,7 @@ Log(const char* aMessageType, StaticPrefs::security_sandbox_windows_log_stackTraceDepth(); if (stackTraceDepth) { msgStream << std::endl << "Stack Trace:"; - MozStackWalk(StackFrameToOStringStream, aFramesToSkip, stackTraceDepth, + MozStackWalk(StackFrameToOStringStream, aFirstFramePC, stackTraceDepth, &msgStream); } } diff --git a/security/sandbox/chromium-shim/sandbox/win/loggingTypes.h b/security/sandbox/chromium-shim/sandbox/win/loggingTypes.h index c9b74c14ec85..7b75648fc9b1 100644 --- a/security/sandbox/chromium-shim/sandbox/win/loggingTypes.h +++ b/security/sandbox/chromium-shim/sandbox/win/loggingTypes.h @@ -18,7 +18,7 @@ typedef void (*LogFunction) (const char* aMessageType, const char* aFunctionName, const char* aContext, const bool aShouldLogStackTrace, - uint32_t aFramesToSkip); + const void* aFirstFramePC); typedef void (*ProvideLogFunctionCb) (LogFunction aLogFunction); } // sandboxing diff --git a/security/sandbox/chromium-shim/sandbox/win/sandboxLogging.cpp b/security/sandbox/chromium-shim/sandbox/win/sandboxLogging.cpp index fa2314f69562..a556f9e77237 100644 --- a/security/sandbox/chromium-shim/sandbox/win/sandboxLogging.cpp +++ b/security/sandbox/chromium-shim/sandbox/win/sandboxLogging.cpp @@ -8,6 +8,8 @@ #include "base/strings/utf_string_conversions.h" #include "sandbox/win/src/sandbox_policy.h" +#include "mozilla/Attributes.h" +#include "mozilla/StackWalk.h" namespace mozilla { namespace sandboxing { @@ -20,34 +22,39 @@ ProvideLogFunction(LogFunction aLogFunction) sLogFunction = aLogFunction; } -void -LogBlocked(const char* aFunctionName, const char* aContext, uint32_t aFramesToSkip) +static void +LogBlocked(const char* aFunctionName, const char* aContext, const void* aFirstFramePC) { if (sLogFunction) { sLogFunction("BLOCKED", aFunctionName, aContext, - /* aShouldLogStackTrace */ true, aFramesToSkip); + /* aShouldLogStackTrace */ true, aFirstFramePC); } } -void +MOZ_NEVER_INLINE void +LogBlocked(const char* aFunctionName, const char* aContext) +{ + if (sLogFunction) { + LogBlocked(aFunctionName, aContext, CallerPC()); + } +} + +MOZ_NEVER_INLINE void LogBlocked(const char* aFunctionName, const wchar_t* aContext) { if (sLogFunction) { - // Skip an extra frame to allow for this function. - LogBlocked(aFunctionName, base::WideToUTF8(aContext).c_str(), - /* aFramesToSkip */ 3); + LogBlocked(aFunctionName, base::WideToUTF8(aContext).c_str(), CallerPC()); } } -void +MOZ_NEVER_INLINE void LogBlocked(const char* aFunctionName, const wchar_t* aContext, uint16_t aLengthInBytes) { if (sLogFunction) { - // Skip an extra frame to allow for this function. LogBlocked(aFunctionName, base::WideToUTF8(std::wstring(aContext, aLengthInBytes / sizeof(wchar_t))).c_str(), - /* aFramesToSkip */ 3); + CallerPC()); } } @@ -56,7 +63,7 @@ LogAllowed(const char* aFunctionName, const char* aContext) { if (sLogFunction) { sLogFunction("Broker ALLOWED", aFunctionName, aContext, - /* aShouldLogStackTrace */ false, /* aFramesToSkip */ 0); + /* aShouldLogStackTrace */ false, nullptr); } } diff --git a/security/sandbox/chromium-shim/sandbox/win/sandboxLogging.h b/security/sandbox/chromium-shim/sandbox/win/sandboxLogging.h index 365afa574c19..31c4ddb076f8 100644 --- a/security/sandbox/chromium-shim/sandbox/win/sandboxLogging.h +++ b/security/sandbox/chromium-shim/sandbox/win/sandboxLogging.h @@ -25,11 +25,10 @@ namespace sandboxing { void ProvideLogFunction(LogFunction aLogFunction); // Log a "BLOCKED" msg to the browser console and, if DEBUG build, stderr. -// If the logging of a stack trace is enabled then the default aFramesToSkip -// will start from our caller's caller, which should normally be the function -// that triggered the interception. -void LogBlocked(const char* aFunctionName, const char* aContext = nullptr, - uint32_t aFramesToSkip = 2); +// If the logging of a stack trace is enabled then a trace starting from the +// caller of the relevant LogBlocked overload will be logged, which should +// normally be the function that triggered the interception. +void LogBlocked(const char* aFunctionName, const char* aContext = nullptr); // Convenience functions to convert to char*. void LogBlocked(const char* aFunctionName, const wchar_t* aContext); diff --git a/security/sandbox/linux/Sandbox.cpp b/security/sandbox/linux/Sandbox.cpp index 772c6b806a20..354866ef9374 100644 --- a/security/sandbox/linux/Sandbox.cpp +++ b/security/sandbox/linux/Sandbox.cpp @@ -36,8 +36,10 @@ #include "mozilla/Array.h" #include "mozilla/Atomics.h" +#include "mozilla/Attributes.h" #include "mozilla/Range.h" #include "mozilla/SandboxInfo.h" +#include "mozilla/StackWalk.h" #include "mozilla/Span.h" #include "mozilla/UniquePtr.h" #include "mozilla/Unused.h" @@ -116,7 +118,8 @@ static bool ContextIsError(const ucontext_t* aContext, int aError) { * that it could be in async signal context (e.g., intercepting an * open() called from an async signal handler). */ -static void SigSysHandler(int nr, siginfo_t* info, void* void_context) { +MOZ_NEVER_INLINE static void SigSysHandler(int nr, siginfo_t* info, + void* void_context) { ucontext_t* ctx = static_cast(void_context); // This shouldn't ever be null, but the Chromium handler checks for // that and refrains from crashing, so let's not crash release builds: @@ -149,7 +152,7 @@ static void SigSysHandler(int nr, siginfo_t* info, void* void_context) { // Bug 1017393: record syscall number somewhere useful. info->si_addr = reinterpret_cast(report.mSyscall); - gSandboxCrashFunc(nr, info, &savedCtx); + gSandboxCrashFunc(nr, info, &savedCtx, CallerPC()); _exit(127); } } diff --git a/security/sandbox/linux/SandboxInternal.h b/security/sandbox/linux/SandboxInternal.h index bcd13c6019ab..27934f9a8edf 100644 --- a/security/sandbox/linux/SandboxInternal.h +++ b/security/sandbox/linux/SandboxInternal.h @@ -19,7 +19,7 @@ namespace mozilla { // its caller in libmozsandbox. // See also bug 1101170. -typedef void (*SandboxCrashFunc)(int, siginfo_t*, void*); +typedef void (*SandboxCrashFunc)(int, siginfo_t*, void*, const void*); extern MOZ_EXPORT SandboxCrashFunc gSandboxCrashFunc; extern const sock_fprog* gSetSandboxFilter; diff --git a/security/sandbox/linux/glue/SandboxCrash.cpp b/security/sandbox/linux/glue/SandboxCrash.cpp index 6868d4094a0b..d478cfa8b81f 100644 --- a/security/sandbox/linux/glue/SandboxCrash.cpp +++ b/security/sandbox/linux/glue/SandboxCrash.cpp @@ -81,19 +81,17 @@ static void SandboxPrintStackFrame(uint32_t aFrameNumber, void* aPC, void* aSP, SANDBOX_LOG_ERROR("frame %s", buf); } -static void SandboxLogCStack() { - // Skip 3 frames: one for this module, one for the signal handler in - // libmozsandbox, and one for the signal trampoline. - // +static void SandboxLogCStack(const void* aFirstFramePC) { // Warning: this might not print any stack frames. MozStackWalk // can't walk past the signal trampoline on ARM (bug 968531), and // x86 frame pointer walking may or may not work (bug 1082276). - MozStackWalk(SandboxPrintStackFrame, /* skip */ 3, /* max */ 0, nullptr); + MozStackWalk(SandboxPrintStackFrame, aFirstFramePC, /* max */ 0, nullptr); SANDBOX_LOG_ERROR("end of stack."); } -static void SandboxCrash(int nr, siginfo_t* info, void* void_context) { +static void SandboxCrash(int nr, siginfo_t* info, void* void_context, + const void* aFirstFramePC) { pid_t pid = getpid(), tid = syscall(__NR_gettid); bool dumped = CrashReporter::WriteMinidumpForSigInfo(nr, info, void_context); @@ -101,7 +99,7 @@ static void SandboxCrash(int nr, siginfo_t* info, void* void_context) { SANDBOX_LOG_ERROR( "crash reporter is disabled (or failed);" " trying stack trace:"); - SandboxLogCStack(); + SandboxLogCStack(aFirstFramePC); } // Do this last, in case it crashes or deadlocks. diff --git a/toolkit/xre/nsSigHandlers.cpp b/toolkit/xre/nsSigHandlers.cpp index e90dad23d456..068e32cadba7 100644 --- a/toolkit/xre/nsSigHandlers.cpp +++ b/toolkit/xre/nsSigHandlers.cpp @@ -56,6 +56,7 @@ unsigned int _gdb_sleep_duration = 300; # include # include "nsISupportsUtils.h" +# include "mozilla/Attributes.h" # include "mozilla/StackWalk.h" static const char* gProgname = "huh?"; @@ -78,12 +79,12 @@ static void PrintStackFrame(uint32_t aFrameNumber, void* aPC, void* aSP, } } -void ah_crap_handler(int signum) { +void common_crap_handler(int signum, const void* aFirstFramePC) { printf("\nProgram %s (pid = %d) received signal %d.\n", gProgname, getpid(), signum); printf("Stack:\n"); - MozStackWalk(PrintStackFrame, /* skipFrames */ 2, /* maxFrames */ 0, nullptr); + MozStackWalk(PrintStackFrame, aFirstFramePC, /* maxFrames */ 0, nullptr); printf("Sleeping for %d seconds.\n", _gdb_sleep_duration); printf("Type 'gdb %s %d' to attach your debugger to this thread.\n", @@ -99,10 +100,14 @@ void ah_crap_handler(int signum) { _exit(signum); } -void child_ah_crap_handler(int signum) { +MOZ_NEVER_INLINE void ah_crap_handler(int signum) { + common_crap_handler(signum, CallerPC()); +} + +MOZ_NEVER_INLINE void child_ah_crap_handler(int signum) { if (!getenv("MOZ_DONT_UNBLOCK_PARENT_ON_CHILD_CRASH")) close(kClientChannelFd); - ah_crap_handler(signum); + common_crap_handler(signum, CallerPC()); } # endif // CRAWL_STACK_ON_SIGSEGV diff --git a/xpcom/base/nsTraceRefcnt.cpp b/xpcom/base/nsTraceRefcnt.cpp index d59e3e1d791f..86fe502cb534 100644 --- a/xpcom/base/nsTraceRefcnt.cpp +++ b/xpcom/base/nsTraceRefcnt.cpp @@ -5,6 +5,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "nsTraceRefcnt.h" +#include "mozilla/Attributes.h" #include "mozilla/AutoRestore.h" #include "mozilla/CycleCollectedJSContext.h" #include "mozilla/IntegerPrintfMacros.h" @@ -136,7 +137,8 @@ static FILE* gRefcntsLog = nullptr; static FILE* gAllocLog = nullptr; static FILE* gCOMPtrLog = nullptr; -static void WalkTheStackSavingLocations(std::vector& aLocations); +static void WalkTheStackSavingLocations(std::vector& aLocations, + const void* aFirstFramePC); struct SerialNumberRecord { SerialNumberRecord() @@ -451,13 +453,13 @@ void nsTraceRefcnt::ResetStatistics() { gBloatView = nullptr; } -static intptr_t GetSerialNumber(void* aPtr, bool aCreate) { +static intptr_t GetSerialNumber(void* aPtr, bool aCreate, void* aFirstFramePC) { if (!aCreate) { auto record = gSerialNumbers->Get(aPtr); return record ? record->serialNumber : 0; } - gSerialNumbers->WithEntryHandle(aPtr, [](auto&& entry) { + gSerialNumbers->WithEntryHandle(aPtr, [aFirstFramePC](auto&& entry) { if (entry) { MOZ_CRASH( "If an object already has a serial number, we should be destroying " @@ -465,7 +467,7 @@ static intptr_t GetSerialNumber(void* aPtr, bool aCreate) { } auto& record = entry.Insert(MakeUnique()); - WalkTheStackSavingLocations(record->allocationStack); + WalkTheStackSavingLocations(record->allocationStack, aFirstFramePC); if (gLogJSStacks) { record->SaveJSStack(); } @@ -771,22 +773,20 @@ static void RecordStackFrame(uint32_t /*aFrameNumber*/, void* aPC, * OOM crashes. Therefore, this should only be used for things like refcount * logging which walk the stack extremely frequently. */ -static void WalkTheStackCached(FILE* aStream) { +static void WalkTheStackCached(FILE* aStream, const void* aFirstFramePC) { if (!gCodeAddressService) { gCodeAddressService = new CodeAddressService<>(); } - MozStackWalk(PrintStackFrameCached, /* skipFrames */ 2, /* maxFrames */ 0, + MozStackWalk(PrintStackFrameCached, aFirstFramePC, /* maxFrames */ 0, aStream); } -static void WalkTheStackSavingLocations(std::vector& aLocations) { +static void WalkTheStackSavingLocations(std::vector& aLocations, + const void* aFirstFramePC) { if (!gCodeAddressService) { gCodeAddressService = new CodeAddressService<>(); } - static const int kFramesToSkip = 0 + // this frame gets inlined - 1 + // GetSerialNumber - 1; // NS_LogCtor - MozStackWalk(RecordStackFrame, kFramesToSkip, /* maxFrames */ 0, &aLocations); + MozStackWalk(RecordStackFrame, aFirstFramePC, /* maxFrames */ 0, &aLocations); } //---------------------------------------------------------------------- @@ -873,7 +873,7 @@ void LogTerm() { } // namespace mozilla -EXPORT_XPCOM_API(void) +EXPORT_XPCOM_API(MOZ_NEVER_INLINE void) NS_LogAddRef(void* aPtr, nsrefcnt aRefcnt, const char* aClass, uint32_t aClassSize) { ASSERT_ACTIVITY_IS_LEGAL; @@ -899,7 +899,7 @@ NS_LogAddRef(void* aPtr, nsrefcnt aRefcnt, const char* aClass, bool loggingThisType = (!gTypesToLog || gTypesToLog->Contains(aClass)); intptr_t serialno = 0; if (gSerialNumbers && loggingThisType) { - serialno = GetSerialNumber(aPtr, aRefcnt == 1); + serialno = GetSerialNumber(aPtr, aRefcnt == 1, CallerPC()); MOZ_ASSERT(serialno != 0, "Serial number requested for unrecognized pointer! " "Are you memmoving a refcounted object?"); @@ -913,7 +913,7 @@ NS_LogAddRef(void* aPtr, nsrefcnt aRefcnt, const char* aClass, if (aRefcnt == 1 && gAllocLog && loggingThisType && loggingThisObject) { fprintf(gAllocLog, "\n<%s> %p %" PRIdPTR " Create [thread %p]\n", aClass, aPtr, serialno, PR_GetCurrentThread()); - WalkTheStackCached(gAllocLog); + WalkTheStackCached(gAllocLog, CallerPC()); } if (gRefcntsLog && loggingThisType && loggingThisObject) { @@ -921,13 +921,13 @@ NS_LogAddRef(void* aPtr, nsrefcnt aRefcnt, const char* aClass, fprintf(gRefcntsLog, "\n<%s> %p %" PRIuPTR " AddRef %" PRIuPTR " [thread %p]\n", aClass, aPtr, serialno, aRefcnt, PR_GetCurrentThread()); - WalkTheStackCached(gRefcntsLog); + WalkTheStackCached(gRefcntsLog, CallerPC()); fflush(gRefcntsLog); } } } -EXPORT_XPCOM_API(void) +EXPORT_XPCOM_API(MOZ_NEVER_INLINE void) NS_LogRelease(void* aPtr, nsrefcnt aRefcnt, const char* aClass) { ASSERT_ACTIVITY_IS_LEGAL; if (!gInitialized) { @@ -949,7 +949,7 @@ NS_LogRelease(void* aPtr, nsrefcnt aRefcnt, const char* aClass) { bool loggingThisType = (!gTypesToLog || gTypesToLog->Contains(aClass)); intptr_t serialno = 0; if (gSerialNumbers && loggingThisType) { - serialno = GetSerialNumber(aPtr, false); + serialno = GetSerialNumber(aPtr, false, CallerPC()); MOZ_ASSERT(serialno != 0, "Serial number requested for unrecognized pointer! " "Are you memmoving a refcounted object?"); @@ -965,7 +965,7 @@ NS_LogRelease(void* aPtr, nsrefcnt aRefcnt, const char* aClass) { fprintf(gRefcntsLog, "\n<%s> %p %" PRIuPTR " Release %" PRIuPTR " [thread %p]\n", aClass, aPtr, serialno, aRefcnt, PR_GetCurrentThread()); - WalkTheStackCached(gRefcntsLog); + WalkTheStackCached(gRefcntsLog, CallerPC()); fflush(gRefcntsLog); } @@ -975,7 +975,7 @@ NS_LogRelease(void* aPtr, nsrefcnt aRefcnt, const char* aClass) { if (aRefcnt == 0 && gAllocLog && loggingThisType && loggingThisObject) { fprintf(gAllocLog, "\n<%s> %p %" PRIdPTR " Destroy [thread %p]\n", aClass, aPtr, serialno, PR_GetCurrentThread()); - WalkTheStackCached(gAllocLog); + WalkTheStackCached(gAllocLog, CallerPC()); } if (aRefcnt == 0 && gSerialNumbers && loggingThisType) { @@ -984,7 +984,7 @@ NS_LogRelease(void* aPtr, nsrefcnt aRefcnt, const char* aClass) { } } -EXPORT_XPCOM_API(void) +EXPORT_XPCOM_API(MOZ_NEVER_INLINE void) NS_LogCtor(void* aPtr, const char* aType, uint32_t aInstanceSize) { ASSERT_ACTIVITY_IS_LEGAL; if (!gInitialized) { @@ -1007,7 +1007,7 @@ NS_LogCtor(void* aPtr, const char* aType, uint32_t aInstanceSize) { bool loggingThisType = (!gTypesToLog || gTypesToLog->Contains(aType)); intptr_t serialno = 0; if (gSerialNumbers && loggingThisType) { - serialno = GetSerialNumber(aPtr, true); + serialno = GetSerialNumber(aPtr, true, CallerPC()); MOZ_ASSERT(serialno != 0, "GetSerialNumber should never return 0 when passed true"); } @@ -1016,11 +1016,11 @@ NS_LogCtor(void* aPtr, const char* aType, uint32_t aInstanceSize) { if (gAllocLog && loggingThisType && loggingThisObject) { fprintf(gAllocLog, "\n<%s> %p %" PRIdPTR " Ctor (%d)\n", aType, aPtr, serialno, aInstanceSize); - WalkTheStackCached(gAllocLog); + WalkTheStackCached(gAllocLog, CallerPC()); } } -EXPORT_XPCOM_API(void) +EXPORT_XPCOM_API(MOZ_NEVER_INLINE void) NS_LogDtor(void* aPtr, const char* aType, uint32_t aInstanceSize) { ASSERT_ACTIVITY_IS_LEGAL; if (!gInitialized) { @@ -1043,7 +1043,7 @@ NS_LogDtor(void* aPtr, const char* aType, uint32_t aInstanceSize) { bool loggingThisType = (!gTypesToLog || gTypesToLog->Contains(aType)); intptr_t serialno = 0; if (gSerialNumbers && loggingThisType) { - serialno = GetSerialNumber(aPtr, false); + serialno = GetSerialNumber(aPtr, false, CallerPC()); MOZ_ASSERT(serialno != 0, "Serial number requested for unrecognized pointer! " "Are you memmoving a MOZ_COUNT_CTOR-tracked object?"); @@ -1057,11 +1057,11 @@ NS_LogDtor(void* aPtr, const char* aType, uint32_t aInstanceSize) { if (gAllocLog && loggingThisType && loggingThisObject) { fprintf(gAllocLog, "\n<%s> %p %" PRIdPTR " Dtor (%d)\n", aType, aPtr, serialno, aInstanceSize); - WalkTheStackCached(gAllocLog); + WalkTheStackCached(gAllocLog, CallerPC()); } } -EXPORT_XPCOM_API(void) +EXPORT_XPCOM_API(MOZ_NEVER_INLINE void) NS_LogCOMPtrAddRef(void* aCOMPtr, nsISupports* aObject) { #ifdef HAVE_CPP_DYNAMIC_CAST_TO_VOID_PTR // Get the most-derived object. @@ -1079,7 +1079,7 @@ NS_LogCOMPtrAddRef(void* aCOMPtr, nsISupports* aObject) { if (gLogging == FullLogging) { AutoTraceLogLock lock; - intptr_t serialno = GetSerialNumber(object, false); + intptr_t serialno = GetSerialNumber(object, false, CallerPC()); if (serialno == 0) { return; } @@ -1091,13 +1091,13 @@ NS_LogCOMPtrAddRef(void* aCOMPtr, nsISupports* aObject) { if (gCOMPtrLog && loggingThisObject) { fprintf(gCOMPtrLog, "\n %p %" PRIdPTR " nsCOMPtrAddRef %d %p\n", object, serialno, count, aCOMPtr); - WalkTheStackCached(gCOMPtrLog); + WalkTheStackCached(gCOMPtrLog, CallerPC()); } } #endif // HAVE_CPP_DYNAMIC_CAST_TO_VOID_PTR } -EXPORT_XPCOM_API(void) +EXPORT_XPCOM_API(MOZ_NEVER_INLINE void) NS_LogCOMPtrRelease(void* aCOMPtr, nsISupports* aObject) { #ifdef HAVE_CPP_DYNAMIC_CAST_TO_VOID_PTR // Get the most-derived object. @@ -1115,7 +1115,7 @@ NS_LogCOMPtrRelease(void* aCOMPtr, nsISupports* aObject) { if (gLogging == FullLogging) { AutoTraceLogLock lock; - intptr_t serialno = GetSerialNumber(object, false); + intptr_t serialno = GetSerialNumber(object, false, CallerPC()); if (serialno == 0) { return; } @@ -1127,7 +1127,7 @@ NS_LogCOMPtrRelease(void* aCOMPtr, nsISupports* aObject) { if (gCOMPtrLog && loggingThisObject) { fprintf(gCOMPtrLog, "\n %p %" PRIdPTR " nsCOMPtrRelease %d %p\n", object, serialno, count, aCOMPtr); - WalkTheStackCached(gCOMPtrLog); + WalkTheStackCached(gCOMPtrLog, CallerPC()); } } #endif // HAVE_CPP_DYNAMIC_CAST_TO_VOID_PTR diff --git a/xpcom/build/LateWriteChecks.cpp b/xpcom/build/LateWriteChecks.cpp index a9b37c295a0a..66bfd6f81615 100644 --- a/xpcom/build/LateWriteChecks.cpp +++ b/xpcom/build/LateWriteChecks.cpp @@ -129,8 +129,7 @@ void LateWriteObserver::Observe( // concurrently from many writes, so we use multiple temporary files. std::vector rawStack; - MozStackWalk(RecordStackWalker, /* skipFrames */ 0, /* maxFrames */ 0, - &rawStack); + MozStackWalk(RecordStackWalker, nullptr, /* maxFrames */ 0, &rawStack); mozilla::Telemetry::ProcessedStack stack = mozilla::Telemetry::GetStackAndModules(rawStack); diff --git a/xpcom/threads/BlockingResourceBase.cpp b/xpcom/threads/BlockingResourceBase.cpp index 4fab2a8df1db..8daf17d66aae 100644 --- a/xpcom/threads/BlockingResourceBase.cpp +++ b/xpcom/threads/BlockingResourceBase.cpp @@ -16,6 +16,7 @@ # include "nsTHashtable.h" # endif +# include "mozilla/Attributes.h" # include "mozilla/CondVar.h" # include "mozilla/DeadlockDetector.h" # include "mozilla/RecursiveMutex.h" @@ -55,20 +56,16 @@ void BlockingResourceBase::StackWalkCallback(uint32_t aFrameNumber, void* aPc, # endif } -void BlockingResourceBase::GetStackTrace(AcquisitionState& aState) { +void BlockingResourceBase::GetStackTrace(AcquisitionState& aState, + const void* aFirstFramePC) { # ifndef MOZ_CALLSTACK_DISABLED - // Skip this function and the calling function. - const uint32_t kSkipFrames = 2; - // Clear the array... aState.reset(); // ...and create a new one; this also puts the state to 'acquired' status // regardless of whether we obtain a stack trace or not. aState.emplace(); - // NB: Ignore the return value, there's nothing useful we can do if this - // this fails. - MozStackWalk(StackWalkCallback, kSkipFrames, kAcquisitionStateStackSize, + MozStackWalk(StackWalkCallback, aFirstFramePC, kAcquisitionStateStackSize, aState.ptr()); # endif } @@ -212,7 +209,7 @@ void BlockingResourceBase::Shutdown() { sDeadlockDetector = 0; } -void BlockingResourceBase::CheckAcquire() { +MOZ_NEVER_INLINE void BlockingResourceBase::CheckAcquire() { if (mType == eCondVar) { MOZ_ASSERT_UNREACHABLE( "FIXME bug 456272: annots. to allow CheckAcquire()ing condvars"); @@ -228,7 +225,7 @@ void BlockingResourceBase::CheckAcquire() { # ifndef MOZ_CALLSTACK_DISABLED // Update the current stack before printing. - GetStackTrace(mAcquired); + GetStackTrace(mAcquired, CallerPC()); # endif fputs("###!!! ERROR: Potential deadlock detected:\n", stderr); @@ -251,7 +248,7 @@ void BlockingResourceBase::CheckAcquire() { } } -void BlockingResourceBase::Acquire() { +MOZ_NEVER_INLINE void BlockingResourceBase::Acquire() { if (mType == eCondVar) { MOZ_ASSERT_UNREACHABLE( "FIXME bug 456272: annots. to allow Acquire()ing condvars"); @@ -265,7 +262,7 @@ void BlockingResourceBase::Acquire() { mAcquired = true; # else // Take a stack snapshot. - GetStackTrace(mAcquired); + GetStackTrace(mAcquired, CallerPC()); MOZ_ASSERT(IsAcquired()); if (!mFirstSeen) { diff --git a/xpcom/threads/BlockingResourceBase.h b/xpcom/threads/BlockingResourceBase.h index 312256591af2..abf89d53a806 100644 --- a/xpcom/threads/BlockingResourceBase.h +++ b/xpcom/threads/BlockingResourceBase.h @@ -316,7 +316,8 @@ class BlockingResourceBase { static void StackWalkCallback(uint32_t aFrameNumber, void* aPc, void* aSp, void* aClosure); - static void GetStackTrace(AcquisitionState& aState); + static void GetStackTrace(AcquisitionState& aState, + const void* aFirstFramePC); # ifdef MOZILLA_INTERNAL_API // so it can call BlockingResourceBase::Shutdown()