From patchwork Fri Jun 17 21:20:02 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Merrill X-Patchwork-Id: 1644949 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=IzQMt4ob; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4LPsQR6gzdz9sFw for ; Sat, 18 Jun 2022 07:20:31 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 16F933857B8E for ; Fri, 17 Jun 2022 21:20:28 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 16F933857B8E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1655500828; bh=cuQ7ZcA1fYSJdWvVO95uyliFTUxQj76ceH2Fow8zJAo=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=IzQMt4obQaGdENLR2Wmn5KPGktZWd4rLNB5hT90xgRNxbBX4GC6etmafsYyPfszsx k0yPTI0UgUngSM7XG+e00I8VKakOloeaYq9u75qsHtvfQPEiUgJ77k10CR7lRSElGU nQQsGfwWqrFffpIhUGKZtcX2yrmi3hB2r2lu0cpE= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 0D1763858434 for ; Fri, 17 Jun 2022 21:20:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0D1763858434 Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-142-1jQZ-3AJNFC_PFNZ0_Si_A-1; Fri, 17 Jun 2022 17:20:05 -0400 X-MC-Unique: 1jQZ-3AJNFC_PFNZ0_Si_A-1 Received: by mail-qv1-f71.google.com with SMTP id s11-20020a0562140cab00b0046e7d2b24b3so5385246qvs.16 for ; Fri, 17 Jun 2022 14:20:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=cuQ7ZcA1fYSJdWvVO95uyliFTUxQj76ceH2Fow8zJAo=; b=WtFmERjX57b01OiVR5Umso3owlHz2YCbg5oDm4ohPgxp4uJCTdUm921Mlans+CpIO+ keWKxQ0PET8V6JkSCoea6HBB5zkrIiu9NET6mdbquxYRG0CnpG06kYPy+nmWoYjxWqvW ceYffq8VHOARc3end2NiWS1bhdII+J+OOKCPTOedzowyZtP0yMJZ1v7Ev07LfiHq8fhz BJvgO2qPeQltJGIYrfTtdQ0cpNeGuiNgggvc+IpPtXRvS9mUAHO55/OWTUzo+2rb7aFQ 6ILGD/X80/dHG/iVHTlZz72ql1JNrdZE9xmuhskSyJBUDK1tn4kSLmEbYaT0Cw4MMDam BNgw== X-Gm-Message-State: AJIora9fWh5IbUYX/XSClT6degVF6HdGeWqtdWmWQ+4ShhJvC8g0HQMS 5TpMV5MX4FnBMQZCn3ChxZdFN3UlPGgim8S9pBi/0oFnoRpJudbim+g5opYxkvTq1kA552HejP/ tmQO980Hmbx4qqnOiraTvJ5f62d8o9prSvQQzHQp9u/21PotN8HF+7HeooNL6kwXEkw== X-Received: by 2002:a05:6214:20c8:b0:470:2b1e:99b with SMTP id 8-20020a05621420c800b004702b1e099bmr3109705qve.111.1655500804859; Fri, 17 Jun 2022 14:20:04 -0700 (PDT) X-Google-Smtp-Source: AGRyM1sOtsJWqG1b+kfBagK8+YQWQkfyr5EsiH/OnmtvsdUCungNtb9DvYU9LEA73xnHUaeBi/Q/8w== X-Received: by 2002:a05:6214:20c8:b0:470:2b1e:99b with SMTP id 8-20020a05621420c800b004702b1e099bmr3109684qve.111.1655500804456; Fri, 17 Jun 2022 14:20:04 -0700 (PDT) Received: from barrymore.redhat.com (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id u7-20020a05622a198700b002f90a33c78csm5291820qtc.67.2022.06.17.14.20.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 17 Jun 2022 14:20:04 -0700 (PDT) To: gcc-patches@gcc.gnu.org, jakub@redhat.com Subject: [PATCH RFA] ubsan: do return check with -fsanitize=unreachable Date: Fri, 17 Jun 2022 17:20:02 -0400 Message-Id: <20220617212002.3747825-1-jason@redhat.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-13.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jason Merrill via Gcc-patches From: Jason Merrill Reply-To: Jason Merrill Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" Related to PR104642, the current situation where we get less return checking with just -fsanitize=unreachable than no sanitize flags seems undesirable; I propose that we do return checking when -fsanitize=unreachable. Looks like clang just traps on missing return if not -fsanitize=return, but the approach in this patch seems more helpful to me if we're already sanitizing other should-be-unreachable code. I'm assuming that the difference in treatment of SANITIZE_UNREACHABLE and SANITIZE_RETURN with regard to loop optimization is deliberate. This assumes Jakub's -fsanitize-trap patch. gcc/ChangeLog: * doc/invoke.texi: Note that -fsanitize=unreachable implies -fsanitize=return. * opts.cc (finish_options): Make that so. gcc/cp/ChangeLog: * cp-gimplify.cc (cp_maybe_instrument_return): Remove return vs. unreachable handling. gcc/testsuite/ChangeLog: * g++.dg/ubsan/return-8c.C: New test. --- gcc/doc/invoke.texi | 2 ++ gcc/cp/cp-gimplify.cc | 12 ------------ gcc/opts.cc | 10 ++++++++++ gcc/testsuite/g++.dg/ubsan/return-8c.C | 15 +++++++++++++++ 4 files changed, 27 insertions(+), 12 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ubsan/return-8c.C base-commit: 0f96ac43fa0a5fdbfce317b274233852d5b46d23 prerequisite-patch-id: fa35013a253eae78fe744794172aeed26fe6f166 diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 50f57877477..e572158a1ba 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -15946,6 +15946,8 @@ built with this option turned on will issue an error message when the end of a non-void function is reached without actually returning a value. This option works in C++ only. +This check is also enabled by -fsanitize=unreachable. + @item -fsanitize=signed-integer-overflow @opindex fsanitize=signed-integer-overflow This option enables signed integer overflow checking. We check that diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc index 6f84d157c98..5c2eb61842c 100644 --- a/gcc/cp/cp-gimplify.cc +++ b/gcc/cp/cp-gimplify.cc @@ -1806,18 +1806,6 @@ cp_maybe_instrument_return (tree fndecl) || !targetm.warn_func_return (fndecl)) return; - if (!sanitize_flags_p (SANITIZE_RETURN, fndecl) - /* Don't add __builtin_unreachable () if not optimizing, it will not - improve any optimizations in that case, just break UB code. - Don't add it if -fsanitize=unreachable -fno-sanitize=return either, - UBSan covers this with ubsan_instrument_return above where sufficient - information is provided, while the __builtin_unreachable () below - if return sanitization is disabled will just result in hard to - understand runtime error without location. */ - && (!optimize - || sanitize_flags_p (SANITIZE_UNREACHABLE, fndecl))) - return; - tree t = DECL_SAVED_TREE (fndecl); while (t) { diff --git a/gcc/opts.cc b/gcc/opts.cc index 062782ac700..a7b02b0f693 100644 --- a/gcc/opts.cc +++ b/gcc/opts.cc @@ -1254,6 +1254,16 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, if (opts->x_flag_sanitize & ~(SANITIZE_LEAK | SANITIZE_UNREACHABLE)) opts->x_flag_aggressive_loop_optimizations = 0; + /* -fsanitize=unreachable implies -fsanitize=return, but without affecting + aggressive loop optimizations. */ + if ((opts->x_flag_sanitize & (SANITIZE_UNREACHABLE | SANITIZE_RETURN)) + == SANITIZE_UNREACHABLE) + { + opts->x_flag_sanitize |= SANITIZE_RETURN; + if (opts->x_flag_sanitize_trap & SANITIZE_UNREACHABLE) + opts->x_flag_sanitize_trap |= SANITIZE_RETURN; + } + /* Enable -fsanitize-address-use-after-scope if either address sanitizer is enabled. */ if (opts->x_flag_sanitize diff --git a/gcc/testsuite/g++.dg/ubsan/return-8c.C b/gcc/testsuite/g++.dg/ubsan/return-8c.C new file mode 100644 index 00000000000..a67f086d452 --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/return-8c.C @@ -0,0 +1,15 @@ +// PR c++/104642 + +// -fsanitize=unreachable should imply -fsanitize=return. + +// { dg-do run } +// { dg-shouldfail { *-*-* } } +// { dg-additional-options "-O -fsanitize=unreachable" } + +bool b; + +int f() { + if (b) return 42; +} // { dg-warning "-Wreturn-type" } + +int main() { f(); }