[DTrace-devel] [PATCH v2] build: add gdb and valgrind support to the run-dtrace wrapper

[Kris Van Hees]

On Wed, May 17, 2023 at 08:47:22PM +0100, Nick Alcock via DTrace-devel wrote:
> Now, rather than having to mess around setting SYSLIBDIR and
> LD_LIBRARY_PATH yourself to debug a just-built dtrace, you can
> just run DTRACE_GDB=t build/run-dtrace or
> DTRACE_VALGRIND=t build/run-dtrace
> 
> GDB is run with --args. Valgrind is run with no args other that the
> just-built dtrace's for now.
> 
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

... but ...

> ---
>  cmd/Build | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/cmd/Build b/cmd/Build
> index 644a3449d2ffc..be1514410d641 100644
> --- a/cmd/Build
> +++ b/cmd/Build
> @@ -27,16 +27,24 @@ bpf_dump_LIBS = -lelf
>  
>  $(objdir)/run-dtrace: $(DTRACE)
>  	printf > $@ "\
> -#!/bin/sh\n\
> +#!/bin/bash\n\

I still think /usr/bin/bash is the more appropriate pathname, given that quite
often /bin is a symbolic link to /usr/bin, so the canonical location is
/usr/bin.  Obviously, almost always both will work, but I think it is better
practice to refer to the actual installed location rather than to the location
that works because the distro provides that symbolic link.

But I'll let it stay as /bin/bash because the same (IMHO wrong) practice has
been used in many other scripts.

>  # run-dtrace -- A wrapper to run DTrace out of the build tree, or from /usr/sbin\n\
>  #               if this is a check-installed run\n\
>  \n\
> -if [ -z \$$_DTRACE_CHECK_INSTALLED ]; then\n\
> +if [[ -z \$$_DTRACE_CHECK_INSTALLED ]]; then\n\
>      export DTRACE_OPT_SYSLIBDIR=$(objdir)/dlibs\n\
>      export LD_LIBRARY_PATH=$(objdir)\n\
> -    exec $(objdir)/dtrace \"\$$@\"\n\
> +    dtrace_cmd=$(objdir)/dtrace\n\
>  else\n\
> -    exec /usr/sbin/dtrace \"\$$@\"\n\
> +    dtrace_cmd=/usr/sbin/dtrace\n\
> +fi\n\
> +\n\
> +if [[ -n \$$DTRACE_GDB ]]; then\n\
> +    exec gdb --args \$$dtrace_cmd \"\$$@\"\n\
> +elif [[ -n \$$DTRACE_VALGRIND ]]; then\n\
> +     exec valgrind \$$dtrace_cmd \"\$$@\"\n\

Here (and the remainder indented lines) are indented 5 spaces instead of 4
as above?  I'm changing this to be 4 spaces as I integrate the patch :)

> +else\n\
> +     exec \$$dtrace_cmd \"\$$@\"\n\
>  fi\n"
>  	chmod a+x $(objdir)/run-dtrace
>  
> 
> base-commit: f543fa6706c0b31364356c01bf3de63e3cce8ad1
> prerequisite-patch-id: fa667248b7b02e92ee6b3807123ebcf2ac0acc38
> prerequisite-patch-id: 0cec5f1a31d528a493d6143644c1e6b78c7f403b
> -- 
> 2.39.1.268.g9de2f9a303
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel