public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Pierre-Marie de Rodat <derodat@adacore.com>
To: gcc-patches@gcc.gnu.org
Cc: Bob Duff <duff@adacore.com>
Subject: [Ada] Avoid uninitialized variable in bounded containers
Date: Wed, 18 Sep 2019 08:40:00 -0000	[thread overview]
Message-ID: <20190918083944.GA145016@adacore.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 951 bytes --]

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  <duff@adacore.com>

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.

[-- Attachment #2: patch.diff --]
[-- Type: text/x-diff, Size: 5117 bytes --]

--- 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


                 reply	other threads:[~2019-09-18  8:39 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190918083944.GA145016@adacore.com \
    --to=derodat@adacore.com \
    --cc=duff@adacore.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).