>>> On 04.10.17 at 15:28, <andrew.coop...@citrix.com> wrote: > On 04/10/17 13:03, Jan Beulich wrote: >>>>> On 03.10.17 at 20:07, <andrew.coop...@citrix.com> wrote: >>> --- a/xen/Kconfig >>> +++ b/xen/Kconfig >>> @@ -38,4 +38,10 @@ config LTO >>> >>> If unsure, say N. >>> >>> +# >>> +# For architectures that know their GCC __int128 support is sound >>> +# >>> +config ARCH_SUPPORTS_INT128 >>> + bool >> Why GCC? What about Clang? > > This came straight from Linux. I can s/GCC/compiler/ if you like?
Yes please (provided it's usable with Clang). >>> --- a/xen/Kconfig.debug >>> +++ b/xen/Kconfig.debug >>> @@ -121,6 +121,14 @@ config SCRUB_DEBUG >>> Verify that pages that need to be scrubbed before being allocated to >>> a guest are indeed scrubbed. >>> >>> +config UBSAN >>> + bool "Undefined behaviour sanitizer" >>> + depends on X86 >> I think we should switch away from this model of explicitly stating >> architectures, and instead have HAVE_* symbols selected by each >> architecture supporting it, and the main symbol then depending on >> the HAVE_* one. Us having only two architectures right now >> doesn't make this a big difference, but Linux has (partially?) >> switched to that model for a reason, I think. > > I'm not fussed. Which would you prefer? I'd prefer the combination HAVE_UBSAN plus UBSAN, as outlined. >>> --- a/xen/Rules.mk >>> +++ b/xen/Rules.mk >>> @@ -119,6 +119,10 @@ ifeq ($(CONFIG_GCOV),y) >>> $(filter-out %.init.o $(nogcov-y),$(obj-y) $(obj-bin-y) $(extra-y)): >>> CFLAGS += >>> -fprofile-arcs -ftest-coverage >>> endif >>> >>> +ifeq ($(CONFIG_UBSAN),y) >>> +$(filter-out %.init.o $(noubsan-y),$(obj-y) $(obj-bin-y) $(extra-y)): >>> CFLAGS += -fsanitize=undefined >>> +endif >> You have no users of noubsan-y, other than what Wei's RFC patch >> had. What about this part? >> Also neither you nor he explain why *.init.o are unilaterally >> excluded. > > The answer is complicated. If you want it to work with .init. files, > then the following change is required: > > diff --git a/xen/Rules.mk b/xen/Rules.mk > index cafc67b..9ce5b56 100644 > --- a/xen/Rules.mk > +++ b/xen/Rules.mk > @@ -189,7 +189,7 @@ $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): > %.init.o: %.o Makefile > .text|.text.*|.data|.data.*|.bss) \ > test $$sz != 0 || continue; \ > echo "Error: size of $<:$$name is 0x$$sz" >&2; \ > - exit $$(expr $$idx + 1);; \ > + # exit $$(expr $$idx + 1);; \ > esac; \ > done > $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section > .$(s)=.init.$(s)) $< $@ > > I was debating whether to keep or remove the noubsan, but I figured that > keeping it would be more flexible for developing with. Could you mention this in the commit message then, please? >>> --- a/xen/include/xen/compiler.h >>> +++ b/xen/include/xen/compiler.h >>> @@ -15,6 +15,7 @@ >>> #define noinline __attribute__((__noinline__)) >>> >>> #define noreturn __attribute__((__noreturn__)) >>> +#define __noreturn noreturn >> Please let's avoid new name space violations if at all possibly, or >> at least restrict them to individual source files where eliminating >> them would be undesirable. > > This is entirely down to how much we want to diverge the Linux code. > For ubsan, I've gone out of my way not to modify the Linux code at all. Except for the #include-s. > I can see an argument for making this local to the file in question. > However, that needs to be weighed up against other Linux source we > choose to take. As there's no reasonable chance for us to ever be able to take a Linux file completely unmodified, if putting such #define-s into individual files is too restrictive for your taste, how about making those files identify themselves (by, say, #define LINUX_SOURCE) and enabling such compatibility things only for those? Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
