[Lldb-commits] [PATCH] D131335: [lldb] abi_tag support 3/3 - Use mangle tree API to determine approximate mangled matches

Tue Aug 16 07:53:48 PDT 2022

Michael137 added a comment.

Thanks for taking the time to look at this

> I would like to understand what (if any) are the issues with extending the approach #2 to cover non-member functions as well. For what it's worth, I don't think #2 is a hack. I might actually say that's "using DWARF as it was meant to be used" -- the mangled name is there for us to use (*). Before LLDB came around, I doubt anybody on the DWARF committee imagined people would try round-tripping the DWARF information back into a "regular" C++ compiler (and expecting it to recreate "perfect" mangled names for templates and all).

There are no issues that I know of with this. My hesitation with this approach was just that if we eventually fix the way we construct template decls, we will need to make sure we continue attaching the labels correctly. Maybe this isn't a big deal if we note it in a comment

> I don't mean to offend, but if anything, I would say that #1 is a hack. What it essentially does is pretend that abi tags don't exist. The way I understand it, the main reason for the introduction of abi tags was to enable one to have two versions of the same function (class, etc.) co-exist in the same binary/program. This essentially defeats that. I haven't looked at the implementation too closely, but I don't see how one could properly disambiguate two differently-tagged versions of the same functions using this approach. It may be a hack that we have to live with, but I am not convinced of that (yet).

The way this patch allows distinguishing differently-tagged versions is by checking the `Attrs` Node of the Itanium mangle tree. But you're right in that this change does rely on a follow-up patch that attaches `AbiTagAttr` nodes to the `FunctionDecl`s we generate, which we were planning on doing. But I guess if we are going to attach labels to FunctionDecls anyway, we could just attach the `AsmLabel` instead and thus cover all types of attributes trivially.

> Regarding #3, yes, there are definite problems regarding the way we represent templates. The root cause is the fundamental mismatch between what clang wants (a completely accurate description of all objects "as if written in source") versus what DWARF provides (a description of the most important properties of entities that are present in the binary). Where the problem lies depends on your point of view. However, even if we come up with a better way to represent the debug information in AST, it's not clear to me why that would be incompatible with #2. Since we don't have the actual source code for the template function, we will never be able to provide a fully generic definition of it -- we will always need to deal with specific instantiations of that template. So what we just need is the attach a (asm) label to a specific template instantiation. I would hope that wouldn't be too much to ask of clang to support.

This mismatch currently causes problems with unqualified lookups, especially with import-std-module enabled, so it would be nice to fix eventually. We don't need a fully generic definition, just the generic prototype to get the mangled name correctly. But as you say, this can be done with an asm() label with less trouble.

I can open a separate diff with approach #2 and see from there.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131335