Skip to content

Commit

Permalink
Bug 1384819 (part 2) - Tweak FramePointerStackWalk() arguments. r=gla…
Browse files Browse the repository at this point in the history
…ndium.

This patch does he following.

- Avoids some unnecessary casting.

- Renames the |bp| parameter as |aBp|.

- Makes the no-op FramePointerStackWalk() signature match the real one.
  (Clearly it's dead code in all built configurations!)
  • Loading branch information
nnethercote committed Jul 27, 2017
1 parent 926aa41 commit 7c7711e
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 20 deletions.
10 changes: 4 additions & 6 deletions memory/replace/dmd/DMD.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ StackTrace::Get(Thread* aT)
// frame pointer, and GetStackTop() for the stack end.
CONTEXT context;
RtlCaptureContext(&context);
void* fp = reinterpret_cast<void*>(context.Ebp);
void** fp = reinterpret_cast<void**>(context.Ebp);

// Offset 0x18 from the FS segment register gives a pointer to the thread
// information block for the current thread.
Expand All @@ -785,16 +785,15 @@ StackTrace::Get(Thread* aT)
#endif
void* stackEnd = static_cast<void*>(pTib->StackBase);
bool ok = FramePointerStackWalk(StackWalkCallback, /* skipFrames = */ 0,
MaxFrames, &tmp,
reinterpret_cast<void**>(fp), stackEnd);
MaxFrames, &tmp, fp, stackEnd);
#elif defined(XP_MACOSX)
// This avoids MozStackWalk(), which has become unusably slow on Mac due to
// changes in libunwind.
//
// This code is cribbed from the Gecko Profiler, which also uses
// FramePointerStackWalk() on Mac: Registers::SyncPopulate() for the frame
// pointer, and GetStackTop() for the stack end.
void* fp;
void** fp;
asm (
// Dereference %rbp to get previous %rbp
"movq (%%rbp), %0\n\t"
Expand All @@ -803,8 +802,7 @@ StackTrace::Get(Thread* aT)
);
void* stackEnd = pthread_get_stackaddr_np(pthread_self());
bool ok = FramePointerStackWalk(StackWalkCallback, /* skipFrames = */ 0,
MaxFrames, &tmp,
reinterpret_cast<void**>(fp), stackEnd);
MaxFrames, &tmp, fp, stackEnd);
#else
#if defined(XP_WIN) && defined(_M_X64)
int skipFrames = 1;
Expand Down
29 changes: 15 additions & 14 deletions mozglue/misc/StackWalk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1175,33 +1175,33 @@ MozDescribeCodeAddress(void* aPC, MozCodeAddressDetails* aDetails)
namespace mozilla {
bool
FramePointerStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
uint32_t aMaxFrames, void* aClosure, void** bp,
uint32_t aMaxFrames, void* aClosure, void** aBp,
void* aStackEnd)
{
// Stack walking code courtesy Kipp's "leaky".

int32_t skip = aSkipFrames;
uint32_t numFrames = 0;
while (bp) {
void** next = (void**)*bp;
// bp may not be a frame pointer on i386 if code was compiled with
while (aBp) {
void** next = (void**)*aBp;
// aBp may not be a frame pointer on i386 if code was compiled with
// -fomit-frame-pointer, so do some sanity checks.
// (bp should be a frame pointer on ppc(64) but checking anyway may help
// (aBp should be a frame pointer on ppc(64) but checking anyway may help
// a little if the stack has been corrupted.)
// We don't need to check against the begining of the stack because
// we can assume that bp > sp
if (next <= bp ||
// we can assume that aBp > sp
if (next <= aBp ||
next > aStackEnd ||
(uintptr_t(next) & 3)) {
break;
}
#if (defined(__ppc__) && defined(XP_MACOSX)) || defined(__powerpc64__)
// ppc mac or powerpc64 linux
void* pc = *(bp + 2);
bp += 3;
void* pc = *(aBp + 2);
aBp += 3;
#else // i386 or powerpc32 linux
void* pc = *(bp + 1);
bp += 2;
void* pc = *(aBp + 1);
aBp += 2;
#endif
if (IsCriticalAddress(pc)) {
return false;
Expand All @@ -1212,12 +1212,12 @@ FramePointerStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
// but this should be sufficient for our use the SP
// to order elements on the stack.
numFrames++;
(*aCallback)(numFrames, pc, bp, aClosure);
(*aCallback)(numFrames, pc, aBp, aClosure);
if (aMaxFrames != 0 && numFrames == aMaxFrames) {
break;
}
}
bp = next;
aBp = next;
}
return numFrames != 0;
}
Expand All @@ -1228,7 +1228,8 @@ FramePointerStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
namespace mozilla {
MFBT_API bool
FramePointerStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
void* aClosure, void** aBp)
uint32_t aMaxFrames, void* aClosure, void** aBp,
void* aStackEnd)
{
return false;
}
Expand Down

0 comments on commit 7c7711e

Please sign in to comment.