[Bolt] Solving pie support issue

[JohnLee1243]

Now PIE is default supported after clang 14. It cause parsing error when using perf2bolt. The reason is the base address can not get correctly. Fix the method of geting base address. If SegInfo.Alignment is not equal to pagesize, alignDown(SegInfo.FileOffset, SegInfo.Alignment) can not equal to FileOffset. So the SegInfo.FileOffset and FileOffset should be aligned by SegInfo.Alignment first and then judge whether they are equal.
The .text segment's offset from base address in VAS is aligned by pagesize. So MMapAddress's offset from base address is alignDown(SegInfo.Address, pagesize) instead of
alignDown(SegInfo.Address, SegInfo.Alignment). So the base address calculate way should be changed.

[JohnLee1243]

@maksfb I meet a bug when doing perf2bolt and It happens when PIE was supported. I did some modification of the function getBaseAddressForMapping, and this functions was wrotten by you. Can you get me some suggestions if my modification is proper. Thanks.

[rafaelauler]

Hi @JohnLee1243, thanks for your contribution and thanks for the test infra provided to make sure perf2bolt runs fine.

I couldn't repro the problem where the segment info alignment is different than page size alignment with the test case provided. Every time I build it, the linker aligns the segment to have alignment of 0x1000, which is a page size alignment, making the proposed change unnecessary. But I do see that in a scenario where segment is not aligned to page size, your patch makes perfect sense. Do you know how can we make the linker align differently than page size, so we can test the change properly?

I tried in the linker script aligning the sections to a different value, and indeed the sections seem all aligned to something smaller than page size. But when we look at the segment (program headers table), they're all aligned at page size.

Now, regarding the perf2bolt tests: I think we eventually need to test perf2bolt, but I admit I'm slightly concerned about how stable these tests will be. Since the test success will depend on perf successfully collecting samples on a binary that runs for a limited amount of time, not only the test needs to run for a bit of extra time to allow perf to sample it several times, it can also become a brittle test if perf fails to sample it for any reason. I like the idea of making the test conditional on finding perf in the machine, I think it's a good first step. We can just commit a test like this and "see if it breaks often", but the idea of having brittle tests makes me think they might not be worth the pain. I'll discuss this with the team.

[maksfb]

@JohnLee1243, sorry I missed your ping. Could you please add a test case to bolt/unittests/Core/BinaryContext.cpp? This way we can test getBaseAddressForMapping() interface without relying on the test system and its dynamic loader.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[JohnLee1243]

Hi @JohnLee1243, thanks for your contribution and thanks for the test infra provided to make sure perf2bolt runs fine.

I couldn't repro the problem where the segment info alignment is different than page size alignment with the test case provided. Every time I build it, the linker aligns the segment to have alignment of 0x1000, which is a page size alignment, making the proposed change unnecessary. But I do see that in a scenario where segment is not aligned to page size, your patch makes perfect sense. Do you know how can we make the linker align differently than page size, so we can test the change properly?

I tried in the linker script aligning the sections to a different value, and indeed the sections seem all aligned to something smaller than page size. But when we look at the segment (program headers table), they're all aligned at page size.

Now, regarding the perf2bolt tests: I think we eventually need to test perf2bolt, but I admit I'm slightly concerned about how stable these tests will be. Since the test success will depend on perf successfully collecting samples on a binary that runs for a limited amount of time, not only the test needs to run for a bit of extra time to allow perf to sample it several times, it can also become a brittle test if perf fails to sample it for any reason. I like the idea of making the test conditional on finding perf in the machine, I think it's a good first step. We can just commit a test like this and "see if it breaks often", but the idea of having brittle tests makes me think they might not be worth the pain. I'll discuss this with the team.

I use the link options -Wl,-z,max-page-size=0x1000 to restrict the alignment in segment info ( assuming real page size is 0x10000 ). However, it can reduce the alignment, but I am not sure it can work on increasing the alignment and make it larger than real page size.

[rafaelauler]

Thanks!

Could you move the LIT test "bolt/test/perf_test.test" under a new folder called "bolt/test/perf2bolt" with tests that only run when perf is available?

[rafaelauler]

Thanks