Re: [PATCH v2] lib: drop (replace) debug_build()

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 23/12/2020 16:05, Jan Beulich wrote:
> Its expansion shouldn't be tied to NDEBUG - down the road we may want to
> allow enabling assertions independently of CONFIG_DEBUG. Replace the few
> uses by a new xen_build_info() helper, subsuming gcov_string at the same
> time (while replacing the stale CONFIG_GCOV used there) and also adding
> CONFIG_UBSAN indication.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
For the Arm code:

Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

> ---
> v2: Introduce xen_build_info() including also gcov and ubsan info.
>
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -175,14 +175,14 @@ static void print_xen_info(void)
>   {
>       char taint_str[TAINT_STRING_MAX_LEN];
>   
> -    printk("----[ Xen-%d.%d%s  %s  debug=%c " gcov_string "  %s ]----\n",
> +    printk("----[ Xen-%d.%d%s  %s  %s  %s ]----\n",
>              xen_major_version(), xen_minor_version(), xen_extra_version(),
>   #ifdef CONFIG_ARM_32
>              "arm32",
>   #else
>              "arm64",
>   #endif
> -           debug_build() ? 'y' : 'n', print_tainted(taint_str));
> +           xen_build_info(), print_tainted(taint_str));
>   }
>   
>   #ifdef CONFIG_ARM_32
> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -29,9 +29,9 @@ static void print_xen_info(void)
>   {
>       char taint_str[TAINT_STRING_MAX_LEN];
>   
> -    printk("----[ Xen-%d.%d%s  x86_64  debug=%c " gcov_string "  %s ]----\n",
> +    printk("----[ Xen-%d.%d%s  x86_64  %s  %s ]----\n",
>              xen_major_version(), xen_minor_version(), xen_extra_version(),
> -           debug_build() ? 'y' : 'n', print_tainted(taint_str));
> +           xen_build_info(), print_tainted(taint_str));
>   }
>   
>   enum context { CTXT_hypervisor, CTXT_pv_guest, CTXT_hvm_guest };
> --- a/xen/common/version.c
> +++ b/xen/common/version.c
> @@ -70,6 +70,30 @@ const char *xen_deny(void)
>       return "<denied>";
>   }
>   
> +static const char build_info[] =
> +    "debug="
> +#ifdef CONFIG_DEBUG
> +    "y"
> +#else
> +    "n"
> +#endif
> +#ifdef CONFIG_COVERAGE
> +# ifdef __clang__
> +    " llvmcov=y"
> +# else
> +    " gcov=y"
> +# endif
> +#endif
> +#ifdef CONFIG_UBSAN
> +    " ubsan=y"
> +#endif
> +    "";
> +
> +const char *xen_build_info(void)
> +{
> +    return build_info;
> +}
> +
>   static const void *build_id_p __read_mostly;
>   static unsigned int build_id_len __read_mostly;
>   
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -1002,10 +1002,10 @@ void __init console_init_preirq(void)
>       spin_lock(&console_lock);
>       __putstr(xen_banner());
>       spin_unlock(&console_lock);
> -    printk("Xen version %d.%d%s (%s@%s) (%s) debug=%c " gcov_string " %s\n",
> +    printk("Xen version %d.%d%s (%s@%s) (%s) %s %s\n",
>              xen_major_version(), xen_minor_version(), xen_extra_version(),
> -           xen_compile_by(), xen_compile_domain(),
> -           xen_compiler(), debug_build() ? 'y' : 'n', xen_compile_date());
> +           xen_compile_by(), xen_compile_domain(), xen_compiler(),
> +           xen_build_info(), xen_compile_date());
>       printk("Latest ChangeSet: %s\n", xen_changeset());
>   
>       /* Locate and print the buildid, if applicable. */
> --- a/xen/include/xen/lib.h
> +++ b/xen/include/xen/lib.h
> @@ -48,21 +48,13 @@
>   #define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond))
>   #endif
>   
> -#ifdef CONFIG_GCOV
> -#define gcov_string "gcov=y"
> -#else
> -#define gcov_string ""
> -#endif
> -
>   #ifndef NDEBUG
>   #define ASSERT(p) \
>       do { if ( unlikely(!(p)) ) assert_failed(#p); } while (0)
>   #define ASSERT_UNREACHABLE() assert_failed("unreachable")
> -#define debug_build() 1
>   #else
>   #define ASSERT(p) do { if ( 0 && (p) ) {} } while (0)
>   #define ASSERT_UNREACHABLE() do { } while (0)
> -#define debug_build() 0
>   #endif
>   
>   #define ABS(_x) ({                              \
> --- a/xen/include/xen/version.h
> +++ b/xen/include/xen/version.h
> @@ -16,6 +16,7 @@ const char *xen_extra_version(void);
>   const char *xen_changeset(void);
>   const char *xen_banner(void);
>   const char *xen_deny(void);
> +const char *xen_build_info(void);
>   int xen_build_id(const void **p, unsigned int *len);
>   
>   #ifdef BUILD_ID
--
Julien Grall