This is an archive of the discontinued LLVM Phabricator instance.

[yaml2obj] - Change the order of implicitly created sections.
ClosedPublic

Authored by grimar on Feb 18 2020, 12:38 AM.

Details

Summary

.dynsym and .dynstr are allocatable and therefore normally are placed
before non-allocatable .strtab, .shstrtab, .symtab sections.
But we are placing them after currently what creates a mix of
alloc/non-alloc sections and does not look normal.

Diff Detail

Event Timeline

grimar created this revision.Feb 18 2020, 12:38 AM
MaskRay accepted this revision.Feb 18 2020, 3:08 PM

There is a typo in the title. I think you meant Change the order of implicitly created sections.

(I think default can be removed.)

This revision is now accepted and ready to land.Feb 18 2020, 3:08 PM
MaskRay added inline comments.Feb 18 2020, 3:16 PM
llvm/test/tools/yaml2obj/ELF/implicit-sections-info.yaml
58

Indent, i.e. CASE2-SAME: 1

grimar retitled this revision from [yaml2obj] - Change the default order if implicitly created sections. to [yaml2obj] - Change the order of implicitly created sections..Feb 19 2020, 12:23 AM
jhenderson added inline comments.Feb 19 2020, 2:26 AM
llvm/test/tools/yaml2obj/ELF/implicit-sections-info.yaml
58

I know I suggested this behaviour previously, but I realise that there is a slight flaw in it, as it will also match "Info: 1234" or "Info: 2211" for example. I think you need ^$ regex markers too.

grimar updated this revision to Diff 245370.Feb 19 2020, 3:39 AM
grimar marked an inline comment as done.
  • Addressed review comments.
llvm/test/tools/yaml2obj/ELF/implicit-sections-info.yaml
58

I am not sure how to use {{^}} with -SAME
The following does not work (and I think it is correct that it is not):

# CASE1:      Info:
# CASE1-SAME:       {{^}}2{{$}}

I think we can use -NOT, I did it here. Looks fine?
(I've only applied this approach here as it is the only test I added in this diff, we can update other places independently).

grimar planned changes to this revision.Feb 19 2020, 3:41 AM

Will reupload.

grimar updated this revision to Diff 245371.Feb 19 2020, 3:49 AM
  • Now with the change at the right place.
This revision is now accepted and ready to land.Feb 19 2020, 3:49 AM
jhenderson accepted this revision.Feb 19 2020, 3:54 AM
jhenderson added inline comments.
llvm/test/tools/yaml2obj/ELF/implicit-sections-info.yaml
58

Looks fine to me.

I think you need a space after the ^ pattern with CHECK-SAME, though I'm not certain: CASE1-SAME: {{^}} 2{{$}}. My understanding is that the ^ character matches the start of a line, and also the end of the previous match, because of the way FileCheck's system is set up. Whitespace will be significant (but canoncalized to a single space).

grimar marked an inline comment as done.Feb 19 2020, 4:02 AM
grimar added inline comments.
llvm/test/tools/yaml2obj/ELF/implicit-sections-info.yaml
58

{{^}} 2{{$}} works! As well as {{^}} 2{{$}} Thanks!

This revision was automatically updated to reflect the committed changes.