From patchwork Thu Nov 12 10:59:43 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnaud Charlet X-Patchwork-Id: 543306 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id AEA641402B7 for ; Thu, 12 Nov 2015 21:59:58 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=ipcMoEE7; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:mime-version:content-type; q=dns; s=default; b=AtWRgc9ab9g0iW5+ilezp8QAlpo1fbltxKSXcIzieZcLRMK2se 0pf7AOEANOJfRa8RmlV0Y+t7TxOCYRK49SZ/jcN8CEcfm4PIYfyORrP8KB6PkY1K +9zrxgEvdAkLfqR2GvG/3pcJPY9BH2FOVRqk09zC05UvsUM6LV7Zt+xSo= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:mime-version:content-type; s= default; bh=ysUMt3Xq+NuGVBragbyStlDDAhM=; b=ipcMoEE7geIj8Bwf3QcF h2gcAamiKzAIP+RIVrHpMMjbt5EIMeBm1iWzd39uHLfc2Jhkj4rSNtiSmmWUxFYY WWjAvwKpTk/zSvla/aVpMO4wFSwFJ1PBaRffxwsK+8yi40ojkOironjn8X9is5Wv c4FuvFsDU08gyHSsoGanGSU= Received: (qmail 112877 invoked by alias); 12 Nov 2015 10:59:47 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 112866 invoked by uid 89); 12 Nov 2015 10:59:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.3 required=5.0 tests=AWL, BAYES_50, KAM_ASCII_DIVIDERS, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW, T_FILL_THIS_FORM_SHORT autolearn=no version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 12 Nov 2015 10:59:45 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 624F429824; Thu, 12 Nov 2015 05:59:43 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id fHSynIplPrGR; Thu, 12 Nov 2015 05:59:43 -0500 (EST) Received: from tron.gnat.com (tron.gnat.com [205.232.38.10]) by rock.gnat.com (Postfix) with ESMTP id 490072981D; Thu, 12 Nov 2015 05:59:43 -0500 (EST) Received: by tron.gnat.com (Postfix, from userid 4192) id 45FBA29E; Thu, 12 Nov 2015 05:59:43 -0500 (EST) Date: Thu, 12 Nov 2015 05:59:43 -0500 From: Arnaud Charlet To: gcc-patches@gcc.gnu.org Cc: Eric Botcazou Subject: [Ada] Warn when a non-imported constant overlays a constant Message-ID: <20151112105943.GA102663@adacore.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) The compiler warns when a variable overlays a constant because of an address clause on the former. This change makes the compiler issue the same warning when a non-imported constant overlays a constant. The patch also removes an old pessimization whereby overlaid objects would be treated as volatile by the compiler in some circumstances, for example preventing them from being put into read-only memory if they are constant. The compiler must issue the warning: consovl3.adb:4:03: warning: constant "C" may be modified via address clause at line 5 on the followig code: with Q; use Q; procedure Consovl3 is A : constant Natural := 0; for A'Address use C'Address; begin null; end; package Q is C : constant Natural := 1; end Q; Tested on x86_64-pc-linux-gnu, committed on trunk 2015-11-12 Eric Botcazou * einfo.ads (Overlays_Constant): Document usage for E_Constant. * freeze.adb (Warn_Overlay): Small reformatting. (Check_Address_Clause): Deal specifically with deferred constants. For a variable or a non-imported constant overlaying a constant object and with initialization value, either remove the initialization or issue a warning. Fix a couple of typos. * sem_util.adb (Note_Possible_Modification): Overhaul the condition for the warning on modified constants and use Find_Overlaid_Entity instead of doing it manually. * sem_ch13.adb (Analyze_Attribute_Definition_Clause): Compute and set Overlays_Constant once on entry. Do not treat the overlaid entity as volatile. Do not issue the warning on modified constants here. * gcc-interface/decl.c (gnat_to_gnu_entity) : Remove over-restrictive condition for the special treatment of deferred constants. : Remove obsolete associated code. Index: einfo.ads =================================================================== --- einfo.ads (revision 230223) +++ einfo.ads (working copy) @@ -3638,8 +3638,9 @@ -- Points to the component in the base type. -- Overlays_Constant (Flag243) --- Defined in all entities. Set only for a variable for which there is --- an address clause which causes the variable to overlay a constant. +-- Defined in all entities. Set only for E_Constant or E_Variable for +-- which there is an address clause which causes the entity to overlay +-- a constant object. -- Overridden_Operation (Node26) -- Defined in subprograms. For overriding operations, points to the Index: freeze.adb =================================================================== --- freeze.adb (revision 230223) +++ freeze.adb (working copy) @@ -207,10 +207,7 @@ -- this to have a Freeze_Node, so ensure it doesn't. Do the same for any -- Full_View or Corresponding_Record_Type. - procedure Warn_Overlay - (Expr : Node_Id; - Typ : Entity_Id; - Nam : Node_Id); + procedure Warn_Overlay (Expr : Node_Id; Typ : Entity_Id; Nam : Node_Id); -- Expr is the expression for an address clause for entity Nam whose type -- is Typ. If Typ has a default initialization, and there is no explicit -- initialization in the source declaration, check whether the address @@ -598,16 +595,25 @@ -------------------------- procedure Check_Address_Clause (E : Entity_Id) is - Addr : constant Node_Id := Address_Clause (E); + Addr : constant Node_Id := Address_Clause (E); + Typ : constant Entity_Id := Etype (E); + Decl : Node_Id; Expr : Node_Id; - Decl : constant Node_Id := Declaration_Node (E); - Loc : constant Source_Ptr := Sloc (Decl); - Typ : constant Entity_Id := Etype (E); + Init : Node_Id; Lhs : Node_Id; Tag_Assign : Node_Id; begin if Present (Addr) then + + -- For a deferred constant, the initialization value is on full view + + if Ekind (E) = E_Constant and then Present (Full_View (E)) then + Decl := Declaration_Node (Full_View (E)); + else + Decl := Declaration_Node (E); + end if; + Expr := Expression (Addr); if Needs_Constant_Address (Decl, Typ) then @@ -656,29 +662,72 @@ Warn_Overlay (Expr, Typ, Name (Addr)); end if; - if Present (Expression (Decl)) then + Init := Expression (Decl); + -- If a variable, or a non-imported constant, overlays a constant + -- object and has an initialization value, then the initialization + -- may end up writing into read-only memory. Detect the cases of + -- statically identical values and remove the initialization. In + -- the other cases, give a warning. We will give other warnings + -- later for the variable if it is assigned. + + if (Ekind (E) = E_Variable + or else (Ekind (E) = E_Constant + and then not Is_Imported (E))) + and then Overlays_Constant (E) + and then Present (Init) + then + declare + O_Ent : Entity_Id; + Off : Boolean; + begin + Find_Overlaid_Entity (Addr, O_Ent, Off); + + if Ekind (O_Ent) = E_Constant + and then Etype (O_Ent) = Typ + and then Present (Constant_Value (O_Ent)) + and then Compile_Time_Compare ( + Init, + Constant_Value (O_Ent), + Assume_Valid => True) = EQ + then + Set_No_Initialization (Decl); + return; + + elsif Comes_From_Source (Init) + and then Address_Clause_Overlay_Warnings + then + Error_Msg_Sloc := Sloc (Addr); + Error_Msg_NE + ("??constant& may be modified via address clause#", + Decl, O_Ent); + end if; + end; + end if; + + if Present (Init) then + -- Capture initialization value at point of declaration, -- and make explicit assignment legal, because object may -- be a constant. - Remove_Side_Effects (Expression (Decl)); - Lhs := New_Occurrence_Of (E, Loc); + Remove_Side_Effects (Init); + Lhs := New_Occurrence_Of (E, Sloc (Decl)); Set_Assignment_OK (Lhs); - -- Move initialization to freeze actions (once the object has - -- been frozen, and the address clause alignment check has been + -- Move initialization to freeze actions, once the object has + -- been frozen and the address clause alignment check has been -- performed. Append_Freeze_Action (E, - Make_Assignment_Statement (Loc, + Make_Assignment_Statement (Sloc (Decl), Name => Lhs, Expression => Expression (Decl))); Set_No_Initialization (Decl); -- If the objet is tagged, check whether the tag must be - -- reassigned expliitly. + -- reassigned explicitly. Tag_Assign := Make_Tag_Assignment (Decl); if Present (Tag_Assign) then @@ -8128,11 +8177,7 @@ -- Warn_Overlay -- ------------------ - procedure Warn_Overlay - (Expr : Node_Id; - Typ : Entity_Id; - Nam : Entity_Id) - is + procedure Warn_Overlay (Expr : Node_Id; Typ : Entity_Id; Nam : Entity_Id) is Ent : constant Entity_Id := Entity (Nam); -- The object to which the address clause applies Index: sem_util.adb =================================================================== --- sem_util.adb (revision 230223) +++ sem_util.adb (working copy) @@ -16258,27 +16258,22 @@ -- If we are sure this is a modification from source, and we know -- this modifies a constant, then give an appropriate warning. - if Overlays_Constant (Ent) - and then (Modification_Comes_From_Source and Sure) + if Sure + and then Modification_Comes_From_Source + and then Overlays_Constant (Ent) + and then Address_Clause_Overlay_Warnings then declare - A : constant Node_Id := Address_Clause (Ent); + Addr : constant Node_Id := Address_Clause (Ent); + O_Ent : Entity_Id; + Off : Boolean; begin - if Present (A) then - declare - Exp : constant Node_Id := Expression (A); - begin - if Nkind (Exp) = N_Attribute_Reference - and then Attribute_Name (Exp) = Name_Address - and then Is_Entity_Name (Prefix (Exp)) - then - Error_Msg_Sloc := Sloc (A); - Error_Msg_NE - ("constant& may be modified via address " - & "clause#??", N, Entity (Prefix (Exp))); - end if; - end; - end if; + Find_Overlaid_Entity (Addr, O_Ent, Off); + + Error_Msg_Sloc := Sloc (Addr); + Error_Msg_NE + ("??constant& may be modified via address clause#", + N, O_Ent); end; end if; Index: sem_ch13.adb =================================================================== --- sem_ch13.adb (revision 230225) +++ sem_ch13.adb (working copy) @@ -4724,6 +4724,12 @@ Find_Overlaid_Entity (N, O_Ent, Off); + -- If the object overlays a constant view, mark it so + + if Present (O_Ent) and then Is_Constant_Object (O_Ent) then + Set_Overlays_Constant (U_Ent); + end if; + -- Overlaying controlled objects is erroneous. -- Emit warning but continue analysis because program is -- itself legal, and back-end must see address clause. @@ -4743,12 +4749,12 @@ -- Issue an unconditional warning for a constant overlaying -- a variable. For the reverse case, we will issue it only - -- if the variable is modified, see below. + -- if the variable is modified. - elsif Address_Clause_Overlay_Warnings + elsif Ekind (U_Ent) = E_Constant and then Present (O_Ent) - and then Ekind (U_Ent) = E_Constant - and then not Is_Constant_Object (O_Ent) + and then not Overlays_Constant (U_Ent) + and then Address_Clause_Overlay_Warnings then Error_Msg_N ("??constant overlays a variable", Expr); @@ -4767,34 +4773,6 @@ Note_Possible_Modification (Nam, Sure => False); - -- Here we are checking for explicit overlap of one variable - -- by another, and if we find this then mark the overlapped - -- variable as also being volatile to prevent unwanted - -- optimizations. This is a significant pessimization so - -- avoid it when there is an offset, i.e. when the object - -- is composite; they cannot be optimized easily anyway. - - if Present (O_Ent) - and then Is_Object (O_Ent) - and then not Off - - -- The following test is an expedient solution to what - -- is really a problem in CodePeer. Suppressing the - -- Set_Treat_As_Volatile call here prevents later - -- generation (in some cases) of trees that CodePeer - -- should, but currently does not, handle correctly. - -- This test should probably be removed when CodePeer - -- is improved, just because we want the tree CodePeer - -- analyzes to match the tree for which we generate code - -- as closely as is practical. ??? - - and then not CodePeer_Mode - then - -- ??? O_Ent might not be in current unit - - Set_Treat_As_Volatile (O_Ent); - end if; - -- Legality checks on the address clause for initialized -- objects is deferred until the freeze point, because -- a subsequent pragma might indicate that the object @@ -4867,39 +4845,12 @@ -- Furthermore, by removing the test, we handle the -- aspect case properly. - if Address_Clause_Overlay_Warnings - and then Present (O_Ent) + if Present (O_Ent) and then Is_Object (O_Ent) + and then not Is_Generic_Type (Etype (U_Ent)) + and then Address_Clause_Overlay_Warnings then - if not Is_Generic_Type (Etype (U_Ent)) then - Address_Clause_Checks.Append ((N, U_Ent, O_Ent, Off)); - end if; - - -- If variable overlays a constant view, and we are - -- warning on overlays, then mark the variable as - -- overlaying a constant and warn immediately if it - -- is initialized. We will give other warnings later - -- if the variable is assigned. - - if Is_Constant_Object (O_Ent) - and then Ekind (U_Ent) = E_Variable - then - declare - Init : constant Node_Id := - Expression (Declaration_Node (U_Ent)); - begin - Set_Overlays_Constant (U_Ent); - - if Present (Init) - and then Comes_From_Source (Init) - then - Error_Msg_Sloc := Sloc (N); - Error_Msg_NE - ("??constant& may be modified via address " - & "clause#", Declaration_Node (U_Ent), O_Ent); - end if; - end; - end if; + Address_Clause_Checks.Append ((N, U_Ent, O_Ent, Off)); end if; end; Index: gcc-interface/decl.c =================================================================== --- gcc-interface/decl.c (revision 230227) +++ gcc-interface/decl.c (working copy) @@ -506,7 +506,6 @@ /* Ignore constant definitions already marked with the error node. See the N_Object_Declaration case of gnat_to_gnu for the rationale. */ if (definition - && gnu_expr && present_gnu_tree (gnat_entity) && get_gnu_tree (gnat_entity) == error_mark_node) { @@ -1186,13 +1185,6 @@ } } - /* If this is a deferred constant, the initializer is attached to - the full view. */ - if (kind == E_Constant && Present (Full_View (gnat_entity))) - gnu_expr - = gnat_to_gnu - (Expression (Declaration_Node (Full_View (gnat_entity)))); - /* If we don't have an initializing expression for the underlying variable, the initializing expression for the pointer is the specified address. Otherwise, we have to make a COMPOUND_EXPR