From patchwork Wed Sep 18 08:39:44 2019 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: 1163832 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-509170-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="ffoQBZDa"; dkim-atps=neutral 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 46YD2S6dklz9s4Y for ; Wed, 18 Sep 2019 18:40:32 +1000 (AEST) 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=Qh76aVt3jM6JCWfvkJ1HB7GaxVfYdrHvYsgn0dH2jL9xK+OUm6 HbPWJHKOUZlMWwKGx5BDYskdM3Rr0iFLZ14cfOakpdsKUHslJjWJUhJB+dA1pVDw NgqWcQj7Mpb6vrjt9KZA5VyM0I9+TZ471x/fni02GTmdUaZdC3xu245p4= 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=CY81dgmRoV/9jCZCztFieV7Wgm0=; b=ffoQBZDaRet4Ib5WyyY0 knTpk94qOKvztD4G2DLJY/PegmmI17t6ABDJm0pXHsQHUU0A8wwvhgXCLwbt+xHq 9Dh5Aj7EZn9BHh0nK+6GzxXs1uXqfpEU4csChuijrNnhBw3l07vXQPW60ugA6D75 Fe085mn7Q8lIFj58LaF3A+8= 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-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 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 04:39:44 -0400 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-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes 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. --- 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