[PATCH] D82274: [llvm-objcopy] Emit error if removing symbol table referenced by SHT_GROUP section

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 25 03:44:52 PDT 2020


jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

In D82274#2113598 <https://reviews.llvm.org/D82274#2113598>, @jubnzv wrote:

> I agree, this makes it cleaner.
>
> I also found a strange behavior of `{{.*}}` regex in group.test. It seems that it doesn't match an empty sequence.
>
> For example, when I trying to match the second line here (empty `Flg` field):
>
>   [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
>   [ 1] .group            GROUP           0000000000000000 000040 000008 04     0  0   4
>
>
> This regex works as expected:
>
>   # SECTIONS-NEXT: .group    {{.+}} {{.+}}  {{.+}} {{.+}} {{.+}}     0  0
>
>
> But this one doesn't match the line:
>
>   # SECTIONS-NEXT: .group    {{.+}} {{.+}}  {{.+}} {{.+}} {{.+}} {{.*}} 0  0
>


Without investigating it in too much depth, I suspect you've tripped over FileCheck's canonicalization of whitespace. Essentially, FileCheck normalises all consecutive whitespace in both the pattern and the input to match against into a single space character. Therefore, if your input is something like `xyz      abc`, and your pattern is `xyz  abc` FileCheck will treat them both as `xyz abc`, meaning it will match. I suspect, but haven't checked, that this is applied before interpreting regexes. Therefore `xyz {{.*}} abc` will be treated as `xyz ` a regex matching an empty string and ` abc` in sequence. Thus the final pattern string being used to match with will contain 2 spaces (one either side of the "empty" regex). That means that it won't match `xyz abc`, or `xyz     abc` or even `xyz  abc` (because those are all treated as having only one space). You can use --strict-whitespace to disable this canonicalization. Take a look at the documentation for details. I hope that makes sense.

Anyway, the change as-is LGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82274/new/

https://reviews.llvm.org/D82274





More information about the llvm-commits mailing list