wenlei added a comment.
> It isn't common when the program is built with the frame pointer omission
> disabled, but can still happen with third-party static libs built with frame
> pointer omitted.
Did you mean disabled -> enabled?
> This could happen to frame-pointer-based unwinding and the callee functions
> that do not have the frame pointer chain set up.
So this leads to frame pointer being used to volatile register and assume it
contains frame pointer would lead to bad frame address when unwinding the stack
during profiling, is that what you observed?
================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:523-531
+ auto I = Binary->getIndexForAddr(FrameAddr);
+ FrameAddr = I ? Binary->getAddressforIndex(I - 1) : 0;
+ // Stop at an invalid return address caused by bad unwinding. This could
+ // happen to frame-pointer-based unwinding and the callee functions that
+ // do not have the frame pointer chain set up.
+ if (!FrameAddr || !Binary->addressIsCall(FrameAddr)) {
+ NumStackSamplesWithInvalidReturnAddress++;
----------------
Can we hide all this complexity into `getCallAddrFromFrameAddr`? i.e. we could
use `bool getCallAddrFromFrameAddr(uint64_t FrameAddr, uint64_t &CallAddr)`.
================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:529
+ if (!FrameAddr || !Binary->addressIsCall(FrameAddr)) {
+ NumStackSamplesWithInvalidReturnAddress++;
+ break;
----------------
Should we be using a warning too? For truncated context due to missing probe
for merged callsite, we used warning. I think this is things of similar nature,
should we also handle them in similar/consistent fashion- i.e. with warning
instead of stats?
And if we expect duplicated warnings, would be good to deduplicated before
printing.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109638/new/
https://reviews.llvm.org/D109638
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
