Bug 1698719 - Remove aSkipFrames argument to both FramePointerStackWalk and MozStackWalkThread. r=gsvelto,gerald

In the case of FramePointerStackwalk, the caller gives a pointer to the
top-most frame to walk from. There isn't really a reason to give a
number of frames to skip, as the right frame pointer could be given in
the first place if that was really necessary. And in practice, it's
hasn't been used so far.

In the case of MozStackWalkThread, the caller presumably doesn't know
what the thread the stack is being walked for is doing, and it would be
a guesswork to pass a valid number of frames to skip. In practice, it's
also not used.

The aSkipFrames is already a footgun on MozStackWalk (and we're going to
change that in bug 1515229), we don't need to keep a footgun on these
other stack walking methods.

Differential Revision: https://phabricator.services.mozilla.com/D108563
This commit is contained in:
Mike Hommey 2021-03-17 00:21:39 +00:00
Родитель e0c536d241
Коммит ec7f538e0c
6 изменённых файлов: 48 добавлений и 35 удалений

Просмотреть файл

@ -643,8 +643,7 @@ static uint32_t gGCStackTraceTableWhenSizeExceeds = 4 * 1024;
PNT_TIB pTib = reinterpret_cast<PNT_TIB>(NtCurrentTeb());
void* stackEnd = static_cast<void*>(pTib->StackBase);
FramePointerStackWalk(StackWalkCallback, /* skipFrames = */ 0, MaxFrames,
&tmp, fp, stackEnd);
FramePointerStackWalk(StackWalkCallback, MaxFrames, &tmp, fp, stackEnd);
#elif defined(XP_MACOSX)
// This avoids MozStackWalk(), which has become unusably slow on Mac due to
// changes in libunwind.
@ -662,8 +661,7 @@ static uint32_t gGCStackTraceTableWhenSizeExceeds = 4 * 1024;
asm("ldr %0, [x29]\n\t" : "=r"(fp));
# endif
void* stackEnd = pthread_get_stackaddr_np(pthread_self());
FramePointerStackWalk(StackWalkCallback, /* skipFrames = */ 0, MaxFrames,
&tmp, fp, stackEnd);
FramePointerStackWalk(StackWalkCallback, MaxFrames, &tmp, fp, stackEnd);
#else
# if defined(XP_WIN) && defined(_M_X64)
int skipFrames = 1;

Просмотреть файл

@ -216,8 +216,7 @@ void StackTrace::Fill() {
PNT_TIB pTib = reinterpret_cast<PNT_TIB>(NtCurrentTeb());
void* stackEnd = static_cast<void*>(pTib->StackBase);
FramePointerStackWalk(StackWalkCallback, /* aSkipFrames = */ 0, kMaxFrames,
this, fp, stackEnd);
FramePointerStackWalk(StackWalkCallback, kMaxFrames, this, fp, stackEnd);
#elif defined(XP_MACOSX)
// This avoids MozStackWalk(), which has become unusably slow on Mac due to
// changes in libunwind.
@ -231,8 +230,7 @@ void StackTrace::Fill() {
"movq (%%rbp), %0\n\t"
: "=r"(fp));
void* stackEnd = pthread_get_stackaddr_np(pthread_self());
FramePointerStackWalk(StackWalkCallback, /* skipFrames = */ 0, kMaxFrames,
this, fp, stackEnd);
FramePointerStackWalk(StackWalkCallback, kMaxFrames, this, fp, stackEnd);
#else
MozStackWalk(StackWalkCallback, /* aSkipFrames = */ 0, kMaxFrames, this);
#endif

Просмотреть файл

@ -1427,8 +1427,8 @@ static void DoFramePointerBacktrace(PSLockRef aLock,
const void* stackEnd = aRegisteredThread.StackTop();
if (aRegs.mFP >= aRegs.mSP && aRegs.mFP <= stackEnd) {
FramePointerStackWalk(StackWalkCallback, /* skipFrames */ 0, maxFrames,
&aNativeStack, reinterpret_cast<void**>(aRegs.mFP),
FramePointerStackWalk(StackWalkCallback, maxFrames, &aNativeStack,
reinterpret_cast<void**>(aRegs.mFP),
const_cast<void*>(stackEnd));
}
}
@ -1453,8 +1453,8 @@ static void DoMozStackWalkBacktrace(PSLockRef aLock,
HANDLE thread = GetThreadHandle(aRegisteredThread.GetPlatformData());
MOZ_ASSERT(thread);
MozStackWalkThread(StackWalkCallback, /* skipFrames */ 0, maxFrames,
&aNativeStack, thread, /* context */ nullptr);
MozStackWalkThread(StackWalkCallback, maxFrames, &aNativeStack, thread,
/* context */ nullptr);
}
#endif

Просмотреть файл

@ -376,7 +376,7 @@ static void WalkStackMain64(struct WalkStackData* aData) {
* whose in memory address doesn't match its in-file address.
*/
MFBT_API void MozStackWalkThread(MozWalkStackCallback aCallback,
static void DoMozStackWalkThread(MozWalkStackCallback aCallback,
uint32_t aSkipFrames, uint32_t aMaxFrames,
void* aClosure, HANDLE aThread,
CONTEXT* aContext) {
@ -425,10 +425,17 @@ MFBT_API void MozStackWalkThread(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);
}
MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
uint32_t aMaxFrames, void* aClosure) {
MozStackWalkThread(aCallback, aSkipFrames, aMaxFrames, aClosure, nullptr,
nullptr);
DoMozStackWalkThread(aCallback, aSkipFrames, aMaxFrames, aClosure, nullptr,
nullptr);
}
static BOOL CALLBACK callbackEspecial64(PCSTR aModuleName, DWORD64 aModuleBase,
@ -676,6 +683,11 @@ void DemangleSymbol(const char* aSymbol, char* aBuffer, int aBufLen) {
# if ((defined(__i386) || defined(PPC) || defined(__ppc__)) && \
(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);
MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
uint32_t aMaxFrames, void* aClosure) {
// Get the frame pointer
@ -714,8 +726,8 @@ MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
# else
# error Unsupported configuration
# endif
FramePointerStackWalk(aCallback, aSkipFrames, aMaxFrames, aClosure, bp,
stackEnd);
DoFramePointerStackWalk(aCallback, aSkipFrames, aMaxFrames, aClosure, bp,
stackEnd);
}
# elif defined(HAVE__UNWIND_BACKTRACE)
@ -838,11 +850,11 @@ MFBT_API bool MozDescribeCodeAddress(void* aPC,
#endif
#if defined(XP_WIN) || defined(XP_MACOSX) || defined(XP_LINUX)
namespace mozilla {
MOZ_ASAN_BLACKLIST
void FramePointerStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
uint32_t aMaxFrames, void* aClosure, void** aBp,
void* aStackEnd) {
static void DoFramePointerStackWalk(MozWalkStackCallback aCallback,
uint32_t aSkipFrames, uint32_t aMaxFrames,
void* aClosure, void** aBp,
void* aStackEnd) {
// Stack walking code courtesy Kipp's "leaky".
int32_t skip = aSkipFrames;
@ -880,15 +892,23 @@ void FramePointerStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
aBp = next;
}
}
namespace mozilla {
void FramePointerStackWalk(MozWalkStackCallback aCallback, uint32_t aMaxFrames,
void* aClosure, void** aBp, void* aStackEnd) {
DoFramePointerStackWalk(aCallback, /* aSkipFrames = */ 0, aMaxFrames,
aClosure, aBp, aStackEnd);
}
} // namespace mozilla
#else
namespace mozilla {
MFBT_API void FramePointerStackWalk(MozWalkStackCallback aCallback,
uint32_t aSkipFrames, uint32_t aMaxFrames,
void* aClosure, void** aBp,
void* aStackEnd) {}
uint32_t aMaxFrames, void* aClosure,
void** aBp, void* aStackEnd) {}
} // namespace mozilla
#endif

Просмотреть файл

@ -57,7 +57,6 @@ MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
* the caller of MozStackWalk to main (or above).
*
* @param aCallback Same as for MozStackWalk().
* @param aSkipFrames Same as for MozStackWalk().
* @param aMaxFrames Same as for MozStackWalk().
* @param aClosure Same as for MozStackWalk().
* @param aThread The handle of the thread whose stack is to be walked.
@ -67,9 +66,8 @@ MFBT_API void MozStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
* null, the CONTEXT will be re-obtained.
*/
MFBT_API void MozStackWalkThread(MozWalkStackCallback aCallback,
uint32_t aSkipFrames, uint32_t aMaxFrames,
void* aClosure, HANDLE aThread,
CONTEXT* aContext);
uint32_t aMaxFrames, void* aClosure,
HANDLE aThread, CONTEXT* aContext);
#else
@ -164,9 +162,8 @@ MFBT_API void MozFormatCodeAddressDetails(
namespace mozilla {
MFBT_API void FramePointerStackWalk(MozWalkStackCallback aCallback,
uint32_t aSkipFrames, uint32_t aMaxFrames,
void* aClosure, void** aBp,
void* aStackEnd);
uint32_t aMaxFrames, void* aClosure,
void** aBp, void* aStackEnd);
#if defined(XP_LINUX) || defined(XP_FREEBSD)
MFBT_API void DemangleSymbol(const char* aSymbol, char* aBuffer, int aBufLen);

Просмотреть файл

@ -1982,8 +1982,8 @@ static void DoFramePointerBacktrace(PSLockRef aLock,
const void* stackEnd = aRegisteredThread.StackTop();
if (aRegs.mFP >= aRegs.mSP && aRegs.mFP <= stackEnd) {
FramePointerStackWalk(StackWalkCallback, /* skipFrames */ 0, maxFrames,
&aNativeStack, reinterpret_cast<void**>(aRegs.mFP),
FramePointerStackWalk(StackWalkCallback, maxFrames, &aNativeStack,
reinterpret_cast<void**>(aRegs.mFP),
const_cast<void*>(stackEnd));
}
}
@ -2008,8 +2008,8 @@ static void DoMozStackWalkBacktrace(PSLockRef aLock,
HANDLE thread = GetThreadHandle(aRegisteredThread.GetPlatformData());
MOZ_ASSERT(thread);
MozStackWalkThread(StackWalkCallback, /* skipFrames */ 0, maxFrames,
&aNativeStack, thread, /* context */ nullptr);
MozStackWalkThread(StackWalkCallback, maxFrames, &aNativeStack, thread,
/* context */ nullptr);
}
#endif