clayborg added inline comments.
================
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 =
----------------
labath wrote:
> aadsm wrote:
> > labath wrote:
> > > clayborg wrote:
> > > > 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.
> > > Your understanding is correct, at least at the object file level -- I'm
> > > not sure whether `IsWeak()` implies `IsExternal()` inside lldb. However,
> > > I would actually argue for removal of IsWeak() for a different reason --
> > > any weak definition of mmap is not going to be used by the process since
> > > libc already has a strong definition of that symbol.
> > >
> > > If we really end up in a situation where we only have a weak mmap symbol
> > > around, then this is probably a sufficiently strange setup that we don't
> > > want to be randomly calling that function.
> > All (with the exception of the one overriden by the leak sanitizer) mmap
> > symbols in the symbol table are Weak bound but none are External bound,
> > this was the main reason that lead me to investigate the difference between
> > the two.
> >
> > Not sure how to check how lldb interprets the Weak overall, but I think it
> > kind of does, because that's what I saw when I dumped the symbol table:
> > ```
> > (lldb) target modules dump symtab libc.so.6
> > Debug symbol
> > |Synthetic symbol
> > ||Externally Visible
> > |||
> > Index UserID DSX Type File Address/Value Load Address
> > Size Flags Name
> > ------- ------ --- --------------- ------------------ ------------------
> > ------------------ ---------- ----------------------------------
> > ...
> > [ 6945] 6947 X Code 0x000000000010b5e0 0x00007ffff69db5e0
> > 0x00000000000000f9 0x00000012 __mmap
> > ...
> > ```
> >
> > Another solution I thought was to add a IsLocal to the Symbol (and use
> > !IsLocal) but then not really sure how to implement this for all Symbols
> > not ELFSymbol.
> Interesting. I guess I should have verified my assumptions. With that in
> mind, I think checking for both weak and external (strong) symbols is fine.
I think this is a bug in the ObjectFileELF where it is setting weak but not
external for weak symbols. Weak symbols would be useless if they weren't
external as the first one to get used should end up winning and all other weak
symbols should resolve to the first symbol to come along that matches when dyld
does a lookup. So I think we should verify this with some linker folks and
remove IsWeak from this if statement, and fix the ObjectFileELF symbol table
parsing code.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87868/new/
https://reviews.llvm.org/D87868
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
