This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Linkerscript: support all kinds of sorting (including nested).
ClosedPublic

Authored by grimar on Aug 1 2016, 9:39 AM.

Details

Summary

Previously we supported only sorting by name.
I found that SORT_BY_ALIGNMENT is also often used in combination with nested sorting:
https://searchcode.com/codesearch/view/85938978/
https://searchcode.com/codesearch/view/47611074/

When there are nested section sorting commands in linker script, there can be at most 1
level of nesting for section sorting commands.

  1. SORT_BY_NAME (SORT_BY_ALIGNMENT (wildcard section pattern)). It will sort the input

sections by name first, then by alignment if 2 sections have the same name.

  1. SORT_BY_ALIGNMENT (SORT_BY_NAME (wildcard section pattern)). It will sort the input

sections by alignment first, then by name if 2 sections have the same alignment.

  1. SORT_BY_NAME (SORT_BY_NAME (wildcard section pattern)) is treated the same as SORT_

BY_NAME (wildcard section pattern).

  1. SORT_BY_ALIGNMENT (SORT_BY_ALIGNMENT (wildcard section pattern)) is treated the

same as SORT_BY_ALIGNMENT (wildcard section pattern).

  1. All other nested section sorting commands are invalid.

Patch implements that all above.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 66332.Aug 1 2016, 9:39 AM
grimar retitled this revision from to [ELF] - Linkerscript: support all kinds of sorting (including nested)..
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, grimar, davide, evgeny777.
ruiu added inline comments.Aug 1 2016, 1:26 PM
ELF/LinkerScript.cpp
742 ↗(On Diff #66332)

Because we have limited number of patterns, we probably should handle all pattern as a grammar rather than a computed result. I think more straightforward code is something like this.

if (skip("SORT") || skip("SORT_BY_NAME)) {
  expect("(");
  if (skip("SORT_by_alignment") {
    InCmd->Sort = NameAlign;
    expect("(");
    readInputFilePattern(InCmd, Keep);
    expect(")");
  } else {
    InCmd->Sort = Name;
    readInputFilePattern(InCmd, Keep);
  }
  expect(")");
  return;
}

if (skip("SORT_BY_ALIGNMENT")) {
  expect("(");
  if (skip("SORT") || skip("SORT_BY_NAME")) {
    InCmd->Sort = AlignName;
    expect("(");
    readInputFilePattern(InCmd, Keep);
    expect(")");
  } else {
    InCmd->Sort = Align;
    readInputFilePattern(InCmd, Keep);
  }
  expect(")");
  return;
}

readInputFilePattern(InCmd, Keep);
grimar updated this revision to Diff 66445.Aug 2 2016, 2:32 AM
  • Addressed review comments.
grimar added inline comments.Aug 2 2016, 2:37 AM
ELF/LinkerScript.cpp
742 ↗(On Diff #66332)

Ok.

ruiu accepted this revision.Aug 2 2016, 9:12 AM
ruiu edited edge metadata.

LGTM. I think there's a way to simplify it, but it can be done in a follow-up patch.

This revision is now accepted and ready to land.Aug 2 2016, 9:12 AM
This revision was automatically updated to reflect the committed changes.