Re: [PATCH bpf-next 2/5] bpftool: Fix bug for long instructions in program CFG dumps

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2023-03-24 19:54 UTC-0700 ~ Stanislav Fomichev <sdf@xxxxxxxxxx>
> 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@xxxxxxxxxxxxx>
>> ---
>>   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.



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux