On 06/07/2016 06:19 AM, Mark Wielaard wrote: > Hi Richard, > > On Fri, 2016-06-03 at 13:48 -0700, Richard Henderson wrote: >> I'm using the following to aid in debugging objects similar to what's >> produced by the llvm-bpf backend. > > Thanks. Is there a document/URL on BPF data structures and how it gets > embedded into ELF files to include in the source comments for reference?
No. The best I could do is point you at http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/samples/bpf/bpf_load.c and then you can read the code. ;-) > Also can I trick you into submitting a GNU-style ChangeLog for it too? > Personally I like those because they review by listing what to expect. > Although there seem to be more and more abolitionists out there these > days. Heh. And I'm one of them. But yes, I can write such a thing. > Nice :) Normally we just take the latest elf.h from glibc. > Would you mind submitting that part to libc-al...@sourceware.org for > inclusion? That would also make sure that other projects pick up the > same EM constant as standard for BPF ELF files. Yeah, I'll submit it for binutils and llvm too. >> What's lacking so far are test cases. Although there needn't be too >> many of those, considering the simplicity of the target. > > A simple testcase would be great. If only for those of us who have never > actually seen an BPF ELF file. Note that we do allow binary files in the > testsuite for tests that only read data. But please add a note in the > test harness how to regenerate them. > > I think we need a configure check to see if we have <linux/bpf.h>. Or we > could create our own constants file and struct bpf_insn to be > independent from the specific kernel headers installed. > >> +static const char class_string[8][8] = { >> + [BPF_LD] = "ld", >> + [BPF_LDX] = "ldx", >> + [BPF_ST] = "st", >> + [BPF_STX] = "stx", >> + [BPF_ALU] = "alu", >> + [BPF_JMP] = "jmp", >> + [BPF_RET] = "6", >> + [BPF_ALU64] = "alu64", >> +}; > > Why is RET 6? The "old" bpf format, from *bsd, used opcode 6 for return. In the new ebpf format, it's completely unused. >> + /* ??? We really should pass in CTX, so that we can detect >> + wrong endianness and do some swapping. */ > > Note that the disasm EBL hook is an internal API, so you should feel > free to just pass in the ctx, ebl or elf to the function if you need it > to determine whether the ELF was big or little endian. All users are > internal. Ok. >> + code = i.code; >> + class = BPF_CLASS(code); >> + op = BPF_OP(code) | BPF_SRC(code); > > Do the various BPF_XXX macros clamp the values so that the fmt array > accesses below don't overflow? We don't want crashes even if someone > feeds bogus code (struct bpf_insn) values. Yes, they do mask values. > Of course if we have a small BPF testcase ELF file we can always just > run a fuzzer on it to make sure. Fair enough. It would be small enough to just add all 256 values that fill the opcode byte. r~
