clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
We probably don't need to check for IsWeak() as we wouldn't want an internal +
weak symbol (if there is such a thing) to match and be used.
================
Comment at: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:36
bool lldb_private::InferiorCallMmap(Process *process, addr_t &allocated_addr,
addr_t addr, addr_t length, unsigned prot,
----------------
Might be nice to return a llvm::Error here instead of pool? Maybe to help
explain what went wrong? like any of:
"thread required to call mmap"
"couldn't find any symbols named 'mmap'"
"no external symbols named 'mmap' were found"
"mmap call failed" (for when it returns UINT32_MAX for 4 byte addresses or
UINT64_MAX for 8 byte addresses"
It have been nice to see a nice error message during the expression evaluation
prior to this fix. At the very least we should log to the expression log
channel that mmap failed if we choose not to return an error.
================
Comment at: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:54
+ if (sc_list.GetContextAtIndex(i, sc) &&
+ (sc.symbol->IsExternal() || sc.symbol->IsWeak())) {
const uint32_t range_scope =
----------------
aadsm wrote:
> clayborg wrote:
> > Why are we checking "IsWeak()" here?
> A weak symbol is also an external symbol, but it's weak in the sense that
> another external symbol with the same name will take precedence over it (as
> far as I understood).
I think we only need to check for external here. Any weak symbol will also need
to be external, but if it isn't we don't want that symbol.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87868/new/
https://reviews.llvm.org/D87868
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits