This is an archive of the discontinued LLVM Phabricator instance.

Correctly recognize bitfields when emitting dwarf
ClosedPublic

Authored by mahkoh on Feb 9 2021, 5:56 AM.

Details

Summary

The code incorrectly assumed that full-sized bitfields are located at a
byte boundary.

This fixes https://bugs.llvm.org/show_bug.cgi?id=44601

Diff Detail

Event Timeline

mahkoh created this revision.Feb 9 2021, 5:56 AM
mahkoh requested review of this revision.Feb 9 2021, 5:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 9 2021, 5:56 AM

@aprantl / @JDevlieghere you folks might want to check this out from a backwards compatibility/validation perspective.

aprantl accepted this revision.Feb 12 2021, 9:08 AM

Let's watch the LLDB / GDB bots carefully after landing this, but it seems to make sense to me.

llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
1624

This line definitely looks good.

This revision is now accepted and ready to land.Feb 12 2021, 9:08 AM

This makes sense, I was surprised we did not catch this on the lldb side but it looks like we do ok with this case because it appears like a regular field.

shafik accepted this revision.Feb 12 2021, 9:42 AM

I do not have commit access. Please commit this for me.

jmorse added a subscriber: jmorse.Apr 1 2022, 10:27 AM

This has come up on my radar, still applies cleanly, and passes all the debug-info tests. I'm out of time in my timezone, but without objection will commit next week (unless someone else gets there first).

Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2022, 10:27 AM
jmorse added a comment.Apr 4 2022, 5:45 AM

Green dragon mailed me a test failure:

https://green.lab.llvm.org/green/job/lldb-cmake/42719/testReport/junit/lldb-api/lang_objc_bitfield_ivars/TestBitfieldIvars_py/

Which I can't replicate locally due to lack of objc sdk stuff. Fiddling with the input though, it looks like "field1" and "field2" of "@interface HasBitfield" don't get the DIFlagBitField flag, and thus don't get recognised as being bitfields as a result of this patch (and were before?).

That suggests there's more work to do / something exciting happening with flags in the frontend, however that's way out of my comfort zone. I'll revert later today if no-one has any other suggestions. A potential hacky workaround would be

bool IsBitfield = DT->isBitField() || (FieldSize && Size != FieldSize);

@jmorse feel free to ask me or @shafik for help if you need help reproducing. Happy to serve as a human compiler explorer!

I opened https://reviews.llvm.org/D140195 which adds the DIFlagBitField to objective-c members.

This should fix the problem in LLDB and we should be able to bring back this patch.