amccarth added a comment.

I don't have any specific code comments, but I do have a couple general 
questions and points to consider.

1. `isFoundHeuristically` is very generic.  It's true that it's a heuristic 
approach, but it's a very specific heuristic.  Might there be other heuristic 
approaches in the future?  Should we name it something more specific like 
`isRaSearch`?

2. `max_iterations` means how many stack positions the heuristic will scan 
before giving up, right?  Are there any alignment issues here?  Should we 
assert that the return address hint is a multiple of the stack alignment?

3. The 100 for `max_iterations` is probably fine, but I wonder if there's a way 
to determine a more specific limit without just guessing.  What things could be 
on the stack between the hint and the actual return address?  It seems like 
only arguments for a call that the current function is preparing to make.  The 
standard says that the actual number of parameters is implementation-defined, 
but that it suggests a minimum of 256.  Should `max_iterations` be 256?  Is 
there much risk in making it bigger than it needs to be?

4. Is checking for executable permission slow?  Would it be worth doing some 
culling or caching?  I imagine a lot of non-return address values on the stack 
will be simple small numbers, like 0 or 1, which, for Windows, would never be a 
valid executable address.



================
Comment at: include/lldb/Symbol/SymbolFile.h:254
+  /// variables and spilled registers, but it should not include paramenters, 
as
+  /// they are considered to be a part of the callers frame.
+  virtual llvm::Expected<lldb::addr_t> GetOwnFrameSize(Symbol &symbol) {
----------------
Typos:

paramenters -> parameters
callers -> caller's



Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66638



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

Reply via email to