From patchwork Thu Dec 7 09:28:55 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 175061 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp4655761vqy; Thu, 7 Dec 2023 01:29:24 -0800 (PST) X-Google-Smtp-Source: AGHT+IGE75LSmQssewt72VeFlUs6LgCExLYiRfAl6vjhmHJeO4QHJgx+AqDd3/F/qhutCQAIFQ4i X-Received: by 2002:a1f:ecc1:0:b0:4b3:db2:f722 with SMTP id k184-20020a1fecc1000000b004b30db2f722mr1851074vkh.27.1701941364566; Thu, 07 Dec 2023 01:29:24 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1701941364; cv=pass; d=google.com; s=arc-20160816; b=vlwHufB43Rv7HClaEU9s13cVyJtJpiCmieYT+8hsjb5ydBx+4GBzc+au2jYKIr0WuA +3egyR0E7248sH1m4VXvCCXsmUY5CDKrOv5oaq6pAr2tvWOPRJUJ04IbIzEovZD8AJnX qStWk+/5cKC+8LwPkLQLIkK5JN7bQbmQB80Ad3OGcVjiNYJQiX5IcrDx8TZONEL7Zrq5 HrwRC8C+CKGHwCeBuvEo3MH3VzNvSeu2Gkh+ElFhHn/8Q5DuxIFLy0VS8fSaMdxXwDfB EgQZ8QrdQIfZXEF2NWed4U1LQSKxDnXvqiFlfyUtCF8pk/60in++ftR0B00OViJUuwnz Doqw== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:reply-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-disposition :mime-version:message-id:subject:cc:to:from:date:dkim-signature :arc-filter:dmarc-filter:delivered-to; bh=vWAhP8O5sqvx0aP6SeAtf81MMfiCdWrXtKNtQ6IENy0=; fh=drb8jPahgz959MrupsiUi8XeCFozkYVF0F6zLW2CcMA=; b=IX1V2LIJEbZg60I0d6utJVf/6/R6uqDJOOpemd3tXETVxzfxWHzEh1uFZm1HnhnF8j ISWnXojDr6iE0leUUf8elUS8h2CENp5QTLiPXl6s43crnjhpvINxmMiu2B/5ixYysguD zzD9P7CNDKqwc0GRhI+9JXqOYnWHQ9n06xFlLemNBTayapG77wZGEBU3UdL5WNvxPTAJ QQjDpVEQKmUOX/aFx6vHBoqKo4BOZD4FddyfL/rhu1yeO1/n25QohtLTcUdBIxYEjfks 7ztWGrHgIgl6B/Uj7+7vchzMu3caoMtznCzEeN8SzB3b/CLIL6OLMqcm2GQI4lvDv5zI JPDg== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=WcItwzWq; arc=pass (i=1); spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from server2.sourceware.org (server2.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id fy27-20020a05622a5a1b00b00419695d7c82si1002782qtb.525.2023.12.07.01.29.24 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Dec 2023 01:29:24 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=WcItwzWq; arc=pass (i=1); spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 55BE9385AC19 for ; Thu, 7 Dec 2023 09:29:24 +0000 (GMT) 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.129.124]) by sourceware.org (Postfix) with ESMTPS id B19B93858D20 for ; Thu, 7 Dec 2023 09:29:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B19B93858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org B19B93858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701941342; cv=none; b=aitRC+z048ibCjK6nrifJKa19KdMmunoi1R2pX+PZBtPgRJ5hlS9ibqzTrSrFP93Xengstg0e1Y/z2br689afxgoy2/YeSfM+ADvRsI7IEvofdnCrDdaQ0FplVpt7MaGnnLCehtgQjwdqzhu0NHmigESpBObenC55z4FnenDoQo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701941342; c=relaxed/simple; bh=2KKag9bRHTn529lvzRLO0xgAwldnRlkA1NUY9PmbrQo=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=XgKAY4gR+GnQKES4l3bs6v5pGdgMbQ5xulixaQILzMK02gVfyNAcQYCk+r3xALKjV9DPejxW4pkWvs8ksma8/BVPcoYZwT/6AyQ0O4PYFAvXwg0WFGdsFc8gZs6Do39Y6zWIT3bfJUl+Njw/E/9RTWqgGSOPP8XnMumr52ErepQ= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1701941340; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type; bh=vWAhP8O5sqvx0aP6SeAtf81MMfiCdWrXtKNtQ6IENy0=; b=WcItwzWqUG2/gd+N0gefaSaUMnTTN/b2oM0xrMGSUS7ivQI8vZ3G/Q72ocpiKjadTillCY 66ldKAznZQwd/eE+5BC0brg1n1O7XktsFfTqeTvKW7i8CPxWuFXeXVKgKnoI6M3Fyjq04F yTSOD6pwb6J4EadEW2ETijito+tg618= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-615-MsVMhgDZOoWvS_42czD5fw-1; Thu, 07 Dec 2023 04:28:58 -0500 X-MC-Unique: MsVMhgDZOoWvS_42czD5fw-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (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 mimecast-mx02.redhat.com (Postfix) with ESMTPS id 97D29102F221 for ; Thu, 7 Dec 2023 09:28:58 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.195.157]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3EBE7492BC6; Thu, 7 Dec 2023 09:28:58 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 3B79StZ1162697 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 7 Dec 2023 10:28:56 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 3B79StPf162696; Thu, 7 Dec 2023 10:28:55 +0100 Date: Thu, 7 Dec 2023 10:28:55 +0100 From: Jakub Jelinek To: Jason Merrill Cc: gcc-patches@gcc.gnu.org, Marek Polacek Subject: [PATCH] c++: Unshare folded SAVE_EXPR arguments during cp_fold [PR112727] Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-3.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, 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.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Jakub Jelinek Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784614868261863073 X-GMAIL-MSGID: 1784614868261863073 Hi! The following testcase is miscompiled because two ubsan instrumentations run into each other. The first one is the shift instrumentation. Before the C++ FE calls it, it wraps the 2 shift arguments with cp_save_expr, so that side-effects in them aren't evaluated multiple times. And, ubsan_instrument_shift itself uses unshare_expr on any uses of the operands to make sure further modifications in them don't affect other copies of them (the only not unshared ones are the one the caller then uses for the actual operation after the instrumentation, which means there is no tree sharing). Now, if there are side-effects in the first operand like say function call, cp_save_expr wraps it into a SAVE_EXPR, and ubsan_instrument_shift in this mode emits something like if (..., SAVE_EXPR , SAVE_EXPR > const) __ubsan_handle_shift_out_of_bounds (..., SAVE_EXPR , ...); and caller adds SAVE_EXPR << SAVE_EXPR after it in a COMPOUND_EXPR. So far so good. If there are no side-effects and cp_save_expr doesn't create SAVE_EXPR, everything is ok as well because of the unshare_expr. We have if (..., SAVE_EXPR > const) __ubsan_handle_shift_out_of_bounds (..., ptr->something[i], ...); and ptr->something[i] << SAVE_EXPR where ptr->something[i] is unshared. In the testcase below, the !x->s[j] ? 1 : 0 expression is wrapped initially into a SAVE_EXPR though, and unshare_expr doesn't unshare SAVE_EXPRs nor anything used in them for obvious reasons, so we end up with: if (..., SAVE_EXPR (x)->s[j] ? 1 : 0>, SAVE_EXPR > const) __ubsan_handle_shift_out_of_bounds (..., SAVE_EXPR (x)->s[j] ? 1 : 0>, ...); and SAVE_EXPR (x)->s[j] ? 1 : 0> << SAVE_EXPR So far good as well. But later during cp_fold of the SAVE_EXPR we find out that VIEW_CONVERT_EXPR(x)->s[j] ? 0 : 1 is actually invariant (has TREE_READONLY set) and so cp_fold simplifies the above to if (..., SAVE_EXPR > const) __ubsan_handle_shift_out_of_bounds (..., (bool) VIEW_CONVERT_EXPR(x)->s[j] ? 0 : 1, ...); and ((bool) VIEW_CONVERT_EXPR(x)->s[j] ? 0 : 1) << SAVE_EXPR with the s[j] ARRAY_REFs and other expressions shared in between the two uses (and obviously the expression optimized away from the COMPOUND_EXPR in the if condition. Then comes another ubsan instrumentation at genericization time, this time to instrument the ARRAY_REFs with strict bounds checking, and replaces the s[j] in there with s[.UBSAN_BOUNDS (0B, SAVE_EXPR, 8), SAVE_EXPR] As the trees are shared, it does that just once though. And as the if body is gimplified first, the SAVE_EXPR is evaluated inside of the if body and when it is used again after the if, it uses a potentially uninitialized value of j.1 (always uninitialized if the shift count isn't out of bounds). The following patch fixes that by unshare_expr unsharing the folded argument of a SAVE_EXPR if we've folded the SAVE_EXPR into an invariant and it is used more than once. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2023-12-07 Jakub Jelinek PR sanitizer/112727 * cp-gimplify.cc (cp_fold): If SAVE_EXPR has been previously folded, unshare_expr what is returned. * c-c++-common/ubsan/pr112727.c: New test. Jakub --- gcc/cp/cp-gimplify.cc.jj 2023-12-05 09:06:06.112878408 +0100 +++ gcc/cp/cp-gimplify.cc 2023-12-06 22:32:46.379370223 +0100 @@ -2906,7 +2906,14 @@ cp_fold (tree x, fold_flags_t flags) fold_cache = hash_map::create_ggc (101); if (tree *cached = fold_cache->get (x)) - return *cached; + { + /* unshare_expr doesn't recurse into SAVE_EXPRs. If SAVE_EXPR's + argument has been folded into a tree invariant, make sure it is + unshared. See PR112727. */ + if (TREE_CODE (x) == SAVE_EXPR && *cached != x) + return unshare_expr (*cached); + return *cached; + } uid_sensitive_constexpr_evaluation_checker c; --- gcc/testsuite/c-c++-common/ubsan/pr112727.c.jj 2023-12-06 22:35:46.012819991 +0100 +++ gcc/testsuite/c-c++-common/ubsan/pr112727.c 2023-12-06 22:35:16.708236026 +0100 @@ -0,0 +1,17 @@ +/* PR sanitizer/112727 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fsanitize=shift-exponent,bounds-strict -Wuninitialized" } */ + +#ifndef __cplusplus +#define bool _Bool +#endif + +struct S { bool s[8]; }; + +void +foo (const struct S *x) +{ + unsigned n = 0; + for (unsigned j = 0; j < 8; j++) + n |= ((!x->s[j]) ? 1 : 0) << (16 + j); +}