[llvm] [DWARFVerifier] Verify that DW_AT_LLVM_stmt_sequence is set correctly (PR #152807)

[David Blaikie via llvm-commits]

================
@@ -0,0 +1,1640 @@
+# Object file copied from llvm/test/tools/dsymutil/ARM/stmt-seq-macho.test
+# Then manually tempered with some of the value of the attribute
+# I hope there are easier ways to construct tests like this.
+
+# RUN: yaml2obj %s -o verify_stmt_seq.o
+# RUN: not llvm-dwarfdump -verify -debug-info verify_stmt_seq.o | FileCheck %s --check-prefix=CHECK_INVALID --implicit-check-not=error:
+# RUN: llvm-dwarfdump -debug-line -verbose -debug-info verify_stmt_seq.o | FileCheck %s --check-prefix=CHECK_DEBUG_LINE
+
+
+# CHECK_INVALID: error: DW_AT_LLVM_stmt_sequence offset 0x00000000 is not within the line table bounds [0x0000002e, 0x000000fd)
+# CHECK_INVALID: DW_AT_LLVM_stmt_sequence [DW_FORM_sec_offset]     (0x00000000)
+
+# 0xd3 would be a valid offset, if the line table wan't ill formed with two rows having the same PC (0x8c).
+# CHECK_INVALID: error: DW_AT_LLVM_stmt_sequence offset 0x000000d3 does not point to a valid sequence offset in the line table
+# CHECK_INVALID: DW_AT_LLVM_stmt_sequence [DW_FORM_sec_offset]     (0x000000d3)
+
+# CHECK_DEBUG_LINE:      0x000000d3: 05 DW_LNS_set_column (85)
+# CHECK_DEBUG_LINE-NEXT: 0x000000d5: 0a DW_LNS_set_prologue_end
+# CHECK_DEBUG_LINE-NEXT: 0x000000d6: 00 DW_LNE_set_address (0x000000000000008c)
+# CHECK_DEBUG_LINE-NEXT: 0x000000e1: 03 DW_LNS_advance_line (30)
+# CHECK_DEBUG_LINE-NEXT: 0x000000e3: 01 DW_LNS_copy
+# CHECK_DEBUG_LINE-NEXT:             0x000000000000008c     30     85      1   0             0       0  is_stmt prologue_end
+# CHECK_DEBUG_LINE-NEXT: 0x000000e4: 00 DW_LNE_end_sequence
+# CHECK_DEBUG_LINE-NEXT:             0x000000000000008c     30     85      1   0             0       0  is_stmt end_sequence
----------------
dwblaikie wrote:

> > not emitting the stmt_sequence in the first place might be nice but difficult to implement
> 
> #154986 already does this, it counts the number of instructions emitted by `AsmPrinter`, and skips `DW_AT_LLVM_stmt_sequence` if it is less than 2 (branch + nop for icf thunks). Approve it if you could.

Hmm - think we might be miscommunicating... 

/zero/ length sequences could be omitted (though the function still has an address that might be meaningful/interesting - so I wouldn't want to omit DW_AT_low_pc/high_pc from such a function) but you're suggesting skipping stmt_sequence on 2 byte lengths? 

Are these branch+nop instructions relaxable by the linker, so what's being emitted as a 2 byte sequence is being relaxed to zero bytes? And that's the complex problem we're trying to grapple with?

> > for older content generated
> 
> Nice catch on backward compatibility. I guess we should skip the verification in this PR as well. There could be some workflow where it verifies dSYM before symbolicating, and declaring it illegal out of no where could break them.
> 
> > it'd still be nice to fix the zero-length problem given the issues it creates for DWARF anyway.
> 
> That's a bit tricky, I spent last week investigating the issue we were discussing in #154986 and [here](https://github.com/llvm/llvm-project/pull/110192#issuecomment-2427949464), but I haven't found a nice fix yet. While I'll keep working on it, let me know if you have more insights please.

Last time I looked I'd tried to do something more surgical to only affect zero-length functions - but that's probably overly pedantic and something that does the thing that PS4 and MacOS targets already do is probably fine - namely, they, I think, set TrapUnreachable to true, and NoTrapAfterNoReturn to true:

https://github.com/llvm/llvm-project/blob/2429a8f71ff2080116b8a0e46541d1fb80351219/llvm/include/llvm/Target/TargetOptions.h#L268-L273

Maybe those two settings could/should even go away and be replaced by hardcoded/non-optional behavior... not sure.

https://github.com/llvm/llvm-project/pull/152807