aadsm 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: > 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. 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