[PATCH] D66613: [support][llvm-objcopy] Add support for shell wildcards

Fri Oct 11 16:51:35 PDT 2019

rupprecht added a comment.

In D66613#1659632 <https://reviews.llvm.org/D66613#1659632>, @alexshap wrote:

> khm, I don't insist, but personally I would split out the changes in lib/Support and the corresponding unit tests into a separate patch

Ack; I'll submit them separately, but I'm leaving them together in the patch for now, e.g. to show how the added glob support is needed by llvm-objcopy tests.

In D66613#1643926 <https://reviews.llvm.org/D66613#1643926>, @MaskRay wrote:

> I just realized that you can remove the glob (I'd call it glob, not wildcard) change from this patch, and just use `lib/Support/Regex.cpp:GlobPattern`.
>
> `GlobPattern` is currently used by lld to do version script/dynamic list matching. In version scripts/dynamic lists, `[:` and `[=` are syntax error (ld.bfd), and I don't think anyone using `[.`. But to make it fully `fnmatch(pat, str, 0)` capable (in case someone uses character classes like `[[:digit:]]`), you can add these enhancement to a separate change.

In D66613#1703550 <https://reviews.llvm.org/D66613#1703550>, @evgeny777 wrote:

> I wonder if you can use GlobPattern (llvm/Support/GlobPattern.h) or extend this class. One of the reasons we use it in lld is because of much better performance (see D26241 <https://reviews.llvm.org/D26241>),
>  This may not be the case for llvm-objcopy, but still using original pattern matcher looks nicer than translating to regexps.

OK, removed the regex transformation and using glob instead.

I decided to leave out character classes for now, so the changes to glob are actually pretty minimal. lld tests are still passing w/ these glob changes. I hope the test coverage is sufficient there.

================
Comment at: llvm/docs/CommandGuide/llvm-objcopy.rst:134
+
+  Allow wildcard syntax for symbol-related flags. On by default for
+  section-related flags. Incompatible with --regex.
----------------
MaskRay wrote:
> >  On by default for section-related flags.
> 
> This is another thing I'm not sure if we want to do.
> 
> I'd like users to specify `-w` to get wildcard semantics for section options. I checked the ruby/nacl/netbsd links and they all appear to use -w
Those were just easy-to-find examples of wildcard usage. There are others using wildcards w/o `-w`, e.g.

https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/master/Makefile.rules#66
```
cmd_ec_elf_to_flat ?= $(OBJCOPY) --set-section-flags .roshared=share -R .dram* \
				-O binary $< $@
cmd_ec_elf_to_flat_dram ?= $(OBJCOPY) -j .dram* -O binary $< $@
```

Older kernel versions: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/firmware/efi/libstub/Makefile?h=linux-4.4.y#n57
```
STUBCOPY_FLAGS-y		:= -R .debug* -R *ksymtab* -R *kcrctab*
```

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66613