[DwarfDebug] Skip entries to big for 16 bit size field in Dwarf < 5.

[fhahn]

Summary

Nothing prevents entries from being bigger than the 16 bit size field in
Dwarf < 5. For entries that are too big, there is nothing we can do
other than not emitting them, unless I missed something.

This fixes PR41038.

The minimal test case attached to the bug report is 500 KB as bitcode and
2.2 MB as IR. Not sure if it makes sense to attach it?

[aprantl]

Would generating the testcase on-the-fly inside the RUN: line or perhaps via python be an option? Otherwise I guess this is fine, too.

[probinson]

Generating the test on the fly with Python seems plausible. Note you don't need 16K components, just enough components to make the location description exceed 16K bytes. If each component is (say) a 64-bit field with a max-int constant value, that's something like 11 bytes of description per field, so ~1500 fields ought to do it.

• Duh, not 16K bytes. 64K bytes. ~6000 fields.

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

1987 ↗

(On Diff #191195)

We have to emit a zero length here, otherwise the location-list entry will be syntactically incorrect.

[fhahn]

Add test generated using Python and set size to 0.

[fhahn]

In D59518#1434686, @probinson wrote:

Generating the test on the fly with Python seems plausible. Note you don't need 16K components, just enough components to make the location description exceed 16K bytes. If each component is (say) a 64-bit field with a max-int constant value, that's something like 11 bytes of description per field, so ~1500 fields ought to do it.

• Duh, not 16K bytes. 64K bytes. ~6000 fields.

Thanks! I've added a script that generates a crashing test case. 11K seemed to be the lowest threshold.

[probinson]

llvm/test/MC/X86/dwarf-size-field-overflow.test

48 ↗

(On Diff #191294)

If you make this (i * 1000000) you will need about half as many fields, which will make the test run a little faster.
DWARF expressions will encode the constant value in the smallest possible size, regardless of the size of the associated field; so 0 encodes in 1 byte, 512 in 2 bytes, and so on, even for a 64-bit field. That's why larger constant values will mean you need fewer fields.
It's a bit picky, but as it's trivial to make this test run faster (many fewer lines to generate and parse etc) it seems worthwhile.

[fhahn]

Reduce number of dbg value calls to 4000.

[probinson]

LGTM

llvm/test/MC/X86/dwarf-size-field-overflow.test

9 ↗

(On Diff #191294)

I could wish for something more obviously "no expression here" but that's not your doing.

[fhahn]

Thanks for all the feedback!

llvm/test/MC/X86/dwarf-size-field-overflow.test

9 ↗

(On Diff #191294)

Yes something that makes it more obvious would be great!

[probinson]

Filed PR41152 for the unobvious display of a zero-length location expression.

[dblaikie]

llvm/trunk/test/MC/X86/dwarf-size-field-overflow.test

8–9

This looks incorrect - is it?

By setting the value to zero, now it refers to a different location list than the one that's intended, right? (it refers to the first one)

So while it doesn't crash, it produces the wrong description (describing one variable as being at the location of another variable).

Seems like if we're going to support this (out of curiosity - where did this come up in the wild? Would it be reasonable to reject this with a report_fatal_error, perhaps? (I know, not elegant/nice, but figured I'd ask)) perhaps we need to keep track of the location offsets as they are added and not include a DW_AT_location at all if its offset would be too large? Not sure if that's possible (if we have the necessary information about all prior location lists to compute their size) - might require delaying adding DW_AT_locations.

(@aprantl @probinson - am I misreading this? What're your thoughts on this?)

[probinson]

llvm/trunk/test/MC/X86/dwarf-size-field-overflow.test

8–9

I read this entry as being a location list at offset 0x0 in the location-list section, and that list has one entry for address range 0x0 to 0x8, with a zero-length location description (the entity has no location).
It would be nice if the dump explicitly indicated the zero-length location description, but I believe it's syntactically correct DWARF.
PR41038 describes how it came up in a Swift program.

[aprantl]

We could side-step the issue by emitting a single DW_OP_nop to indicate that something went wrong.

[dblaikie]

llvm/trunk/test/MC/X86/dwarf-size-field-overflow.test

8–9

Ah, thanks - I see my misreading. I thought this was emitting the DW_AT_location value, the offset within the debug_loc section, and that that offset was too large.

My mistake.

Should we skip the entry entirely then, rather than emit an entry with no location/empty location? (Ideally, I suppose, then - if we only have one entry and its too large, we should skip the list entirely too)

Out of curiosity, because I can't quite spot it - where is this part of debug_loc defined in DWARFv4? I don't see any mention of a location description being prepended by its length there - but I'm guessing I'm misreading/missing it in there somewhere.

2.6.2 says that a location list entry consists of a beginning address, an ending address, and then a single location description. 2.6.1 which describes location descriptions doesn't appear to say anything about a length prefix?

[probinson]

Out of curiosity, because I can't quite spot it - where is this part of debug_loc defined in DWARFv4? I don't see any mention of a location description being prepended by its length there - but I'm guessing I'm misreading/missing it in there somewhere.

No, it's just spectacularly hard to find. In the v4 spec this tidbit is hidden away in section 7.7.3, second paragraph. "A location list entry consists of two address offsets followed by a 2-byte length, followed by a block of contiguous bytes that contains a DWARF location description."

[dblaikie]

In D59518#1443166, @probinson wrote:

Out of curiosity, because I can't quite spot it - where is this part of debug_loc defined in DWARFv4? I don't see any mention of a location description being prepended by its length there - but I'm guessing I'm misreading/missing it in there somewhere.

No, it's just spectacularly hard to find. In the v4 spec this tidbit is hidden away in section 7.7.3, second paragraph. "A location list entry consists of two address offsets followed by a 2-byte length, followed by a block of contiguous bytes that contains a DWARF location description."

Ah, thanks!

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

 }
 
 void DwarfDebug::emitDebugLocEntryLocation(const DebugLocStream::Entry &Entry,
                                            const DwarfCompileUnit *CU) {
   // Emit the size.
   Asm->OutStreamer->AddComment("Loc expr size");
   if (getDwarfVersion() >= 5)
     Asm->EmitULEB128(DebugLocs.getBytes(Entry).size());
-  else
+  else if (DebugLocs.getBytes(Entry).size() <= std::numeric_limits<uint16_t>::max())
     Asm->emitInt16(DebugLocs.getBytes(Entry).size());
+  else {
+    // The entry is too big to fit into 16 bit, drop it as there is nothing we
+    // can do.
+    Asm->emitInt16(0);
+    return;
+  }
   // Emit the entry.
   APByteStreamer Streamer(*Asm);
   emitDebugLocEntry(Streamer, Entry, CU);
 }
 
 // Emit the common part of the DWARF 5 range/locations list tables header.
 static void emitListsTableHeaderStart(AsmPrinter *Asm, const DwarfFile &Holder,
                                       MCSymbol *TableStart,

llvm/trunk/test/MC/X86/dwarf-size-field-overflow.test

+# This test generates too many debug location entries to fit into 65KB required
+# by DWARF < 5. Check that the location is set to 0 instead of crashing.
+#
+# RUN: %python %s 4000 | llc -filetype=obj -o %t
+# RUN: llvm-dwarfdump %t | FileCheck %s
+#
+# CHECK:       0x0000004d:       DW_TAG_formal_parameter
+# CHECK-NEXT:      DW_AT_location   (0x00000000
+# CHECK-NEXT:           [0x0000000000000000,  0x0000000000000008): )
+# CHECK-NEXT:      DW_AT_name   ("self")
+
+import sys
+
+SKELETON = """define void @test() !dbg !24 {{
+  {}
+  call void @foo(), !dbg !34
+  ret void, !dbg !34
+}}
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+
+declare void @foo()
+
+attributes #0 = {{ nounwind readnone speculatable }}
+
+!llvm.dbg.cu = !{{!0}}
+!llvm.module.flags = !{{!22}}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 4, emissionKind: FullDebug)
+!1 = !DIFile(filename: "asdf.c", directory: "bar")
+!2 = !{{}}
+!6 = !DIModule(scope: null, name: "dag", includePath: "/")
+!8 = distinct !DICompositeType(tag: DW_TAG_structure_type, scope: !6, file: !1, line: 36, size: 2384384, elements: !2, runtimeLang: DW_LANG_C)
+!22 = !{{i32 2, !"Debug Info Version", i32 3}}
+!24 = distinct !DISubprogram(name: "main", linkageName: "main", scope: !6, file: !1, line: 1, type: !25, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!25 = !DISubroutineType(types: !2)
+!30 = !DILocalVariable(name: "self", arg: 1, scope: !24, file: !1, line: 12, type: !8, flags: DIFlagArtificial)
+!34 = !DILocation(line: 12, column: 8, scope: !24)
+"""
+
+CALL = "call void @llvm.dbg.value(metadata i64 {0}, metadata !30, metadata !DIExpression(DW_OP_LLVM_fragment, {0}, 64)), !dbg !34"
+
+if __name__ == '__main__':
+    N = int(sys.argv[1])
+    calls = []
+    for i in range(0, N):
+        calls.append(CALL.format(i * 10**12))
+    print(SKELETON.format('\n'.join(calls)))