[DTrace-devel] [PATCH 3/4] consume: ustack: handle errors from dt_Pobjname() better

[Eugene Loh]

Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
Out of curiosity, could the failure be triggered intentionally? That is, 
does it make sense to introduce a test that fails without this patch but 
passes with it?

On 3/4/24 13:47, Nick Alcock via DTrace-devel wrote:
> dt_Pobjname(), which returns the name of the ELF object corresponding to a
> specific address, can fail, for instance if the process it's being asked
> about is dead.  One call to it in dt_print_ustack() was unguarded, so if the
> process died at the wrong instant you could get output looking like this:
>
>                     FUNCTION:NAME
>                  exit_group:entry
>                `<E9><EF>^W`_Exit+0x1d
>
> because dt_Pobjname() returned an undetected error and now you're printing
> whatever junk was on the stack at the time.
>
> Fix trivial.  (Everything else in this function is already properly guarded.
> This instance has been unguarded since the Solaris days.)
>
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> ---
>   libdtrace/dt_consume.c | 22 ++++++++++++----------
>   1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
> index 9a616e0780bcf..dec2314b5ad9a 100644
> --- a/libdtrace/dt_consume.c
> +++ b/libdtrace/dt_consume.c
> @@ -1223,16 +1223,18 @@ dt_print_ustack(dtrace_hdl_t *dtp, FILE *fp, const char *format,
>   
>   		} else if (pid >= 0 && dt_Plookup_by_addr(dtp, pid, pc[i],
>   							  &name, &sym) == 0) {
> -			dt_Pobjname(dtp, pid, pc[i], objname, sizeof(objname));
> -
> -			if (pc[i] > sym.st_value)
> -				snprintf(c, sizeof(c), "%s`%s+0x%llx",
> -					 dt_basename(objname), name,
> -					 (unsigned long long)(pc[i] - sym.st_value));
> -			else
> -				snprintf(c, sizeof(c), "%s`%s",
> -					 dt_basename(objname), name);
> -
> +			if (dt_Pobjname(dtp, pid, pc[i], objname,
> +					sizeof(objname)) != NULL) {
> +				if (pc[i] > sym.st_value)
> +					snprintf(c, sizeof(c), "%s`%s+0x%llx",
> +						 dt_basename(objname), name,
> +						 (unsigned long long)(pc[i] - sym.st_value));
> +				else
> +					snprintf(c, sizeof(c), "%s`%s",
> +						 dt_basename(objname), name);
> +			} else
> +				snprintf(c, sizeof(c), "0x%llx",
> +				    (unsigned long long)pc[i]);
>   			/* Allocated by Plookup_by_addr. */
>   			free((char *)name);
>   		} else if (str != NULL && str[0] != '\0' && str[0] != '@' &&