labath added a comment.

Thanks for replying Jason.

I think having the augmentation string specify its validity would be great, 
though I am somewhat sceptical of being able to push that idea through -- given 
that eh_frame needs to be understood by a lot of consumers and is actually 
critical for the correctness of some applications (those using exceptions), 
making changes to it seems to be hard -- that's why it's still stuck at version 
1 of debug_frame.

That said, given that current compilers do emit prologue/epilogue unwind info 
correctly, I'm not sure that we need the epilogue augmentation code, without 
additional header information. Given all of this discussion I'll try to find 
some time to do a detailed investigation of the prologue/epilogue state in 
various compiler versions (similar to the DW_AT_low_pc analysis in D78489 
<https://reviews.llvm.org/D78489>), and see what comes out of it.

You're right that the instruction emulation not handling multiple epilogues is 
a bug. It's probably not about _all_ epilogues, only some special ones -- for 
example, the function where we ran into this was so simple it did not have any 
traditional prologue instructions and I suspect this is what threw the code off.

I was planning to look into that separately. My flow of thought which led to 
starting off with this patch was:

- I knew that instruction emulation is broken, but the very first plan we try 
is the "eh_frame augmented" plan.
- It seemed like the augmenter should also be able to handle this function (and 
it's job should be even easier). So I tried to find why that doesn't kick in.
- While stepping through the code, I found this comment 
(UnwindAssembly-x86.cpp:127), which seems to imply that we don't need to 
augment it at all. It stop short of expliticly saying that we should actually 
use that plan, but that's seems implied to me. (it also makes sense -- the 
augmenter is there to add the epilogue instructions, and we already have them, 
so there's nothing to add..)
- So, I figured I'd first fix it so that it actually does what it says it does.

From your comment, it's not clear to me whether you are ok with this patch, or 
if you want to do something differently.

BTW, the call-next-instruction trick is still used, but not by any gcc version 
available on godbolt.org -- clang uses this sequence on i386 PIC. However, 
since clang-3.8, the cfi instruction properly describe how to unwind out of 
that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82378/new/

https://reviews.llvm.org/D82378



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to