Page MenuHomePhabricator

[llvm-dwp] Abort when dwo_id is unset
ClosedPublic

Authored by rupprecht on Feb 11 2019, 4:14 PM.

Details

Summary

An empty dwo_id indicates a degenerate .dwo file that should not have been generated in the first place. Instead of discovering this error later when merging with another degenerate .dwo file, print an error immediately when noticing an unset dwo_id, including the filename of the offending file.

Test case created by compiling a trivial file w/ -fno-split-dwarf-inlining -gmlt -gsplit-dwarf -c prior to r353771

Diff Detail

Repository
rL LLVM

Event Timeline

rupprecht created this revision.Feb 11 2019, 4:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 11 2019, 4:14 PM
dblaikie added inline comments.Feb 11 2019, 4:22 PM
llvm/test/tools/llvm-dwp/X86/missing_dwo_id.test
3 ↗(On Diff #186361)

This error starts with a capital ("C") and the other ("top level DIE is not ..." ) does not. Might be good to pick one way for the convention - I think probably non-capitals is the norm in these sort of diagnostics?

llvm/tools/llvm-dwp/llvm-dwp.cpp
190–191 ↗(On Diff #186361)

In theory a hash could be zero, I think (it's not a blessed/allocated non-value for a hash function, so far as I would know/think) - so I think this would need to be wrapped in Optional (perhaps an an implementation detail of this function, the result structure doesn't have to expose this detail, necessarily - though perhaps callers need to know if this is valid to use or not, even when getCUIdentifiers succeeds)

rupprecht updated this revision to Diff 186390.Feb 11 2019, 7:56 PM
rupprecht marked 2 inline comments as done.
  • Use optional
  • Fix error message casing
dblaikie accepted this revision.Feb 11 2019, 9:13 PM

Looks good!

This revision is now accepted and ready to land.Feb 11 2019, 9:13 PM
This revision was automatically updated to reflect the committed changes.