Michael137 added a comment. In D131335#3712243 <https://reviews.llvm.org/D131335#3712243>, @labath wrote:
> In D131335#3710236 <https://reviews.llvm.org/D131335#3710236>, @Michael137 > wrote: > >> In D131335#3709788 <https://reviews.llvm.org/D131335#3709788>, @labath wrote: >> >>> I haven't been able to keep up with all the lldb happenings lately, so it's >>> possible I have missed something, but can you give a summary (or point me >>> to one) of what is the problem with abi tags in lldb. For example, why >>> isn't the DW_AT_linkage_name string sufficient to find the correct function? >>> >>> I am somewhat sceptical that this is the best way to fix this problem. As >>> you've said yourself, the "alternate name" machinery is a last resort, and >>> the things that it's doing are wrong on several levels (and quite slow). To >>> tell the truth, I've been hoping that we could get rid of it completely one >>> day. Relying on this mechanism for "core" functionality would preclude that >>> from happening... >> >> Unqualified lookup of template functions currently always resorts this >> "fall-back" mechanism. I investigated this mainly with `import-std-module` >> enabled since those were the tests that failed due to D127444 >> <https://reviews.llvm.org/D127444>. There are two parts to it: >> >> 1. During unqualified lookup `clang::Sema` asks LLDB for the decl >> corresponding to the template in question. `DWARFASTParserClang` adds two >> decls to the AST, one non-template `FunctionDecl` marked `extern` and >> another `TemplateFunctionDecl`. Because of unqualified lookup rules in C++ >> `clang::Sema` picks the extern FunctionDecl. This is why the generated IR >> has an unresolved symbol that we then try to find in the object files. >> >> 2. When resolving this external symbol we try to match mangled function >> names using the hand-rolled `CPlusPlusNameParser` which doesn't support ABI >> tags and we fail to find a suitable symbol. >> >> We can fish out the ABI tag from the `DW_AT_linkage_name` when parsing DWARF >> but we would still fail at (2). The plan was to address both. First reduce >> the reliance on `CPlusPlusNameParser` since that fixes the unqualified >> lookup issue trivially and is more robust to C++ syntax changes/attribute >> additions/etc. And then address the unqualified template lookup which has >> had numerous workarounds added to it over the years >> (https://reviews.llvm.org/D33025, >> https://reviews.llvm.org/D61044, https://reviews.llvm.org/D75761, etc.) > > Thanks for the explanation. I have to admit I am surprised that this is > motivated by the `import-std-module` feature. That's the last place I would > expect, as my understanding was that this is basically about providing a > fully faithful AST representation (of the module) -- by having clang compile > itself rather than us trying to roundtrip it through dwarf. > > From your description, it sounds to me like things go wrong right at the > start -- with the two AST Decls. Do you understand why is that happening? I > pretty sure I am missing something, but I can't escape the feeling that this > could be solved by generating a "better" AST. I know that isn't always > possible, but it's not clear to me why it is not be possible **in this case**. > > The way I understand it, for regular (non-templated) functions we are > generating an AST roughly corresponding to `extern void foo(int) > asm("mangled_name");`. That means we tell clang precisely the mangled name it > should use (the one we get from dwarf), and nobody will care whether it > contains an abi tag or anything else. Could we do the same for templated > functions as well? `extern template void foo<int>() asm("whatever");` doesn't > appear to be entirely valid syntax (gcc rejects it outright, while clang just > ignores <https://godbolt.org/z/zKcE5vfEP> the `asm` part, but maybe it > wouldn't be too hard to change that (we wouldn't actually need to accept that > source code, just a way to express something similar in the AST)? > > Would something like that help here? Or am I just completely in the dark? > > (BTW, I really like the ability to do AST-based processing (comparisons) of > mangled names. That seems like it could be very useful in some situations -- > it's just not clear to me why this is one of them.) Thanks for taking a closer look. I'm revisiting the AST construction for template functions again and seeing some discrepancies in the structure of the AST generated by Clang's `-ast-dump` vs LLDB. We're not only dropping the ABI tags from the mangled name but also the template arguments. That's because the decl that `clang::Sema` finds during lookup looks like the following: // main.cpp namespace A { struct B {}; template <typename T> [[gnu::abi_tag("vTest")]] bool tagged(T t) { return true; } int main() { bool res = tagged(A::B{}); } ... FunctionDecl 0x11910ffd0 <<invalid sloc>> <invalid sloc> used tagged 'bool (A::B)' extern |-ParmVarDecl 0x11910ff68 <<invalid sloc>> <invalid sloc> t 'A::B' It seems like the `FunctionTemplateDecl` we create in `DWARFASTParserClang` doesn't actually get attached to the AST properly. E.g., we never call `addDecl` on it. I'm not sure if that's by design, but that is one of the discrepancies; it has pretty much stayed this way since support for template functions was first introduced in 3c2e3ae49093a4f5e0660c96932d83f6d81bd798. This lack of a `FunctionTemplateDecl` is reflected again when emitting the `$__lldb_result_expr` IR via `CodeGen::EmitDirectCall` (which is the component that emits the "incorrectly" mangled function call). - `-ast-dump` yields the following ... | |-ImplicitCastExpr 0x13311c548 <col:28> 'bool (*)(A::B)' <FunctionToPointerDecay> | | `-DeclRefExpr 0x13311c4c0 <col:28> 'bool (A::B)' lvalue Function 0x13311a130 'tagged' 'bool (A::B)' (FunctionTemplate 0x1331166c8 'tagged') ... - while LLDB's AST looks like: ... |-ImplicitCastExpr 0x119110208 '_Bool (*)(struct A::B)' <FunctionToPointerDecay> | `-DeclRefExpr 0x1191101c0 '_Bool (struct A::B)' lvalue Function 0x11910ffd0 'tagged' '_Bool (struct A::B)' ... Note how the `DeclRefExpr` node doesn't have a `FunctionTemplate` attached to it in LLDB's AST. I'll have to see if there's something wrong with how `TypeSystemClang::CreateFunctionTemplateDecl` is used Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131335/new/ https://reviews.llvm.org/D131335 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
