From patchwork Mon Sep 2 17:49:14 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 1979710 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=J0ZBsMFQ; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.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 ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WyGVJ6jb9z1yZ9 for ; Tue, 3 Sep 2024 03:49:44 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 22236385DDED for ; Mon, 2 Sep 2024 17:49:43 +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.133.124]) by sourceware.org (Postfix) with ESMTP id 66A54385DDE5 for ; Mon, 2 Sep 2024 17:49:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 66A54385DDE5 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 66A54385DDE5 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1725299362; cv=none; b=kbHnvEtj65nScxoBrYhC/fMHNZvn1leBOXu4XpNoF9ZMrYXy6k+8tknJ2IGguerHvxOtFMLk8cKXUqaDmPNrKHybpXLm9MmZ9Tz8yCJZnW+BqJOCk+1heQrc/MVFl2nC7NK5PdrLtJE81u+9yxHH5ny9Yb9w3S4cb1J7BCqzjhY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1725299362; c=relaxed/simple; bh=zsikAhaKnKZcZM31kjHU1GGeBiM9yJEVOy/6fn/lWN4=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=LoK+vOsARoP6eej+eK+u5nd37A5A6HifVhHp5b6kwYcBE3Q8J0jGYJpBeoFdChymxLnnN9XXCHod25qj9dwAjKXd9qnMqOv5+mp7eRFwjnnjv+ZIj2XpoLfjAh8Wauh1qFu+8ojvc9GR63FFSzxXIUKDZKoNUdMfaB4xT3Hw8tc= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1725299360; 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=Cnduv5TneaL2MNiRGmnoD0m1KMt6LsGIYjRiMvmoR3E=; b=J0ZBsMFQC4n9jEQK3mDiycgE7+k7TF/glKUAoTDI5eqSBuaJDMx8LB99T2p6okRujOdD9N HhuTNSB2UQLdVWC/9V3Af9um54FirraAYST693x5ySMUyOP3qxAIusWazNOS2FgA+9KBlc QnnyQNz0gII0q7T+iJdHmt5VWivRCBI= Received: from mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-512-6ZqzO-DXNHCg90fDBTx22g-1; Mon, 02 Sep 2024 13:49:19 -0400 X-MC-Unique: 6ZqzO-DXNHCg90fDBTx22g-1 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (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 mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 2C6EF19560AB for ; Mon, 2 Sep 2024 17:49:18 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.45.224.29]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 402AA30001A4; Mon, 2 Sep 2024 17:49:17 +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 482HnELW1894215 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Mon, 2 Sep 2024 19:49:14 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 482HnEJK1894214; Mon, 2 Sep 2024 19:49:14 +0200 Date: Mon, 2 Sep 2024 19:49:14 +0200 From: Jakub Jelinek To: Jason Merrill Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] c++: Fix get_member_function_from_ptrfunc with -fsanitize=bounds [PR116449] Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-3.6 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~incoming=patchwork.ozlabs.org@gcc.gnu.org Hi! The following testcase is miscompiled, because get_member_function_from_ptrfunc emits something like (((FUNCTION.__pfn & 1) != 0) ? ptr + FUNCTION.__delta + FUNCTION.__pfn - 1 : FUNCTION.__pfn) (ptr + FUNCTION.__delta, ...) or so, so FUNCTION tree is used there 5 times. There is if (TREE_SIDE_EFFECTS (function)) function = save_expr (function); but in this case function doesn't have side-effects, just nested ARRAY_REFs. Now, if all the FUNCTION trees would be shared, it would work fine, FUNCTION is evaluated in the first operand of COND_EXPR; but unfortunately that isn't the case, both the BIT_AND_EXPR shortening and conversion to bool done for build_conditional_expr actually unshare_expr that first expression, but none of the other 4 are unshared. With -fsanitize=bounds, .UBSAN_BOUNDS calls are added to the ARRAY_REFs and use SAVE_EXPRs to avoid evaluating the argument multiple times, but because that FUNCTION tree is first used in the second argument of COND_EXPR (i.e. conditionally), the SAVE_EXPR initialization is done just there and then the third argument of COND_EXPR just uses the uninitialized temporary and so does the first argument computation as well. The following patch fixes it by also unsharing the trees that end up in the third COND_EXPR argument and unsharing what is passed as the first argument too. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Guess another possibility would be to somehow arrange for the BIT_AND_EXPR and NE_EXPR not to do unshare_expr (but that would mean build2 most likely rather than cp_build_binary_op), or force FUNCTION into a SAVE_EXPR whenever it isn't say a tcc_declaration or tcc_constant even if it doesn't have side-effects (but that would mean creating the SAVE_EXPR by hand) or create a TARGET_EXPR instead (force_target_expr). 2024-09-02 Jakub Jelinek PR c++/116449 * typeck.cc (get_member_function_from_ptrfunc): Call unshare_expr on *instance_ptrptr and e3. * g++.dg/ubsan/pr116449.C: New test. Jakub --- gcc/cp/typeck.cc.jj 2024-07-26 08:34:18.117159928 +0200 +++ gcc/cp/typeck.cc 2024-09-02 12:35:54.254380135 +0200 @@ -4255,8 +4255,11 @@ get_member_function_from_ptrfunc (tree * /* ...and then the delta in the PMF. */ instance_ptr = fold_build_pointer_plus (instance_ptr, delta); - /* Hand back the adjusted 'this' argument to our caller. */ - *instance_ptrptr = instance_ptr; + /* Hand back the adjusted 'this' argument to our caller. + The e1 computation unfortunately can result in unshare_expr + and we need to make sure the delta tree isn't first evaluated + in the COND_EXPR branch. */ + *instance_ptrptr = unshare_expr (instance_ptr); if (nonvirtual) /* Now just return the pointer. */ @@ -4283,6 +4286,9 @@ get_member_function_from_ptrfunc (tree * cp_build_addr_expr (e2, complain)); e2 = fold_convert (TREE_TYPE (e3), e2); + /* As e1 computation can result in unshare_expr, make sure e3 isn't + shared with the e2 trees. */ + e3 = unshare_expr (e3); e1 = build_conditional_expr (input_location, e1, e2, e3, complain); if (e1 == error_mark_node) return error_mark_node; --- gcc/testsuite/g++.dg/ubsan/pr116449.C.jj 2024-09-02 12:34:18.545629851 +0200 +++ gcc/testsuite/g++.dg/ubsan/pr116449.C 2024-09-02 12:31:49.070581617 +0200 @@ -0,0 +1,14 @@ +// PR c++/116449 +// { dg-do compile } +// { dg-options "-O2 -Wall -fsanitize=undefined" } + +struct C { void foo (int); void bar (); int c[16]; }; +typedef void (C::*P) (); +struct D { P d; }; +static D e[1] = { { &C::bar } }; + +void +C::foo (int x) +{ + (this->*e[c[x]].d) (); +}