This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Fix init_fini_priority.s test.
ClosedPublic

Authored by grimar on Jul 18 2017, 5:14 AM.

Details

Summary

Previous output of readelf for object file used in test was:

[ 3] .init_array       INIT_ARRAY       0000000000000000  00000048
     0000000000000003  0000000000000000  WA       0     0     8
[ 4] .init_array.100   INIT_ARRAY       0000000000000000  0000004b
     0000000000000004  0000000000000000  WA       0     0     1
[ 5] .init_array.5     INIT_ARRAY       0000000000000000  0000004f
     0000000000000001  0000000000000000  WA       0     0     1

Problem is that .init_array sections were merged together.
I believe intention was different - test should show how separate
sections are correctly sorted. Patch makes such section to be unique.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Jul 18 2017, 5:14 AM
ruiu edited edge metadata.Jul 20 2017, 9:37 AM

I don't understand the problem you are trying to fix. Looks like the test is correct to me. It verifies that .{init,fini}_array sections are sorted.

In D35552#816312, @ruiu wrote:

I don't understand the problem you are trying to fix. Looks like the test is correct to me. It verifies that .{init,fini}_array sections are sorted.

I do not think it was intended that test will have 6 input section in total: 3 .init_array and 3 .fini_array.
I believe intention was to create 10 input sections, or am I missing something ?

ruiu added a comment.Jul 20 2017, 10:48 AM

What I don't really understand is that you changed the test input but didn't change the expected output, so this change doesn't demonstrate what was wrong and what it should have been.

In D35552#816422, @ruiu wrote:

What I don't really understand is that you changed the test input but didn't change the expected output, so this change doesn't demonstrate what was wrong and what it should have been.

I can add a check of llvm-mc output to validate that we have 5+5 input sections now and show initial order.
I did not do that because usually we do not check llvm-mc output, but only linker result which is the same with or without patch.

grimar updated this revision to Diff 107642.Jul 21 2017, 2:01 AM
  • Added check of sections that object contains.
ruiu accepted this revision.Jul 24 2017, 10:59 AM

Ah, now I understood what you were trying to do. LGTM

This revision is now accepted and ready to land.Jul 24 2017, 10:59 AM
This revision was automatically updated to reflect the committed changes.