[PATCH] D146595: [clang] Add clang trampoline_mangled_target attribute

Mon Mar 27 12:34:45 PDT 2023

aaron.ballman added a comment.

In D146595#4225259 <https://reviews.llvm.org/D146595#4225259>, @augusto2112 wrote:

> In D146595#4225170 <https://reviews.llvm.org/D146595#4225170>, @aaron.ballman wrote:
>
>> In D146595#4225132 <https://reviews.llvm.org/D146595#4225132>, @aprantl wrote:
>>
>>> In D146595#4225048 <https://reviews.llvm.org/D146595#4225048>, @dblaikie wrote:
>>>
>>>> I know this is a bit of a redirection/scope creep/etc - but I'd quite like to see a solution that is likely to be usable for the "std::function" problem (stepping into std::function should allow you to reach the underlying function - but lldb currently skips any call to a std-namespaced function, I think, so you step right over the whole op() call) that could also cover the Swift needs. Though perhaps they're just sufficiently different problems that there is no generalizing here.
>>>
>>> This patch aims at exposing an existing LLVM IR & DWARF feature in clang. Having a generic solution for the std::function problem is definitely worthwhile investigating, but I'm not sure it needs to prevent supporting the existing mechanism in clang.
>>
>> Why should this feature be limited to DWARF? Don't we have the same kind of stepping behavior issue with PDB files, for example?
>
> There's already support to threading trampoline names from IR to DWARF. If PDB supports the same attribute in some form there's nothing stopping someone to adding support to thread the attribute from IR to PDB as well.

We don't want to have one attribute per debug format, because that's not going to scale. So how do we validate that this attribute should be exposed to users *now* before we know what the story is for other debug formats? The argument "this is for compiler-generated code" doesn't really address the concern I have here -- once the attribute exists, people will start using it if it does useful things. The problem this attribute solves is pretty general to a lot of different kinds of library facilities (at least in C++, a bit less so in C IMO), so it seems that hypothetical situation is plausible. We can document that this only works with DWARF, but that's still not actually solving the problem users have.

I guess what I'm trying to say is: this feels like a specific solution to a general problem and that makes me worried we're painting ourselves into a corner where we're going to need to deprecate this attribute and add the general one in the future. How likely do you think that is (might be more of a question for @dblaikie from the debug info side of things) and do you agree that it seems like users would want this functionality for their own libraries?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595