[committed] d: Call the assertp and boundsp variants for assert and array contract failures.

Iain Buclaw ibuclaw@gdcproject.org
Sun Aug 29 16:34:06 GMT 2021


Hi,

This patch updates the code generator to call _d_assertp or
_d_arrayboundsp when an assert or array bounds check fails respectively.
These functions accept an `immutable(char)*` instead of an
`immutable(char)[]`.  The subtle difference being that the length of the
string no longer has to be included in the generated code.

Bootstrapped and regression tested on x86_64-linux-gnu/-m32/-mx32,
committed to mainline.

Regards,
Iain

---
gcc/d/ChangeLog:

	* d-codegen.cc: Include dmd/module.h.
	(build_filename_from_loc): New function.
	(d_assert_call): Rename to...
	(build_assert_call): ...this.
	(build_array_bounds_call): Call arrayboundsp variant of the array
	bounds failure callback.
	(build_bounds_condition): Rename to...
	(build_bounds_index_condition): ...this.  Update signature.
	(build_bounds_slice_condition): New function.
	(checkaction_trap_p): New function.
	(d_assert_call): Call assertp variant of assert failure callback.
	* d-tree.h (class IndexExp): Declare.
	(class SliceExp): Declare.
	(build_bounds_condition): Remove.
	(build_assert_call): Declare.
	(build_bounds_index_condition): Declare.
	(build_bounds_slice_condition): Declare.
	(checkaction_trap_p): Declare.
	(d_assert_call): Remove.
	* expr.cc (ExprVisitor::visit(IndexExp *)): Call
	build_bounds_index_condition.
	(ExprVisitor::visit(SliceExp *)): Call build_bounds_slice_condition.
	(ExprVisitor::visit(AssertExp *)): Update setting of libcall.
	* runtime.cc (enum d_libcall_type): Add LCT_IMMUTABLE_CHARPTR.
	(get_libcall_type): Handle LCT_IMMUTABLE_CHARPTR.
	* runtime.def (ASSERT): Rename to...
	(ASSERTP): ...this.  Update signature.
	(UNITTEST): Rename to...
	(UNITTESTP): ...this.  Update signature.
	(ARRAY_BOUNDS): Rename to...
	(ARRAYBOUNDSP): ...this.  Updates signature.
	* toir.cc (IRVisitor::visit(SwitchErrorStatement *)): Update call.
---
 gcc/d/d-codegen.cc | 185 ++++++++++++++++++++++++++++++++++-----------
 gcc/d/d-tree.h     |   8 +-
 gcc/d/expr.cc      |  58 ++------------
 gcc/d/runtime.cc   |   5 ++
 gcc/d/runtime.def  |  24 +++---
 gcc/d/toir.cc      |   2 +-
 6 files changed, 173 insertions(+), 109 deletions(-)

diff --git a/gcc/d/d-codegen.cc b/gcc/d/d-codegen.cc
index ad20bd15403..e63365055d3 100644
--- a/gcc/d/d-codegen.cc
+++ b/gcc/d/d-codegen.cc
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "dmd/ctfe.h"
 #include "dmd/declaration.h"
 #include "dmd/identifier.h"
+#include "dmd/module.h"
 #include "dmd/target.h"
 #include "dmd/template.h"
 
@@ -1831,50 +1832,149 @@ void_okay_p (tree t)
   return t;
 }
 
-/* Builds a CALL_EXPR at location LOC in the source file to execute when an
-   array bounds check fails.  */
+/* Builds a STRING_CST representing the filename of location LOC.  When the
+   location is not valid, the name of the source module is used instead.  */
+
+static tree
+build_filename_from_loc (const Loc &loc)
+{
+  const char *filename = loc.filename
+    ? loc.filename : d_function_chain->module->srcfile->toChars ();
+
+  unsigned length = strlen (filename);
+  tree str = build_string (length, filename);
+  TREE_TYPE (str) = make_array_type (Type::tchar, length + 1);
+
+  return build_address (str);
+}
+
+/* Builds a CALL_EXPR at location LOC in the source file to call LIBCALL when
+   an assert check fails.  When calling the msg variant functions, MSG is the
+   error message supplied by the user.  */
 
 tree
-build_array_bounds_call (const Loc &loc)
+build_assert_call (const Loc &loc, libcall_fn libcall, tree msg)
 {
-  switch (global.params.checkAction)
+  tree file;
+  tree line = size_int (loc.linnum);
+
+  switch (libcall)
     {
-    case CHECKACTION_D:
-      return d_assert_call (loc, LIBCALL_ARRAY_BOUNDS);
+    case LIBCALL_ASSERT_MSG:
+    case LIBCALL_UNITTEST_MSG:
+    case LIBCALL_SWITCH_ERROR:
+      /* File location is passed as a D string.  */
+      if (loc.filename)
+	{
+	  unsigned len = strlen (loc.filename);
+	  tree str = build_string (len, loc.filename);
+	  TREE_TYPE (str) = make_array_type (Type::tchar, len);
 
-    case CHECKACTION_C:
-    case CHECKACTION_halt:
-      return build_call_expr (builtin_decl_explicit (BUILT_IN_TRAP), 0);
+	  file = d_array_value (build_ctype (Type::tchar->arrayOf ()),
+				size_int (len), build_address (str));
+	}
+      else
+	file = null_array_node;
+      break;
+
+    case LIBCALL_ASSERTP:
+    case LIBCALL_UNITTESTP:
+      file = build_filename_from_loc (loc);
+      break;
 
     default:
       gcc_unreachable ();
     }
+
+
+  if (msg != NULL_TREE)
+    return build_libcall (libcall, Type::tvoid, 3, msg, file, line);
+  else
+    return build_libcall (libcall, Type::tvoid, 2, file, line);
 }
 
-/* Builds a bounds condition checking that INDEX is between 0 and LEN.
-   The condition returns the INDEX if true, or throws a RangeError.
-   If INCLUSIVE, we allow INDEX == LEN to return true also.  */
+/* Builds a CALL_EXPR at location LOC in the source file to execute when an
+   array bounds check fails.  */
 
 tree
-build_bounds_condition (const Loc &loc, tree index, tree len, bool inclusive)
+build_array_bounds_call (const Loc &loc)
 {
-  if (!array_bounds_check ())
+  /* Terminate the program with a trap if no D runtime present.  */
+  if (checkaction_trap_p ())
+    return build_call_expr (builtin_decl_explicit (BUILT_IN_TRAP), 0);
+  else
+    {
+      return build_libcall (LIBCALL_ARRAYBOUNDSP, Type::tvoid, 2,
+			    build_filename_from_loc (loc),
+			    size_int (loc.linnum));
+    }
+}
+
+/* Builds a bounds condition checking that INDEX is between 0 and LENGTH
+   in the index expression IE.  The condition returns the INDEX if true, or
+   throws a `RangeError`.  */
+
+tree
+build_bounds_index_condition (IndexExp *ie, tree index, tree length)
+{
+  if (ie->indexIsInBounds || !array_bounds_check ())
     return index;
 
   /* Prevent multiple evaluations of the index.  */
   index = d_save_expr (index);
 
-  /* Generate INDEX >= LEN && throw RangeError.
+  /* Generate INDEX >= LENGTH && throw RangeError.
      No need to check whether INDEX >= 0 as the front-end should
      have already taken care of implicit casts to unsigned.  */
-  tree condition = fold_build2 (inclusive ? GT_EXPR : GE_EXPR,
-				d_bool_type, index, len);
-  /* Terminate the program with a trap if no D runtime present.  */
-  tree boundserr = build_array_bounds_call (loc);
+  tree condition = fold_build2 (GE_EXPR, d_bool_type, index, length);
+  tree boundserr = build_array_bounds_call (ie->e2->loc);
 
   return build_condition (TREE_TYPE (index), condition, boundserr, index);
 }
 
+/* Builds a bounds condition checking that the range LOWER..UPPER do not overlap
+   the slice expression SE of the source array length LENGTH.  The condition
+   returns the new array length if true, or throws an `ArraySliceError`.  */
+
+tree
+build_bounds_slice_condition (SliceExp *se, tree lower, tree upper, tree length)
+{
+  if (array_bounds_check ())
+    {
+      tree condition = NULL_TREE;
+
+      /* Enforces that `upper <= length`.  */
+      if (!se->upperIsInBounds && length != NULL_TREE)
+	condition = fold_build2 (GT_EXPR, d_bool_type, upper, length);
+      else
+	length = integer_zero_node;
+
+      /* Enforces that `lower <= upper`.  No need to check `lower <= length` as
+	 we've already ensured that `upper <= length`.  */
+      if (!se->lowerIsLessThanUpper)
+	{
+	  tree lwr_cond = fold_build2 (GT_EXPR, d_bool_type, lower, upper);
+
+	  if (condition != NULL_TREE)
+	    condition = build_boolop (TRUTH_ORIF_EXPR, condition, lwr_cond);
+	  else
+	    condition = lwr_cond;
+	}
+
+      if (condition != NULL_TREE)
+	{
+	  tree boundserr = build_array_bounds_call (se->loc);
+	  upper = build_condition (TREE_TYPE (upper), condition,
+				   boundserr, upper);
+	}
+    }
+
+  /* Need to ensure lower always gets evaluated first, as it may be a function
+     call.  Generates (lower, upper) - lower.  */
+  return fold_build2 (MINUS_EXPR, TREE_TYPE (upper),
+		      compound_expr (lower, upper), lower);
+}
+
 /* Returns TRUE if array bounds checking code generation is turned on.  */
 
 bool
@@ -1905,6 +2005,26 @@ array_bounds_check (void)
     }
 }
 
+/* Returns TRUE if we terminate the program with a trap if an array bounds or
+   contract check fails.  */
+
+bool
+checkaction_trap_p (void)
+{
+  switch (global.params.checkAction)
+    {
+    case CHECKACTION_D:
+      return false;
+
+    case CHECKACTION_C:
+    case CHECKACTION_halt:
+      return true;
+
+    default:
+      gcc_unreachable ();
+    }
+}
+
 /* Returns the TypeFunction class for Type T.
    Assumes T is already ->toBasetype().  */
 
@@ -2093,33 +2213,6 @@ d_build_call (TypeFunction *tf, tree callable, tree object,
   return compound_expr (saved_args, result);
 }
 
-/* Builds a call to AssertError or AssertErrorMsg.  */
-
-tree
-d_assert_call (const Loc &loc, libcall_fn libcall, tree msg)
-{
-  tree file;
-  tree line = size_int (loc.linnum);
-
-  /* File location is passed as a D string.  */
-  if (loc.filename)
-    {
-      unsigned len = strlen (loc.filename);
-      tree str = build_string (len, loc.filename);
-      TREE_TYPE (str) = make_array_type (Type::tchar, len);
-
-      file = d_array_value (build_ctype (Type::tchar->arrayOf ()),
-			    size_int (len), build_address (str));
-    }
-  else
-    file = null_array_node;
-
-  if (msg != NULL)
-    return build_libcall (libcall, Type::tvoid, 3, msg, file, line);
-  else
-    return build_libcall (libcall, Type::tvoid, 2, file, line);
-}
-
 /* Build and return the correct call to fmod depending on TYPE.
    ARG0 and ARG1 are the arguments pass to the function.  */
 
diff --git a/gcc/d/d-tree.h b/gcc/d/d-tree.h
index f210b8b1a6e..9b90ca530e6 100644
--- a/gcc/d/d-tree.h
+++ b/gcc/d/d-tree.h
@@ -31,6 +31,8 @@ class TypeInfoDeclaration;
 class VarDeclaration;
 class Expression;
 class ClassReferenceExp;
+class IndexExp;
+class SliceExp;
 class Module;
 class Statement;
 class Type;
@@ -575,15 +577,17 @@ extern tree build_array_set (tree, tree, tree);
 extern tree build_array_from_val (Type *, tree);
 extern tree build_array_from_exprs (Type *, Expressions *, bool);
 extern tree void_okay_p (tree);
+extern tree build_assert_call (const Loc &, libcall_fn, tree = NULL_TREE);
 extern tree build_array_bounds_call (const Loc &);
-extern tree build_bounds_condition (const Loc &, tree, tree, bool);
+extern tree build_bounds_index_condition (IndexExp *, tree, tree);
+extern tree build_bounds_slice_condition (SliceExp *, tree, tree, tree);
 extern bool array_bounds_check (void);
+extern bool checkaction_trap_p (void);
 extern tree bind_expr (tree, tree);
 extern TypeFunction *get_function_type (Type *);
 extern bool call_by_alias_p (FuncDeclaration *, FuncDeclaration *);
 extern tree d_build_call_expr (FuncDeclaration *, tree, Expressions *);
 extern tree d_build_call (TypeFunction *, tree, tree, Expressions *);
-extern tree d_assert_call (const Loc &, libcall_fn, tree = NULL_TREE);
 extern tree build_float_modulus (tree, tree, tree);
 extern tree build_vthis_function (tree, tree);
 extern tree error_no_frame_access (Dsymbol *);
diff --git a/gcc/d/expr.cc b/gcc/d/expr.cc
index e293cf2a4cd..ea21bd5a8a1 100644
--- a/gcc/d/expr.cc
+++ b/gcc/d/expr.cc
@@ -1304,8 +1304,8 @@ public:
 
 	/* If it's a static array and the index is constant, the front end has
 	   already checked the bounds.  */
-	if (tb1->ty != Tpointer && !e->indexIsInBounds)
-	  index = build_bounds_condition (e->e2->loc, index, length, false);
+	if (tb1->ty != Tpointer)
+	  index = build_bounds_index_condition (e, index, length);
 
 	/* Index the .ptr.  */
 	ptr = void_okay_p (ptr);
@@ -1412,8 +1412,6 @@ public:
 	ptr = build_array_index (void_okay_p (ptr), lwr_tree);
 	ptr = build_nop (ptrtype, ptr);
       }
-    else
-      lwr_tree = NULL_TREE;
 
     /* Nothing more to do for static arrays, their bounds checking has been
        done at compile-time.  */
@@ -1426,46 +1424,8 @@ public:
       gcc_assert (tb->ty == Tarray);
 
     /* Generate bounds checking code.  */
-    tree newlength;
-
-    if (!e->upperIsInBounds)
-      {
-	if (length)
-	  {
-	    newlength = build_bounds_condition (e->upr->loc, upr_tree,
-						length, true);
-	  }
-	else
-	  {
-	    /* Still need to check bounds lwr <= upr for pointers.  */
-	    gcc_assert (tb1->ty == Tpointer);
-	    newlength = upr_tree;
-	  }
-      }
-    else
-      newlength = upr_tree;
-
-    if (lwr_tree)
-      {
-	/* Enforces lwr <= upr.  No need to check lwr <= length as
-	   we've already ensured that upr <= length.  */
-	if (!e->lowerIsLessThanUpper)
-	  {
-	    tree cond = build_bounds_condition (e->lwr->loc, lwr_tree,
-						upr_tree, true);
-
-	    /* When bounds checking is off, the index value is
-	       returned directly.  */
-	    if (cond != lwr_tree)
-	      newlength = compound_expr (cond, newlength);
-	  }
-
-	/* Need to ensure lwr always gets evaluated first, as it may be a
-	   function call.  Generates (lwr, upr) - lwr.  */
-	newlength = fold_build2 (MINUS_EXPR, TREE_TYPE (newlength),
-				 compound_expr (lwr_tree, newlength), lwr_tree);
-      }
-
+    tree newlength = build_bounds_slice_condition (e, lwr_tree, upr_tree,
+						   length);
     tree result = d_array_value (build_ctype (e->type), newlength, ptr);
     this->result_ = compound_expr (array, result);
   }
@@ -2023,8 +1983,7 @@ public:
     tree assert_pass = void_node;
     tree assert_fail;
 
-    if (global.params.useAssert == CHECKENABLEon
-	&& global.params.checkAction == CHECKACTION_D)
+    if (global.params.useAssert == CHECKENABLEon && !checkaction_trap_p ())
       {
 	/* Generate: ((bool) e1  ? (void)0 : _d_assert (...))
 		 or: (e1 != null ? e1._invariant() : _d_assert (...))  */
@@ -2037,10 +1996,10 @@ public:
 	    libcall = unittest_p ? LIBCALL_UNITTEST_MSG : LIBCALL_ASSERT_MSG;
 	  }
 	else
-	  libcall = unittest_p ? LIBCALL_UNITTEST : LIBCALL_ASSERT;
+	  libcall = unittest_p ? LIBCALL_UNITTESTP : LIBCALL_ASSERTP;
 
 	/* Build a call to _d_assert().  */
-	assert_fail = d_assert_call (e->loc, libcall, tmsg);
+	assert_fail = build_assert_call (e->loc, libcall, tmsg);
 
 	if (global.params.useInvariants == CHECKENABLEon)
 	  {
@@ -2068,8 +2027,7 @@ public:
 	      }
 	  }
       }
-    else if (global.params.useAssert == CHECKENABLEon
-	     && global.params.checkAction == CHECKACTION_C)
+    else if (global.params.useAssert == CHECKENABLEon && checkaction_trap_p ())
       {
 	/* Generate: __builtin_trap()  */
 	tree fn = builtin_decl_explicit (BUILT_IN_TRAP);
diff --git a/gcc/d/runtime.cc b/gcc/d/runtime.cc
index d789fcae63b..5206b53b865 100644
--- a/gcc/d/runtime.cc
+++ b/gcc/d/runtime.cc
@@ -51,6 +51,7 @@ enum d_libcall_type
   LCT_ARRAY_VOID,	    /* void[]		    */
   LCT_ARRAY_SIZE_T,	    /* size_t[]		    */
   LCT_ARRAY_BYTE,	    /* byte[]		    */
+  LCT_IMMUTABLE_CHARPTR,    /* immutable(char*)	    */
   LCT_ARRAY_STRING,	    /* string[]		    */
   LCT_ARRAY_WSTRING,	    /* wstring[]	    */
   LCT_ARRAY_DSTRING,	    /* dstring[]	    */
@@ -200,6 +201,10 @@ get_libcall_type (d_libcall_type type)
       libcall_types[type] = Type::tint8->arrayOf ()->pointerTo ();
       break;
 
+    case LCT_IMMUTABLE_CHARPTR:
+      libcall_types[type] = Type::tchar->pointerTo ()->immutableOf ();
+      break;
+
     default:
       gcc_unreachable ();
     }
diff --git a/gcc/d/runtime.def b/gcc/d/runtime.def
index f872cfc3f4e..0296233f4e2 100644
--- a/gcc/d/runtime.def
+++ b/gcc/d/runtime.def
@@ -26,27 +26,31 @@ along with GCC; see the file COPYING3.  If not see
    extern(C) - for those that are not, ensure to use correct mangling.  */
 
 /* Helper macros for parameter building.  */
-#define P0()		    0
-#define P1(T1)		    1, LCT_ ## T1
-#define P2(T1, T2)	    2, LCT_ ## T1, LCT_ ## T2
-#define P3(T1, T2, T3)	    3, LCT_ ## T1, LCT_ ## T2, LCT_ ## T3
-#define P4(T1, T2, T3, T4)  4, LCT_ ## T1, LCT_ ## T2, LCT_ ## T3, LCT_ ## T4
-#define RT(T1)		    LCT_ ## T1
+#define P0()	0
+#define P1(T1)	1, LCT_ ## T1
+#define P2(T1, T2) \
+		2, LCT_ ## T1, LCT_ ## T2
+#define P3(T1, T2, T3) \
+		3, LCT_ ## T1, LCT_ ## T2, LCT_ ## T3
+#define P4(T1, T2, T3, T4) \
+		4, LCT_ ## T1, LCT_ ## T2, LCT_ ## T3, LCT_ ## T4
+#define RT(T1)	LCT_ ## T1
 
 /* Used when an assert() contract fails.  */
-DEF_D_RUNTIME (ASSERT, "_d_assert", RT(VOID), P2(STRING, UINT), ECF_NORETURN)
+DEF_D_RUNTIME (ASSERTP, "_d_assertp", RT(VOID), P2(IMMUTABLE_CHARPTR, UINT),
+	       ECF_NORETURN)
 DEF_D_RUNTIME (ASSERT_MSG, "_d_assert_msg", RT(VOID), P3(STRING, STRING, UINT),
 	       ECF_NORETURN)
 
 /* Used when an assert() contract fails in a unittest function.  */
-DEF_D_RUNTIME (UNITTEST, "_d_unittest", RT(VOID), P2(STRING, UINT),
+DEF_D_RUNTIME (UNITTESTP, "_d_unittestp", RT(VOID), P2(IMMUTABLE_CHARPTR, UINT),
 	       ECF_NORETURN)
 DEF_D_RUNTIME (UNITTEST_MSG, "_d_unittest_msg", RT(VOID),
 	       P3(STRING, STRING, UINT), ECF_NORETURN)
 
 /* Used when an array index outside the bounds of its range.  */
-DEF_D_RUNTIME (ARRAY_BOUNDS, "_d_arraybounds", RT(VOID), P2(STRING, UINT),
-	       ECF_NORETURN)
+DEF_D_RUNTIME (ARRAYBOUNDSP, "_d_arrayboundsp", RT(VOID),
+	       P2(IMMUTABLE_CHARPTR, UINT), ECF_NORETURN)
 
 /* Used when calling new on a class.  */
 DEF_D_RUNTIME (NEWCLASS, "_d_newclass", RT(OBJECT), P1(CONST_CLASSINFO), 0)
diff --git a/gcc/d/toir.cc b/gcc/d/toir.cc
index eaee6f7e803..1c9da101f39 100644
--- a/gcc/d/toir.cc
+++ b/gcc/d/toir.cc
@@ -992,7 +992,7 @@ public:
 
   void visit (SwitchErrorStatement *s)
   {
-    add_stmt (d_assert_call (s->loc, LIBCALL_SWITCH_ERROR));
+    add_stmt (build_assert_call (s->loc, LIBCALL_SWITCH_ERROR));
   }
 
   /* A return statement exits the current function and supplies its return
-- 
2.30.2



More information about the Gcc-patches mailing list