[PATCH] perf annotate: Fix BUG_ON in basic block processing

[PATCH] perf annotate: Fix BUG_ON in basic block processing

[Andi Kleen]

Hit with perf annotate --code-with-type --stdio

perf: util/annotate-data.c:1299: find_data_type_insn: Assertion
`!(bb->begin->al.offset == -1 || bb->end->al.offset == -1)' failed.

This makes sure to skip non instruction disasm lines in all cases that
add basic blocks. Also added some extra BUG_ONs for this case.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/annotate-data.c | 10 +++++++++-
 tools/perf/util/annotate.c      |  4 +++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index c3ffb8cad4fc..51ca86ef2677 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -986,7 +986,15 @@ static void prepend_basic_blocks(struct list_head *this_blocks,
 	}
 
 	/* Point to the insn before the last when adding this block to full_blocks */
-	last_bb->end = list_prev_entry(last_bb->end, al.node);
+	do {
+		/* Nothing useful found */
+		if (last_bb->end == last_bb->begin) {
+			list_del(&last_bb->list);
+			free(last_bb);
+			goto out;
+		}
+		last_bb->end = list_prev_entry(last_bb->end, al.node);
+	} while (last_bb->end->al.offset == -1);
 
 out:
 	list_splice(this_blocks, full_blocks);
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 18ce9f37053e..1c226f257245 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -2966,6 +2966,7 @@ static int add_basic_block(struct basic_block_data *bb_data,
 
 	if (dl == NULL)
 		return -1;
+	BUG_ON(dl->al.offset == -1);
 
 	if (!is_new_basic_block(bb_data, dl))
 		return 0;
@@ -3006,7 +3007,7 @@ static bool process_basic_block(struct basic_block_data *bb_data,
 
 	last_dl = list_last_entry(&notes->src->source,
 				  struct disasm_line, al.node);
-	if (last_dl->al.offset == -1)
+	while (last_dl && last_dl->al.offset == -1)
 		last_dl = annotation__prev_asm_line(notes, last_dl);
 
 	if (last_dl == NULL)
@@ -3052,6 +3053,7 @@ static bool process_basic_block(struct basic_block_data *bb_data,
 		break;
 
 	}
+	BUG_ON(dl->al.offset == -1);
 	link->bb->end = dl;
 	return found;
 }
-- 
2.49.0

Re: [PATCH] perf annotate: Fix BUG_ON in basic block processing

[Namhyung Kim]

On Thu, Apr 10, 2025 at 11:38:52AM -0700, Andi Kleen wrote:
> Hit with perf annotate --code-with-type --stdio
> 
> perf: util/annotate-data.c:1299: find_data_type_insn: Assertion
> `!(bb->begin->al.offset == -1 || bb->end->al.offset == -1)' failed.
> 
> This makes sure to skip non instruction disasm lines in all cases that
> add basic blocks. Also added some extra BUG_ONs for this case.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
>  tools/perf/util/annotate-data.c | 10 +++++++++-
>  tools/perf/util/annotate.c      |  4 +++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index c3ffb8cad4fc..51ca86ef2677 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -986,7 +986,15 @@ static void prepend_basic_blocks(struct list_head *this_blocks,
>  	}
>  
>  	/* Point to the insn before the last when adding this block to full_blocks */
> -	last_bb->end = list_prev_entry(last_bb->end, al.node);
> +	do {
> +		/* Nothing useful found */
> +		if (last_bb->end == last_bb->begin) {
> +			list_del(&last_bb->list);
> +			free(last_bb);
> +			goto out;
> +		}
> +		last_bb->end = list_prev_entry(last_bb->end, al.node);
> +	} while (last_bb->end->al.offset == -1);
>  
>  out:
>  	list_splice(this_blocks, full_blocks);
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 18ce9f37053e..1c226f257245 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -2966,6 +2966,7 @@ static int add_basic_block(struct basic_block_data *bb_data,
>  
>  	if (dl == NULL)
>  		return -1;
> +	BUG_ON(dl->al.offset == -1);
>  
>  	if (!is_new_basic_block(bb_data, dl))
>  		return 0;
> @@ -3006,7 +3007,7 @@ static bool process_basic_block(struct basic_block_data *bb_data,
>  
>  	last_dl = list_last_entry(&notes->src->source,
>  				  struct disasm_line, al.node);
> -	if (last_dl->al.offset == -1)
> +	while (last_dl && last_dl->al.offset == -1)
>  		last_dl = annotation__prev_asm_line(notes, last_dl);
>  
>  	if (last_dl == NULL)
> @@ -3052,6 +3053,7 @@ static bool process_basic_block(struct basic_block_data *bb_data,
>  		break;
>  
>  	}
> +	BUG_ON(dl->al.offset == -1);
>  	link->bb->end = dl;
>  	return found;
>  }
> -- 
> 2.49.0
>