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

Wed Mar 22 15:59:08 PDT 2023

augusto2112 added a comment.

> This is quite fragile, for a few reasons.
> It's too easy for the user to get undiagnosed typos. e.g., they want to dispatch to bar but accidentally dispatch to barf or bar() instead.
> How does this work with overloaded functions? Templated functions with specializations?

> I can see some minor utility to this for some kinds of libraries perhaps, but I'm on the fence about adding the attribute. Is there a reason we need the user to write this attribute at all? e.g., could we be smarter about inline function definitions rather than making this the user's problem?

> Yeah, this seems a bit burdensome for both the frontend (encoding more information, the author has to duplicate the name of the implementation function so there's a risk they get out of sync?)

> So looking at the DWARF spec for this - I see what it's going for, but yeah, I wouldn't want to use a textual representation either at the source or bitcode level, ideally... - perhaps this should just be implemented in LLVM when something's being lowered to a trampoline we could emit DWARF that describes that, without needing source annotations? Or are there situations where it depends on how the user wrote the code as to how we want the debugger to dehave?

Right, I probably should've been clearer on this: the attribute is meant to be used mainly in compiler generated code, not really to be used directly by people (similar to how `external_source_symbol` is used). The main motivation is to annotate the C++ header file that the Swift compiler emits to enable interop with C++. I'd be happy to make the attribute name really specific (as it shouldn't be something users need to type very often), and make it clear on the documentation that this is meant to be used by tooling.

> What about function overloading & namespaces? Is the debugger expected to figure out which `bar` function the DWARF/user is referring to?

The symbol name that's passed in the attribute is supposed to be the mangled name, so there shouldn't be any ambiguity to resolve.

> is the debugger expected to jump directly and not call the implementation? Is it expected to continue into the implementation and break at the function? What if the function isn't called, or there's more than one call in the function, etc?)...

I'm also working on an lldb patch to support the DW_AT_trampoline attribute, where we could discuss the details on how the behavior of the debugger should be, but the way I'm imagining it right now would be that stepping into a trampoline should step through directly to the trampoline target instead, and stepping out of the trampoline target should also step out of the trampoline as well . If the function isn't called, maybe it should behave like it does for functions with no debug info, where we immediately step out of the trampoline. If there's more than one call to the trampoline target, I'm not sure what should happen, I guess we'd step into the trampoline target multiple times (that'd be a pretty odd trampoline though). We could also have a setting where you can enable/disable the trampoline stepping behavior.

================
Comment at: clang/include/clang/Basic/Attr.td:774
+def Trampoline : InheritableAttr {
+  let Spellings = [GNU<"trampoline">];
+  let Subjects = SubjectList<[Function, ObjCMethod]>;
----------------
aaron.ballman wrote:
> Why does this not have a `[[]]` spelling? New attributes typically should be using the `Clang` spelling unless there's a compelling reason not to.
> 
> Also, I'm a bit worried that the identifier `trampoline` is pretty generic and can mean different things to different folks, so we might want a more debug info-specific name for this (https://en.wikipedia.org/wiki/Trampoline_(computing)).
> Why does this not have a `[[]]` spelling? New attributes typically should be using the `Clang` spelling unless there's a compelling reason not to.

Ok, I'll update it to the `Clang` spelling.

> Also, I'm a bit worried that the identifier `trampoline` is pretty generic and can mean different things to different folks, so we might want a more debug info-specific name for this (https://en.wikipedia.org/wiki/Trampoline_(computing)).

What about `debugger_trampoline_target`? `debugger_trampoline_mangled_target`?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595