diff mbox series

[bpf] bpf: Use option "help" in the llvm-objcopy test

Message ID 20180720053410.3891870-1-kafai@fb.com
State Accepted, archived
Delegated to: BPF Maintainers
Headers show
Series [bpf] bpf: Use option "help" in the llvm-objcopy test | expand

Commit Message

Martin KaFai Lau July 20, 2018, 5:34 a.m. UTC
I noticed the "--version" option of the llvm-objcopy command has recently
disappeared from the master llvm branch.  It is currently used as a BTF
support test in tools/testing/selftests/bpf/Makefile.

This patch replaces it with "--help" which should be
less error prone in the future.

Fixes: c0fa1b6c3efc ("bpf: btf: Add BTF tests")
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/testing/selftests/bpf/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Daniel Borkmann July 20, 2018, 8:37 a.m. UTC | #1
On 07/20/2018 07:34 AM, Martin KaFai Lau wrote:
> I noticed the "--version" option of the llvm-objcopy command has recently
> disappeared from the master llvm branch.  It is currently used as a BTF
> support test in tools/testing/selftests/bpf/Makefile.
> 
> This patch replaces it with "--help" which should be
> less error prone in the future.
> 
> Fixes: c0fa1b6c3efc ("bpf: btf: Add BTF tests")
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>

Looks good, ran into the same recently as well from llvm git. Wondering whether
the "--version" removal there was by accident or on purpose. In any case, applied
to bpf tree, thanks! If we make another change to the Makefile in near future,
we should also make a comment there that the llvm-objcopy is used by pahole -J
internally to make it a bit more clear in case someone is wondering why it's not
used in the Makefile itself.

Thanks,
Daniel
Yonghong Song July 25, 2018, 12:30 a.m. UTC | #2
On 7/20/18 1:37 AM, Daniel Borkmann wrote:
> On 07/20/2018 07:34 AM, Martin KaFai Lau wrote:
>> I noticed the "--version" option of the llvm-objcopy command has recently
>> disappeared from the master llvm branch.  It is currently used as a BTF
>> support test in tools/testing/selftests/bpf/Makefile.
>>
>> This patch replaces it with "--help" which should be
>> less error prone in the future.
>>
>> Fixes: c0fa1b6c3efc ("bpf: btf: Add BTF tests")
>> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> 
> Looks good, ran into the same recently as well from llvm git. Wondering whether
> the "--version" removal there was by accident or on purpose. In any case, applied

The option "--version" seems removed by accident.
On 6.0.0, the option handling is done by llvm.
      static cl::opt<std::string>
              OutputFormat("O", cl::desc("Set output format to one of the
                           following:"
                           "\n\tbinary"));
      cl::ParseCommandLineOptions(argc, argv, "llvm objcopy utility\n");
That is, the options are defined through llvm option handling system and
option "--version" is handled by llvm automatically.

In 7.0.0, llvm-objcopy tries to handle the options itself. 
Unfortunately, it did not define "version" option in its option file, so
"llvm-objcopy --version" won't work any more.

I will raise a bug or fix the issue properly.

> to bpf tree, thanks! If we make another change to the Makefile in near future,
> we should also make a comment there that the llvm-objcopy is used by pahole -J
> internally to make it a bit more clear in case someone is wondering why it's not
> used in the Makefile itself.
> 
> Thanks,
> Daniel
>
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 7a6214e9ae58..a362e3d7abc6 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -105,7 +105,7 @@  $(OUTPUT)/test_xdp_noinline.o: CLANG_FLAGS += -fno-inline
 
 BTF_LLC_PROBE := $(shell $(LLC) -march=bpf -mattr=help 2>&1 | grep dwarfris)
 BTF_PAHOLE_PROBE := $(shell $(BTF_PAHOLE) --help 2>&1 | grep BTF)
-BTF_OBJCOPY_PROBE := $(shell $(LLVM_OBJCOPY) --version 2>&1 | grep LLVM)
+BTF_OBJCOPY_PROBE := $(shell $(LLVM_OBJCOPY) --help 2>&1 | grep -i 'usage.*llvm')
 
 ifneq ($(BTF_LLC_PROBE),)
 ifneq ($(BTF_PAHOLE_PROBE),)