Page MenuHomePhabricator

[DebugInfo] Use the 'getTypeAlignIfRequired' function to get DW_AT_alignment correct when attribute((__aligned__)) is present.
ClosedPublic

Authored by MaggieYi on Apr 19 2022, 8:28 AM.

Details

Summary

The detailed description of the issue could be found in
https://bugs.llvm.org/show_bug.cgi?id=51277.

In the original code, the 'getDeclAlignIfRequired' function is used.
The 'getDeclAlignIfRequired' function will return the max alignment
of all aligned attributes if the type has aligned attributes. The
function doesn’t consider the type at all.

The 'getTypeAlignIfRequired' function uses the type’s alignment value,
which also used by the 'alignof' function. I think we should use the
function of 'getTypeAlignIfRequired'.

Diff Detail

Event Timeline

MaggieYi created this revision.Apr 19 2022, 8:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 19 2022, 8:28 AM
MaggieYi requested review of this revision.Apr 19 2022, 8:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 19 2022, 8:28 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Could you include some static_asserts(+alignof) in the test case to demonstrate these are the alignments that the language is computing too?

MaggieYi updated this revision to Diff 424257.Apr 21 2022, 11:55 AM

Thanks David, I have modified the test following your advice.

dblaikie accepted this revision.Apr 21 2022, 8:48 PM

Looks good, thanks!

clang/test/CodeGenCXX/debug-info-struct-align.cpp
12

You can drop the "struct" here and from other references to these types (in mt1/mt2 and the alignof calls)

This revision is now accepted and ready to land.Apr 21 2022, 8:48 PM
MaggieYi updated this revision to Diff 424420.Apr 22 2022, 2:37 AM
MaggieYi added inline comments.Apr 22 2022, 2:41 AM
clang/test/CodeGenCXX/debug-info-struct-align.cpp
12

Thanks David. All unnecessary structs have been removed.

This revision was landed with ongoing or failed builds.Apr 22 2022, 4:16 AM
This revision was automatically updated to reflect the committed changes.