diff mbox series

[Ada] Better warning on access to string at negative or null index

Message ID 20170906102728.GA116496@adacore.com
State New
Headers show
Series [Ada] Better warning on access to string at negative or null index | expand

Commit Message

Arnaud Charlet Sept. 6, 2017, 10:27 a.m. UTC
The warning issued when accessing a string at a negative or null index
was misleading, suggesting to use S'First - 1 as correct index, which
it is obviously not. Add a detection for negative or null index when
accessing a standard string, so that an appropriate warning is issued.
Also add a corresponding warning for other arrays, which is currently
not triggered by this detection mechanism under -gnatww

The following compilation shows the new warning:

     $ gcc -c cstr.adb

     1. procedure Cstr (X : in out String; J : Integer := -1) is
     2. begin
     3.    X(0 .. J) := "";
             |
        >>> warning: string index should be positive
        >>> warning: static expression fails Constraint_Check

     4.    X(0) := 'c';
             |
        >>> warning: string index should be positive
        >>> warning: static expression fails Constraint_Check

     5.    X(0 .. 4) := "hello";
             1    3
        >>> warning: string index should be positive
        >>> warning: static expression fails Constraint_Check
        >>> warning: index for "X" may assume lower bound of 1
        >>> warning: suggested replacement: "X'First + 3"

     6. end Cstr;

Tested on x86_64-pc-linux-gnu, committed on trunk

2017-09-06  Yannick Moy  <moy@adacore.com>

	* sem_warn.adb (Warn_On_Suspicious_Index): Improve warning when the
	literal index used to access a string is null or negative.
diff mbox series

Patch

Index: sem_warn.adb
===================================================================
--- sem_warn.adb	(revision 251772)
+++ sem_warn.adb	(working copy)
@@ -46,6 +46,7 @@ 
 with Snames;   use Snames;
 with Stand;    use Stand;
 with Stringt;  use Stringt;
+with Tbuild;   use Tbuild;
 with Uintp;    use Uintp;
 
 package body Sem_Warn is
@@ -3878,6 +3879,13 @@ 
          procedure Warn1;
          --  Generate first warning line
 
+         procedure Warn_On_Index_Below_Lower_Bound;
+         --  Generate a warning on indexing the array with a literal value
+         --  below the lower bound of the index type.
+
+         procedure Warn_On_Literal_Index;
+         --  Generate a warning on indexing the array with a literal value
+
          ----------------------
          -- Length_Reference --
          ----------------------
@@ -3903,21 +3911,31 @@ 
               ("?w?index for& may assume lower bound of^", X, Ent);
          end Warn1;
 
-      --  Start of processing for Test_Suspicious_Index
+         -------------------------------------
+         -- Warn_On_Index_Below_Lower_Bound --
+         -------------------------------------
 
-      begin
-         --  Nothing to do if subscript does not come from source (we don't
-         --  want to give garbage warnings on compiler expanded code, e.g. the
-         --  loops generated for slice assignments. Such junk warnings would
-         --  be placed on source constructs with no subscript in sight).
+         procedure Warn_On_Index_Below_Lower_Bound is
+         begin
+            if Is_Standard_String_Type (Typ) then
+               Discard_Node
+                 (Compile_Time_Constraint_Error
+                   (N   => X,
+                    Msg => "?w?string index should be positive"));
+            else
+               Discard_Node
+                 (Compile_Time_Constraint_Error
+                   (N   => X,
+                    Msg => "?w?index out of the allowed range"));
+            end if;
+         end Warn_On_Index_Below_Lower_Bound;
 
-         if not Comes_From_Source (Original_Node (X)) then
-            return;
-         end if;
+         ---------------------------
+         -- Warn_On_Literal_Index --
+         ---------------------------
 
-         --  Case where subscript is a constant integer
-
-         if Nkind (X) = N_Integer_Literal then
+         procedure Warn_On_Literal_Index is
+         begin
             Warn1;
 
             --  Case where original form of subscript is an integer literal
@@ -4037,7 +4055,35 @@ 
                Error_Msg_FE -- CODEFIX
                  ("\?w?suggested replacement: `&~`", Original_Node (X), Ent);
             end if;
+         end Warn_On_Literal_Index;
 
+      --  Start of processing for Test_Suspicious_Index
+
+      begin
+         --  Nothing to do if subscript does not come from source (we don't
+         --  want to give garbage warnings on compiler expanded code, e.g. the
+         --  loops generated for slice assignments. Such junk warnings would
+         --  be placed on source constructs with no subscript in sight).
+
+         if not Comes_From_Source (Original_Node (X)) then
+            return;
+         end if;
+
+         --  Case where subscript is a constant integer
+
+         if Nkind (X) = N_Integer_Literal then
+
+            --  Case where subscript is lower than the lowest possible bound.
+            --  This might be the case for example when programmers try to
+            --  access a string at index 0, as they are used to in other
+            --  programming languages like C.
+
+            if Intval (X) < Low_Bound then
+               Warn_On_Index_Below_Lower_Bound;
+            else
+               Warn_On_Literal_Index;
+            end if;
+
          --  Case where subscript is of the form X'Length
 
          elsif Length_Reference (X) then