From patchwork Wed Jul 6 13:31:22 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pierre-Marie de Rodat X-Patchwork-Id: 55775 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8A66C3850863 for ; Wed, 6 Jul 2022 13:32:04 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8A66C3850863 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1657114324; bh=Yz23ZoPs1UFNDgBHJJlPkDxAsBQ2FJBu/EWv8d1YW6Q=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=rf7EpWxFRJ9YDO8adswvoQWPa/jEDOHPz5yGjNe6NOsohanYEonGFpdZmzWvQMnig DyIflnHWuRGjdyBFvSDAH4POEG9chv7ogwrGSFJ27YxzX7/B6lnzjjJFiuSnRaQmsl Hotu41m7EFGLufHYPs7MiUedXw/yS81ouAmZPIBw= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) by sourceware.org (Postfix) with ESMTPS id 70CF0385703E for ; Wed, 6 Jul 2022 13:31:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 70CF0385703E Received: by mail-wm1-x331.google.com with SMTP id u12-20020a05600c210c00b003a02b16d2b8so8999958wml.2 for ; Wed, 06 Jul 2022 06:31:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-disposition; bh=Yz23ZoPs1UFNDgBHJJlPkDxAsBQ2FJBu/EWv8d1YW6Q=; b=bPklKrrhWyfePjGbhMd9ht0OsyoR/TwkL/OhdWw/8qul8cP1Qr72zhmN8Luyl7szvZ WT9/WGfjfYkanU730FLYTKcmnJy6kCEB7BtDGzGbXQ/i2qpzfnn2brDDVxxM7W+EtH+m TS9xKgRGFUCMAlnBXjpIrdPWYD+P/nhLrPxFQ/lZjf2vyjjc9iT0Nejyn+x+amUc6KdC C+dKpxNgOTAE/6mFedN9Q5LWSMDE04K+vT7Oj0kpVMGxbwKrLMHGPylfDc8k7Eu5hM2t bFhibIEoA7ErtWhG/qXNHNblqlIcNuOmRKl6IRBwizE5Mbqjmh0/uadpuYniLiNJB2M8 bA4Q== X-Gm-Message-State: AJIora8BUx6z0F2Yjn5ZC997mSYyRPxj2gVJ5wgxnVhfBDXRxWLqVFK7 FsAVXMduiz1l31xa6aQ8IerCFMT1b6sqkQ== X-Google-Smtp-Source: AGRyM1tDdAIG3YwuoIE3gqfT5JWLwswO6g3H69rEvDaoMp5D6F2nIh5j4klVDTVqzI41fcYl5oyUPw== X-Received: by 2002:a1c:f018:0:b0:3a0:3f8d:d71e with SMTP id a24-20020a1cf018000000b003a03f8dd71emr43327777wmb.104.1657114283101; Wed, 06 Jul 2022 06:31:23 -0700 (PDT) Received: from adacore.com ([45.147.211.82]) by smtp.gmail.com with ESMTPSA id g14-20020a05600c4ece00b0039c99f61e5bsm25050741wmq.5.2022.07.06.06.31.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Jul 2022 06:31:22 -0700 (PDT) Date: Wed, 6 Jul 2022 13:31:22 +0000 To: gcc-patches@gcc.gnu.org Subject: [Ada] Fix incorrect itype sharing for case expression in limited type return Message-ID: <20220706133122.GA2202722@adacore.com> MIME-Version: 1.0 Content-Disposition: inline X-Spam-Status: No, score=-13.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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: Pierre-Marie de Rodat via Gcc-patches From: Pierre-Marie de Rodat Reply-To: Pierre-Marie de Rodat Cc: Eric Botcazou Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" The compiler aborts with an internal error in gigi, but the problem is an itype incorrectly shared between several branches of an if_statement that has been created for a Build-In-Place return. Three branches of this if_statement contain an allocator statement and the latter two have been obtained as the result of calling New_Copy_Tree on the first; now the initialization expression of the first had also been obtained as the result of calling New_Copy_Tree on the original tree, and these chained calls to New_Copy_Tree run afoul of an issue with the copy of itypes after the rewrite of an aggregate as an expression with actions. Fixing this issue looks quite delicate, so this fixes the incorrect sharing by replacing the chained calls to New_Copy_Tree with repeated calls on the original expression, which is more elegant in any case. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * exp_ch3.adb (Make_Allocator_For_BIP_Return): New local function. (Expand_N_Object_Declaration): Use it to build the three allocators for a Build-In-Place return with an unconstrained type. Update the head comment after other recent changes. diff --git a/gcc/ada/exp_ch3.adb b/gcc/ada/exp_ch3.adb --- a/gcc/ada/exp_ch3.adb +++ b/gcc/ada/exp_ch3.adb @@ -7980,16 +7980,11 @@ package body Exp_Ch3 is -- the value one, then the caller has passed access to an -- existing object for use as the return object. If the value -- is two, then the return object must be allocated on the - -- secondary stack. Otherwise, the object must be allocated in - -- a storage pool. We generate an if statement to test the - -- implicit allocation formal and initialize a local access - -- value appropriately, creating allocators in the secondary - -- stack and global heap cases. The special formal also exists - -- and must be tested when the function has a tagged result, - -- even when the result subtype is constrained, because in - -- general such functions can be called in dispatching contexts - -- and must be handled similarly to functions with a class-wide - -- result. + -- secondary stack. If the value is three, then the return + -- object must be allocated on the heap. Otherwise, the object + -- must be allocated in a storage pool. We generate an if + -- statement to test the BIP_Alloc_Form formal and initialize + -- a local access value appropriately. if Needs_BIP_Alloc_Form (Func_Id) then declare @@ -8005,6 +8000,73 @@ package body Exp_Ch3 is Pool_Id : constant Entity_Id := Make_Temporary (Loc, 'P'); + function Make_Allocator_For_BIP_Return return Node_Id; + -- Make an allocator for the BIP return being processed + + ----------------------------------- + -- Make_Allocator_For_BIP_Return -- + ----------------------------------- + + function Make_Allocator_For_BIP_Return return Node_Id is + Alloc : Node_Id; + + begin + if Present (Expr_Q) + and then not Is_Delayed_Aggregate (Expr_Q) + and then not No_Initialization (N) + then + -- Always use the type of the expression for the + -- qualified expression, rather than the result type. + -- In general we cannot always use the result type + -- for the allocator, because the expression might be + -- of a specific type, such as in the case of an + -- aggregate or even a nonlimited object when the + -- result type is a limited class-wide interface type. + + Alloc := + Make_Allocator (Loc, + Expression => + Make_Qualified_Expression (Loc, + Subtype_Mark => + New_Occurrence_Of (Etype (Expr_Q), Loc), + Expression => New_Copy_Tree (Expr_Q))); + + else + -- If the function returns a class-wide type we cannot + -- use the return type for the allocator. Instead we + -- use the type of the expression, which must be an + -- aggregate of a definite type. + + if Is_Class_Wide_Type (Ret_Obj_Typ) then + Alloc := + Make_Allocator (Loc, + Expression => + New_Occurrence_Of (Etype (Expr_Q), Loc)); + + else + Alloc := + Make_Allocator (Loc, + Expression => + New_Occurrence_Of (Ret_Obj_Typ, Loc)); + end if; + + -- If the object requires default initialization then + -- that will happen later following the elaboration of + -- the object renaming. If we don't turn it off here + -- then the object will be default initialized twice. + + Set_No_Initialization (Alloc); + end if; + + -- Set the flag indicating that the allocator came from + -- a build-in-place return statement, so we can avoid + -- adjusting the allocated object. + + Set_Alloc_For_BIP_Return (Alloc); + + return Alloc; + end Make_Allocator_For_BIP_Return; + Alloc_Obj_Id : Entity_Id; Alloc_Obj_Decl : Node_Id; Alloc_Stmt : Node_Id; @@ -8049,71 +8111,15 @@ package body Exp_Ch3 is Insert_Action (N, Alloc_Obj_Decl); - -- Create allocators for both the secondary stack and - -- global heap. If there's an initialization expression, - -- then create these as initialized allocators. - - if Present (Expr_Q) - and then not Is_Delayed_Aggregate (Expr_Q) - and then not No_Initialization (N) - then - -- Always use the type of the expression for the - -- qualified expression, rather than the result type. - -- In general we cannot always use the result type - -- for the allocator, because the expression might be - -- of a specific type, such as in the case of an - -- aggregate or even a nonlimited object when the - -- result type is a limited class-wide interface type. - - Heap_Allocator := - Make_Allocator (Loc, - Expression => - Make_Qualified_Expression (Loc, - Subtype_Mark => - New_Occurrence_Of (Etype (Expr_Q), Loc), - Expression => New_Copy_Tree (Expr_Q))); - - else - -- If the function returns a class-wide type we cannot - -- use the return type for the allocator. Instead we - -- use the type of the expression, which must be an - -- aggregate of a definite type. + -- First create the Heap_Allocator - if Is_Class_Wide_Type (Ret_Obj_Typ) then - Heap_Allocator := - Make_Allocator (Loc, - Expression => - New_Occurrence_Of (Etype (Expr_Q), Loc)); - - else - Heap_Allocator := - Make_Allocator (Loc, - Expression => - New_Occurrence_Of (Ret_Obj_Typ, Loc)); - end if; - - -- If the object requires default initialization then - -- that will happen later following the elaboration of - -- the object renaming. If we don't turn it off here - -- then the object will be default initialized twice. - - Set_No_Initialization (Heap_Allocator); - end if; - - -- Set the flag indicating that the allocator came from - -- a build-in-place return statement, so we can avoid - -- adjusting the allocated object. Note that this flag - -- will be inherited by the copies made below. - - Set_Alloc_For_BIP_Return (Heap_Allocator); + Heap_Allocator := Make_Allocator_For_BIP_Return; -- The Pool_Allocator is just like the Heap_Allocator, -- except we set Storage_Pool and Procedure_To_Call so -- it will use the user-defined storage pool. - Pool_Allocator := New_Copy_Tree (Heap_Allocator); - - pragma Assert (Alloc_For_BIP_Return (Pool_Allocator)); + Pool_Allocator := Make_Allocator_For_BIP_Return; -- Do not generate the renaming of the build-in-place -- pool parameter on ZFP because the parameter is not @@ -8154,9 +8160,7 @@ package body Exp_Ch3 is -- allocation. else - SS_Allocator := New_Copy_Tree (Heap_Allocator); - - pragma Assert (Alloc_For_BIP_Return (SS_Allocator)); + SS_Allocator := Make_Allocator_For_BIP_Return; -- The heap and pool allocators are marked as -- Comes_From_Source since they correspond to an