This is an archive of the discontinued LLVM Phabricator instance.

[BOLT] Add writable segment for allocatable sections
ClosedPublic

Authored by yota9 on Feb 6 2023, 5:18 AM.

Details

Summary

The data reorder pass needs to create new data section in writable segment.
Also the golang support create 2 new data sections, one of them contains
relocations in PIC binaries so the section must have writable rights.
Currently BOLT creates only one new segment that contains new sections
with RX rights, also create RW segment if there any new writable
sections were allocated during BOLT binary processing.

Vladislav Khmelevsky,
Advanced Software Technology Lab, Huawei

Diff Detail

Event Timeline

yota9 created this revision.Feb 6 2023, 5:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 6 2023, 5:18 AM
yota9 requested review of this revision.Feb 6 2023, 5:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 6 2023, 5:18 AM
yota9 edited the summary of this revision. (Show Details)Feb 6 2023, 5:46 AM
yota9 updated this revision to Diff 497664.Feb 15 2023, 6:51 AM

Add test. The test is using reorder data pass and .data section which must appear in the writable segment.
@rafauler @maksfb gentle ping. Also please check D144095 D143748 . Thank you!

yota9 updated this revision to Diff 497684.Feb 15 2023, 8:11 AM

clang-format

yota9 updated this revision to Diff 497685.Feb 15 2023, 8:14 AM

nit test

yota9 edited the summary of this revision. (Show Details)Feb 15 2023, 11:22 AM
yota9 updated this revision to Diff 501149.Feb 28 2023, 8:06 AM

Fix last segment size

yota9 updated this revision to Diff 501226.Feb 28 2023, 10:50 AM

Add static flag to test

rafauler added inline comments.Feb 28 2023, 2:43 PM
bolt/lib/Rewrite/RewriteInstance.cpp
4170

Is .eh_frame_hdr part of this new writable segment? Shouldn't it go to the read-only segment? Why is eh_frame_hdr being mapped to the last segment?

yota9 added inline comments.Feb 28 2023, 2:52 PM
bolt/lib/Rewrite/RewriteInstance.cpp
4170

Because currently eh_frame_hdr is appended to the binary after all bianary sections were already maped. I think the patch with rewrite option would fix this behaviour, but for now we can go as-is since currently it only affects instrumentation (not really due to RWX), reorder data pass and golang binaries. As rewrite option review would probably take some time I propose to submit this patch and continue with other golang staff in parallel. Rewrite patch would remove most of this patch except for a tests..

rafauler accepted this revision.Mar 13 2023, 3:10 PM

nit: spellcheck summary
other than that, LGTM.

This revision is now accepted and ready to land.Mar 13 2023, 3:10 PM
This revision was automatically updated to reflect the committed changes.