Re: [PATCH v3 03/14] configure, build: make valgrind optional

[Kris Van Hees <kris.van.hees@oracle.com>]

On Wed, Oct 30, 2024 at 12:12:25PM +0000, Nick Alcock wrote:
> We fail building if <valgrind/valgrind.h> is not available, which is
> ridiculous given that the only reason we need it is to make valgrind go away
> at suitable moments to let us drop uprobes.
> 
> A suitable new configure check (using a new check-header-macro-rule
> function) lets us check for <valgrind/valgrind.h> and disable it if not
> present: as usual, defining HAVE_VALGRIND or passing it to configure will
> also suffice to override the check.
> 
> Bug: https://github.com/oracle/dtrace-utils/issues/80
> Signed-off-by: Nick Alcock <nick.alcock@oracle.com>

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

> ---
>  Makeconfig          | 36 ++++++++++++++++++++++++++++++++++++
>  configure           |  1 +
>  libdtrace/dt_work.c | 16 ++++++++++++++--
>  3 files changed, 51 insertions(+), 2 deletions(-)
> 
> diff --git a/Makeconfig b/Makeconfig
> index 346078598624..752bd79b01fc 100644
> --- a/Makeconfig
> +++ b/Makeconfig
> @@ -163,6 +163,41 @@ $(CONFIG_H): $(objdir)/.config/config.$(1).h
>  $(CONFIG_MK): $(objdir)/.config/config.$(1).mk
>  endef
>  
> +# Generate a makefile rule to check for the presence of MACRO
> +# in HEADER and emit an appropriate header file fragment into a file
> +# under $(objdir)/.config.
> +#
> +# The first argument must be suitable for a filename fragment,
> +# for a makefile rule name and for a #define.
> +#
> +# Syntax: $(call check-header-macro-rule,name,macro,header)
> +define check-header-macro-rule
> +$(objdir)/.config/config.$(1).h $(objdir)/.config/config.$(1).mk: $(objdir)/.config/.dir.stamp
> +	case x$(HAVE_$(1)) in \
> +	xyes) echo '#define HAVE_$(1) 1' > $(objdir)/.config/config.$(1).h; \
> +	     echo 'HAVE_$(1)=y' > $(objdir)/.config/config.$(1).mk;; \
> +	xno) echo '/* #undef HAVE_$(1) */' > $(objdir)/.config/config.$(1).h; \
> +	     echo '# HAVE_$(1) undefined' > $(objdir)/.config/config.$(1).mk;; \
> +	*) if printf '#include <%s.h>\n#ifndef %s\n#error %s not found.\n#endif' "$(3)" "$(2)" "$(2)" | \
> +	       $(CC) $(filter-out --coverage,$(CFLAGS) $(LDFLAGS)) -Iinclude -D_GNU_SOURCE -Werror=implicit-function-declaration -c -o /dev/null -x c - >/dev/null 2>&1; then \
> +	       echo '#define HAVE_$(1) 1' > $(objdir)/.config/config.$(1).h; \
> +	       echo 'HAVE_$(1)=y' > $(objdir)/.config/config.$(1).mk; \
> +	   else \
> +	       echo '/* #undef HAVE_$(1) */' > $(objdir)/.config/config.$(1).h; \
> +	       echo '# HAVE_$(1) undefined' > $(objdir)/.config/config.$(1).mk; \
> +	   fi;; \
> +	*) echo "HAVE_$(1) must be yes or no, not $(HAVE_$(1))" >&2; \
> +	   false;; \
> +	esac
> +	rm -f $(CONFIG_H)
> +	rm -f $(CONFIG_MK)
> +
> +$(eval $(call make-override-help,HAVE_$(1), presence of preprocessor macro $(2) in $(3).h))
> +
> +$(CONFIG_H): $(objdir)/.config/config.$(1).h
> +$(CONFIG_MK): $(objdir)/.config/config.$(1).mk
> +endef
> +
>  # Generate a makefile rule to check for support for OPTION in BPFC and emit an
>  # appropriate header file fragment into a file under $(objdir)/.config.
>  #
> @@ -229,6 +264,7 @@ $(eval $(call check-header-rule,DIS1,disassembler(NULL),disasm))
>  $(eval $(call check-header-rule,DIS4,disassembler(0,0,0,NULL),disasm))
>  $(eval $(call check-header-rule,INITDISINFO3,init_disassemble_info(NULL,NULL,NULL),disasm))
>  $(eval $(call check-header-rule,INITDISINFO4,init_disassemble_info(NULL,NULL,NULL,NULL),disasm))
> +$(eval $(call check-header-macro-rule,VALGRIND,VALGRIND_NON_SIMD_CALL0,valgrind/valgrind))
>  $(eval $(call check-bpfc-option-rule,BPFV3,-mcpu=v3))
>  $(eval $(call check-bpfc-option-rule,BPFMASM,-masm=normal))
>  
> diff --git a/configure b/configure
> index c44c77383dd0..8cb09942703b 100755
> --- a/configure
> +++ b/configure
> @@ -169,6 +169,7 @@ for option in "$@"; do
>          HAVE_FUSE_NUMA=*) write_config_var FUSE_NUMA "$option";;
>          HAVE_CLOSE_RANGE=*) write_config_var CLOSE_RANGE "$option";;
>          HAVE_GETTID=*) write_config_var GETTID "$option";;
> +        HAVE_VALGRIND=*) write_config_var VALGRIND "$option";;
>          HAVE_BPFV3=*) write_config_var BPFV3 "$option";;
>          HAVE_BPFMASM=*) write_config_var BPFMASM "$option";;
>          *) echo "Unknown option $option" >&2
> diff --git a/libdtrace/dt_work.c b/libdtrace/dt_work.c
> index 11335345a6d7..fd57b59513f1 100644
> --- a/libdtrace/dt_work.c
> +++ b/libdtrace/dt_work.c
> @@ -13,7 +13,9 @@
>  #include <port.h>
>  #include <linux/perf_event.h>
>  #include <sys/epoll.h>
> +#ifdef HAVE_VALGRIND
>  #include <valgrind/valgrind.h>
> +#endif
>  #include <dt_impl.h>
>  #include <dt_aggregate.h>
>  #include <dt_peb.h>
> @@ -187,9 +189,11 @@ beginend_child(void *arg) {
>  	read(args->tochild[0], &cmd, sizeof(cmd));
>  	if (cmd != CMD_BEGIN)
>  		exit(1);
> +#ifdef HAVE_VALGRIND
>  	if (RUNNING_ON_VALGRIND)
>  		VALGRIND_NON_SIMD_CALL0(BEGIN_probe);
>  	else
> +#endif
>  		BEGIN_probe();
>  	cmd++;
>  	write(args->frchild[1], &cmd, sizeof(cmd));
> @@ -198,9 +202,11 @@ beginend_child(void *arg) {
>  	read(args->tochild[0], &cmd, sizeof(cmd));
>  	if (cmd != CMD_END)
>  		exit(1);
> +#ifdef HAVE_VALGRIND
>  	if (RUNNING_ON_VALGRIND)
>  		VALGRIND_NON_SIMD_CALL0(END_probe);
>  	else
> +#endif
>  		END_probe();
>  	cmd++;
>  	write(args->frchild[1], &cmd, sizeof(cmd));
> @@ -300,9 +306,12 @@ dtrace_go(dtrace_hdl_t *dtp, uint_t cflags)
>  		}
>  		if (cmd != CMD_BEGIN + 1)
>  			return -1;
> -	} else if (RUNNING_ON_VALGRIND)
> +	}
> +#ifdef HAVE_VALGRIND
> +	else if (RUNNING_ON_VALGRIND)
>  		VALGRIND_NON_SIMD_CALL0(BEGIN_probe);
>  	else
> +#endif
>  		BEGIN_probe();
>  
>  	dtp->dt_active = 1;
> @@ -344,9 +353,12 @@ dtrace_stop(dtrace_hdl_t *dtp)
>  			return -1;
>  		pthread_join(args->thr, NULL);
>  		dt_free(dtp, args);
> -	} else if (RUNNING_ON_VALGRIND)
> +	}
> +#ifdef HAVE_VALGRIND
> +	else if (RUNNING_ON_VALGRIND)
>  		VALGRIND_NON_SIMD_CALL0(END_probe);
>  	else
> +#endif
>  		END_probe();
>  
>  	dtp->dt_stopped = 1;
> -- 
> 2.46.0.278.g36e3a12567
>