From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32103 invoked by alias); 18 Nov 2014 19:36:13 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 32079 invoked by uid 89); 18 Nov 2014 19:36:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail-ob0-f182.google.com Received: from mail-ob0-f182.google.com (HELO mail-ob0-f182.google.com) (209.85.214.182) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Tue, 18 Nov 2014 19:36:09 +0000 Received: by mail-ob0-f182.google.com with SMTP id m8so405523obr.27 for ; Tue, 18 Nov 2014 11:36:08 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=lUqIMCD7l9eQl2Xxw9Ct1JtnxBXw6uWsMN6fgMOGiD8=; b=Wg7wWN9lw9elWEFIAapbq5aMnC2TZjqWe39T2eaYiW4luXqEWe5xQs0HLjsDIPoBAC xYlU1cX3TaTZaR10WpwiiRViJmDLwiROrQiNqFjoa4BVp1TacTt0paj/rAQ+uIx6sAcF 5DQKr8sNqcLbVLuT1npozqpB8TwI8qFPqTX/cZLDFe5NEiri2nBORAunDCw6a5OHi2/S xCx9tQKqI0EUzgi5jhtGef5iNprf8u43FmhS5hYjzlBP51hKPqHfQ9JioCiMaIFtYYB5 dhDAyJmTNw0h7wieLUze23iMY1+USJVTG0MlLvNjKgLwN6dtmu9LUN1xdIBxu6aCCrQM 0y1g== X-Gm-Message-State: ALoCoQka+LG2p1+fzeatHiDp1nQWTJhlC0W+3bqccczPnvn3Q5TYDuY9Sy5s8VzU1/p/Ac/DN8KT MIME-Version: 1.0 X-Received: by 10.60.160.232 with SMTP id xn8mr31485676oeb.29.1416339367855; Tue, 18 Nov 2014 11:36:07 -0800 (PST) Received: by 10.182.143.71 with HTTP; Tue, 18 Nov 2014 11:36:07 -0800 (PST) In-Reply-To: <20141118075317.GF1745@tucnak.redhat.com> References: <542A56C0.2030506@samsung.com> <20140930173340.GI1986@tucnak.redhat.com> <20140930173913.GJ1986@tucnak.redhat.com> <543BADAB.4090000@samsung.com> <20141017161334.GF10376@tucnak.redhat.com> <20141118064740.GE1745@tucnak.redhat.com> <20141118075317.GF1745@tucnak.redhat.com> Date: Tue, 18 Nov 2014 20:25:00 -0000 Message-ID: Subject: Re: [PATCH] -fsanitize-recover=list From: Alexey Samsonov To: Jakub Jelinek Cc: Yury Gribov , Marek Polacek , GCC Patches , Dmitry Vyukov , Konstantin Serebryany Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2014-11/txt/msg02350.txt.bz2 On Mon, Nov 17, 2014 at 11:53 PM, Jakub Jelinek 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=: 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,. >> 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