From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 100293 invoked by alias); 18 Sep 2019 08:39:48 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 100230 invoked by uid 89); 18 Sep 2019 08:39:47 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.2 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=duffadacorecom, Duff, U*duff, duff@adacore.com 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 ESMTP; Wed, 18 Sep 2019 08:39:46 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id BDDF4117C77; Wed, 18 Sep 2019 04:39:44 -0400 (EDT) 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 T6qRZDvFYJwG; Wed, 18 Sep 2019 04:39:44 -0400 (EDT) Received: from tron.gnat.com (tron.gnat.com [205.232.38.10]) by rock.gnat.com (Postfix) with ESMTP id AC723117C49; Wed, 18 Sep 2019 04:39:44 -0400 (EDT) Received: by tron.gnat.com (Postfix, from userid 4862) id A842D702; Wed, 18 Sep 2019 04:39:44 -0400 (EDT) Date: Wed, 18 Sep 2019 08:40:00 -0000 From: Pierre-Marie de Rodat To: gcc-patches@gcc.gnu.org Cc: Bob Duff Subject: [Ada] Avoid uninitialized variable in bounded containers Message-ID: <20190918083944.GA145016@adacore.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="2oS5YaxWCcQjTEyO" Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes X-SW-Source: 2019-09/txt/msg01069.txt.bz2 --2oS5YaxWCcQjTEyO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 951 In function Copy in Ada.Containers.Bounded_Ordered_Sets and other bounded containers packages, remove a possible use of an uninitialized variable. This was not a bug, because the uninitialized variable could be used only if checks are suppressed, and the checks would have failed, leading to erroneous execution. However, it seems more robust this way, and is probably equally efficient, and avoids a warning that is given if checks are suppressed, and the -Wall switch is given, and optimization is turned on. Tested on x86_64-pc-linux-gnu, committed on trunk 2019-09-18 Bob Duff gcc/ada/ * libgnat/a-cbhama.adb, libgnat/a-cbhase.adb, libgnat/a-cbmutr.adb, libgnat/a-cborma.adb, libgnat/a-cborse.adb, libgnat/a-cobove.adb (Copy): Avoid reading the uninitialized variable C in the Checks = False case. Change variable to be a constant. gcc/testsuite/ * gnat.dg/containers1.adb, gnat.dg/containers1.ads: New testcase. --2oS5YaxWCcQjTEyO Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="patch.diff" Content-length: 5117 --- gcc/ada/libgnat/a-cbhama.adb +++ gcc/ada/libgnat/a-cbhama.adb @@ -262,18 +262,14 @@ package body Ada.Containers.Bounded_Hashed_Maps is Capacity : Count_Type := 0; Modulus : Hash_Type := 0) return Map is - C : Count_Type; + C : constant Count_Type := + (if Capacity = 0 then Source.Length + else Capacity); M : Hash_Type; begin - if Capacity = 0 then - C := Source.Length; - - elsif Capacity >= Source.Length then - C := Capacity; - - elsif Checks then - raise Capacity_Error with "Capacity value too small"; + if Checks and then C < Source.Length then + raise Capacity_Error with "Capacity too small"; end if; if Modulus = 0 then --- gcc/ada/libgnat/a-cbhase.adb +++ gcc/ada/libgnat/a-cbhase.adb @@ -254,16 +254,14 @@ package body Ada.Containers.Bounded_Hashed_Sets is Capacity : Count_Type := 0; Modulus : Hash_Type := 0) return Set is - C : Count_Type; + C : constant Count_Type := + (if Capacity = 0 then Source.Length + else Capacity); M : Hash_Type; begin - if Capacity = 0 then - C := Source.Length; - elsif Capacity >= Source.Length then - C := Capacity; - elsif Checks then - raise Capacity_Error with "Capacity value too small"; + if Checks and then C < Source.Length then + raise Capacity_Error with "Capacity too small"; end if; if Modulus = 0 then --- gcc/ada/libgnat/a-cbmutr.adb +++ gcc/ada/libgnat/a-cbmutr.adb @@ -625,15 +625,12 @@ package body Ada.Containers.Bounded_Multiway_Trees is (Source : Tree; Capacity : Count_Type := 0) return Tree is - C : Count_Type; - + C : constant Count_Type := + (if Capacity = 0 then Source.Count + else Capacity); begin - if Capacity = 0 then - C := Source.Count; - elsif Capacity >= Source.Count then - C := Capacity; - elsif Checks then - raise Capacity_Error with "Capacity value too small"; + if Checks and then C < Source.Count then + raise Capacity_Error with "Capacity too small"; end if; return Target : Tree (Capacity => C) do --- gcc/ada/libgnat/a-cborma.adb +++ gcc/ada/libgnat/a-cborma.adb @@ -464,17 +464,12 @@ package body Ada.Containers.Bounded_Ordered_Maps is ---------- function Copy (Source : Map; Capacity : Count_Type := 0) return Map is - C : Count_Type; - + C : constant Count_Type := + (if Capacity = 0 then Source.Length + else Capacity); begin - if Capacity = 0 then - C := Source.Length; - - elsif Capacity >= Source.Length then - C := Capacity; - - elsif Checks then - raise Capacity_Error with "Capacity value too small"; + if Checks and then C < Source.Length then + raise Capacity_Error with "Capacity too small"; end if; return Target : Map (Capacity => C) do --- gcc/ada/libgnat/a-cborse.adb +++ gcc/ada/libgnat/a-cborse.adb @@ -442,15 +442,12 @@ package body Ada.Containers.Bounded_Ordered_Sets is ---------- function Copy (Source : Set; Capacity : Count_Type := 0) return Set is - C : Count_Type; - + C : constant Count_Type := + (if Capacity = 0 then Source.Length + else Capacity); begin - if Capacity = 0 then - C := Source.Length; - elsif Capacity >= Source.Length then - C := Capacity; - elsif Checks then - raise Capacity_Error with "Capacity value too small"; + if Checks and then C < Source.Length then + raise Capacity_Error with "Capacity too small"; end if; return Target : Set (Capacity => C) do --- gcc/ada/libgnat/a-cobove.adb +++ gcc/ada/libgnat/a-cobove.adb @@ -451,18 +451,12 @@ package body Ada.Containers.Bounded_Vectors is (Source : Vector; Capacity : Count_Type := 0) return Vector is - C : Count_Type; - + C : constant Count_Type := + (if Capacity = 0 then Source.Length + else Capacity); begin - if Capacity = 0 then - C := Source.Length; - - elsif Capacity >= Source.Length then - C := Capacity; - - elsif Checks then - raise Capacity_Error - with "Requested capacity is less than Source length"; + if Checks and then C < Source.Length then + raise Capacity_Error with "Capacity too small"; end if; return Target : Vector (C) do --- /dev/null new file mode 100644 +++ gcc/testsuite/gnat.dg/containers1.adb @@ -0,0 +1,5 @@ +-- { dg-do compile } +-- { dg-options "-Wall -O2" } +package body Containers1 is + procedure Dummy is null; +end Containers1; --- /dev/null new file mode 100644 +++ gcc/testsuite/gnat.dg/containers1.ads @@ -0,0 +1,6 @@ +with Ada.Containers.Bounded_Ordered_Sets; use Ada.Containers; +package Containers1 is + pragma Suppress (All_Checks); + package Sets is new Bounded_Ordered_Sets (Boolean); + procedure Dummy; +end Containers1; \ No newline at end of file --2oS5YaxWCcQjTEyO--