capstone-engine / capstone

Capstone disassembly/disassembler framework for ARM, ARM64 (ARMv8), Alpha, BPF, Ethereum VM, HPPA, LoongArch, M68K, M680X, Mips, MOS65XX, PPC, RISC-V(rv32G/rv64G), SH, Sparc, SystemZ, TMS320C64X, TriCore, Webassembly, XCore and X86.
http://www.capstone-engine.org
7.61k stars 1.56k forks source link

[BPF] `bpf_insn` collides with `libbpf` definition. #2316

Closed grahamwoodward closed 7 months ago

grahamwoodward commented 7 months ago

I've cloned, checked out, built and installed Capstone on my aarch64 VM like so

git clone https://github.com/capstone-engine/capstone.git
CAPSTONE_ARCHS="arm aarch64" ./make.sh install

and I'm trying to build an eBPF program I'm writing, that looks at retired instructions and uses Capstone to disassemble.

However I'm hitting this build issue

In file included from /usr/include/capstone/capstone.h:371:
/usr/include/capstone/bpf.h:94:9: error: use of 'bpf_insn' with tag type that does not match previous declaration
typedef enum bpf_insn {
...
...
/usr/include/bpf/libbpf.h:268:8: note: previous use is here
struct bpf_insn;

Work environment

Questions Answers
OS/arch/bits Ubunto AArch64
Architecture armv8.4
Source of Capstone git clone https://github.com/capstone-engine/capstone.git
Version/git commit b4fde983de9d14c038afef88e79fe1111388e569

Expected behavior

Unless I'm doing something wrong then I didn't expect the build failure

Actual behavior

Seems like the enum defined in Capstone clashes with the installed BPF header?

Steps to reproduce the behavior

I'm compiling the eBPF tool I'm working on, which is #including the bpf/libbpf.h and hence the compiler issue...not sure whether it's easier to isolate

https://facebookmicrosites.github.io/bpf/blog/2020/02/20/bcc-to-libbpf-howto-guide.html

grahamwoodward commented 7 months ago

The installed version of libbpf is (I think) v1.3...the GitHub repo for libbpf still has the struct bpf_insn; but at line 334

https://github.com/libbpf/libbpf/blob/6d3595d215b014d3eddb88038d686e1c20781534/src/libbpf.h#L334

grahamwoodward commented 7 months ago

@Rot127 sorry to tag you direct, just wondering if you had any thoughts on this?

Rot127 commented 7 months ago

There seems to be something off with the includes. Can you try to cherry-prick this commit: https://github.com/Rot127/capstone/commit/7d746b5c10de454f2807c63987fb8fe507d79afa

Rot127 commented 7 months ago

Also possibly related: https://github.com/capstone-engine/capstone/pull/2307

Rot127 commented 7 months ago

Just saw you use make. Please use cmake. The Makefile method is considered deprecated. Also, please share a minimal example for me to test.

Rot127 commented 7 months ago

@grahamwoodward Regarding https://github.com/capstone-engine/capstone/pull/2307#issuecomment-2049064082, yes, please try https://github.com/Rot127/capstone/commit/7d746b5c10de454f2807c63987fb8fe507d79afa. But it likely doesn't work.

Your problem comes likely from including headers in an order which creates the conflict.

If you have one file where the CS header and the libbpf header are included in the same file (or indirectly through other headers), you would need to change this.

Please write the result in this issue, not the PR.

Regarding https://github.com/capstone-engine/capstone/pull/2307#issuecomment-2049063196

This is not an option for us, unfortunately. You can do it of course locally, but we can not change the name that easily.

grahamwoodward commented 7 months ago

hmm so how do we fix it? The perf tool I'm working on might be made public with a dependency on Capstone and BPF so this will need fixing somehow

grahamwoodward commented 7 months ago

Also

Regarding #2307 (comment)

This is not an option for us, unfortunately. You can do it of course locally, but we can not change the name that easily.

why can't this be done? Because of people using Capstone would need to update their code to accommodate the enum name change?

Rot127 commented 7 months ago

why can't this be done? Because of people using Capstone would need to update their code to accommodate the enum name change?

Yes, we would need to change the names for all the archs in the API. So we are consistent with naming. This in turn would force everyone to fix their code with the new names. And Capstone tries to be as stable as possible between versions.

hmm so how do we fix it?

As far as I can see, you will need to do something like this. So you can add the cs_ prefix with the macro when you need it.

grahamwoodward commented 7 months ago

why can't this be done? Because of people using Capstone would need to update their code to accommodate the enum name change?

Yes, we would need to change the names for all the archs in the API. So we are consistent with naming. This in turn would force everyone to fix their code with the new names. And Capstone tries to be as stable as possible between versions.

It's only the "bpf arch" that needs the change, i.e in include/capstone/bpf.h

hmm so how do we fix it?

As far as I can see, you will need to do something like this. So you can add the cs_ prefix with the macro when you need it.

I'm not convinced this works...but going to play with it to see whether I can use this trick

grahamwoodward commented 7 months ago

I'm not convinced this works...but going to play with it to see whether I can use this trick

hhmm this might be working actually...


+   8 #include <capstone/capstone.h>
+   9 #define bpf_insn cs_bpf_insn
+  10   #include <linux/bpf.h>
+  11   #include <bpf/libbpf.h>
+  12 #undef cs_bpf_insn
   13
Rot127 commented 7 months ago

Please close the issue if the macro work around works for you.

grahamwoodward commented 7 months ago

Closing as no fix planned. Workaround available as per https://github.com/capstone-engine/capstone/issues/2316#issuecomment-2049281992

user202729 commented 2 weeks ago

I just come across this while trying to compile perf from source with both libbfd and capstone enabled:

git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
cd linux/tools/perf
make -f Makefile.perf prefix=/usr lib=lib/perf perfexecdir=lib/perf WERROR=0 NO_SDT=1 BUILD_BPF_SKEL=1 PYTHON=python PYTHON_CONFIG=python-config BUILD_NONDISTRO=1

and get the error

  CC      util/disasm.o
In file included from /usr/include/capstone/capstone.h:383,
                 from util/disasm.c:1354:
/usr/include/capstone/bpf.h:94:14: error: ‘bpf_insn’ defined as wrong kind of tag
   94 | typedef enum bpf_insn {
      |              ^~~~~~~~

As mentioned above, the cause is the clash between definition in capstone and definition in Linux kernel.

Since no fix is planned I suppose I should suggest the fix to perf-tools.