Hi, Thanks for your patch, Huang. It looks good to me. Just one comment: can you write a testcase, similar to the others used for linker script testing, with your example? Alternatively, you can modify lld/test/elf/linkerscript/sections-with-wildcards.test to test your case. This will make your patch complete and ready for commit, and will ensure we do not regress on this bug if this code is ever rewritten. Best regards, Rafael Auler On Fri, Aug 21, 2015 at 8:48 PM, Davide Italiano <davide at freebsd.org> wrote:> On Fri, Aug 21, 2015 at 4:41 AM, zan jyu Wong via llvm-dev > <llvm-dev at lists.llvm.org> wrote: >> Hi, >> >> I've found a crash in lld when using linker script with wildcard matching. >> An example linker script: >> >> INPUT(os/main.o os/foo.o os/startup.o) >> OUTPUT(k.bin) >> >> SECTIONS >> { >> . = 0x0 >> .text : { *startup.o (.text) } >> .text.2 : { *(.tex*) } >> } >> >> I've wrote up a patch to fix this crash. >> > > The patch looks good to me. > CC:ing Rafael and Michael as they wrote large part of the linker script support. > > >> Index: tools/lld/lib/ReaderWriter/LinkerScript.cpp >> <+>UTF-8 >> ==================================================================>> --- tools/lld/lib/ReaderWriter/LinkerScript.cpp (revision >> 8570c61c3fce7de2f655a20f8b184efa1bd97c00) >> +++ tools/lld/lib/ReaderWriter/LinkerScript.cpp (revision ) >> @@ -2557,7 +2557,7 @@ >> switch (*j) { >> case '*': >> while (!wildcardMatch(pattern.drop_front(j - pattern.begin() + 1), >> - name.drop_front(i - name.begin() + 1))) { >> + name.drop_front(i - name.begin()))) { >> if (i == name.end()) >> return false; >> ++i; >> @@ -2565,6 +2565,7 @@ >> break; >> case '?': >> // Matches any character >> + ++i; >> break; >> case '[': { >> // Matches a range of characters specified between brackets >> @@ -2577,20 +2578,22 @@ >> return false; >> >> j = pattern.begin() + end; >> + ++i; >> break; >> } >> case '\\': >> ++j; >> if (*j != *i) >> return false; >> + ++i; >> break; >> default: >> // No wildcard character means we must match exactly the same char >> if (*j != *i) >> return false; >> + ++i; >> break; >> } >> - ++i; >> } >> >> // If our pattern has't consumed the entire string, it is not a match >> >> Cheers, >> Huang >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> > > > > -- > Davide > > "There are no solved problems; there are only problems that are more > or less solved" -- Henri Poincare
Hi, I've wrote a test, which is modified from lld/test/elf/linkerscript/sections-with-wildcards.tes. Index: tools/lld/test/elf/linkerscript/filename-with-wildcards.test <+>UTF-8 ==================================================================--- tools/lld/test/elf/linkerscript/filename-with-wildcards.test (revision ) +++ tools/lld/test/elf/linkerscript/filename-with-wildcards.test (revision ) @@ -0,0 +1,49 @@ +/* +Tests a linker script that uses the SECTIONS command with rules containing +wildcards that matching input object files. +*/ + +ENTRY(_start) + +SECTIONS +{ + . = 0x500000; + .foo : { *p1.o(.text .rodata*) } + .bar : { *(.text .rodata*) } +} + +/* +RUN: mkdir -p %T +RUN: yaml2obj -format=elf %p/Inputs/prog1.o.yaml -o=%T/p1.o +RUN: yaml2obj -format=elf %p/Inputs/prog2.o.yaml -o=%T/p2.o +RUN: yaml2obj -format=elf %p/Inputs/prog3.o.yaml -o=%T/p3.o +RUN: cd %T + +RUN: lld -flavor gnu -target x86_64 -T %s p1.o p2.o p3.o \ +RUN: -static -o %t1 +RUN: llvm-readobj -s %t1 | FileCheck -check-prefix CHECKSECTIONS %s + +CHECKSECTIONS: Index: 1 +CHECKSECTIONS: Name: .foo +CHECKSECTIONS: Address: 0x500000 +CHECKSECTIONS: Size: 33 + +CHECKSECTIONS: Index: 2 +CHECKSECTIONS: Name: .bar +CHECKSECTIONS: Address: 0x500030 +CHECKSECTIONS: Size: 52 + +RUN: llvm-readobj -symbols %t1 | FileCheck -check-prefix CHECKSYMS %s + +CHECKSYMS: Name: main +CHECKSYMS-NEXT: Value: 0x500000 + +CHECKSYMS: Name: prog2 +CHECKSYMS-NEXT: Value: 0x500030 + +CHECKSYMS: Name: write +CHECKSYMS-NEXT: Value: 0x500040 + +CHECKSYMS: Name: _start +CHECKSYMS-NEXT: Value: 0x500048 +*/ Cheers, Huang On Sat, Aug 22, 2015 at 8:17 AM, Rafael Auler <rafaelauler at gmail.com> wrote:> Hi, > > Thanks for your patch, Huang. It looks good to me. Just one comment: > can you write a testcase, similar to the others used for linker script > testing, with your example? Alternatively, you can modify > lld/test/elf/linkerscript/sections-with-wildcards.test to test your > case. This will make your patch complete and ready for commit, and > will ensure we do not regress on this bug if this code is ever > rewritten. > > Best regards, > Rafael Auler > > On Fri, Aug 21, 2015 at 8:48 PM, Davide Italiano <davide at freebsd.org> > wrote: > > On Fri, Aug 21, 2015 at 4:41 AM, zan jyu Wong via llvm-dev > > <llvm-dev at lists.llvm.org> wrote: > >> Hi, > >> > >> I've found a crash in lld when using linker script with wildcard > matching. > >> An example linker script: > >> > >> INPUT(os/main.o os/foo.o os/startup.o) > >> OUTPUT(k.bin) > >> > >> SECTIONS > >> { > >> . = 0x0 > >> .text : { *startup.o (.text) } > >> .text.2 : { *(.tex*) } > >> } > >> > >> I've wrote up a patch to fix this crash. > >> > > > > The patch looks good to me. > > CC:ing Rafael and Michael as they wrote large part of the linker script > support. > > > > > >> Index: tools/lld/lib/ReaderWriter/LinkerScript.cpp > >> <+>UTF-8 > >> ==================================================================> >> --- tools/lld/lib/ReaderWriter/LinkerScript.cpp (revision > >> 8570c61c3fce7de2f655a20f8b184efa1bd97c00) > >> +++ tools/lld/lib/ReaderWriter/LinkerScript.cpp (revision ) > >> @@ -2557,7 +2557,7 @@ > >> switch (*j) { > >> case '*': > >> while (!wildcardMatch(pattern.drop_front(j - pattern.begin() + > 1), > >> - name.drop_front(i - name.begin() + 1))) { > >> + name.drop_front(i - name.begin()))) { > >> if (i == name.end()) > >> return false; > >> ++i; > >> @@ -2565,6 +2565,7 @@ > >> break; > >> case '?': > >> // Matches any character > >> + ++i; > >> break; > >> case '[': { > >> // Matches a range of characters specified between brackets > >> @@ -2577,20 +2578,22 @@ > >> return false; > >> > >> j = pattern.begin() + end; > >> + ++i; > >> break; > >> } > >> case '\\': > >> ++j; > >> if (*j != *i) > >> return false; > >> + ++i; > >> break; > >> default: > >> // No wildcard character means we must match exactly the same > char > >> if (*j != *i) > >> return false; > >> + ++i; > >> break; > >> } > >> - ++i; > >> } > >> > >> // If our pattern has't consumed the entire string, it is not a match > >> > >> Cheers, > >> Huang > >> > >> _______________________________________________ > >> LLVM Developers mailing list > >> llvm-dev at lists.llvm.org > >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > >> > > > > > > > > -- > > Davide > > > > "There are no solved problems; there are only problems that are more > > or less solved" -- Henri Poincare >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150822/db69b9c9/attachment.html>
On Sat, Aug 22, 2015 at 3:13 AM, zan jyu Wong via llvm-dev <llvm-dev at lists.llvm.org> wrote:> Hi, > > I've wrote a test, which is modified from > lld/test/elf/linkerscript/sections-with-wildcards.tes. > > Index: tools/lld/test/elf/linkerscript/filename-with-wildcards.test > <+>UTF-8 > ==================================================================> --- tools/lld/test/elf/linkerscript/filename-with-wildcards.test > (revision ) > +++ tools/lld/test/elf/linkerscript/filename-with-wildcards.test > (revision ) > @@ -0,0 +1,49 @@ > +/* > +Tests a linker script that uses the SECTIONS command with rules containing > +wildcards that matching input object files. > +*/ > + > +ENTRY(_start) > + > +SECTIONS > +{ > + . = 0x500000; > + .foo : { *p1.o(.text .rodata*) } > + .bar : { *(.text .rodata*) } > +} > + > +/* > +RUN: mkdir -p %T > +RUN: yaml2obj -format=elf %p/Inputs/prog1.o.yaml -o=%T/p1.o > +RUN: yaml2obj -format=elf %p/Inputs/prog2.o.yaml -o=%T/p2.o > +RUN: yaml2obj -format=elf %p/Inputs/prog3.o.yaml -o=%T/p3.o > +RUN: cd %T > + > +RUN: lld -flavor gnu -target x86_64 -T %s p1.o p2.o p3.o \ > +RUN: -static -o %t1 > +RUN: llvm-readobj -s %t1 | FileCheck -check-prefix CHECKSECTIONS %s > + > +CHECKSECTIONS: Index: 1 > +CHECKSECTIONS: Name: .foo > +CHECKSECTIONS: Address: 0x500000 > +CHECKSECTIONS: Size: 33 > + > +CHECKSECTIONS: Index: 2 > +CHECKSECTIONS: Name: .bar > +CHECKSECTIONS: Address: 0x500030 > +CHECKSECTIONS: Size: 52 > + > +RUN: llvm-readobj -symbols %t1 | FileCheck -check-prefix CHECKSYMS %s > + > +CHECKSYMS: Name: main > +CHECKSYMS-NEXT: Value: 0x500000 > + > +CHECKSYMS: Name: prog2 > +CHECKSYMS-NEXT: Value: 0x500030 > + > +CHECKSYMS: Name: write > +CHECKSYMS-NEXT: Value: 0x500040 > + > +CHECKSYMS: Name: _start > +CHECKSYMS-NEXT: Value: 0x500048 > +*/ > > Cheers, > Huang > > On Sat, Aug 22, 2015 at 8:17 AM, Rafael Auler <rafaelauler at gmail.com> wrote: >> >> Hi, >> >> Thanks for your patch, Huang. It looks good to me. Just one comment: >> can you write a testcase, similar to the others used for linker script >> testing, with your example? Alternatively, you can modify >> lld/test/elf/linkerscript/sections-with-wildcards.test to test your >> case. This will make your patch complete and ready for commit, and >> will ensure we do not regress on this bug if this code is ever >> rewritten. >> >> Best regards, >> Rafael Auler >> >> On Fri, Aug 21, 2015 at 8:48 PM, Davide Italiano <davide at freebsd.org> >> wrote: >> > On Fri, Aug 21, 2015 at 4:41 AM, zan jyu Wong via llvm-dev >> > <llvm-dev at lists.llvm.org> wrote: >> >> Hi, >> >> >> >> I've found a crash in lld when using linker script with wildcard >> >> matching. >> >> An example linker script: >> >> >> >> INPUT(os/main.o os/foo.o os/startup.o) >> >> OUTPUT(k.bin) >> >> >> >> SECTIONS >> >> { >> >> . = 0x0 >> >> .text : { *startup.o (.text) } >> >> .text.2 : { *(.tex*) } >> >> } >> >> >> >> I've wrote up a patch to fix this crash. >> >> >> > >> > The patch looks good to me. >> > CC:ing Rafael and Michael as they wrote large part of the linker script >> > support. >> > >> > >> >> Index: tools/lld/lib/ReaderWriter/LinkerScript.cpp >> >> <+>UTF-8 >> >> ==================================================================>> >> --- tools/lld/lib/ReaderWriter/LinkerScript.cpp (revision >> >> 8570c61c3fce7de2f655a20f8b184efa1bd97c00) >> >> +++ tools/lld/lib/ReaderWriter/LinkerScript.cpp (revision ) >> >> @@ -2557,7 +2557,7 @@ >> >> switch (*j) { >> >> case '*': >> >> while (!wildcardMatch(pattern.drop_front(j - pattern.begin() + >> >> 1), >> >> - name.drop_front(i - name.begin() + 1))) { >> >> + name.drop_front(i - name.begin()))) { >> >> if (i == name.end()) >> >> return false; >> >> ++i; >> >> @@ -2565,6 +2565,7 @@ >> >> break; >> >> case '?': >> >> // Matches any character >> >> + ++i; >> >> break; >> >> case '[': { >> >> // Matches a range of characters specified between brackets >> >> @@ -2577,20 +2578,22 @@ >> >> return false; >> >> >> >> j = pattern.begin() + end; >> >> + ++i; >> >> break; >> >> } >> >> case '\\': >> >> ++j; >> >> if (*j != *i) >> >> return false; >> >> + ++i; >> >> break; >> >> default: >> >> // No wildcard character means we must match exactly the same >> >> char >> >> if (*j != *i) >> >> return false; >> >> + ++i; >> >> break; >> >> } >> >> - ++i; >> >> } >> >> >> >> // If our pattern has't consumed the entire string, it is not a >> >> match >> >> >> >> Cheers, >> >> Huang >> >> >> >> _______________________________________________ >> >> LLVM Developers mailing list >> >> llvm-dev at lists.llvm.org >> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >> >> > >> > >> > >> > -- >> > Davide >> > >> > "There are no solved problems; there are only problems that are more >> > or less solved" -- Henri Poincare > > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >I committed the patch and the test as r245792. Thank you for your contribution. -- Davide "There are no solved problems; there are only problems that are more or less solved" -- Henri Poincare