[PATCH 1/2] PR c/68757: fix uninitialied src_range for various builtins

[PATCH 1/2] PR c/68757: fix uninitialied src_range for various builtins

[David Malcolm]

This fixes various uninitialized src_range of c_expr, this time
in the various builtins that are parsed via c_parser_get_builtin_args.

Bootstrapped&regrtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/c/ChangeLog:
	PR c/68757
	* c-parser.c (c_parser_get_builtin_args): Add
	"out_close_paren_loc" param, and write back to it.
	(c_parser_postfix_expression): Capture the closing
	parenthesis location for RID_CHOOSE_EXPR,
	RID_BUILTIN_CALL_WITH_STATIC_CHAIN, RID_BUILTIN_COMPLEX,
	RID_BUILTIN_SHUFFLE and use it to set the source range
	for such expressions; within RID_BUILTIN_COMPLEX set
	the underlying location.

gcc/testsuite/ChangeLog:
	PR c/68757
	* gcc.dg/plugin/diagnostic-test-expressions-1.c
	(test_builtin_choose_expr): New test function.
	(test_builtin_call_with_static_chain): Likewise.
	(test_builtin_complex): Likewise.
	(test_builtin_shuffle): Likewise.
---
 gcc/c/c-parser.c                                   | 39 ++++++++++++-----
 .../gcc.dg/plugin/diagnostic-test-expressions-1.c  | 50 ++++++++++++++++++++++
 2 files changed, 78 insertions(+), 11 deletions(-)

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index c7d15f9..8ea0e95 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -6933,11 +6933,14 @@ c_parser_alignof_expression (c_parser *parser)
    for the middle-end nodes like COMPLEX_EXPR, VEC_PERM_EXPR and
    others.  The name of the builtin is passed using BNAME parameter.
    Function returns true if there were no errors while parsing and
-   stores the arguments in CEXPR_LIST.  */
+   stores the arguments in CEXPR_LIST.  If it returns true,
+   *OUT_CLOSE_PAREN_LOC is written to with the location of the closing
+   parenthesis.  */
 static bool
 c_parser_get_builtin_args (c_parser *parser, const char *bname,
 			   vec<c_expr_t, va_gc> **ret_cexpr_list,
-			   bool choose_expr_p)
+			   bool choose_expr_p,
+			   location_t *out_close_paren_loc)
 {
   location_t loc = c_parser_peek_token (parser)->location;
   vec<c_expr_t, va_gc> *cexpr_list;
@@ -6955,6 +6958,7 @@ c_parser_get_builtin_args (c_parser *parser, const char *bname,
 
   if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN))
     {
+      *out_close_paren_loc = c_parser_peek_token (parser)->location;
       c_parser_consume_token (parser);
       return true;
     }
@@ -6974,6 +6978,7 @@ c_parser_get_builtin_args (c_parser *parser, const char *bname,
       vec_safe_push (cexpr_list, expr);
     }
 
+  *out_close_paren_loc = c_parser_peek_token (parser)->location;
   if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>"))
     return false;
 
@@ -7594,11 +7599,13 @@ c_parser_postfix_expression (c_parser *parser)
 	    vec<c_expr_t, va_gc> *cexpr_list;
 	    c_expr_t *e1_p, *e2_p, *e3_p;
 	    tree c;
+	    location_t close_paren_loc;
 
 	    c_parser_consume_token (parser);
 	    if (!c_parser_get_builtin_args (parser,
 					    "__builtin_choose_expr",
-					    &cexpr_list, true))
+					    &cexpr_list, true,
+					    &close_paren_loc))
 	      {
 		expr.value = error_mark_node;
 		break;
@@ -7626,6 +7633,7 @@ c_parser_postfix_expression (c_parser *parser)
 			" a constant");
 	    constant_expression_warning (c);
 	    expr = integer_zerop (c) ? *e3_p : *e2_p;
+	    set_c_expr_source_range (&expr, loc, close_paren_loc);
 	    break;
 	  }
 	case RID_TYPES_COMPATIBLE_P:
@@ -7677,11 +7685,13 @@ c_parser_postfix_expression (c_parser *parser)
 	    vec<c_expr_t, va_gc> *cexpr_list;
 	    c_expr_t *e2_p;
 	    tree chain_value;
+	    location_t close_paren_loc;
 
 	    c_parser_consume_token (parser);
 	    if (!c_parser_get_builtin_args (parser,
 					    "__builtin_call_with_static_chain",
-					    &cexpr_list, false))
+					    &cexpr_list, false,
+					    &close_paren_loc))
 	      {
 		expr.value = error_mark_node;
 		break;
@@ -7710,17 +7720,20 @@ c_parser_postfix_expression (c_parser *parser)
 			"must be a pointer type");
 	    else
 	      CALL_EXPR_STATIC_CHAIN (expr.value) = chain_value;
+	    set_c_expr_source_range (&expr, loc, close_paren_loc);
 	    break;
 	  }
 	case RID_BUILTIN_COMPLEX:
 	  {
 	    vec<c_expr_t, va_gc> *cexpr_list;
 	    c_expr_t *e1_p, *e2_p;
+	    location_t close_paren_loc;
 
 	    c_parser_consume_token (parser);
 	    if (!c_parser_get_builtin_args (parser,
 					    "__builtin_complex",
-					    &cexpr_list, false))
+					    &cexpr_list, false,
+					    &close_paren_loc))
 	      {
 		expr.value = error_mark_node;
 		break;
@@ -7765,11 +7778,12 @@ c_parser_postfix_expression (c_parser *parser)
 	      }
 	    pedwarn_c90 (loc, OPT_Wpedantic,
 			 "ISO C90 does not support complex types");
-	    expr.value = build2 (COMPLEX_EXPR,
-				 build_complex_type
-				   (TYPE_MAIN_VARIANT
-				     (TREE_TYPE (e1_p->value))),
-				 e1_p->value, e2_p->value);
+	    expr.value = build2_loc (loc, COMPLEX_EXPR,
+				     build_complex_type
+				     (TYPE_MAIN_VARIANT
+				      (TREE_TYPE (e1_p->value))),
+				     e1_p->value, e2_p->value);
+	    set_c_expr_source_range (&expr, loc, close_paren_loc);
 	    break;
 	  }
 	case RID_BUILTIN_SHUFFLE:
@@ -7777,11 +7791,13 @@ c_parser_postfix_expression (c_parser *parser)
 	    vec<c_expr_t, va_gc> *cexpr_list;
 	    unsigned int i;
 	    c_expr_t *p;
+	    location_t close_paren_loc;
 
 	    c_parser_consume_token (parser);
 	    if (!c_parser_get_builtin_args (parser,
 					    "__builtin_shuffle",
-					    &cexpr_list, false))
+					    &cexpr_list, false,
+					    &close_paren_loc))
 	      {
 		expr.value = error_mark_node;
 		break;
@@ -7808,6 +7824,7 @@ c_parser_postfix_expression (c_parser *parser)
 			       "%<__builtin_shuffle%>");
 		expr.value = error_mark_node;
 	      }
+	    set_c_expr_source_range (&expr, loc, close_paren_loc);
 	    break;
 	  }
 	case RID_AT_SELECTOR:
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
index 0d8c7c5..023385b 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
@@ -518,6 +518,56 @@ void test_builtin_offsetof (int i)
    { dg-end-multiline-output "" } */
 }
 
+void test_builtin_choose_expr (int i)
+{
+  __emit_expression_range (0,  __builtin_choose_expr (1, i, i) + i);  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0,  __builtin_choose_expr (1, i, i) + i);
+                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
+   { dg-end-multiline-output "" } */
+
+  __emit_expression_range (0,  i + __builtin_choose_expr (1, i, i));  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0,  i + __builtin_choose_expr (1, i, i));
+                                ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+}
+
+extern int f (int);
+void test_builtin_call_with_static_chain (int i, void *ptr)
+{
+  __emit_expression_range (0, __builtin_call_with_static_chain (f (i), ptr));  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0, __builtin_call_with_static_chain (f (i), ptr));
+                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+}
+
+void test_builtin_complex (float i, float j)
+{
+  __emit_expression_range (0,  __builtin_complex (i, j) );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0,  __builtin_complex (i, j) );
+                                ^~~~~~~~~~~~~~~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+}
+
+typedef int v4si __attribute__ ((vector_size (16)));
+void test_builtin_shuffle (v4si a, v4si b, v4si mask)
+{
+  __emit_expression_range (0,  __builtin_shuffle (a, mask) );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0,  __builtin_shuffle (a, mask) );
+                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+
+  __emit_expression_range (0,  __builtin_shuffle (a, b, mask) );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0,  __builtin_shuffle (a, b, mask) );
+                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+}
+
 /* Examples of non-trivial expressions.  ****************************/
 
 extern double sqrt (double x);
-- 
1.8.5.3

[PATCH 2/2] C: fix uninitialized ranges for __alignof__

[David Malcolm]

This fixes another uninitialized src_range of a c_expr, this time
in __alignof__ expressions.

Bootstrapped&regrtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/c/ChangeLog:
	* c-parser.c (c_parser_alignof_expression): Capture location of
	closing parenthesis (if any), or of end of unary expression, and
	use it to build a src_range for the expression.

gcc/testsuite/ChangeLog:
	* gcc.dg/plugin/diagnostic-test-expressions-1.c (test_alignof):
	New test function.
---
 gcc/c/c-parser.c                                   | 16 +++++++++----
 .../gcc.dg/plugin/diagnostic-test-expressions-1.c  | 27 ++++++++++++++++++++++
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 8ea0e95..4611e5b 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -6853,7 +6853,8 @@ static struct c_expr
 c_parser_alignof_expression (c_parser *parser)
 {
   struct c_expr expr;
-  location_t loc = c_parser_peek_token (parser)->location;
+  location_t start_loc = c_parser_peek_token (parser)->location;
+  location_t end_loc;
   tree alignof_spelling = c_parser_peek_token (parser)->value;
   gcc_assert (c_parser_next_token_is_keyword (parser, RID_ALIGNOF));
   bool is_c11_alignof = strcmp (IDENTIFIER_POINTER (alignof_spelling),
@@ -6864,10 +6865,10 @@ c_parser_alignof_expression (c_parser *parser)
   if (is_c11_alignof)
     {
       if (flag_isoc99)
-	pedwarn_c99 (loc, OPT_Wpedantic, "ISO C99 does not support %qE",
+	pedwarn_c99 (start_loc, OPT_Wpedantic, "ISO C99 does not support %qE",
 		     alignof_spelling);
       else
-	pedwarn_c99 (loc, OPT_Wpedantic, "ISO C90 does not support %qE",
+	pedwarn_c99 (start_loc, OPT_Wpedantic, "ISO C90 does not support %qE",
 		     alignof_spelling);
     }
   c_parser_consume_token (parser);
@@ -6884,6 +6885,7 @@ c_parser_alignof_expression (c_parser *parser)
       c_parser_consume_token (parser);
       loc = c_parser_peek_token (parser)->location;
       type_name = c_parser_type_name (parser);
+      end_loc = c_parser_peek_token (parser)->location;
       c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>");
       if (type_name == NULL)
 	{
@@ -6910,21 +6912,25 @@ c_parser_alignof_expression (c_parser *parser)
 					    false, is_c11_alignof, 1);
       ret.original_code = ERROR_MARK;
       ret.original_type = NULL;
+      set_c_expr_source_range (&ret, start_loc, end_loc);
       return ret;
     }
   else
     {
       struct c_expr ret;
       expr = c_parser_unary_expression (parser);
+      end_loc = expr.src_range.m_finish;
     alignof_expr:
       mark_exp_read (expr.value);
       c_inhibit_evaluation_warnings--;
       in_alignof--;
-      pedwarn (loc, OPT_Wpedantic, "ISO C does not allow %<%E (expression)%>",
+      pedwarn (start_loc,
+	       OPT_Wpedantic, "ISO C does not allow %<%E (expression)%>",
 	       alignof_spelling);
-      ret.value = c_alignof_expr (loc, expr.value);
+      ret.value = c_alignof_expr (start_loc, expr.value);
       ret.original_code = ERROR_MARK;
       ret.original_type = NULL;
+      set_c_expr_source_range (&ret, start_loc, end_loc);
       return ret;
     }
 }
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
index 023385b..175b2ea 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
@@ -568,6 +568,33 @@ void test_builtin_shuffle (v4si a, v4si b, v4si mask)
    { dg-end-multiline-output "" } */
 }
 
+void test_alignof (int param)
+{
+  __emit_expression_range (0, __alignof__ (int) + param );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0, __alignof__ (int) + param );
+                               ~~~~~~~~~~~~~~~~~~^~~~~~~
+   { dg-end-multiline-output "" } */
+
+  __emit_expression_range (0,  param + __alignof__ (int) );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0,  param + __alignof__ (int) );
+                                ~~~~~~^~~~~~~~~~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+
+  __emit_expression_range (0,  __alignof__ (param) + param );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0,  __alignof__ (param) + param );
+                                ~~~~~~~~~~~~~~~~~~~~^~~~~~~
+   { dg-end-multiline-output "" } */
+
+  __emit_expression_range (0,  param + __alignof__ (param) );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0,  param + __alignof__ (param) );
+                                ~~~~~~^~~~~~~~~~~~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+}
+
 /* Examples of non-trivial expressions.  ****************************/
 
 extern double sqrt (double x);
-- 
1.8.5.3

Re: [PATCH 1/2] PR c/68757: fix uninitialied src_range for various builtins

[Bernd Schmidt]

On 12/08/2015 04:43 PM, David Malcolm wrote:
> This fixes various uninitialized src_range of c_expr, this time
> in the various builtins that are parsed via c_parser_get_builtin_args.
>
> Bootstrapped&regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?

I think both of these patches are OK. Some questions though.

>   			" a constant");
>   	    constant_expression_warning (c);
>   	    expr = integer_zerop (c) ? *e3_p : *e2_p;
> +	    set_c_expr_source_range (&expr, loc, close_paren_loc);

If that had an uninitialized range, it implies that the *eN_p 
expressions also have uninitialized parts. Correct? If so, I think we 
should fix that.

Also, what happened to the idea of a constructor for c_expr_t?


Bernd

Re: [PATCH 1/2] PR c/68757: fix uninitialied src_range for various builtins

[David Malcolm]

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

On Tue, 2015-12-08 at 16:38 +0100, Bernd Schmidt wrote:
> On 12/08/2015 04:43 PM, David Malcolm wrote:
> > This fixes various uninitialized src_range of c_expr, this time
> > in the various builtins that are parsed via c_parser_get_builtin_args.
> >
> > Bootstrapped&regrtested on x86_64-pc-linux-gnu.
> >
> > OK for trunk?
> 
> I think both of these patches are OK. Some questions though.
> 
> >   			" a constant");
> >   	    constant_expression_warning (c);
> >   	    expr = integer_zerop (c) ? *e3_p : *e2_p;
> > +	    set_c_expr_source_range (&expr, loc, close_paren_loc);
> 
> If that had an uninitialized range, it implies that the *eN_p 
> expressions also have uninitialized parts. Correct? If so, I think we 
> should fix that.

Hmmm... re-reading the patch, I think my description was sloppy; sorry.

I believe uninitialized data affected RID_BUILTIN_COMPLEX and
RID_BUILTIN_SHUFFLE, whereas RID_CHOOSE_EXPR and
RID_BUILTIN_CALL_WITH_STATIC_CHAIN copied src_range data from
subexpressions.

The fix for the uninitialized data made me think "what should the source
range of RID_BUILTIN_COMPLEX be?"  Hence I noticed that each of the
places where c_parser_get_builtin_args was used didn't have an explicit
choice about src_range, and that it ought to use the builtin args in its
range, and so I updated these.

Within RID_CHOOSE_EXPR and RID_BUILTIN_CALL_WITH_STATIC_CHAIN, *eN_P
come from c_parser_get_builtin_args, and reviewing that function, it
purely gets them by copying them from the return value of
c_parser_expr_no_commas, so there's no uninitialized data introduced
there.  [There could be an argument that for RID_CHOOSE_EXPR we should
use the range of the chosen expression, which was the status quo for
that case; the patch makes it use the range of the full
__builtin_choose_expr () for consistency with other the builtins].


(I believe the proposed ChangeLog does accurately reflect the change;
just the title may need tweaking).

> Also, what happened to the idea of a constructor for c_expr_t?

I actually implemented something like this when implementing these two
patches.  

Work-in-progress patch attached, which introduces an INVALID_LOCATION
value for source_location/location_t, and uses it to "poison" the
initial value of c_expr's src_range, with lots of assertions to verify
that it's been overwritten by time we use it.

It doesn't fully work yet, but much of gcc.dg survives with this (and
it's what I used to detect the alignof issue in patch 2).  Does this
look like something I should pursue?

[-- Attachment #2: 0001-FIXME-add-INVALID_LOCATION-and-use-it-to-poison-src_.patch --]
[-- Type: text/x-patch, Size: 7331 bytes --]

From cf8aa843a7bb5f173eb8107b1c721e964b9e0677 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Mon, 7 Dec 2015 13:47:03 -0500
Subject: [PATCH] WIP: add INVALID_LOCATION and use it to poison src_range

---
 gcc/c/c-parser.c          |  8 +++++++-
 gcc/c/c-tree.h            | 26 ++++++++++++++++++++++++++
 gcc/c/c-typeck.c          |  2 ++
 gcc/tree.c                |  6 ++++++
 libcpp/include/line-map.h | 10 +++++++++-
 libcpp/line-map.c         |  7 +++++++
 6 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 4611e5b..cded9fe 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -63,6 +63,8 @@ void
 set_c_expr_source_range (c_expr *expr,
 			 location_t start, location_t finish)
 {
+  gcc_assert (start != INVALID_LOCATION);
+  gcc_assert (finish != INVALID_LOCATION);
   expr->src_range.m_start = start;
   expr->src_range.m_finish = finish;
   if (expr->value)
@@ -73,6 +75,8 @@ void
 set_c_expr_source_range (c_expr *expr,
 			 source_range src_range)
 {
+  gcc_assert (src_range.m_start != INVALID_LOCATION);
+  gcc_assert (src_range.m_finish != INVALID_LOCATION);
   expr->src_range = src_range;
   if (expr->value)
     set_source_range (expr->value, src_range);
@@ -6145,6 +6149,8 @@ c_parser_expr_no_commas (c_parser *parser, struct c_expr *after,
   c_parser_consume_token (parser);
   exp_location = c_parser_peek_token (parser)->location;
   rhs = c_parser_expr_no_commas (parser, NULL);
+  gcc_assert (rhs.src_range.m_start != INVALID_LOCATION);
+  gcc_assert (rhs.src_range.m_finish != INVALID_LOCATION);
   rhs = convert_lvalue_to_rvalue (exp_location, rhs, true, true);
   
   ret.value = build_modify_expr (op_location, lhs.value, lhs.original_type,
@@ -6917,7 +6923,6 @@ c_parser_alignof_expression (c_parser *parser)
     }
   else
     {
-      struct c_expr ret;
       expr = c_parser_unary_expression (parser);
       end_loc = expr.src_range.m_finish;
     alignof_expr:
@@ -6927,6 +6932,7 @@ c_parser_alignof_expression (c_parser *parser)
       pedwarn (start_loc,
 	       OPT_Wpedantic, "ISO C does not allow %<%E (expression)%>",
 	       alignof_spelling);
+      struct c_expr ret;
       ret.value = c_alignof_expr (start_loc, expr.value);
       ret.original_code = ERROR_MARK;
       ret.original_type = NULL;
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 00e72b1..30625ee 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -118,6 +118,32 @@ along with GCC; see the file COPYING3.  If not see
    for code generation alongside a tree representing its value.  */
 struct c_expr
 {
+  c_expr ()
+  {
+    /* In a checkeed build, poison the initial value of src_range so
+       that attempts to use uninitialized data fail with an assertion,
+       without needing to run under Valgrind.  */
+#if CHECKING_P
+    src_range.m_start = INVALID_LOCATION;
+    src_range.m_finish = INVALID_LOCATION;
+#endif
+  }
+
+  c_expr (const c_expr &other)
+  {
+    value = other.value;
+    original_code = other.original_code;
+    original_type = other.original_type;
+#if CHECKING_P
+    if (value != error_mark_node)
+      {
+	gcc_assert (other.src_range.m_start != INVALID_LOCATION);
+	gcc_assert (other.src_range.m_finish != INVALID_LOCATION);
+      }
+#endif
+    src_range = other.src_range;
+  }
+
   /* The value of the expression.  */
   tree value;
   /* Record the original unary/binary operator of an expression, which may
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index b691072..97064ae 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -6821,6 +6821,8 @@ digest_init (location_t init_loc, tree type, tree init, tree origtype,
 	  expr.value = inside_init;
 	  expr.original_code = (strict_string ? STRING_CST : ERROR_MARK);
 	  expr.original_type = NULL;
+	  expr.src_range.m_start = init_loc;
+	  expr.src_range.m_finish = init_loc;
 	  maybe_warn_string_init (init_loc, type, expr);
 
 	  if (TYPE_DOMAIN (type) && !TYPE_MAX_VALUE (TYPE_DOMAIN (type)))
diff --git a/gcc/tree.c b/gcc/tree.c
index c8b3ab8..db429c7 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -13938,6 +13938,9 @@ set_block (location_t loc, tree block)
 location_t
 set_source_range (tree expr, location_t start, location_t finish)
 {
+  gcc_assert (start != INVALID_LOCATION);
+  gcc_assert (finish != INVALID_LOCATION);
+
   source_range src_range;
   src_range.m_start = start;
   src_range.m_finish = finish;
@@ -13950,6 +13953,9 @@ set_source_range (tree expr, source_range src_range)
   if (!EXPR_P (expr))
     return UNKNOWN_LOCATION;
 
+  gcc_assert (src_range.m_start != INVALID_LOCATION);
+  gcc_assert (src_range.m_finish != INVALID_LOCATION);
+
   location_t pure_loc = get_pure_location (EXPR_LOCATION (expr));
   location_t adhoc = COMBINE_LOCATION_DATA (line_table,
 					    pure_loc,
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 73c583e..e3cc0d4 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -160,7 +160,7 @@ typedef unsigned int linenum_type;
   -----------+-------------------------------+-------------------------------
   0x80000000 | Start of ad-hoc values; the lower 31 bits are used as an index
   ...        | into the line_table->location_adhoc_data_map.data array.
-  0xffffffff | UINT_MAX                      |
+  0xffffffff | UINT_MAX == INVALID_LOCATION  |
   -----------+-------------------------------+-------------------------------
 
    Examples of location encoding.
@@ -260,6 +260,10 @@ typedef unsigned int linenum_type;
    worked example in libcpp/location-example.txt.  */
 typedef unsigned int source_location;
 
+/* A bogus value, used for detecting uninitialized data in checked
+   builds.  */
+#define INVALID_LOCATION ((source_location) UINT_MAX)
+
 /* A range of source locations.
 
    Ranges are closed:
@@ -1004,6 +1008,10 @@ COMBINE_LOCATION_DATA (struct line_maps *set,
 		       source_range src_range,
 		       void *block)
 {
+  linemap_assert (loc != INVALID_LOCATION);
+  linemap_assert (src_range.m_start != INVALID_LOCATION);
+  linemap_assert (src_range.m_finish != INVALID_LOCATION);
+
   return get_combined_adhoc_loc (set, loc, src_range, block);
 }
 
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 333e835..76c1ec8 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -84,6 +84,10 @@ location_adhoc_data_eq (const void *l1, const void *l2)
       (const struct location_adhoc_data *) l1;
   const struct location_adhoc_data *lb2 =
       (const struct location_adhoc_data *) l2;
+  linemap_assert (lb1->src_range.m_start != INVALID_LOCATION);
+  linemap_assert (lb1->src_range.m_finish != INVALID_LOCATION);
+  linemap_assert (lb2->src_range.m_start != INVALID_LOCATION);
+  linemap_assert (lb2->src_range.m_finish != INVALID_LOCATION);
   return (lb1->locus == lb2->locus
 	  && lb1->src_range.m_start == lb2->src_range.m_start
 	  && lb1->src_range.m_finish == lb2->src_range.m_finish
@@ -166,6 +170,9 @@ get_combined_adhoc_loc (struct line_maps *set,
   struct location_adhoc_data lb;
   struct location_adhoc_data **slot;
 
+  linemap_assert (src_range.m_start != INVALID_LOCATION);
+  linemap_assert (src_range.m_finish != INVALID_LOCATION);
+
   if (IS_ADHOC_LOC (locus))
     locus
       = set->location_adhoc_data_map.data[locus & MAX_SOURCE_LOCATION].locus;
-- 
1.8.5.3

Re: [PATCH 1/2] PR c/68757: fix uninitialied src_range for various builtins

[Bernd Schmidt]

On 12/08/2015 05:02 PM, David Malcolm wrote:
>
> I actually implemented something like this when implementing these two
> patches.
>
> Work-in-progress patch attached, which introduces an INVALID_LOCATION
> value for source_location/location_t, and uses it to "poison" the
> initial value of c_expr's src_range, with lots of assertions to verify
> that it's been overwritten by time we use it.
>
> It doesn't fully work yet, but much of gcc.dg survives with this (and
> it's what I used to detect the alignof issue in patch 2).  Does this
> look like something I should pursue?

Most definitely IMO.


Bernd