[PATCH] D153869: [DWARFv5][DWARFLinker] avoid stripping template names for .debug_names.

[Felipe de Azevedo Piovezan via Phabricator via llvm-commits]

fdeazeve added a comment.

Sorry to come late to this, but this patch is regressing dsym debugging with DWARF 5 (which I am actively working on enabling) when compared to DWARF 4 dsym debugging.
LLDB relies heavily on querying these stripped names, and this patch creates a number of DWARF 5 failures in dsym debugging flows.

Was this patch submitted mostly to address the verifier failure? Or is there some other use case in mind?
I think we can make a strong enough case to simply disable this verifier check for now (see below)

I was going to post a review to make dsymutil keep its prior behavior (but leaving other users of DWARFLinker unchanged), however `llvm-dwarfutil` doesn't know how to consume YAML debug maps, so I can't  update this test without other changes.

With apologies for the small wall of text, but this is the commit message I had typed prior to stumbling on this test:

  [dsymutil] Add template-arguments-stripped names in debug_names
  
  When LLDB needs to evaluate an expression like `foo(42)` and `foo` is defined as
  `template<typename A>foo(A)`, LLDB queries the accelerator table for the name
  `foo`. Note that the string `foo` itself is neither:
  
  1. `AT_name`, which in this case would be `foo<int>`
  2. nor `AT_linkage`, which in this case would be the mangled name of `foo<int>`.
  
  As such, it is guaranteed that LLDB's query will fail, since those are the only
  two attributes blessed by the DWARF standard to be used as keys in debug_names.
  
  Even if LLDB were to attempt conjuring the string `foo<int>`, this is not
  enough. The *contents* of the `AT_name` attribute are not standardized. For
  example, how many white spaces separate nested templates like `<<<`?
  
  With DWARF <= 4 and apple_names, `dsymutil` takes it upon itself to fix this
  issue by adding the key `foo` into the table, linking the string `foo` to every
  DIE corresponding to a specialization of this function. Note that this _enables_
  more functionality in LLDB, but it is not a great solution, as we now have a
  debug linker doing more than just linking & optimizing: it is enabling
  functionality that would otherwise not be possible if the debug information were
  left in object files.
  
  This patch brings this dsymutil behavior to debug_names, so that we don't create
  regressions by moving from DWARF 4 to DWARF 5 in dsym-based debugging. This is
  slightly non-standard; as mentioned above, only AT_name and AT_linkage_name are
  allowed to be keys. However, we believe this to be acceptable:
  
  1. This change only affects dsym-based debugging, which already expects this
  behavior.
  2. The standard says that "A producer may choose to implement additional rules
  for what names are placed in the index, and may communicate those rules to a
  cooperating consumer via an augmentation string".
  3. There have been favourable discussions about making AT_name be the basename
  of the function without any template parameters [1]. In fact, some patches were
  put upstream to just that for templated *types*, though the motivation there
  seems to be "make debug info smaller". See [2][3][4].
  4. These same discussions [1] also suggest this temporary fix as a stop-gap
  measure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153869