[PATCH] -fsanitize-recover=list

[Alexey Samsonov]

On Mon, Nov 17, 2014 at 10:47 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Mon, Nov 17, 2014 at 06:40:00PM -0800, Alexey Samsonov wrote:
> > I've just prepared a patch implementing -fsanitize-recover=<list> in
> > Clang (http://reviews.llvm.org/D6302), writing here to make sure we're
> > on
> > the same page w.r.t. flag semantics:
> >
> > * -fsanitize-recover: Enable recovery for all checks enabled by
> > -fsanitize= which support it.
> > * -fno-sanitize-recover: Disable recovery for all checks.
>
> That is not what I think we've agreed on and what is implemented in GCC.
> -fsanitize-recover only enables recovery of the undefined plus
> undefined-like sanitizers, in particular it doesn't enable recover from
> kernel-address, because -fsanitize-recover should be a deprecated option
> and kernel-address never used it before.

Hm, indeed, I messed it up. In the older thread we agree that plain
-f(no-)sanitize-recover
should be a deprecated synonym for the current meaning of these flags,
which only specify recoverability for UBSan-related checks :-/

Has GCC already shipped with existing implementation? I'm just curious
if it's convenient to have flags that would enable/disable recovery for all
sanitizers (analogous to -Werror flags which exist in a standalone form, but
may accept specific warnings, so that one can write
  "$(CC) foo.cc -Wno-error -Werror=sign-compare"

> So, in GCC -fsanitize-recover stands for
> -fsanitize-recover=undefined,float-divide-by-zero,float-cast-overflow
> and -fno-sanitize-recover stands for
> -fno-sanitize-recover=undefined,float-divide-by-zero,float-cast-overflow
>
> > * -fsanitize-recover=<list>: Enable recovery for all selected checks
> > or group of checks. It is forbidden to list unrecoverable sanitizers
> > here (e.g., "-fsanitize-recover=address" will produce an error).
>
> We only error on
> -fsanitize-recover=address
> -fsanitize-recover=thread
> -fsanitize-recover=leak

Right, it's a good idea to error on sanitizer kinds we don't have
recovery for. I will
add the errors for TSan/MSan/LSan etc.

> but not say on
> -fsanitize-recover=unreachable
> which is part of undefined; unreachable isn't recoverable silently.
> Likewise -fsanitize-recover=return.  Otherwise one couldn't use
> -fsanitize-recover=undefined which is useful.

Can't this be fixed by checking the actual values of -fsanitize-recover= flag
before expanding the sanitizer groups (i.e. turning "undefined" into
"null,unreachable,return,....")?

We should probably be able to distinguish between -fsanitize-recover=undefined,
and -fsanitize-recover=unreachable,<another checks from undefined>.
In the first case we can enable recovery
for all parts of "undefined" that support it. In the second, we can
produce an error as user
explicitly tried to enable recovery for sanitizer that doesn't support it.

This is only a diagnostic quality issue, though.

>
> > * -fno-sanitize-recover=<list>: Disable recovery for selected checks
> > or group of checks.
>
>         Jakub

-- 
Alexey Samsonov, Mountain View, CA