Re: [PATCH] -fsanitize-recover=list
On Thu, Dec 18, 2014 at 05:34:12PM -0800, Alexey Samsonov wrote: Reviving this thread. What do you think of the following idea: 1) we keep -fsanitize-recover and -fno-sanitize-recover as deprecated synonyms for -f(no-)?sanitize=ubsan-like checks 2) we introduce -fsanitize-recover=list and -fno-sanitize-recover=list, where list may contain specific sanitizers (address) or group of sanitizers (undefined). 3) we introduce group of sanitizers called all, which is, well, all available sanitizers. It can *not* be used in -fsanitize=all flag, but can be used to easily disable all previously enabled sanitizers (-fno-sanitize=all), or to enable/disable recovery for all enabled sanitizers (-fsanitize-recover=all / -fno-sanitize-recover=all). So in esence a new group (like undefined) that is forbidden for -fsanitize= option, but allowed for all the others? Sounds fine to me. Can one still mix it with others (of course, it doesn't make much sense, but would mean fewer exceptions in the option parser and thus more easily understandable), like -fno-sanitize=enum,all,bool or do you prefer to only allow it as the sole argument of those options? Jakub
Re: [PATCH] -fsanitize-recover=list
Hi Jakub, On Tue, Nov 18, 2014 at 11:36 AM, Alexey Samsonov samso...@google.com wrote: On Mon, Nov 17, 2014 at 11:53 PM, Jakub Jelinek ja...@redhat.com wrote: On Mon, Nov 17, 2014 at 11:39:47PM -0800, Alexey Samsonov wrote: 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 Well, no sanitizer is on by default, so you can just use the same list for -fno-sanitize-recover= or -fsanitize-recover= as for -fsanitize= if you want. Yeah, but it may look somewhat redundant. Also, re-using the exact same list may lead to diagnostic messages (if you write -fsanitize=unreachable,null -fsanitize-recover=unreachable,null). Reviving this thread. What do you think of the following idea: 1) we keep -fsanitize-recover and -fno-sanitize-recover as deprecated synonyms for -f(no-)?sanitize=ubsan-like checks 2) we introduce -fsanitize-recover=list and -fno-sanitize-recover=list, where list may contain specific sanitizers (address) or group of sanitizers (undefined). 3) we introduce group of sanitizers called all, which is, well, all available sanitizers. It can *not* be used in -fsanitize=all flag, but can be used to easily disable all previously enabled sanitizers (-fno-sanitize=all), or to enable/disable recovery for all enabled sanitizers (-fsanitize-recover=all / -fno-sanitize-recover=all). This would be a handy shortcut, and will save users from copying the same list twice for -fsanitize= and -fsanitize-recover= flags, and from potential diagnostic problems I mention. 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. But in that case you would need to diagnose it already when parsing the option, or add code that would remember what bits have been set from other option sets. In the former case, you'd diagnose even -fsanitize-recover=unreachable -fno-sanitize-recover=undefined even when in that case unreachable in the end is not recoverable. We don't error out for -fsanitize-recover=address -fno-sanitize-recover=address because in the end address is not recovered. OK, that's a good question: should we diagnose -fsanitize-recover=address if it was later disabled by -fno-sanitize-recover=address? There is an argument for doing this: -fsanitize-recover=address is unsupported, so this flag shouldn't have been provided in the first place. It makes much more sense to delete it rather than override it. It couldn't be passed down from some global project-wide configuration (like CFLAGS), because it wouldn't work anywhere. The only case where overriding might come in handy is re-using the same list for -fsanitize= and -fsanitize-recover= flags that you mention: # SANITIZERS is a list of sanitizers we want to enable.
Re: [PATCH] -fsanitize-recover=list
On Mon, Nov 17, 2014 at 11:53 PM, Jakub Jelinek ja...@redhat.com wrote:
On Mon, Nov 17, 2014 at 11:39:47PM -0800, Alexey Samsonov wrote:
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
Well, no sanitizer is on by default, so you can just use the same
list for -fno-sanitize-recover= or -fsanitize-recover= as for -fsanitize=
if you want.
Yeah, but it may look somewhat redundant. Also, re-using the exact
same list may lead to diagnostic messages (if you write
-fsanitize=unreachable,null -fsanitize-recover=unreachable,null).
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.
But in that case you would need to diagnose it already when parsing the
option, or add code that would remember what bits have been set from other
option sets.
In the former case, you'd diagnose even
-fsanitize-recover=unreachable -fno-sanitize-recover=undefined
even when in that case unreachable in the end is not recoverable.
We don't error out for
-fsanitize-recover=address -fno-sanitize-recover=address
because in the end address is not recovered.
OK, that's a good question: should we diagnose -fsanitize-recover=address
if it was later disabled by -fno-sanitize-recover=address? There is an argument
for doing this: -fsanitize-recover=address is unsupported, so this
flag shouldn't
have been provided in the first place. It makes much more sense to
delete it rather
than override it. It couldn't be passed down from some global
project-wide configuration
(like CFLAGS), because it wouldn't work anywhere.
The only case where overriding might come in handy is re-using the same list for
-fsanitize= and -fsanitize-recover= flags that you mention:
# SANITIZERS is a list of sanitizers we want to enable.
CFLAGS = ${CFLAGS} -fsanitize=${SANITIZERS} -fsanitize-recover=${SANITIZERS}
# Oops - we produce an error if ${SANITIZERS} contains address.
However, even if we decide to *not* diagnose unrecoverable sanitizer
kinds disabled later
in the command line, we can still implement warnings for unreachable properly.
You can scan -fsanitize-recover flags from right to left and keep
track of all sanitizers that
would be disabled. When you see -fsanitize=unreachable (with
literal unreachable as a value),
you diagnose the error only if unreachable wouldn't be disabled
later by some -fno-sanitize-recover flag.
FWIW, we implement this logic in Clang for regular -fsanitize= flags.
--
Alexey Samsonov, Mountain View, CA
Re: [PATCH] -fsanitize-recover=list
Hi Jakub,
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.
* -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).
* -fno-sanitize-recover=list: Disable recovery for selected checks
or group of checks.
We maintain the set of recoverable sanitizers. Initially it contains
all UBSan checks (except for unreachable and return). We then
update this set as we
parse flags listed above, from left to right. -fsanitize-recover are
-fsanitize flags are independent. If a sanitizer is listed as
recoverable, but is not enabled via -fsanitize=,
we just ignore corresponding -fsanitize-recover flag.
Examples:
* $(CC) -fsanitize=undefined,thread -fsanitize-recover=thread //
Use UBSan and TSan, both of them are recoverable (Note: in reality,
-fsanitize-recover=thread is not implemented in Clang, this example
just illustrates intended semantics).
* $(CC) -fsanitize=undefined -fno-sanitize-recover
-fsanitize-recover=signed-integer-overflow // Recover only from
signed-integer-overflow check, all other UBSan checks are fatal.
Sounds good?
As for the generated code, I'm at the stage where I can implement the
following: if a single UBSan hander is used to report multiple error
kinds (__ubsan_handle_type_mismatch is used for
-fsanitize=null,alignment,object-size), and these kinds have different
recoverability, then we emit two handler calls like this:
$ (CC) -fsanitize=undefined -fno-sanitize-recover=alignment //
null and object-size are recoverable by default.
bool IsNonNull = ...;
bool IsAligned = ...;
bool HasCorrectSize = ...;
bool OK = IsAligned IsNonNull HasCorrectSize;
if (UNLIKELY(!OK)) {
// setup arguments for UBSan handler call
if (!IsAligned) {
__ubsan_handle_type_mismatch_abort(args);
__builtin_unreachable();
}
__ubsan_handle_type_mismatch(args);
}
// user code
On Fri, Oct 17, 2014 at 9:13 AM, Jakub Jelinek ja...@redhat.com wrote:
On Mon, Oct 13, 2014 at 02:47:07PM +0400, Yury Gribov wrote:
On 09/30/2014 09:39 PM, Jakub Jelinek wrote:
LGTM, will hack it up soon in GCC then.
Do you plan to work on this in near future?
Here is only very lightly tested patch, didn't get to updating
documentation though, plus there is no testsuite coverage for it.
Supposedly, most of the tests that use -fno-sanitize-recover
or -fsanitize-recover in dg-options should be changed to use
-fno-sanitize-recover= or -fsanitize-recover (in some cases for
the kind that is enabled with -fsanitize= only, in other cases
perhaps for something covering that and some other options),
plus perhaps some new smallish tests that test that if you e.g.
do -fsanitize=undefined -fno-sanitize-recover=divide , that
you can recover from several say out of bound shifts, but that
the first divide will terminate, etc.
Yuri or Marek, would you have spare time for that?
There is one not so nice thing, if one requests e.g.
-fsanitize=null,alignment -fno-sanitize-recover=null
-fsanitize-recover=alignment
(or vice versa), as a single call is used for both alignment and null
checks, if both are tested, it needs to pick something, the patch
right now picks recovery rather than abort if the two bits
in flag_sanitize_recover disagree. In theory, we could in that case
just use two separate calls rather than sharing one call, doing the
check and conditional branch to *_abort () first and if that wasn't true,
do the other check.
Also, the patch enables -fsanitize-recover by default for
-fsanitize-recover=undefined,float-divide-by-zero,float-cast-overflow
but not for kernel-address and doesn't check
(flag_sanitize_recover SANITIZE_KERNEL_ADDRESS) in asan.c, you'd
need to add that afterwards.
2014-10-17 Jakub Jelinek ja...@redhat.com
* common.opt (flag_sanitize_recover): New variable.
(fsanitize-recover): Remove Var/Init, deprecate.
(fsanitize-recover=): New option.
* opts.c (finish_options): Use opts-x_flag_sanitize
instead of flag_sanitize. Formatting.
(common_handle_option): Handle OPT_fsanitize_recover_
and OPT_fsanitize_recover. Use opts-x_flag_sanitize
instead of flag_sanitize.
* asan.c (pass_sanopt::execute): Fix up formatting.
* ubsan.c (ubsan_expand_bounds_ifn, ubsan_expand_null_ifn,
ubsan_expand_objsize_ifn, ubsan_build_overflow_builtin,
instrument_bool_enum_load, ubsan_instrument_float_cast,
instrument_nonnull_arg, instrument_nonnull_return): Check
bits in flag_sanitize_recover bitmask instead of
flag_sanitize_recover as bool flag.
c-family/
Re: [PATCH] -fsanitize-recover=list
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. 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 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. * -fno-sanitize-recover=list: Disable recovery for selected checks or group of checks. Jakub
Re: [PATCH] -fsanitize-recover=list
As for the generated code, I'm at the stage where I can implement the following: if a single UBSan hander is used to report multiple error kinds (__ubsan_handle_type_mismatch is used for -fsanitize=null,alignment,object-size), and these kinds have different recoverability, then we emit two handler calls like this: Nice, I think we do not yet have this in gcc. -Y
Re: [PATCH] -fsanitize-recover=list
On Mon, Nov 17, 2014 at 10:47 PM, Jakub Jelinek ja...@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
Re: [PATCH] -fsanitize-recover=list
On Mon, Nov 17, 2014 at 11:39:47PM -0800, Alexey Samsonov wrote: 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 Well, no sanitizer is on by default, so you can just use the same list for -fno-sanitize-recover= or -fsanitize-recover= as for -fsanitize= if you want. 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. But in that case you would need to diagnose it already when parsing the option, or add code that would remember what bits have been set from other option sets. In the former case, you'd diagnose even -fsanitize-recover=unreachable -fno-sanitize-recover=undefined even when in that case unreachable in the end is not recoverable. We don't error out for -fsanitize-recover=address -fno-sanitize-recover=address because in the end address is not recovered. Jakub
Re: [PATCH] -fsanitize-recover=list
On 10/17/2014 08:13 PM, Jakub Jelinek wrote: On Mon, Oct 13, 2014 at 02:47:07PM +0400, Yury Gribov wrote: On 09/30/2014 09:39 PM, Jakub Jelinek wrote: LGTM, will hack it up soon in GCC then. Do you plan to work on this in near future? Here is only very lightly tested patch, didn't get to updating documentation though, plus there is no testsuite coverage for it. Supposedly, most of the tests that use -fno-sanitize-recover or -fsanitize-recover in dg-options should be changed to use -fno-sanitize-recover= or -fsanitize-recover Here is an updated patch. I've slightly changed opts.c chunk, added docs and updated tests. (in some cases for the kind that is enabled with -fsanitize= only, in other cases perhaps for something covering that and some other options), Depending on what? I've just passed contents of -fsanitize= to -fsanitize-recover=, seems to work fine. plus perhaps some new smallish tests that test that if you e.g. do -fsanitize=undefined -fno-sanitize-recover=divide , that you can recover from several say out of bound shifts, but that the first divide will terminate, etc. Did couple of those. There is one not so nice thing, if one requests e.g. -fsanitize=null,alignment -fno-sanitize-recover=null -fsanitize-recover=alignment (or vice versa), as a single call is used for both alignment and null checks, if both are tested, it needs to pick something, the patch right now picks recovery rather than abort if the two bits in flag_sanitize_recover disagree. In theory, we could in that case just use two separate calls rather than sharing one call, doing the check and conditional branch to *_abort () first and if that wasn't true, do the other check. Either that or just issue warning for now. -Y From 956b59533590fdda49af3afb33063047b0415567 Mon Sep 17 00:00:00 2001 From: Yury Gribov y.gri...@samsung.com Date: Tue, 21 Oct 2014 17:09:37 +0400 Subject: [PATCH] New syntax for -fsanitize-recover. 2014-10-22 Jakub Jelinek ja...@redhat.com Yury Gribov y.gri...@samsung.com gcc/ * common.opt (flag_sanitize_recover): New variable. (fsanitize-recover): Remove Var/Init, deprecate. (fsanitize-recover=): New option. * doc/invoke.texi (fsanitize-recover): Update docs. * opts.c (finish_options): Use opts-x_flag_sanitize instead of flag_sanitize. Prohibit -fsanitize-recover for anything besides UBSan. Formatting. (common_handle_option): Handle OPT_fsanitize_recover_ and OPT_fsanitize_recover. Use opts-x_flag_sanitize instead of flag_sanitize. * asan.c (pass_sanopt::execute): Fix up formatting. * ubsan.c (ubsan_expand_bounds_ifn, ubsan_expand_null_ifn, ubsan_expand_objsize_ifn, ubsan_build_overflow_builtin, instrument_bool_enum_load, ubsan_instrument_float_cast, instrument_nonnull_arg, instrument_nonnull_return): Check bits in flag_sanitize_recover bitmask instead of flag_sanitize_recover as bool flag. gcc/c-family/ * c-ubsan.c (ubsan_instrument_division, ubsan_instrument_shift, ubsan_instrument_vla): Check bits in flag_sanitize_recover bitmask instead of flag_sanitize_recover as bool flag. gcc/testsuite/ * c-c++-common/ubsan/align-1.c: Updated cmdline options. * c-c++-common/ubsan/align-3.c: Likewise. * c-c++-common/ubsan/bounds-1.c: Likewise. * c-c++-common/ubsan/div-by-zero-7.c: Likewise. * c-c++-common/ubsan/float-cast-overflow-10.c: Likewise. * c-c++-common/ubsan/float-cast-overflow-7.c: Likewise. * c-c++-common/ubsan/float-cast-overflow-8.c: Likewise. * c-c++-common/ubsan/float-cast-overflow-9.c: Likewise. * c-c++-common/ubsan/nonnull-2.c: Likewise. * c-c++-common/ubsan/nonnull-3.c: Likewise. * c-c++-common/ubsan/object-size-3.c: Likewise. * c-c++-common/ubsan/overflow-1.c: Likewise. * c-c++-common/ubsan/overflow-add-1.c: Likewise. * c-c++-common/ubsan/overflow-add-3.c: Likewise. * c-c++-common/ubsan/overflow-mul-1.c: Likewise. * c-c++-common/ubsan/overflow-mul-3.c: Likewise. * c-c++-common/ubsan/overflow-negate-2.c: Likewise. * c-c++-common/ubsan/overflow-sub-1.c: Likewise. * c-c++-common/ubsan/pr59503.c: Likewise. * c-c++-common/ubsan/pr60613-1.c: Likewise. * c-c++-common/ubsan/save-expr-1.c: Likewise. * c-c++-common/ubsan/shift-3.c: Likewise. * c-c++-common/ubsan/shift-6.c: Likewise. * c-c++-common/ubsan/undefined-1.c: Likewise. * c-c++-common/ubsan/vla-2.c: Likewise. * c-c++-common/ubsan/vla-3.c: Likewise. * c-c++-common/ubsan/vla-4.c: Likewise. * g++.dg/ubsan/cxx11-shift-1.C: Likewise. * g++.dg/ubsan/return-2.C: Likewise. * c-c++-common/ubsan/recovery-1.c: New test. * c-c++-common/ubsan/recovery-2.c: New test. * c-c++-common/ubsan/recovery-3.c: New test. * c-c++-common/ubsan/recovery-common.inc: New file. --- gcc/asan.c |6 +-- gcc/c-family/c-ubsan.c |6 +-- gcc/common.opt | 12 - gcc/doc/invoke.texi| 31 +++ gcc/opts.c |
Re: [PATCH] -fsanitize-recover=list
On Wed, Oct 22, 2014 at 11:59:06AM +0400, Yury Gribov wrote: (in some cases for the kind that is enabled with -fsanitize= only, in other cases perhaps for something covering that and some other options), Depending on what? I've just passed contents of -fsanitize= to -fsanitize-recover=, seems to work fine. I meant pick randomly a few testcases where you -fno-sanitize-recover= a superset of what is specified in -fsanitize= , but it is not a big deal. 2014-10-22 Jakub Jelinek ja...@redhat.com Yury Gribov y.gri...@samsung.com gcc/ * common.opt (flag_sanitize_recover): New variable. (fsanitize-recover): Remove Var/Init, deprecate. (fsanitize-recover=): New option. * doc/invoke.texi (fsanitize-recover): Update docs. * opts.c (finish_options): Use opts-x_flag_sanitize instead of flag_sanitize. Prohibit -fsanitize-recover for anything besides UBSan. Formatting. (common_handle_option): Handle OPT_fsanitize_recover_ and OPT_fsanitize_recover. Use opts-x_flag_sanitize instead of flag_sanitize. * asan.c (pass_sanopt::execute): Fix up formatting. * ubsan.c (ubsan_expand_bounds_ifn, ubsan_expand_null_ifn, ubsan_expand_objsize_ifn, ubsan_build_overflow_builtin, instrument_bool_enum_load, ubsan_instrument_float_cast, instrument_nonnull_arg, instrument_nonnull_return): Check bits in flag_sanitize_recover bitmask instead of flag_sanitize_recover as bool flag. gcc/c-family/ * c-ubsan.c (ubsan_instrument_division, ubsan_instrument_shift, ubsan_instrument_vla): Check bits in flag_sanitize_recover bitmask instead of flag_sanitize_recover as bool flag. gcc/testsuite/ * c-c++-common/ubsan/align-1.c: Updated cmdline options. * c-c++-common/ubsan/align-3.c: Likewise. * c-c++-common/ubsan/bounds-1.c: Likewise. * c-c++-common/ubsan/div-by-zero-7.c: Likewise. * c-c++-common/ubsan/float-cast-overflow-10.c: Likewise. * c-c++-common/ubsan/float-cast-overflow-7.c: Likewise. * c-c++-common/ubsan/float-cast-overflow-8.c: Likewise. * c-c++-common/ubsan/float-cast-overflow-9.c: Likewise. * c-c++-common/ubsan/nonnull-2.c: Likewise. * c-c++-common/ubsan/nonnull-3.c: Likewise. * c-c++-common/ubsan/object-size-3.c: Likewise. * c-c++-common/ubsan/overflow-1.c: Likewise. * c-c++-common/ubsan/overflow-add-1.c: Likewise. * c-c++-common/ubsan/overflow-add-3.c: Likewise. * c-c++-common/ubsan/overflow-mul-1.c: Likewise. * c-c++-common/ubsan/overflow-mul-3.c: Likewise. * c-c++-common/ubsan/overflow-negate-2.c: Likewise. * c-c++-common/ubsan/overflow-sub-1.c: Likewise. * c-c++-common/ubsan/pr59503.c: Likewise. * c-c++-common/ubsan/pr60613-1.c: Likewise. * c-c++-common/ubsan/save-expr-1.c: Likewise. * c-c++-common/ubsan/shift-3.c: Likewise. * c-c++-common/ubsan/shift-6.c: Likewise. * c-c++-common/ubsan/undefined-1.c: Likewise. * c-c++-common/ubsan/vla-2.c: Likewise. * c-c++-common/ubsan/vla-3.c: Likewise. * c-c++-common/ubsan/vla-4.c: Likewise. * g++.dg/ubsan/cxx11-shift-1.C: Likewise. * g++.dg/ubsan/return-2.C: Likewise. * c-c++-common/ubsan/recovery-1.c: New test. * c-c++-common/ubsan/recovery-2.c: New test. * c-c++-common/ubsan/recovery-3.c: New test. * c-c++-common/ubsan/recovery-common.inc: New file. Ok, thanks. Jakub
Re: [PATCH] -fsanitize-recover=list
On 10/17/2014 08:13 PM, Jakub Jelinek wrote: On Mon, Oct 13, 2014 at 02:47:07PM +0400, Yury Gribov wrote: On 09/30/2014 09:39 PM, Jakub Jelinek wrote: LGTM, will hack it up soon in GCC then. Do you plan to work on this in near future? Here is only very lightly tested patch, didn't get to updating documentation though, plus there is no testsuite coverage for it. Supposedly, most of the tests that use -fno-sanitize-recover or -fsanitize-recover in dg-options should be changed to use -fno-sanitize-recover= or -fsanitize-recover (in some cases for the kind that is enabled with -fsanitize= only, in other cases perhaps for something covering that and some other options), plus perhaps some new smallish tests that test that if you e.g. do -fsanitize=undefined -fno-sanitize-recover=divide , that you can recover from several say out of bound shifts, but that the first divide will terminate, etc. Yuri or Marek, would you have spare time for that? I'll take on this! -Y
