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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/trunk/test/MC/X86/dwarf-size-field-overflow.test:8-9
+# CHECK:       0x0000004d:       DW_TAG_formal_parameter
+# CHECK-NEXT:      DW_AT_location   (0x00000000
+# CHECK-NEXT:           [0x0000000000000000,  0x0000000000000008): )
+# CHECK-NEXT:      DW_AT_name   ("self")
----------------
probinson wrote:
> dblaikie wrote:
> > 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?)
> 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.
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?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59518