perf build: Fix up broken capstone feature detection fast path

PATCH In review Arnaldo Carvalho de Melo 3 participants 4 messages Aug 14, 2024 – Aug 15, 2024

Share & tools

Arnaldo Carvalho de Melo

author

2024-08-14 13:44 UTC #0 lore raw

PATCH 1/1 perf build: Fix up broken capstone feature detection fast path

The capstone devel headers define 'struct bpf_insn' in a way that clashes with what is in the libbpf devel headers, so we so far need to avoid including both.

This is happening on the tools/build/feature/test-all.c file, where we try building all the expected set of libraries to be normally available on a system:

⬢[acme@toolbox perf-tools-next]$ cat /tmp/build/perf-tools-next/feature/test-all.make.output In file included from test-bpf.c:3,

                   from test-all.c:150:
  /home/acme/git/perf-tools-next/tools/include/uapi/linux/bpf.h:77:8: error: ‘bpf_insn’ defined as wrong kind of tag
     77 | struct bpf_insn {
        |        ^~~~~~~~

⬢[acme@toolbox perf-tools-next]$ cat /tmp/build/perf-tools-next/feature/test-all.make.output

When doing so there is a trick where we define main to be main_test_libcapstone, then include the individual tools/build/feture/test-libcapstone.c capability query test, and then we undef 'main' because we'll do it all over again with the next expected library to be tested (at this time 'lzma').

To complete this mechanism we need to, in test-all.c 'main' routine, to call main_test_libcapstone(), which isn't being done, so the effect of adding references to capstone in test-all.c are not achieved.

The only thing that is happening is that test-all.c is failing to build and thus all the tests will have to be done individually, which nullifies the test-all.c single build speedup.

So lets remove references to capstone from test-all.c to see if this makes it build again so that we get faster builds or go on fixing up whatever is preventing us to get that benefit.

Nothing: after this fix we get a clean test-all.c build and get the build speedup back:

⬢[acme@toolbox perf-tools-next]$ cat /tmp/build/perf-tools-next/feature/test-all.make.output ⬢[acme@toolbox perf-tools-next]$ cat /tmp/build/perf-tools-next/feature/test-all.
test-all.bin test-all.d test-all.make.output
⬢[acme@toolbox perf-tools-next]$ cat /tmp/build/perf-tools-next/feature/test-all.make.output ⬢[acme@toolbox perf-tools-next]$ ldd /tmp/build/perf-tools-next/feature/test-all.bin
linux-vdso.so.1 (0x00007f13277a1000)
libpython3.12.so.1.0 => /lib64/libpython3.12.so.1.0 (0x00007f1326e00000) libm.so.6 => /lib64/libm.so.6 (0x00007f13274be000)
libtraceevent.so.1 => /lib64/libtraceevent.so.1 (0x00007f1327496000) libtracefs.so.1 => /lib64/libtracefs.so.1 (0x00007f132746f000) libcrypto.so.3 => /lib64/libcrypto.so.3 (0x00007f1326800000) libunwind-x86_64.so.8 => /lib64/libunwind-x86_64.so.8 (0x00007f1327452000) libunwind.so.8 => /lib64/libunwind.so.8 (0x00007f1327436000) liblzma.so.5 => /lib64/liblzma.so.5 (0x00007f1327403000)
libdw.so.1 => /lib64/libdw.so.1 (0x00007f1326d6f000)
libz.so.1 => /lib64/libz.so.1 (0x00007f13273e2000)
libelf.so.1 => /lib64/libelf.so.1 (0x00007f1326d53000)
libnuma.so.1 => /lib64/libnuma.so.1 (0x00007f13273d4000)
libslang.so.2 => /lib64/libslang.so.2 (0x00007f1326400000) libperl.so.5.38 => /lib64/libperl.so.5.38 (0x00007f1326000000) libc.so.6 => /lib64/libc.so.6 (0x00007f1325e0f000)
libzstd.so.1 => /lib64/libzstd.so.1 (0x00007f1326741000) /lib64/ld-linux-x86-64.so.2 (0x00007f13277a3000)
libbz2.so.1 => /lib64/libbz2.so.1 (0x00007f1326d3f000)
libcrypt.so.2 => /lib64/libcrypt.so.2 (0x00007f1326d07000)
⬢[acme@toolbox perf-tools-next]$

And when having capstone-devel installed we get it detected and linked with perf, allowing us to benefit from the features that it enables:

⬢[acme@toolbox perf-tools-next]$ rpm -q capstone-devel capstone-devel-5.0.1-3.fc40.x86_64
⬢[acme@toolbox perf-tools-next]$ ldd /tmp/build/perf-tools-next/perf | grep capstone
libcapstone.so.5 => /lib64/libcapstone.so.5 (0x00007fe6a5c00000)
⬢[acme@toolbox perf-tools-next]$ /tmp/build/perf-tools-next/perf -vv | grep cap
libcapstone: [ on ] # HAVE_LIBCAPSTONE_SUPPORT
⬢[acme@toolbox perf-tools-next]$

Fixes 8b767db3309595a2 ("perf: build: introduce the libcapstone")

Cc Adrian Hunter <[email protected]>

Cc Andi Kleen <[email protected]>

Cc Changbin Du <[email protected]>

Cc Ian Rogers <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Kan Liang <[email protected]>

Cc Namhyung Kim <[email protected]>

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/build/feature/test-all.c | 4 ----
 1 file changed, 4 deletions(-)

tools/build/feature/test-all.c +0 -4

index dd0a18c2ef8fc080..6f4bf386a3b5c4b0 100644
@@ -134,10 +134,6 @@
 #undef main
 #endif
 
-#define main main_test_libcapstone
-# include "test-libcapstone.c"
-#undef main
-
 #define main main_test_lzma
 # include "test-lzma.c"
 #undef main

Signature · 1 line

2.45.2

duchangbin

2024-08-15 03:26 UTC #1 lore raw

Hi, Arnaldo,

How about workaround it by rename the 'bpf_insn' in capstione?

Change test-libcapstone.c as:

#define bpf_insn capstone_bpf_insn
#include <capstone/capstone.h>
#undef bpf_insn

I haven't tried it but seems feasible.

Quoted 105 lines · Arnaldo Carvalho de Melo

The capstone devel headers define 'struct bpf_insn' in a way that clashes with what is in the libbpf devel headers, so we so far need to avoid including both.
This is happening on the tools/build/feature/test-all.c file, where we try building all the expected set of libraries to be normally available on a system:
⬢[acme@toolbox perf-tools-next]$ cat /tmp/build/perf-tools-next/feature/test-all.make.output In file included from test-bpf.c:3,
                   from test-all.c:150:
  /home/acme/git/perf-tools-next/tools/include/uapi/linux/bpf.h:77:8: error: ‘bpf_insn’ defined as wrong kind of tag
     77 | struct bpf_insn {
        |        ^~~~~~~~
⬢[acme@toolbox perf-tools-next]$ cat /tmp/build/perf-tools-next/feature/test-all.make.output
When doing so there is a trick where we define main to be main_test_libcapstone, then include the individual tools/build/feture/test-libcapstone.c capability query test, and then we undef 'main' because we'll do it all over again with the next expected library to be tested (at this time 'lzma').
To complete this mechanism we need to, in test-all.c 'main' routine, to call main_test_libcapstone(), which isn't being done, so the effect of adding references to capstone in test-all.c are not achieved.
The only thing that is happening is that test-all.c is failing to build and thus all the tests will have to be done individually, which nullifies the test-all.c single build speedup.
So lets remove references to capstone from test-all.c to see if this makes it build again so that we get faster builds or go on fixing up whatever is preventing us to get that benefit.
Nothing: after this fix we get a clean test-all.c build and get the build speedup back:
⬢[acme@toolbox perf-tools-next]$ cat /tmp/build/perf-tools-next/feature/test-all.make.output ⬢[acme@toolbox perf-tools-next]$ cat /tmp/build/perf-tools-next/feature/test-all.
test-all.bin test-all.d test-all.make.output
⬢[acme@toolbox perf-tools-next]$ cat /tmp/build/perf-tools-next/feature/test-all.make.output ⬢[acme@toolbox perf-tools-next]$ ldd /tmp/build/perf-tools-next/feature/test-all.bin
linux-vdso.so.1 (0x00007f13277a1000)
libpython3.12.so.1.0 => /lib64/libpython3.12.so.1.0 (0x00007f1326e00000) libm.so.6 => /lib64/libm.so.6 (0x00007f13274be000)
libtraceevent.so.1 => /lib64/libtraceevent.so.1 (0x00007f1327496000) libtracefs.so.1 => /lib64/libtracefs.so.1 (0x00007f132746f000) libcrypto.so.3 => /lib64/libcrypto.so.3 (0x00007f1326800000) libunwind-x86_64.so.8 => /lib64/libunwind-x86_64.so.8 (0x00007f1327452000) libunwind.so.8 => /lib64/libunwind.so.8 (0x00007f1327436000) liblzma.so.5 => /lib64/liblzma.so.5 (0x00007f1327403000)
libdw.so.1 => /lib64/libdw.so.1 (0x00007f1326d6f000)
libz.so.1 => /lib64/libz.so.1 (0x00007f13273e2000)
libelf.so.1 => /lib64/libelf.so.1 (0x00007f1326d53000)
libnuma.so.1 => /lib64/libnuma.so.1 (0x00007f13273d4000)
libslang.so.2 => /lib64/libslang.so.2 (0x00007f1326400000) libperl.so.5.38 => /lib64/libperl.so.5.38 (0x00007f1326000000) libc.so.6 => /lib64/libc.so.6 (0x00007f1325e0f000)
libzstd.so.1 => /lib64/libzstd.so.1 (0x00007f1326741000) /lib64/ld-linux-x86-64.so.2 (0x00007f13277a3000)
libbz2.so.1 => /lib64/libbz2.so.1 (0x00007f1326d3f000)
libcrypt.so.2 => /lib64/libcrypt.so.2 (0x00007f1326d07000)
⬢[acme@toolbox perf-tools-next]$
And when having capstone-devel installed we get it detected and linked with perf, allowing us to benefit from the features that it enables:
⬢[acme@toolbox perf-tools-next]$ rpm -q capstone-devel capstone-devel-5.0.1-3.fc40.x86_64
⬢[acme@toolbox perf-tools-next]$ ldd /tmp/build/perf-tools-next/perf | grep capstone
libcapstone.so.5 => /lib64/libcapstone.so.5 (0x00007fe6a5c00000)
⬢[acme@toolbox perf-tools-next]$ /tmp/build/perf-tools-next/perf -vv | grep cap
libcapstone: [ on ] # HAVE_LIBCAPSTONE_SUPPORT
⬢[acme@toolbox perf-tools-next]$
Fixes 8b767db3309595a2 ("perf: build: introduce the libcapstone")
Cc Adrian Hunter <[email protected]>
Cc Andi Kleen <[email protected]>
Cc Changbin Du <[email protected]>
Cc Ian Rogers <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Kan Liang <[email protected]>
Cc Namhyung Kim <[email protected]>
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>
---
 tools/build/feature/test-all.c | 4 ----
 1 file changed, 4 deletions(-)
tools/build/feature/test-all.c +0 -4
index dd0a18c2ef8fc080..6f4bf386a3b5c4b0 100644
@@ -134,10 +134,6 @@
 #undef main
 #endif
 
-#define main main_test_libcapstone
-# include "test-libcapstone.c"
-#undef main
-
 #define main main_test_lzma
 # include "test-lzma.c"
 #undef main
Signature · 1 line
2.45.2

Signature · 2 lines

Cheers,
Changbin Du

Arnaldo Carvalho de Melo

author

2024-08-15 14:46 UTC #2 lore raw

Quoted 3 lines · duchangbin

Hi, Arnaldo,
How about workaround it by rename the 'bpf_insn' in capstione?

Right, that could help overcome that clash, but then we would also have to call main_test_libcapstone() from test-all.c's main() to get it fully supported.

The thing is, do we think capstone is crucial for building perf these days and is most of the time installed on developer's machines? The idea of test-all.c is to save time by doing just one build test for the things that are almost certain to be installed, so we don't have to do N compilations/links.

The way it is now makes it achieve that, after a while, if capstone support becomes integral to the perf tooling experience, we can change that and add its feature query test to the test-all.c suite.

Since we're seeing also some support for llvm libraries that "compete" with capstone, this may end up not being the case, so I think we should wait a bit there.

- Arnaldo

Quoted 118 lines

Change test-libcapstone.c as:
#define bpf_insn capstone_bpf_insn
#include <capstone/capstone.h>
#undef bpf_insn
I haven't tried it but seems feasible.
Quoted 105 lines · Arnaldo Carvalho de Melo
The capstone devel headers define 'struct bpf_insn' in a way that clashes with what is in the libbpf devel headers, so we so far need to avoid including both.
This is happening on the tools/build/feature/test-all.c file, where we try building all the expected set of libraries to be normally available on a system:
⬢[acme@toolbox perf-tools-next]$ cat /tmp/build/perf-tools-next/feature/test-all.make.output In file included from test-bpf.c:3,
                   from test-all.c:150:
  /home/acme/git/perf-tools-next/tools/include/uapi/linux/bpf.h:77:8: error: ‘bpf_insn’ defined as wrong kind of tag
     77 | struct bpf_insn {
        |        ^~~~~~~~
⬢[acme@toolbox perf-tools-next]$ cat /tmp/build/perf-tools-next/feature/test-all.make.output
When doing so there is a trick where we define main to be main_test_libcapstone, then include the individual tools/build/feture/test-libcapstone.c capability query test, and then we undef 'main' because we'll do it all over again with the next expected library to be tested (at this time 'lzma').
To complete this mechanism we need to, in test-all.c 'main' routine, to call main_test_libcapstone(), which isn't being done, so the effect of adding references to capstone in test-all.c are not achieved.
The only thing that is happening is that test-all.c is failing to build and thus all the tests will have to be done individually, which nullifies the test-all.c single build speedup.
So lets remove references to capstone from test-all.c to see if this makes it build again so that we get faster builds or go on fixing up whatever is preventing us to get that benefit.
Nothing: after this fix we get a clean test-all.c build and get the build speedup back:
⬢[acme@toolbox perf-tools-next]$ cat /tmp/build/perf-tools-next/feature/test-all.make.output ⬢[acme@toolbox perf-tools-next]$ cat /tmp/build/perf-tools-next/feature/test-all.
test-all.bin test-all.d test-all.make.output
⬢[acme@toolbox perf-tools-next]$ cat /tmp/build/perf-tools-next/feature/test-all.make.output ⬢[acme@toolbox perf-tools-next]$ ldd /tmp/build/perf-tools-next/feature/test-all.bin
linux-vdso.so.1 (0x00007f13277a1000)
libpython3.12.so.1.0 => /lib64/libpython3.12.so.1.0 (0x00007f1326e00000) libm.so.6 => /lib64/libm.so.6 (0x00007f13274be000)
libtraceevent.so.1 => /lib64/libtraceevent.so.1 (0x00007f1327496000) libtracefs.so.1 => /lib64/libtracefs.so.1 (0x00007f132746f000) libcrypto.so.3 => /lib64/libcrypto.so.3 (0x00007f1326800000) libunwind-x86_64.so.8 => /lib64/libunwind-x86_64.so.8 (0x00007f1327452000) libunwind.so.8 => /lib64/libunwind.so.8 (0x00007f1327436000) liblzma.so.5 => /lib64/liblzma.so.5 (0x00007f1327403000)
libdw.so.1 => /lib64/libdw.so.1 (0x00007f1326d6f000)
libz.so.1 => /lib64/libz.so.1 (0x00007f13273e2000)
libelf.so.1 => /lib64/libelf.so.1 (0x00007f1326d53000)
libnuma.so.1 => /lib64/libnuma.so.1 (0x00007f13273d4000)
libslang.so.2 => /lib64/libslang.so.2 (0x00007f1326400000) libperl.so.5.38 => /lib64/libperl.so.5.38 (0x00007f1326000000) libc.so.6 => /lib64/libc.so.6 (0x00007f1325e0f000)
libzstd.so.1 => /lib64/libzstd.so.1 (0x00007f1326741000) /lib64/ld-linux-x86-64.so.2 (0x00007f13277a3000)
libbz2.so.1 => /lib64/libbz2.so.1 (0x00007f1326d3f000)
libcrypt.so.2 => /lib64/libcrypt.so.2 (0x00007f1326d07000)
⬢[acme@toolbox perf-tools-next]$
And when having capstone-devel installed we get it detected and linked with perf, allowing us to benefit from the features that it enables:
⬢[acme@toolbox perf-tools-next]$ rpm -q capstone-devel capstone-devel-5.0.1-3.fc40.x86_64
⬢[acme@toolbox perf-tools-next]$ ldd /tmp/build/perf-tools-next/perf | grep capstone
libcapstone.so.5 => /lib64/libcapstone.so.5 (0x00007fe6a5c00000)
⬢[acme@toolbox perf-tools-next]$ /tmp/build/perf-tools-next/perf -vv | grep cap
libcapstone: [ on ] # HAVE_LIBCAPSTONE_SUPPORT
⬢[acme@toolbox perf-tools-next]$
Fixes 8b767db3309595a2 ("perf: build: introduce the libcapstone")
Cc Adrian Hunter <[email protected]>
Cc Andi Kleen <[email protected]>
Cc Changbin Du <[email protected]>
Cc Ian Rogers <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Kan Liang <[email protected]>
Cc Namhyung Kim <[email protected]>
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>
---
 tools/build/feature/test-all.c | 4 ----
 1 file changed, 4 deletions(-)
tools/build/feature/test-all.c +0 -4
index dd0a18c2ef8fc080..6f4bf386a3b5c4b0 100644
@@ -134,10 +134,6 @@
 #undef main
 #endif
 
-#define main main_test_libcapstone
-# include "test-libcapstone.c"
-#undef main
-
 #define main main_test_lzma
 # include "test-lzma.c"
 #undef main
Signature · 1 line
2.45.2
Signature · 2 lines
Cheers,
Changbin Du

Andi Kleen

2024-08-15 17:17 UTC #3 lore raw

In reply to duchangbin

Quoted 11 lines · duchangbin

Hi, Arnaldo,
How about workaround it by rename the 'bpf_insn' in capstione?
Change test-libcapstone.c as:
#define bpf_insn capstone_bpf_insn
#include <capstone/capstone.h>
#undef bpf_insn
I haven't tried it but seems feasible.

Yes that seems like a better fix.

-Andi