Page MenuHomePhabricator

[ELF] - Teach input section wildcard patterns to recognize '?' meta character.
ClosedPublic

Authored by grimar on Feb 16 2016, 5:21 AM.

Details

Summary

`?' - matches any single character
https://sourceware.org/binutils/docs/ld/Input-Section-Wildcards.html

This is used in few linker scripts I saw. Ex:

.init_array     :
  {
    KEEP (*(SORT_BY_INIT_PRIORITY(.init_array.*) SORT_BY_INIT_PRIORITY(.ctors.*)))
    KEEP (*(.init_array EXCLUDE_FILE (*crtbegin.o *crtbegin?.o *crtend.o *crtend?.o ) .ctors))
  }

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 48067.Feb 16 2016, 5:21 AM
grimar retitled this revision from to [ELF] - Teach input section wildcard patterns to recognize '?' meta character..
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, grimar.
rafael edited edge metadata.Feb 18 2016, 7:15 AM

Testcase?

grimar updated this revision to Diff 48318.Feb 18 2016, 8:10 AM
grimar edited edge metadata.
  • Added testcase
grimar updated this revision to Diff 48320.Feb 18 2016, 8:12 AM
  • Fixed comment in testcase
rafael added inline comments.Feb 18 2016, 8:29 AM
ELF/LinkerScript.cpp
66 ↗(On Diff #48320)

Why rename S to W in the same commit?

ruiu added inline comments.Feb 18 2016, 11:52 AM
ELF/LinkerScript.cpp
58–63 ↗(On Diff #48320)

I don't think we need examples because this pattern matching is familiar.

// Returns true if S matches T. S can contain DOS-style wildcard characters.
// The asterisk ('*') matches one or more characacters, and the question
// mark ('?') matches one character.
test/ELF/wildcards.s
1 ↗(On Diff #48320)

If you create a new file, please move the existing test for '*' in linkerscript-sections.s to this file.

grimar updated this revision to Diff 48477.Feb 19 2016, 5:20 AM
grimar marked 3 inline comments as done.
  • Addressed review comments.
ELF/LinkerScript.cpp
66 ↗(On Diff #48320)

We often rename the variables during common patches. See nothing ubnormal here. But reverted it back, if you think it is better to do separatelly.

test/ELF/wildcards.s
1 ↗(On Diff #48320)

I can't do that. Those test uses those file source code.
I removed test from linkerscript-sections.s and added one additional for * here.

rafael added inline comments.Feb 19 2016, 11:49 AM
ELF/LinkerScript.cpp
58 ↗(On Diff #48477)

glob is probably a better known term than DOS-style wildcard.

grimar added inline comments.Feb 19 2016, 11:42 PM
ELF/LinkerScript.cpp
58 ↗(On Diff #48477)

https://sourceware.org/binutils/docs/ld/Input-Section-Wildcards.html
In specs it just called "the wildcard patterns characters" I think.
At least there is a mention that "The wildcard patterns are like those used by the Unix shell." and below each special symbol named as character.

rafael added inline comments.Feb 22 2016, 8:10 AM
ELF/LinkerScript.cpp
58 ↗(On Diff #48477)

"those used by the Unix shell" == glob.

Please don't use DOS-style.

ruiu added inline comments.Feb 22 2016, 2:37 PM
ELF/LinkerScript.cpp
58 ↗(On Diff #48477)

glob sounds fine.

grimar updated this revision to Diff 48786.Feb 23 2016, 12:25 AM
  • s/DOS-style wildcard characters/glob meta-characters
rafael accepted this revision.Feb 23 2016, 11:32 AM
rafael edited edge metadata.

LGTM with a nit. This patch is not changing what '*' does and should not change its description to say it doesn't match 0 characters.

ELF/LinkerScript.cpp
70 ↗(On Diff #48786)

'*' can't really mach 0 characters?

This revision is now accepted and ready to land.Feb 23 2016, 11:32 AM
ruiu accepted this revision.Feb 23 2016, 2:02 PM
ruiu edited edge metadata.

LGTM

ELF/LinkerScript.cpp
70 ↗(On Diff #48786)

That's a good question, and I found that either the code or this comment is wrong. If my understanding is correct, '*' in this function matches zero-or-more. We need to fix either one.

This revision was automatically updated to reflect the committed changes.