Fix inline annotations in generated control-flow graphs for programs with multiple functions

[qmonnet]

The example provided by Christoph on the mailing list, for a separate issue, produces a graph with two functions:

On func_1(), on the right-hand side of the picture, we can observe that the inline annotations are not correct: they should be the annotations for the do_barf() function, but instead they duplicate the lines from handle__sched_process_exec().

There seems to be an issue in the way we print the line information for the control graph: we offset the annotation lines from the start of the program, not from the start of the function.

[qmonnet]

I've been trying the following diff and it seems to address the issue, but it needs more testing. It passes insn down so that we can compute and use the offset from the start of the program, and not the start of the function, when fetching the line information fragments with bpf_prog_linfo__lfind().

diff --git a/tools/bpf/bpftool/cfg.c b/tools/bpf/bpftool/cfg.c
index eec437cca2ea..14a61dbf2823 100644
--- a/tools/bpf/bpftool/cfg.c
+++ b/tools/bpf/bpftool/cfg.c
@@ -302,6 +302,7 @@ static bool func_add_bb_edges(struct func_node *func)
 
 		insn = bb->tail;
 		if (!is_jmp_insn(insn->code) ||
+		    BPF_OP(insn->code) == BPF_CALL ||
 		    BPF_OP(insn->code) == BPF_EXIT) {
 			e->dst = bb_next(bb);
 			e->flags |= EDGE_FLAG_FALLTHROUGH;
@@ -382,7 +383,7 @@ static void cfg_destroy(struct cfg *cfg)
 
 static void
 draw_bb_node(struct func_node *func, struct bb_node *bb, struct dump_data *dd,
-	     bool opcodes, bool linum)
+	     struct bpf_insn *insn, bool opcodes, bool linum)
 {
 	const char *shape;
 
@@ -402,7 +403,7 @@ draw_bb_node(struct func_node *func, struct bb_node *bb, struct dump_data *dd,
 		unsigned int start_idx;
 		printf("{\\\n");
 		start_idx = bb->head - func->start;
-		dump_xlated_for_graph(dd, bb->head, bb->tail, start_idx,
+		dump_xlated_for_graph(dd, bb->head, bb->tail, start_idx, insn,
 				      opcodes, linum);
 		printf("}");
 	}
@@ -431,12 +432,12 @@ static void draw_bb_succ_edges(struct func_node *func, struct bb_node *bb)
 
 static void
 func_output_bb_def(struct func_node *func, struct dump_data *dd,
-		   bool opcodes, bool linum)
+		   struct bpf_insn *insn, bool opcodes, bool linum)
 {
 	struct bb_node *bb;
 
 	list_for_each_entry(bb, &func->bbs, l) {
-		draw_bb_node(func, bb, dd, opcodes, linum);
+		draw_bb_node(func, bb, dd, insn, opcodes, linum);
 	}
 }
 
@@ -457,7 +458,7 @@ static void func_output_edges(struct func_node *func)
 }
 
 static void
-cfg_dump(struct cfg *cfg, struct dump_data *dd, bool opcodes, bool linum)
+cfg_dump(struct cfg *cfg, struct dump_data *dd, struct bpf_insn *insn, bool opcodes, bool linum)
 {
 	struct func_node *func;
 
@@ -465,7 +466,7 @@ cfg_dump(struct cfg *cfg, struct dump_data *dd, bool opcodes, bool linum)
 	list_for_each_entry(func, &cfg->funcs, l) {
 		printf("subgraph \"cluster_%d\" {\n\tstyle=\"dashed\";\n\tcolor=\"black\";\n\tlabel=\"func_%d ()\";\n",
 		       func->idx, func->idx);
-		func_output_bb_def(func, dd, opcodes, linum);
+		func_output_bb_def(func, dd, insn, opcodes, linum);
 		func_output_edges(func);
 		printf("}\n");
 	}
@@ -482,7 +483,7 @@ void dump_xlated_cfg(struct dump_data *dd, void *buf, unsigned int len,
 	if (cfg_build(&cfg, insn, len))
 		return;
 
-	cfg_dump(&cfg, dd, opcodes, linum);
+	cfg_dump(&cfg, dd, insn, opcodes, linum);
 
 	cfg_destroy(&cfg);
 }
diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
index d0094345fb2b..faaeea5f6fd4 100644
--- a/tools/bpf/bpftool/xlated_dumper.c
+++ b/tools/bpf/bpftool/xlated_dumper.c
@@ -365,7 +365,7 @@ void dump_xlated_plain(struct dump_data *dd, void *buf, unsigned int len,
 }
 
 void dump_xlated_for_graph(struct dump_data *dd, void *buf_start, void *buf_end,
-			   unsigned int start_idx,
+			   unsigned int start_idx, struct bpf_insn *insn,
 			   bool opcodes, bool linum)
 {
 	const struct bpf_insn_cbs cbs = {
@@ -385,6 +385,7 @@ void dump_xlated_for_graph(struct dump_data *dd, void *buf_start, void *buf_end,
 	char func_sig[1024];
 
 	for (; cur <= insn_end; cur++) {
+		unsigned int linfo_offset = (unsigned int)(cur - insn);
 		unsigned int insn_off;
 
 		if (double_insn) {
@@ -408,14 +409,14 @@ void dump_xlated_for_graph(struct dump_data *dd, void *buf_start, void *buf_end,
 		if (prog_linfo) {
 			const struct bpf_line_info *linfo;
 
-			linfo = bpf_prog_linfo__lfind(prog_linfo, insn_off, 0);
+			linfo = bpf_prog_linfo__lfind(prog_linfo, linfo_offset, 0);
 			if (linfo && linfo != last_linfo) {
 				btf_dump_linfo_dotlabel(btf, linfo, linum);
 				last_linfo = linfo;
 			}
 		}
 
-		printf("%u: ", insn_off);
+		printf("%u: ", linfo_offset);
 		print_bpf_insn(&cbs, cur, true);
 
 		if (opcodes) {
diff --git a/tools/bpf/bpftool/xlated_dumper.h b/tools/bpf/bpftool/xlated_dumper.h
index db3ba0671501..08acc5c33207 100644
--- a/tools/bpf/bpftool/xlated_dumper.h
+++ b/tools/bpf/bpftool/xlated_dumper.h
@@ -4,6 +4,8 @@
 #ifndef __BPF_TOOL_XLATED_DUMPER_H
 #define __BPF_TOOL_XLATED_DUMPER_H
 
+#include <linux/bpf.h>
+
 #define SYM_MAX_NAME	256
 #define MODULE_MAX_NAME	64
 
@@ -36,7 +38,7 @@ void dump_xlated_json(struct dump_data *dd, void *buf, unsigned int len,
 void dump_xlated_plain(struct dump_data *dd, void *buf, unsigned int len,
 		       bool opcodes, bool linum);
 void dump_xlated_for_graph(struct dump_data *dd, void *buf, void *buf_end,
-			   unsigned int start_index,
+			   unsigned int start_index, struct bpf_insn *insn,
 			   bool opcodes, bool linum);
 
 #endif

[troglodyt]

Have also run some quick tests / read through code.
Looks good / nothing unexpected so far 👍