| Message ID | 20230324230209.161008-3-quentin@isovalent.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpftool: Add inline annotations when dumping program CFGs | expand |
On 03/24, Quentin Monnet wrote: > When dumping the control flow graphs for programs using the 16-byte long > load instruction, we need to skip the second part of this instruction > when looking for the next instruction to process. Otherwise, we end up > printing "BUG_ld_00" from the kernel disassembler in the CFG. > Fixes: efcef17a6d65 ("tools: bpftool: generate .dot graph from CFG > information") > Signed-off-by: Quentin Monnet <quentin@isovalent.com> > --- > tools/bpf/bpftool/xlated_dumper.c | 7 +++++++ > 1 file changed, 7 insertions(+) > diff --git a/tools/bpf/bpftool/xlated_dumper.c > b/tools/bpf/bpftool/xlated_dumper.c > index 6fe3134ae45d..3daa05d9bbb7 100644 > --- a/tools/bpf/bpftool/xlated_dumper.c > +++ b/tools/bpf/bpftool/xlated_dumper.c > @@ -372,8 +372,15 @@ void dump_xlated_for_graph(struct dump_data *dd, > void *buf_start, void *buf_end, > struct bpf_insn *insn_start = buf_start; > struct bpf_insn *insn_end = buf_end; > struct bpf_insn *cur = insn_start; > + bool double_insn = false; > for (; cur <= insn_end; cur++) { > + if (double_insn) { > + double_insn = false; > + continue; > + } > + double_insn = cur->code == (BPF_LD | BPF_IMM | BPF_DW); > + > printf("% 4d: ", (int)(cur - insn_start + start_idx)); > print_bpf_insn(&cbs, cur, true); > if (cur != insn_end) Any reason not to do the following here instead? if (cur->code == (BPF_LD | BPF_IMM | BPF_DW)) cur++; > -- > 2.34.1
2023-03-24 19:54 UTC-0700 ~ Stanislav Fomichev <sdf@google.com> > On 03/24, Quentin Monnet wrote: >> When dumping the control flow graphs for programs using the 16-byte long >> load instruction, we need to skip the second part of this instruction >> when looking for the next instruction to process. Otherwise, we end up >> printing "BUG_ld_00" from the kernel disassembler in the CFG. > >> Fixes: efcef17a6d65 ("tools: bpftool: generate .dot graph from CFG >> information") >> Signed-off-by: Quentin Monnet <quentin@isovalent.com> >> --- >> tools/bpf/bpftool/xlated_dumper.c | 7 +++++++ >> 1 file changed, 7 insertions(+) > >> diff --git a/tools/bpf/bpftool/xlated_dumper.c >> b/tools/bpf/bpftool/xlated_dumper.c >> index 6fe3134ae45d..3daa05d9bbb7 100644 >> --- a/tools/bpf/bpftool/xlated_dumper.c >> +++ b/tools/bpf/bpftool/xlated_dumper.c >> @@ -372,8 +372,15 @@ void dump_xlated_for_graph(struct dump_data *dd, >> void *buf_start, void *buf_end, >> struct bpf_insn *insn_start = buf_start; >> struct bpf_insn *insn_end = buf_end; >> struct bpf_insn *cur = insn_start; >> + bool double_insn = false; > >> for (; cur <= insn_end; cur++) { >> + if (double_insn) { >> + double_insn = false; >> + continue; >> + } >> + double_insn = cur->code == (BPF_LD | BPF_IMM | BPF_DW); >> + >> printf("% 4d: ", (int)(cur - insn_start + start_idx)); >> print_bpf_insn(&cbs, cur, true); >> if (cur != insn_end) > > Any reason not to do the following here instead? > > if (cur->code == (BPF_LD | BPF_IMM | BPF_DW)) > cur++; Yes, I reuse double_insn in patch 5 to print the last 8 raw bytes of the instruction if "opcodes" is passed. I could make it work with your suggestion too, but would likely have to test "cur->code" a second time, I'm not sure we'd gain in readability overall. I'll keep the variable for v2.
diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c index 6fe3134ae45d..3daa05d9bbb7 100644 --- a/tools/bpf/bpftool/xlated_dumper.c +++ b/tools/bpf/bpftool/xlated_dumper.c @@ -372,8 +372,15 @@ void dump_xlated_for_graph(struct dump_data *dd, void *buf_start, void *buf_end, struct bpf_insn *insn_start = buf_start; struct bpf_insn *insn_end = buf_end; struct bpf_insn *cur = insn_start; + bool double_insn = false; for (; cur <= insn_end; cur++) { + if (double_insn) { + double_insn = false; + continue; + } + double_insn = cur->code == (BPF_LD | BPF_IMM | BPF_DW); + printf("% 4d: ", (int)(cur - insn_start + start_idx)); print_bpf_insn(&cbs, cur, true); if (cur != insn_end)
When dumping the control flow graphs for programs using the 16-byte long load instruction, we need to skip the second part of this instruction when looking for the next instruction to process. Otherwise, we end up printing "BUG_ld_00" from the kernel disassembler in the CFG. Fixes: efcef17a6d65 ("tools: bpftool: generate .dot graph from CFG information") Signed-off-by: Quentin Monnet <quentin@isovalent.com> --- tools/bpf/bpftool/xlated_dumper.c | 7 +++++++ 1 file changed, 7 insertions(+)