[Lldb-commits] [PATCH] D131974: [lldb][ClangExpression] Add asm() label to all FunctionDecls we create from DWARF

Thu Aug 18 11:57:08 PDT 2022

Michael137 added a comment.

In D131974#3732727 <https://reviews.llvm.org/D131974#3732727>, @labath wrote:

> In D131974#3726618 <https://reviews.llvm.org/D131974#3726618>, @labath wrote:
>
>> For me this wins on simplicity grounds alone.
>>
>> In D131974#3726475 <https://reviews.llvm.org/D131974#3726475>, @Michael137 wrote:
>>
>>> - Check mangled name and storage class before attaching label
>>
>> I don't think that the storage class check is helping anything. Non-external functions can have DW_AT_linkage_names and ABI tags (although their usefulness is questionable) as well. Of course, that means that the local (indicated by the `L` in the name) mangled name is not necessarily unique (and that's why I said that going off of the address would be more correct). However, this is not something we can fix by letting clang deduce the name for itself.
>
> Any particular reason for not removing the `SC_Extern` check? I'm pretty sure I could create a test case with a `static` abi-tagged function which we wouldn't be able to call with that check in place...

Boiled down to this comment:

> the local (indicated by the `L` in the name) mangled name is not necessarily unique

Attaching it to non-external functions caused `cpp/namespace/TestNamespaceLookup.py` to fail. With a non-static and a static version of `func()` LLDB chose the wrong one. I can follow up on whether this is a bug elsewhere and how feasible it would be to unconditionally attach the label

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131974/new/

https://reviews.llvm.org/D131974