Re: [Xen-devel] [PATCH RFC] xen: add undefined behaviour sanitizer

[Julien Grall <julien.grall@xxxxxxx>]

Hi Wei,

On 03/10/17 17:03, Wei Liu wrote:
> 1. Introduce Kconfig UBSAN option.
> 2. Import and adapt a bunch of hooks from Linux kernel with as little
>     modification as possible.
> 3. Provide mechanism to filter out unwanted files.
>
> This is x86 only at the moment. It should be easy to make it work on
> arm, but I don't have the test environment to figure out what files
> need to be filtered out.
Sadly, it is not trivial on Arm to get UBSAN running. It is not because 
of UBSAN itself but because of the memory layout. We assume that Xen is 
always smaller than 2MB (assembly code included :/).
With UBSAN enabled, Xen binary go way beyond 2MB and will fail to build.

I actually have a development branch with a prototype for UBSAN for Arm 
but I haven't found time to upstream it. I will rebase it on your work.
> Currently two files are filtered out:
>
> 1. arch/x86/setup.o -- xen crashes very early without any output
> 2. drivers/acpi/tables/tbutils.o -- it has requirement to access
>     misaligned memory before current is properly set up.
>
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
>
> Though this is not going to be in 4.10 I think it would be valuable to
> fix the issues it reports.
>
> And example output on x86:
> https://paste.debian.net/988890/
I have got some for Arm too. I will try to post the fixes for Xen 4.10.

[...]

> diff --git a/xen/common/ubsan/ubsan.c b/xen/common/ubsan/ubsan.c
> new file mode 100644
> index 0000000000..aa359e712c
> --- /dev/null
> +++ b/xen/common/ubsan/ubsan.c
> @@ -0,0 +1,486 @@
> +/*
> + * UBSAN error reporting functions
> + *
> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> + * Author: Andrey Ryabinin <ryabinin.a.a@xxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Adapt to Xen by:
> + *  Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> + *  Wei Liu <wei.liu2@xxxxxxxxxx>
> + */
> +
> +#include <xen/bitops.h>
> +#include <xen/kernel.h>
> +#include <xen/lib.h>
> +#include <xen/types.h>
> +#include <xen/sched.h>
> +#include <xen/spinlock.h>
> +#include <xen/percpu.h>
> +
> +#define pr_err(...) printk(XENLOG_ERR __VA_ARGS__)
> +#define dump_stack dump_execution_state
> +#define __noreturn noreturn
> +#define CONFIG_ARCH_SUPPORTS_INT128
I believe this should go in arch specific code. For instance, Arm does 
not support INT128.
> +#define u64 long long unsigned int
> +#define s64 long long int
Would not be better to have a typedef for those 2 and use uint64_t and 
int64_t?
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel