BPF probe compilation under Clang 10 and Clang 11

[fntlnz]

This is what happens when the BPF probe is compiled using Clang 10.

clang --version                                                                                                       
clang version 10.0.0 
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

jump out of range from insn 8 to 584
processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
jump out of range from insn 8 to 584
processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
Thu Jun  4 17:09:56 2020: Runtime error: bpf_load_program() err=22 event=filler/sys_single message=jump out of range from insn 8 to 584
processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
. Exiting.

I also tested against the current LLVM 11 source code and built Clang myself.

clang version 11.0.0 (https://github.com/llvm/llvm-project.git e73d0b5719966ddbeff7a3da70a3cb782c3733ed)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /home/fntlnz/Projects/llvm/llvm-project/build/bin

The situation there is a lot more verbose, you can find the full log here.

This was initially reported on Falco falcosecurity/falco#1251 - since the Falco BPF copy is the one from this repository I think that this is a better home for this issue.

[fntlnz]

Here's an output of llvm objdump of the same program built with different clang versions:

clang 9.0.1 (Red Hat 9.0.1-2.module_el8.2.0+309+0c7b6b03)
https://fs.fntlnz.wtf/sysdig/probe-objdump-clang-9.txt

clang 10.0.0 (Arch Linux)
https://fs.fntlnz.wtf/sysdig/probe-objdump-clang-10.txt

clang 11 (llvm git e73d0b5719966ddbeff7a3da70a3cb782c3733ed)
https://fs.fntlnz.wtf/sysdig/probe-objdump-clang-11.txt

[lumontec]

Hi @fntlnz while compiling the project I also stepped into this problem with clang>9.

I am working with:
kernel_release/headers: 5.4.0-59-generic
clang,llvm,llc: version 10

It seems that starting from clang10 function memset is not translated to llvm builtin equivalent anymore. You can observe it directly inside the llvm IR:
clang9

  call void @llvm.lifetime.start.p0i8(i64 16, i8* nonnull %451) #4
  call void @llvm.memset.p0i8.i64(i8* nonnull align 4 %451, i8 0, i64 16, i1 false) #4
  %452 = getelementptr inbounds %struct.sock, %struct.sock* %407, i64 0, i32 0, i32 11

clang10

  call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %71) #6
  %72 = call i8* @memset(i8* nonnull %71, i32 0, i64 8) #6
  %73 = getelementptr inbounds %struct.pt_regs, %struct.pt_regs* %70, i64 0, i32 14
  ...  
; Function Attrs: nobuiltin nofree
declare dso_local i8* @memset(i8*, i32, i64) local_unnamed_addr #2  

This ends up generating a bogus call instruction inside the _READ function in the bpf bitcode that confuses the verifier (Taken from https://fs.fntlnz.wtf/sysdig/probe-objdump-clang-10.txt):

; 		arg = _READ(regs->di);
     189:	bf 81 00 00 00 00 00 00	r1 = r8
     190:	b7 02 00 00 00 00 00 00	r2 = 0
     191:	b7 03 00 00 08 00 00 00	r3 = 8
     192:	85 10 00 00 ff ff ff ff	call -1
     193:	07 09 00 00 70 00 00 00	r9 += 112

This bug can be easily fixed by forcing the compiler to use the builtin memset by calling __builtin_memset() instead of plain linux memset().
The real problem is that clang>9 seems to implement many other changes breaking a number of functions inside the current implementation of the bpf probe.
Let me know if you are interested in migrating the probe to clang10/11,
I can work on a PR in the case.

[fntlnz]

Hey @lumontec ! Thanks for looking into this, more eyes here are very needed :)

I'm following what you wrote and I tested it with llvm 7 as well as llvm 11 and they both seem to use the builtin memset.

• memset calls on llvm 11, kernel 5.10.5: https://fs.fntlnz.wtf/sysdig/1658-update-jan-8-2021/memset-calls-llvm-11-kernel5_10_5.txt
• memset calls llvm 7, kernel 5.10.5: https://fs.fntlnz.wtf/sysdig/1658-update-jan-8-2021/memset-calls-llvm-7-kernel5_10_5.txt

I haven't been able to reproduce the clang10 point you had with the kernel's memset.

I think that we are seeing two different issues.

From what I can see:

• On kernel < 5.8 using a clang >= 9 leads to load errors, while clang < 9 works
• On kernel >= 5.8 any clang does not work

Here are my load results:

• Clang 11 on kernel 5.10.5: https://fs.fntlnz.wtf/sysdig/1658-update-jan-8-2021/load-llvm11-kernel5_10_5.txt
• Clang 7 on kernel 5.10.5: https://fs.fntlnz.wtf/sysdig/1658-update-jan-8-2021/load-llvm7-kernel5_10_5.txt

[fntlnz]

Here are my objdumps:

clang 11, kernel 5.10.5: https://fs.fntlnz.wtf/sysdig/1658-update-jan-8-2021/clang11-objdump-kernel_5_10_5.txt

clang 7, kernel 5.10.5: https://fs.fntlnz.wtf/sysdig/1658-update-jan-8-2021/clang7-objdump-kernel_5_10_5.txt

[fntlnz]

Here are some llvm ir dumps grep memset for kernel 5.4.69

clang 11, kernel 5.4.69: https://fs.fntlnz.wtf/sysdig/1658-update-jan-8-2021/clang11-memset-kernel_5_4_69.txt

clang 7, kernel 5.4.69: https://fs.fntlnz.wtf/sysdig/1658-update-jan-8-2021/clang7-memset-kernel_5_4_69.txt

Again I only see the builtin memset here.

[lumontec]

I actually get this memset translation error (which is the one that you had in your very first post) only with clang10(5.4.0-59 kernel).
By testing with clang11(5.4.0-59 kernel) i get your same exact behavior, i.e.: failing on verifier at filler/sys_read_x.
I suspect bpf_get_current_task() to be somehow responsible for that, because if i comment that out the verifier now strikes against a new error on filler/sys_poll_e

[fntlnz]

@lumontec yeah this bug evolved a lot with the evolution of kernel and clang combinations.

[fntlnz]

This can be closed now that the driver code was donated to falcosecurity/libs

@lumontec I did another round with the official clang builds taken from https://github.com/llvm/llvm-project/releases and I was able to see the point you had with memset, great job!

I added a Reported-By to give you credit in the commit falcosecurity/libs@8fccf2e

Anyone interested in a followup, it can be found here falcosecurity/libs#22

@lumontec I'd love a review from you in the PR if you are up to it, there are other fixes in there too, two more eyes are always a good thing :)