This is an archive of the discontinued LLVM Phabricator instance.

[AIX] Handle ReadOnlyWithRel kind on AIX.
ClosedPublic

Authored by Esme on Aug 9 2023, 2:18 AM.

Details

Summary

This patch handles the SectionKind of ReadOnlyWithRel.

Diff Detail

Event Timeline

Esme created this revision.Aug 9 2023, 2:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2023, 2:18 AM
Esme requested review of this revision.Aug 9 2023, 2:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2023, 2:18 AM
DiggerLin added inline comments.
llvm/test/CodeGen/PowerPC/aix-relro-section.ll
25

we can delete the Name: <stdin> symbol , it is not relate the patch's functionality

55

we can delete the Name: , it is not relate the patch's functionality

103

ditto

132

ditto

hubert.reinterpretcast added inline comments.
llvm/lib/MC/MCSectionXCOFF.cpp
128

Is this correct for -mxcoff-roptr?

Esme updated this revision to Diff 552639.Aug 23 2023, 2:48 AM

The storage-mapping class should be XCOFF::XMC_RO when -mxcoff-roptr is specified. Add check lines for it.

@Esme, can you add more info about the motivation for doing this (e.g., what fails?) to either the patch description (preferred) or as a comment (replying to this one)?
Information on whether this should be included for LLVM 17 (and why) would be useful.

Esme added a comment.Aug 23 2023, 8:20 PM

@Esme, can you add more info about the motivation for doing this (e.g., what fails?) to either the patch description (preferred) or as a comment (replying to this one)?
Information on whether this should be included for LLVM 17 (and why) would be useful.

The issue was found by Zheng @shchenz during sanitizer enablement on AIX.

$ clang -fsanitize=address -m32 -fsanitize-coverage=control-flow llvm-project/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_control_flow.cpp
fatal error: error in backend: Printing for this SectionKind is unimplemented.
clang: error: clang frontend command failed with exit code 70 (use -v to see invocation)

llvm/lib/MC/MCSectionXCOFF.cpp
44–54

Move the code to here.

84–86

Update this because of the other proposed changes.

127–135

Given the structure of the function, I think the code should be moved to right after the isReadOnly block. Once moved, the XMC_TD case would need to be allowed in this code too.

The issue was found by Zheng @shchenz during sanitizer enablement on AIX.

It sounds like this is not a regression and can be left out of the LLVM 17 release then. Does that sound right?

Esme updated this revision to Diff 553006.Aug 23 2023, 11:53 PM

The issue was found by Zheng @shchenz during sanitizer enablement on AIX.

It sounds like this is not a regression and can be left out of the LLVM 17 release then. Does that sound right?

I think so. This failure currently only occurs with -fsanitize-coverage and support for this option on AIX is not yet complete.

Esme marked 8 inline comments as done.Aug 23 2023, 11:54 PM

It sounds like this is not a regression and can be left out of the LLVM 17 release then. Does that sound right?

I think so. This failure currently only occurs with -fsanitize-coverage and support for this option on AIX is not yet complete.

It is clear from the added test that the failure can occur in other contexts, but we only discovered the issue in new coverage (associated with sanitizer enablement).

This revision is now accepted and ready to land.Aug 24 2023, 7:56 PM
This revision was landed with ongoing or failed builds.Aug 27 2023, 9:25 PM
This revision was automatically updated to reflect the committed changes.