[Lldb-commits] [PATCH] D143652: [lldb][DWARFASTParserClang] Attach linkage name to ctors/dtors if missing

Mon Feb 13 07:29:42 PST 2023

Michael137 added inline comments.

================
Comment at: lldb/test/API/lang/cpp/external_ctor_dtor_lookup/TestExternalCtorDtorLookup.py:28
+# CHECK:           |-CXXConstructorDecl {{.*}} Wrapper 'void ()'
+# CHECK-NEXT:      | `-AsmLabelAttr {{.*}} Implicit "_ZN7WrapperI3FooEC1B4testEv"
+# CHECK-NEXT:      `-CXXDestructorDecl {{.*}} ~Wrapper 'void ()'
----------------
Michael137 wrote:
> labath wrote:
> > I think this could be a bit of a problem, because (as you've probably found out by now) there are multiple versions of a single constructor, and the asm label seems to cause clang to coalesce them. In the simple test case below that doesn't matter, as the two constructors are identical, but things might be different if the class had virtual bases. (i.e., it could cause us to call the wrong constructor and blow up).
> I may be misunderstanding, but wouldn't they just get added as extra `CXXConstructorDecl`s on the AST with distinct `AsmLabel`s? Each constructor subprogram DIE links to some specification, which is the definition of the constructor we should call. That's where we get the linkage name from. Playing around with virtual bases I didn't yet manage to come up with a counterexample of where we would pick the wrong constructor
> 
> Although I did now notice that there's an extra destructor call in some cases where I didn't expect one before. Maybe that's a manifestation of the issue you describe. Investigating...
You probably mean multiple constructor definitions can point to the same specification DIE? In which case we would choose the first one happen to see. To what extent can these definitions differ?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143652