labath added a comment.

The patch description could definitely use more details about the motivation 
for the change, and a description of how it works. Apart from the fact that it 
uses the raSearch feature I introduced a while back, I don't know much about 
how it works (and given the planned changes, I am not going to try to 
understand that), but I think I can provide some details/thoughts about the 
motivation.

(IIUC), the motivating case is some code in kernel32.dll, which ends up setting 
rbp to zero. I don't know why it does that, but I suspect it may have something 
to do with the windows syscall convention (I know that ebp is used in i386 
linux, for instance).  While it would be definitely interesting to understand 
why it does that, I don't think it really matters -- one can never really 
exclude the possibility that some code (inline asm perhaps) will be messing 
with the frame pointer. So, scanning the stack for return addresses as a *last* 
last resort (the architectural default plan is already a kind of a last resort 
option) seems like it could be useful, regardless of the architecture (although 
the actual algorithm may differ somewhat for x86, which stores the return 
address on the stack directly, and say arm, which uses a link register). And 
given that the choice is between "showing nothing" and "showing something 
potentially incorrect", I don't think we need to be extra careful about vetting 
this address. All the checks Greg mentioned seem fine, but I don't see a 
compelling reason to implement them (all of them), especially given that they 
would not be useful in this case -- all we have is a minidump core file with 
some stack memory and a list of loaded modules (not necessarily the modules 
themselves).  In this case, all we can do is to check whether the pointer 
points to some known code region (which is already done IIRC).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124198

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

Reply via email to