This is an archive of the discontinued LLVM Phabricator instance.

[DWARF][DebugInfo] Fix off-by-one error in size of DW_TAG_base_type types
ClosedPublic

Authored by Orlando on Jan 12 2022, 8:54 AM.

Details

Summary

Fix PR53163 by rounding the byte size of DW_TAG_base_type types up. Without
this fix we risk emitting types with a truncated size (including rounding
less-than-byte-sized types' sizes down to zero).

Diff Detail

Event Timeline

Orlando created this revision.Jan 12 2022, 8:54 AM
Orlando requested review of this revision.Jan 12 2022, 8:54 AM
probinson added inline comments.Jan 12 2022, 10:06 AM
llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
1585

You'd think there would be a convenience macro for this, but I'm not finding one. I do have a memory that someone started making some changes to remove the hard-coded assumption about 8-bit bytes, but it's vague and unless someone else can come up with an example of doing this differently, it's not a blocker.

llvm/test/DebugInfo/X86/base-type-size.ll
14

Needs CHECK-NEXT to ensure the name/encoding/byte_size all belong to the same DIE.
(If this isn't the first base_type, that may be slightly tricky...)

Orlando updated this revision to Diff 399592.Jan 13 2022, 2:02 AM
Orlando marked an inline comment as done.

Thanks for taking a look @probinson.

+ Use CHECK-NEXT directives in test
+ Use divideCeil instead of inline maths

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
1585

I had vague memories about hardcoded 8-bit bytes too. Since you mentioned it, I had a look around in DataLayout but I couldn't find anything that would give me the size of a byte and many of the Datalayout methods appear to assume it's 8 bits. In my rummaging around I did find divideCeil at least, which makes this slightly tidier.

Orlando updated this revision to Diff 399598.Jan 13 2022, 2:10 AM

+ Fix comment in test: -g -> -gdwarf-5

This revision is now accepted and ready to land.Jan 13 2022, 8:44 AM