Re: [PATCH v4 2/6] perf dwarf-aux: Fix libdw API contract violations

[Namhyung Kim <namhyung@kernel.org>]

On Sun, May 03, 2026 at 10:10:28AM -0700, Ian Rogers wrote:
> Check return values of `dwarf_decl_line` (where non-optional),
> `dwarf_getfuncs`, and `dwarf_lineaddr` to prevent using uninitialized
> stack variables or incorrectly reporting success on failure.
> 
> For the root DIE in `die_walk_lines()`, `dwarf_decl_line` and
> `die_get_decl_file` are optional and their failures are handled
> gracefully to avoid breaking line walking on valid functions.
> 
> Additionally:
>  - Add NULL pointer protection for `strcmp()` in `die_walk_lines()`
>    when `inf` or `decf` are NULL to prevent crashes on generated
>    code.
>  - Use `dwarf_attr_integrate` in `die_get_data_member_location` to
>    correctly resolve inherited member locations (e.g. via abstract
>    origin or specification).
> 
> Fixes: 57f95bf5f882 ("perf probe: Show correct statement line number by perf probe -l")
> Fixes: 3f4460a28fb2 ("perf probe: Filter out redundant inline-instances")
> Fixes: 75186a9b09e4 ("perf probe: Fix to show lines of sys_ functions correctly")
> Fixes: e0d153c69040 ("perf-probe: Move dwarf library routines to dwarf-aux.{c, h}")
> Fixes: 6243b9dc4c99 ("perf probe: Move dwarf specific functions to dwarf-aux.c")
> Assisted-by: Gemini-CLI:Google Gemini 3
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> v4:
>  - Fix strcmp(NULL) risk and inherited member location fallbacks in dwarf-aux.c.
> ---
>  tools/perf/util/dwarf-aux.c | 34 ++++++++++++++++------------------
>  tools/perf/util/dwarf-aux.h |  5 +++++
>  2 files changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 109a166a6d19..a8ab1c30d0ac 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -125,7 +125,8 @@ int cu_find_lineinfo(Dwarf_Die *cu_die, Dwarf_Addr addr,
>  	    && die_entrypc(&die_mem, &faddr) == 0 &&
>  	    faddr == addr) {
>  		*fname = die_get_decl_file(&die_mem);
> -		dwarf_decl_line(&die_mem, lineno);
> +		if (dwarf_decl_line(&die_mem, lineno) != 0)
> +			return -ENOENT;
>  		goto out;
>  	}
>  
> @@ -459,7 +460,7 @@ int die_get_data_member_location(Dwarf_Die *mb_die, Dwarf_Word *offs)
>  	size_t nexpr;
>  	int ret;
>  
> -	if (dwarf_attr(mb_die, DW_AT_data_member_location, &attr) == NULL)
> +	if (dwarf_attr_integrate(mb_die, DW_AT_data_member_location, &attr) == NULL)
>  		return -ENOENT;
>  
>  	if (dwarf_formudata(&attr, offs) != 0) {
> @@ -795,8 +796,7 @@ static int __die_walk_instances_cb(Dwarf_Die *inst, void *data)
>  
>  	/* Ignore redundant instances */
>  	if (dwarf_tag(inst) == DW_TAG_inlined_subroutine) {
> -		dwarf_decl_line(origin, &tmp);
> -		if (die_get_call_lineno(inst) == tmp) {
> +		if (dwarf_decl_line(origin, &tmp) == 0 && die_get_call_lineno(inst) == tmp) {
>  			tmp = die_get_decl_fileno(origin);
>  			if (die_get_call_fileno(inst) == tmp)
>  				return DIE_FIND_CB_CONTINUE;
> @@ -950,11 +950,6 @@ int die_walk_lines(Dwarf_Die *rt_die, line_walk_callback_t callback, void *data)
>  		cu_die = dwarf_diecu(rt_die, &die_mem, NULL, NULL);
>  		dwarf_decl_line(rt_die, &decl);
>  		decf = die_get_decl_file(rt_die);
> -		if (!decf) {
> -			pr_debug2("Failed to get the declared file name of %s\n",
> -				  dwarf_diename(rt_die));
> -			return -EINVAL;
> -		}

Why is this part removed?


>  	} else
>  		cu_die = rt_die;
>  	if (!cu_die) {
> @@ -998,12 +993,11 @@ int die_walk_lines(Dwarf_Die *rt_die, line_walk_callback_t callback, void *data)
>  			if (die_find_inlinefunc(rt_die, addr, &die_mem)) {
>  				/* Call-site check */
>  				inf = die_get_call_file(&die_mem);
> -				if ((inf && !strcmp(inf, decf)) &&
> +				if ((inf == decf || (inf && decf && !strcmp(inf, decf))) &&
>  				    die_get_call_lineno(&die_mem) == lineno)
>  					goto found;
>  
> -				dwarf_decl_line(&die_mem, &inl);
> -				if (inl != decl ||
> +				if (dwarf_decl_line(&die_mem, &inl) != 0 || inl != decl ||
>  				    decf != die_get_decl_file(&die_mem))
>  					continue;
>  			}
> @@ -1034,8 +1028,10 @@ int die_walk_lines(Dwarf_Die *rt_die, line_walk_callback_t callback, void *data)
>  			.data = data,
>  			.retval = 0,
>  		};
> -		dwarf_getfuncs(cu_die, __die_walk_culines_cb, &param, 0);
> -		ret = param.retval;
> +		if (dwarf_getfuncs(cu_die, __die_walk_culines_cb, &param, 0) < 0)
> +			ret = -EINVAL;
> +		else
> +			ret = param.retval;
>  	}
>  
>  	return ret;
> @@ -1939,10 +1935,12 @@ static bool die_get_postprologue_addr(unsigned long entrypc_idx,
>  			break;
>  	}
>  
> -	dwarf_lineaddr(line, postprologue_addr);
> -	if (*postprologue_addr >= highpc)
> -		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
> -			       postprologue_addr);
> +	if (dwarf_lineaddr(line, postprologue_addr) != 0)
> +		return false;
> +	if (*postprologue_addr >= highpc) {
> +		if (dwarf_lineaddr(dwarf_onesrcline(lines, i - 1), postprologue_addr) != 0)
> +			return false;
> +	}
>  
>  	return true;
>  }
> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
> index a79968a2e573..161f0bf980b6 100644
> --- a/tools/perf/util/dwarf-aux.h
> +++ b/tools/perf/util/dwarf-aux.h
> @@ -10,6 +10,11 @@
>  #include <elfutils/libdwfl.h>
>  #include <elfutils/version.h>
>  
> +static inline const char *die_name(Dwarf_Die *die)
> +{
> +	return dwarf_diename(die) ?: "<unknown>";
> +}
> +

Please move it where it's used.

Thanks,
Namhyung


>  struct strbuf;
>  
>  /* Find the realpath of the target file */
> -- 
> 2.54.0.545.g6539524ca2-goog
>