Diffusion LLVM Github Monorepo edeebad77157

[llvm-objcopy] Add support for shell wildcards
edeebad77157
Actions

Authored by rupprecht on Oct 17 2019, 1:51 PM.

Description

[llvm-objcopy] Add support for shell wildcards

Summary: GNU objcopy accepts the --wildcard flag to allow wildcard matching on symbol-related flags. (Note: it's implicitly true for section flags).

The basic syntax is to allow *, ?, \, and [] which work similarly to how they work in a shell. Additionally, starting a wildcard with ! causes that wildcard to prevent it from matching a flag.

Use an updated GlobPattern in libSupport to handle these patterns. It does not fully match the fnmatch used by GNU objcopy since named character classes (e.g. [[:digit:]]) are not supported, but this should support most existing use cases (mostly just * is what's used anyway).

Reviewers: jhenderson, MaskRay, evgeny777, espindola, alexshap

Reviewed By: MaskRay

Subscribers: nickdesaulniers, emaste, arichardson, hiraditya, jakehehrlich, abrachet, seiya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D66613

llvm-svn: 375169

Details

Committed

rupprecht

Oct 17 2019, 1:51 PM

Reviewer

MaskRay

Differential Revision

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

Parents

rG9c5d76ff4d15: [OPENMP]Dow not emit warnings for uninitialized loop counters.

Branches

Unknown

Tags

Unknown

Event Timeline

rupprecht committed rGedeebad77157: [llvm-objcopy] Add support for shell wildcards (authored by rupprecht).Oct 17 2019, 1:51 PM

I've got a number of post-commit comments. Would you mind creating another review to address them, please?

/llvm/docs/CommandGuide/llvm-objcopy.rst
148	`--regex` -> `:option:--regex` (with backticks surrounding "--regex"). Same goes for the llvm-strip document.
/llvm/test/tools/llvm-objcopy/ELF/wildcard-flags.test
31	What's the point of --strip-debug here and in the llvm-strip case? If it's just to suppress the --strip-all behaviour of llvm-strip, I'd remove it and replace it with --no-strip-all only in llvm-strip for readability.
38	This comment is a little ambiguous, as it seems to imply that the following tests are only looking for literal matches. I'd change it to: "... literal matches. Adding ..." -> "... literal matches, and that adding ..."
/llvm/test/tools/llvm-objcopy/ELF/wildcard-syntax.test
15	I think it might be worth a test case showing the glob behaviour for 0 matching characters, e.g. what happens with `--remove-section='.b*ar'`
16	You also need a test case showing that ? doesn't match 0 or 2+ characters.
29–30	Perhaps a cmp here might be more appropriate? Ditto for the third case.
46–47	Ditto re. cmp.
61	Why? Add that to the comment. (I assume it's "to keep the earlier tests simpler").
65	I think there are possibly some more test cases needed: Perhaps worth a case showing that what "\\" matches. Perhaps worth a case showing that "\*" matches '' followed by any number of characters (i.e. that only the first character is escaped). What about attempting to escape a non-special character (e.g. "\s")? Finally, maybe worth a test case where a character is escaped part way through a glob string, possibly even with glob characters before it.
/llvm/tools/llvm-objcopy/CopyConfig.h
100	Regex and GlobPattern are shared...

Changes (8)

				Path	Packages
	M			llvm/docs/CommandGuide/llvm-objcopy.rst
	M			llvm/docs/CommandGuide/llvm-strip.rst
	A			llvm/test/tools/llvm-objcopy/ELF/wildcard-flags.test
	A			llvm/test/tools/llvm-objcopy/ELF/wildcard-syntax.test
	M			llvm/tools/llvm-objcopy/CommonOpts.td
	M			llvm/tools/llvm-objcopy/CopyConfig.cpp
	M			llvm/tools/llvm-objcopy/CopyConfig.h
	M			llvm/tools/llvm-objcopy/llvm-objcopy.cpp