[PATCH] D82828: [ELF] Don't resolve a relocation in .debug_line referencing an ICF folded symbol to the tombstone value

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I think this is the right fix, but slightly for the wrong reasons. You're describing the problem as the confusing addresses for debuggers, whereas I'm saying it's the inability to set a breakpoint on a folded function's line that is the problem. The two are related but not quite the same. In the "confusing addresses" case, one way to fix it would be to update debuggers to recognise the tombstones and special-case them. That would be fine, except that it doesn't allow a user to set a breakpoint on lives that were folded in (i.e. my point). Hence, it's important to characterise this as "I want to be able to set breakpoints on folded-in functions".

I've suggested comment changes inline to reflect this.

By the way, what about discarded COMDATs? They're similar in concept, but implemented differently, at least at the front-end, so might deserve their own test case?



================
Comment at: lld/ELF/InputSection.cpp:930-932
+      // catches the ICF folded case. However, resolving a relocation in
+      // .debug_line to -1 can leave addresses like -1 and wraparound small
+      // addresses which can confuse debuggers. So exclude .debug_line.
----------------
I recommend:

"However, resolving a relocation in .debug_line to -1 would stop debugger users from setting breakpoints on the folded-in function, so exclude .debug_line."


================
Comment at: lld/test/ELF/debug-dead-reloc-icf.s:29-32
+# .debug_line contribution associated to a folded function can describe
+# different lines from the canonical function. Leaving a tombstone value can
+# cause addresses like UINT64_MAX and some wraparound small addresses which will
+# confuse debuggers. Resolve the relocation to the folded .text.1 to .text
----------------
`#` -> `##`

Similar to the code comment, I recommend:

".debug_line contributions associated with folded functions will describe different lines to the canonical funcion. Leaving a tombstone value would prevent users setting breakpoints in the folded-in function. Instead resolve the relocation to the folded .text.1 to .text."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82828