Re: [Xen-devel] [PATCH v2 1/2] x86/ubsan: Don't perform alignment checking on supporting compilers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.06.19 at 20:25, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -138,7 +138,10 @@ $(filter-out %.init.o $(nocov-y),$(obj-y) $(obj-bin-y) 
> $(extra-y)): CFLAGS += $(
>  endif
>  
>  ifeq ($(CONFIG_UBSAN),y)
> -$(filter-out %.init.o $(noubsan-y),$(obj-y) $(obj-bin-y) $(extra-y)): 
> CFLAGS += -fsanitize=undefined
> +UBSAN_FLAGS += -fsanitize=undefined

Here and in the x86 change below to append to UBSAN_FLAGS. I think we
have more such cases, but I also think we shouldn't extend the badness:
We should start with an empty variable, rather than whatever may have
been inherited from the environment.

Also could this become UBSAN_CFLAGS or CFLAGS_UBSAN? Or perhaps
UBSAN_CFLAGS-y / CFLAGS_UBSAN-y, making adding to it easier?

> +# Any -fno-sanitise= options need to come after any -fsanitise= options
> +$(filter-out %.init.o $(noubsan-y),$(obj-y) $(obj-bin-y) $(extra-y)):\

Could you add a blank before the backslash, for readability?

> --- a/xen/arch/x86/Rules.mk
> +++ b/xen/arch/x86/Rules.mk
> @@ -57,6 +57,10 @@ endif
>  $(call cc-option-add,CFLAGS-stack-boundary,CC,-mpreferred-stack-boundary=3)
>  CFLAGS += $(CFLAGS-stack-boundary)
>  
> +ifeq ($(CONFIG_UBSAN),y)
> +$(call cc-option-add,UBSAN_FLAGS,CC,-fno-sanitize=alignment)
> +endif

Perhaps worth adding a short comment as to the "why"? And perhaps
no need for the ifeq()?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel