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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits