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

Reply via email to