Re: [PATCH][PING] Enable -fsanitize-recover for KASan

[Jakub Jelinek <jakub@redhat.com>]

On Mon, Sep 15, 2014 at 01:38:42PM +0400, Yury Gribov wrote:
> --- a/gcc/builtins.def
> +++ b/gcc/builtins.def
> @@ -176,7 +176,7 @@ along with GCC; see the file COPYING3.  If not see
>    DEF_BUILTIN (ENUM, "__builtin_" NAME, BUILT_IN_NORMAL, TYPE, TYPE,    \
>  	       true, true, true, ATTRS, true, \
>  	      (flag_sanitize & (SANITIZE_ADDRESS | SANITIZE_THREAD \
> -				| SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT)))
> +				| SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT)))

This is too long line after the change.

> --- a/gcc/gcc.c
> +++ b/gcc/gcc.c
> @@ -8236,7 +8236,7 @@ sanitize_spec_function (int argc, const char **argv)
>    if (strcmp (argv[0], "thread") == 0)
>      return (flag_sanitize & SANITIZE_THREAD) ? "" : NULL;
>    if (strcmp (argv[0], "undefined") == 0)
> -    return ((flag_sanitize & (SANITIZE_UNDEFINED | SANITIZE_NONDEFAULT))
> +    return ((flag_sanitize & (SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT))

Likewise.

> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -1551,6 +1551,12 @@ common_handle_option (struct gcc_options *opts,
>  			     | SANITIZE_RETURNS_NONNULL_ATTRIBUTE))
>  	  opts->x_flag_delete_null_pointer_checks = 0;
>  
> +	/* UBSan and KASan enable recovery by default.  */
> +	opts->x_flag_sanitize_recover
> +	  = !!(flag_sanitize & (SANITIZE_UNDEFINED
> +				| SANITIZE_UNDEFINED_NONDEFAULT
> +				| SANITIZE_KERNEL_ADDRESS));
> +

Doesn't this override even user supplied -fsanitize-recover or
-fno-sanitize-recover ?  Have you tried both
-fno-sanitize-recover -fsanitize=kernel-address
and
-fsanitize=kernel-address -fno-sanitize-recover
option orders?

Seems for -fdelete-null-pointer-checks we got it wrong too,
IMHO for -fsanitize={null,{,returns-}nonnull-attribute,undefined}
we want to disable it unconditionally, regardless of whether
that option appears on the command line or not.
And we handle it right for 
-fdelete-null-pointer-checks -fsanitize=undefined
but not for
-fsanitize=undefined -fdelete-null-pointer-checks
Joseph, thoughts where to override it instead (I mean, after all
options are processed)?

In the -fsanitize-recover case, I'd on the other side think that
it should just override the default and not override explicit
user's decision.  Which could be done here, but supposedly guarded
with if (!opts_set->x_flag_sanitize_recover)?

I don't think your proposal will work properly though,
if one compiles with
-fsanitize=undefined -fsanitize=address
you'll just get userland asan with error recovery, which is highly
undesirable (not just that it changes the behavior from how it
behaved before, but especially because libasan doesn't contain
such entrypoints at all).
-fsanitize=undefined,address
or
-fsanitize=address,undefined
is normal supported mode and thus I think you either can't reuse
-fsanitize-recover option for what you want to do, or
asan.c needs to limit it to flag_sanitize & SANITIZE_KERNEL_ADDRESS
mode only.  Depends if you ever want to add recovery for userland
sanitization.

	Jakub