This is an archive of the discontinued LLVM Phabricator instance.

DebugInfo: Permit DW_TAG_structure_type, DW_TAG_member, DW_TAG_typedef tags with empty file names.
ClosedPublic

Authored by pcc on Mar 24 2015, 1:42 PM.

Details

Summary

Some languages, such as Go, have pre-defined structure types (e.g. "string"
is essentially a pointer/length pair) or pre-defined "typedef" types
(e.g. "error" is essentially a typedef for a specific interface type).
Such types do not have associated source location, so a Go frontend would
be correct not to associate a file name with such types.

This change relaxes the DIType verifier to permit unlocated types with
these tags.

Diff Detail

Repository
rL LLVM

Event Timeline

pcc updated this revision to Diff 22597.Mar 24 2015, 1:42 PM
pcc retitled this revision from to DebugInfo: Permit DW_TAG_structure_type, DW_TAG_member, DW_TAG_typedef tags with empty file names..
pcc updated this object.
pcc edited the test plan for this revision. (Show Details)
pcc added a reviewer: dexonsmith.
pcc added a subscriber: Unknown Object (MLST).

Some IR test cases might be nice to have here, to avoid regressing this
functionality.

pcc updated this revision to Diff 22599.Mar 24 2015, 2:09 PM
  • Add test case
dblaikie added inline comments.Mar 24 2015, 2:18 PM
test/DebugInfo/verifier.ll
4 ↗(On Diff #22599)

"this test doesn't crash" is generally not ideal, prefer to test the desired output.

Use llc rather than opt, and ensure that the tags are emitted without file info as desired?

pcc updated this revision to Diff 22620.Mar 24 2015, 5:37 PM
  • Replace with an llc test
probinson added inline comments.
test/DebugInfo/missing-file-line.ll
1 ↗(On Diff #22620)

Generating an object file and going through llvm-dwarfdump means you need
REQUIRES: object-emission
Also, as the test is now architecture-dependent, it needs to go in the DebugInfo/X86 directory.

16 ↗(On Diff #22620)

These checks verify that you accept metadata with no source location; however I think it doesn't verify that what comes back out has no source location?

pcc updated this revision to Diff 22625.Mar 24 2015, 6:25 PM
  • Move test to DebugInfo/X86. Test that the entities have no file/line attrs. Add REQUIRES: object-emission

LGTM with the DW_TAG|NULL chang, and thanks for tolerating the pickiness about debug-info tests!

test/DebugInfo/X86/missing-file-line.ll
22 ↗(On Diff #22625)

I see why this works, but we usually check DW_TAG|NULL to constrain the range to a single DIE. Ditto for the other two places.

dblaikie added inline comments.Mar 25 2015, 10:35 AM
test/DebugInfo/X86/missing-file-line.ll
3 ↗(On Diff #22625)

Ideally just pipe dwarfdump's output straight to FileCheck without the intermediate %t - then just use a single set of CHECK (rather than 3 separate CHECK prefixes) lines for the tests.

This revision was automatically updated to reflect the committed changes.
pcc added inline comments.Mar 25 2015, 10:47 AM
test/DebugInfo/X86/missing-file-line.ll
3 ↗(On Diff #22625)

Previously I used CHECK-DAG, but I couldn't see a way to do the CHECK-NOT checks within a single run of FileCheck while keeping them order independent.

22 ↗(On Diff #22625)

Done

probinson added inline comments.Mar 25 2015, 10:49 AM
test/DebugInfo/X86/missing-file-line.ll
3 ↗(On Diff #22625)

That would hard-code an order for the three DIEs, which is probably okay for the structure/member but I'm not so sure about the typedef.