Re: libcpp/C FE source range patch committed (r230331).

Re: libcpp/C FE source range patch committed (r230331).

[David Edelsohn]

This patch causes numerous new testsuite failure on AIX caused by the
compiler crashing during compilation, e.g.

gcc.c-torture/execute/20020206-1.c

in GCC libcpp

991       linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set));

(gdb) where
#0  _Z11fancy_abortPKciS0_ (
    file=0x11296dc0
<_GLOBAL__F__ZN9text_info9set_rangeEj12source_rangeb+3056>
"/nasfarm/edelsohn/src/src/libcpp/line-map.c", line=991,
    function=0x11296f30
<_GLOBAL__F__ZN9text_info9set_rangeEj12source_rangeb+3424>
"linemap_macro_map_lookup")
    at /nasfarm/edelsohn/src/src/gcc/diagnostic.c:1332
#1  0x100169b4 in _Z14linemap_lookupP9line_mapsj (set=0x70000000, line=991)
    at /nasfarm/edelsohn/src/src/libcpp/line-map.c:991
#2  0x100188f8 in
_Z40linemap_unwind_to_first_non_reserved_locP9line_mapsjPPK8line_map
(set=0x70000000, loc=991, map=0x0)
    at /nasfarm/edelsohn/src/src/libcpp/line-map.c:1629
#3  0x100753c8 in _ZL17expand_location_1jb (loc=889323520,
    expansion_point_p=false) at /nasfarm/edelsohn/src/src/gcc/input.c:158
#4  0x10076488 in _Z48linemap_client_expand_location_to_spelling_pointj (
    loc=991) at /nasfarm/edelsohn/src/src/gcc/input.c:751
#5  0x10019928 in _ZN13rich_location9add_rangeEjjb (this=0x2ff21cd8,
    start=991, finish=889323520, show_caret_p=true)
    at /nasfarm/edelsohn/src/src/libcpp/line-map.c:2012
#6  0x10019a54 in _ZN13rich_locationC2EP9line_mapsj (this=0x2ff21cd8,
    set=0x3df, loc=287928112)
    at /nasfarm/edelsohn/src/src/libcpp/line-map.c:2024
#7  0x1000ed84 in _Z7warningiPKcz (opt=164,
    gmsgid=0x11488d18
<_GLOBAL__F__Z20prepare_call_addressP9tree_nodeP7rtx_defS2-
_PS2_ii+3752> "function call has aggregate value")
    at /nasfarm/edelsohn/src/src/gcc/diagnostic.c:1003
#8  0x1067ebac in _Z11expand_callP9tree_nodeP7rtx_defi (exp=0x700dcf20,
    target=0x700ec080, ignore=0) at /nasfarm/edelsohn/src/src/gcc/calls.c:2476
#9  0x10406858 in
_Z18expand_expr_real_1P9tree_nodeP7rtx_def12machine_mode15expand_modifierPS2_b
(exp=0x700dcf20, target=0x700ec080, tmode=BLKmode,
    modifier=EXPAND_NORMAL, alt_rtl=0x17, inner_reference_p=false)
    at /nasfarm/edelsohn/src/src/gcc/expr.c:10581
#10 0x104158c0 in _Z22store_expr_with_boundsP9tree_nodeP7rtx_defibbS0_ (
    exp=0x700dcf20, target=0x700ec080, call_param_p=0, nontemporal=false,
    reverse=false, btarget=0x700df058)
    at /nasfarm/edelsohn/src/src/gcc/expr.c:5405
#11 0x104178fc in _Z17expand_assignmentP9tree_nodeS0_b (to=0x700df058,
    from=0x700dcf20, nontemporal=false)
    at /nasfarm/edelsohn/src/src/gcc/expr.c:5174
#12 0x106f67b4 in _ZL18expand_gimple_stmtP6gimple (stmt=0x7000e240)
    at /nasfarm/edelsohn/src/src/gcc/cfgexpand.c:6278
#13 0x106f87d8 in _ZL25expand_gimple_basic_blockP15basic_block_defb (
    bb=0x700c7740, disable_tail_calls=false)
    at /nasfarm/edelsohn/src/src/gcc/cfgexpand.c:5679
#14 0x106ffbf4 in _ZN12_GLOBAL__N_111pass_expand7executeEP8function (
    this=0x11296dc0
<_GLOBAL__F__ZN9text_info9set_rangeEj12source_rangeb+3056>,
fun=0x70009138) at /nasfarm/edelsohn/src/src/gcc/cfgexpand.c:6291

Re: libcpp/C FE source range patch committed (r230331).

[David Malcolm]

On Sat, 2015-11-14 at 09:50 -0500, David Edelsohn wrote:
> This patch causes numerous new testsuite failure on AIX caused by the
> compiler crashing during compilation, e.g.
> 
> gcc.c-torture/execute/20020206-1.c
> 
> in GCC libcpp
> 
> 991       linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set));
> 
> (gdb) where
> #0  _Z11fancy_abortPKciS0_ (
>     file=0x11296dc0
> <_GLOBAL__F__ZN9text_info9set_rangeEj12source_rangeb+3056>
> "/nasfarm/edelsohn/src/src/libcpp/line-map.c", line=991,
>     function=0x11296f30
> <_GLOBAL__F__ZN9text_info9set_rangeEj12source_rangeb+3424>
> "linemap_macro_map_lookup")
>     at /nasfarm/edelsohn/src/src/gcc/diagnostic.c:1332
> #1  0x100169b4 in _Z14linemap_lookupP9line_mapsj (set=0x70000000, line=991)
>     at /nasfarm/edelsohn/src/src/libcpp/line-map.c:991
> #2  0x100188f8 in
> _Z40linemap_unwind_to_first_non_reserved_locP9line_mapsjPPK8line_map
> (set=0x70000000, loc=991, map=0x0)
>     at /nasfarm/edelsohn/src/src/libcpp/line-map.c:1629
> #3  0x100753c8 in _ZL17expand_location_1jb (loc=889323520,
>     expansion_point_p=false) at /nasfarm/edelsohn/src/src/gcc/input.c:158
> #4  0x10076488 in _Z48linemap_client_expand_location_to_spelling_pointj (
>     loc=991) at /nasfarm/edelsohn/src/src/gcc/input.c:751
> #5  0x10019928 in _ZN13rich_location9add_rangeEjjb (this=0x2ff21cd8,
>     start=991, finish=889323520, show_caret_p=true)
>     at /nasfarm/edelsohn/src/src/libcpp/line-map.c:2012
> #6  0x10019a54 in _ZN13rich_locationC2EP9line_mapsj (this=0x2ff21cd8,
>     set=0x3df, loc=287928112)
>     at /nasfarm/edelsohn/src/src/libcpp/line-map.c:2024
> #7  0x1000ed84 in _Z7warningiPKcz (opt=164,
>     gmsgid=0x11488d18
> <_GLOBAL__F__Z20prepare_call_addressP9tree_nodeP7rtx_defS2-
> _PS2_ii+3752> "function call has aggregate value")
>     at /nasfarm/edelsohn/src/src/gcc/diagnostic.c:1003
> #8  0x1067ebac in _Z11expand_callP9tree_nodeP7rtx_defi (exp=0x700dcf20,
>     target=0x700ec080, ignore=0) at /nasfarm/edelsohn/src/src/gcc/calls.c:2476
> #9  0x10406858 in
> _Z18expand_expr_real_1P9tree_nodeP7rtx_def12machine_mode15expand_modifierPS2_b
> (exp=0x700dcf20, target=0x700ec080, tmode=BLKmode,
>     modifier=EXPAND_NORMAL, alt_rtl=0x17, inner_reference_p=false)
>     at /nasfarm/edelsohn/src/src/gcc/expr.c:10581
> #10 0x104158c0 in _Z22store_expr_with_boundsP9tree_nodeP7rtx_defibbS0_ (
>     exp=0x700dcf20, target=0x700ec080, call_param_p=0, nontemporal=false,
>     reverse=false, btarget=0x700df058)
>     at /nasfarm/edelsohn/src/src/gcc/expr.c:5405
> #11 0x104178fc in _Z17expand_assignmentP9tree_nodeS0_b (to=0x700df058,
>     from=0x700dcf20, nontemporal=false)
>     at /nasfarm/edelsohn/src/src/gcc/expr.c:5174
> #12 0x106f67b4 in _ZL18expand_gimple_stmtP6gimple (stmt=0x7000e240)
>     at /nasfarm/edelsohn/src/src/gcc/cfgexpand.c:6278
> #13 0x106f87d8 in _ZL25expand_gimple_basic_blockP15basic_block_defb (
>     bb=0x700c7740, disable_tail_calls=false)
>     at /nasfarm/edelsohn/src/src/gcc/cfgexpand.c:5679
> #14 0x106ffbf4 in _ZN12_GLOBAL__N_111pass_expand7executeEP8function (
>     this=0x11296dc0
> <_GLOBAL__F__ZN9text_info9set_rangeEj12source_rangeb+3056>,
> fun=0x70009138) at /nasfarm/edelsohn/src/src/gcc/cfgexpand.c:6291

I attempted to reproduce this on gcc111 (powerpc-ibm-aix7.1.3.0)
  ../src/configure --disable-bootstrap --with-gmp=/opt/cfarm/gmp-latest
--with-mpfr=/opt/cfarm/mpfr-latest --with-mpc=/opt/cfarm/mpc-latest
with latest trunk (r230384).

I saw only one ICE within "make check-gcc", when running
gcc.c-torture/execute/scal-to-vec1.c; specifically:

  /home/dmalcolm/gcc-build/build/gcc/xgcc \
    -B/home/dmalcolm/gcc-build/build/gcc/ \
    /home/dmalcolm/gcc-build/src/gcc/testsuite/gcc.c-torture/execute/scal-to-vec1.c \
    -fno-diagnostics-show-caret -fdiagnostics-color=never \
   -O0  -w  -lm    -o ./scal-to-vec1.exe

and this shows the same assertion failure as your report.


I was able to reproduce that ICE at will under gdb; from what I could
tell from gdb, a seemingly valid location is passed in to warning_at,
but at warning_at, the 32-bit value is seemingly corrupt, and this
eventually leads to an assertion failure in the new code.  The warning
is then discarded (OPT_Wvector_operation_performance).  I can't tell yet
if the data is corrupt, or if gdb is somehow getting confused about the
values as I go up and down the callstack (or indeed if I am), but it's
getting late here.

[PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))

[David Malcolm]

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

On Sat, 2015-11-14 at 23:32 -0500, David Malcolm wrote:
> On Sat, 2015-11-14 at 09:50 -0500, David Edelsohn wrote:
> > This patch causes numerous new testsuite failure on AIX caused by the
> > compiler crashing during compilation, e.g.
> > 
> > gcc.c-torture/execute/20020206-1.c
> > 
> > in GCC libcpp
> > 
> > 991       linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set));
> > 
> > (gdb) where
> > #0  _Z11fancy_abortPKciS0_ (
> >     file=0x11296dc0
> > <_GLOBAL__F__ZN9text_info9set_rangeEj12source_rangeb+3056>
> > "/nasfarm/edelsohn/src/src/libcpp/line-map.c", line=991,
> >     function=0x11296f30
> > <_GLOBAL__F__ZN9text_info9set_rangeEj12source_rangeb+3424>
> > "linemap_macro_map_lookup")
> >     at /nasfarm/edelsohn/src/src/gcc/diagnostic.c:1332
> > #1  0x100169b4 in _Z14linemap_lookupP9line_mapsj (set=0x70000000, line=991)
> >     at /nasfarm/edelsohn/src/src/libcpp/line-map.c:991
> > #2  0x100188f8 in
> > _Z40linemap_unwind_to_first_non_reserved_locP9line_mapsjPPK8line_map
> > (set=0x70000000, loc=991, map=0x0)
> >     at /nasfarm/edelsohn/src/src/libcpp/line-map.c:1629
> > #3  0x100753c8 in _ZL17expand_location_1jb (loc=889323520,
> >     expansion_point_p=false) at /nasfarm/edelsohn/src/src/gcc/input.c:158
> > #4  0x10076488 in _Z48linemap_client_expand_location_to_spelling_pointj (
> >     loc=991) at /nasfarm/edelsohn/src/src/gcc/input.c:751
> > #5  0x10019928 in _ZN13rich_location9add_rangeEjjb (this=0x2ff21cd8,
> >     start=991, finish=889323520, show_caret_p=true)
> >     at /nasfarm/edelsohn/src/src/libcpp/line-map.c:2012
> > #6  0x10019a54 in _ZN13rich_locationC2EP9line_mapsj (this=0x2ff21cd8,
> >     set=0x3df, loc=287928112)
> >     at /nasfarm/edelsohn/src/src/libcpp/line-map.c:2024
> > #7  0x1000ed84 in _Z7warningiPKcz (opt=164,
> >     gmsgid=0x11488d18
> > <_GLOBAL__F__Z20prepare_call_addressP9tree_nodeP7rtx_defS2-
> > _PS2_ii+3752> "function call has aggregate value")
> >     at /nasfarm/edelsohn/src/src/gcc/diagnostic.c:1003
> > #8  0x1067ebac in _Z11expand_callP9tree_nodeP7rtx_defi (exp=0x700dcf20,
> >     target=0x700ec080, ignore=0) at /nasfarm/edelsohn/src/src/gcc/calls.c:2476
> > #9  0x10406858 in
> > _Z18expand_expr_real_1P9tree_nodeP7rtx_def12machine_mode15expand_modifierPS2_b
> > (exp=0x700dcf20, target=0x700ec080, tmode=BLKmode,
> >     modifier=EXPAND_NORMAL, alt_rtl=0x17, inner_reference_p=false)
> >     at /nasfarm/edelsohn/src/src/gcc/expr.c:10581
> > #10 0x104158c0 in _Z22store_expr_with_boundsP9tree_nodeP7rtx_defibbS0_ (
> >     exp=0x700dcf20, target=0x700ec080, call_param_p=0, nontemporal=false,
> >     reverse=false, btarget=0x700df058)
> >     at /nasfarm/edelsohn/src/src/gcc/expr.c:5405
> > #11 0x104178fc in _Z17expand_assignmentP9tree_nodeS0_b (to=0x700df058,
> >     from=0x700dcf20, nontemporal=false)
> >     at /nasfarm/edelsohn/src/src/gcc/expr.c:5174
> > #12 0x106f67b4 in _ZL18expand_gimple_stmtP6gimple (stmt=0x7000e240)
> >     at /nasfarm/edelsohn/src/src/gcc/cfgexpand.c:6278
> > #13 0x106f87d8 in _ZL25expand_gimple_basic_blockP15basic_block_defb (
> >     bb=0x700c7740, disable_tail_calls=false)
> >     at /nasfarm/edelsohn/src/src/gcc/cfgexpand.c:5679
> > #14 0x106ffbf4 in _ZN12_GLOBAL__N_111pass_expand7executeEP8function (
> >     this=0x11296dc0
> > <_GLOBAL__F__ZN9text_info9set_rangeEj12source_rangeb+3056>,
> > fun=0x70009138) at /nasfarm/edelsohn/src/src/gcc/cfgexpand.c:6291
> 
> I attempted to reproduce this on gcc111 (powerpc-ibm-aix7.1.3.0)
>   ../src/configure --disable-bootstrap --with-gmp=/opt/cfarm/gmp-latest
> --with-mpfr=/opt/cfarm/mpfr-latest --with-mpc=/opt/cfarm/mpc-latest
> with latest trunk (r230384).
> 
> I saw only one ICE within "make check-gcc", when running
> gcc.c-torture/execute/scal-to-vec1.c; specifically:
> 
>   /home/dmalcolm/gcc-build/build/gcc/xgcc \
>     -B/home/dmalcolm/gcc-build/build/gcc/ \
>     /home/dmalcolm/gcc-build/src/gcc/testsuite/gcc.c-torture/execute/scal-to-vec1.c \
>     -fno-diagnostics-show-caret -fdiagnostics-color=never \
>    -O0  -w  -lm    -o ./scal-to-vec1.exe
> 
> and this shows the same assertion failure as your report.
> 
> 
> I was able to reproduce that ICE at will under gdb; from what I could
> tell from gdb, a seemingly valid location is passed in to warning_at,
> but at warning_at, the 32-bit value is seemingly corrupt, and this
> eventually leads to an assertion failure in the new code.  The warning
> is then discarded (OPT_Wvector_operation_performance).  I can't tell yet
> if the data is corrupt, or if gdb is somehow getting confused about the
> values as I go up and down the callstack (or indeed if I am), but it's
> getting late here.

The root cause is uninitialized data.  Specifically, the C parser's
struct c_expr gained a "src_range" field, and it turns out there are a
few places where I wasn't initializing this when returning c_expr
instances on the stack, and in some cases the values could get used.  I
was able to reproduce it on x86_64 using valgrind; in each case
--track-origins=yes was able to point either directly to or close to the
issue.  (I'm not quite sure why it wasn't leading to issues on x86_64
linux, does the stack grow in a different direction on ppc AIX?)

The attached patch fixes the two specific testcases mentioned above,
plus one Jakub pointed me to on IRC, plus some I found via review of the
source.  It also fixes a buglet in multiline.exp uncovered when writing
the test coverage: braces within a dg-begin/end-multiline-output need to
be escaped.

Bootstrap&regrtest is ongoing on x86_64-pc-linux-gnu.  OK for trunk if
it succeeds?

I'm working on a followup to fix the remaining places I identified via
review of the source.

Sorry about the breakage.
Dave

[-- Attachment #2: 0001-Fix-uninitialized-src_range-values-for-c_expr.patch --]
[-- Type: text/x-patch, Size: 13193 bytes --]

From 61cc4439ca6ea1d89c6cf505ce3fc91490ed4a4d Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Mon, 16 Nov 2015 14:42:18 -0500
Subject: [PATCH] Fix uninitialized src_range values for c_expr

gcc/c/ChangeLog:
	* c-parser.c (c_parser_braced_init): Set src_range for "ret" to a
	sane pair of values.
	(c_parser_unary_expression): Likewise when handling addresses of
	labels.
	(c_parser_postfix_expression): Likewise for statement expressions,
	for __FUNCTION__, __PRETTY_FUNCTION_ and __func__ keywords, for
	__builtin_va_arg, and for __builtin_offset_of.
	(c_parser_postfix_expression_after_paren_type): Initialize expr's
	src_range using the range of the braced initializer.
	(c_parser_transaction_expression): Set src_range for "ret" to a
	sane pair of values.

gcc/testsuite/ChangeLog:
	* gcc.dg/plugin/diagnostic-test-expressions-1.c (vector): New
	macro.
	(test_braced_init): New function.
	(test_statement_expression): New function.
	(test_address_of_label): New function.
	(test_transaction_expressions): New function.
	(test_keywords): New function.
	(test_builtin_va_arg): New function.
	(test_builtin_offsetof): New function.
	* lib/multiline.exp (_build_multiline_regex): Escape braces.
---
 gcc/c/c-parser.c                                   |  91 ++++++++++------
 .../gcc.dg/plugin/diagnostic-test-expressions-1.c  | 121 +++++++++++++++++++++
 gcc/testsuite/lib/multiline.exp                    |   2 +
 3 files changed, 178 insertions(+), 36 deletions(-)

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index e470234..bcad80c 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -4278,9 +4278,11 @@ c_parser_braced_init (c_parser *parser, tree type, bool nested_p)
       obstack_free (&braced_init_obstack, NULL);
       return ret;
     }
+  location_t close_loc = c_parser_peek_token (parser)->location;
   c_parser_consume_token (parser);
   ret = pop_init_level (brace_loc, 0, &braced_init_obstack);
   obstack_free (&braced_init_obstack, NULL);
+  set_c_expr_source_range (&ret, brace_loc, close_loc);
   return ret;
 }
 
@@ -6723,6 +6725,8 @@ c_parser_unary_expression (c_parser *parser)
 	{
 	  ret.value = finish_label_address_expr
 	    (c_parser_peek_token (parser)->value, op_loc);
+	  set_c_expr_source_range (&ret, op_loc,
+				   c_parser_peek_token (parser)->get_finish ());
 	  c_parser_consume_token (parser);
 	}
       else
@@ -7366,11 +7370,13 @@ c_parser_postfix_expression (c_parser *parser)
 	    }
 	  stmt = c_begin_stmt_expr ();
 	  c_parser_compound_statement_nostart (parser);
+	  location_t close_loc = c_parser_peek_token (parser)->location;
 	  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
 				     "expected %<)%>");
 	  pedwarn (loc, OPT_Wpedantic,
 		   "ISO C forbids braced-groups within expressions");
 	  expr.value = c_finish_stmt_expr (brace_loc, stmt);
+	  set_c_expr_source_range (&expr, loc, close_loc);
 	  mark_exp_read (expr.value);
 	}
       else if (c_token_starts_typename (c_parser_peek_2nd_token (parser)))
@@ -7421,6 +7427,7 @@ c_parser_postfix_expression (c_parser *parser)
 	  expr.value = fname_decl (loc,
 				   c_parser_peek_token (parser)->keyword,
 				   c_parser_peek_token (parser)->value);
+	  set_c_expr_source_range (&expr, loc, loc);
 	  c_parser_consume_token (parser);
 	  break;
 	case RID_PRETTY_FUNCTION_NAME:
@@ -7429,6 +7436,7 @@ c_parser_postfix_expression (c_parser *parser)
 	  expr.value = fname_decl (loc,
 				   c_parser_peek_token (parser)->keyword,
 				   c_parser_peek_token (parser)->value);
+	  set_c_expr_source_range (&expr, loc, loc);
 	  c_parser_consume_token (parser);
 	  break;
 	case RID_C99_FUNCTION_NAME:
@@ -7437,45 +7445,51 @@ c_parser_postfix_expression (c_parser *parser)
 	  expr.value = fname_decl (loc,
 				   c_parser_peek_token (parser)->keyword,
 				   c_parser_peek_token (parser)->value);
+	  set_c_expr_source_range (&expr, loc, loc);
 	  c_parser_consume_token (parser);
 	  break;
 	case RID_VA_ARG:
-	  c_parser_consume_token (parser);
-	  if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
-	    {
-	      expr.value = error_mark_node;
-	      break;
-	    }
-	  e1 = c_parser_expr_no_commas (parser, NULL);
-	  mark_exp_read (e1.value);
-	  e1.value = c_fully_fold (e1.value, false, NULL);
-	  if (!c_parser_require (parser, CPP_COMMA, "expected %<,%>"))
-	    {
-	      c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
-	      expr.value = error_mark_node;
-	      break;
-	    }
-	  loc = c_parser_peek_token (parser)->location;
-	  t1 = c_parser_type_name (parser);
-	  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
-				     "expected %<)%>");
-	  if (t1 == NULL)
-	    {
-	      expr.value = error_mark_node;
-	    }
-	  else
-	    {
-	      tree type_expr = NULL_TREE;
-	      expr.value = c_build_va_arg (loc, e1.value,
-					   groktypename (t1, &type_expr, NULL));
-	      if (type_expr)
-		{
-		  expr.value = build2 (C_MAYBE_CONST_EXPR,
-				       TREE_TYPE (expr.value), type_expr,
-				       expr.value);
-		  C_MAYBE_CONST_EXPR_NON_CONST (expr.value) = true;
-		}
-	    }
+	  {
+	    location_t start_loc = loc;
+	    c_parser_consume_token (parser);
+	    if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
+	      {
+		expr.value = error_mark_node;
+		break;
+	      }
+	    e1 = c_parser_expr_no_commas (parser, NULL);
+	    mark_exp_read (e1.value);
+	    e1.value = c_fully_fold (e1.value, false, NULL);
+	    if (!c_parser_require (parser, CPP_COMMA, "expected %<,%>"))
+	      {
+		c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
+		expr.value = error_mark_node;
+		break;
+	      }
+	    loc = c_parser_peek_token (parser)->location;
+	    t1 = c_parser_type_name (parser);
+	    location_t end_loc = c_parser_peek_token (parser)->get_finish ();
+	    c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
+				       "expected %<)%>");
+	    if (t1 == NULL)
+	      {
+		expr.value = error_mark_node;
+	      }
+	    else
+	      {
+		tree type_expr = NULL_TREE;
+		expr.value = c_build_va_arg (loc, e1.value,
+					     groktypename (t1, &type_expr, NULL));
+		if (type_expr)
+		  {
+		    expr.value = build2 (C_MAYBE_CONST_EXPR,
+					 TREE_TYPE (expr.value), type_expr,
+					 expr.value);
+		    C_MAYBE_CONST_EXPR_NON_CONST (expr.value) = true;
+		  }
+		set_c_expr_source_range (&expr, start_loc, end_loc);
+	      }
+	  }
 	  break;
 	case RID_OFFSETOF:
 	  c_parser_consume_token (parser);
@@ -7561,9 +7575,11 @@ c_parser_postfix_expression (c_parser *parser)
 	      }
 	    else
 	      c_parser_error (parser, "expected identifier");
+	    location_t end_loc = c_parser_peek_token (parser)->get_finish ();
 	    c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
 				       "expected %<)%>");
 	    expr.value = fold_offsetof (offsetof_ref);
+	    set_c_expr_source_range (&expr, loc, end_loc);
 	  }
 	  break;
 	case RID_CHOOSE_EXPR:
@@ -7951,6 +7967,7 @@ c_parser_postfix_expression_after_paren_type (c_parser *parser,
 	       : init.original_code == C_MAYBE_CONST_EXPR);
   non_const |= !type_expr_const;
   expr.value = build_compound_literal (start_loc, type, init.value, non_const);
+  set_c_expr_source_range (&expr, init.src_range);
   expr.original_code = ERROR_MARK;
   expr.original_type = NULL;
   if (type != error_mark_node && type_expr)
@@ -17533,6 +17550,8 @@ c_parser_transaction_expression (c_parser *parser, enum rid keyword)
 	: "%<__transaction_relaxed %> "
 	"without transactional memory support enabled"));
 
+  set_c_expr_source_range (&ret, loc, 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 5485aaf..0d8c7c5 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
@@ -397,6 +397,127 @@ void test_comma_operator (int a, int b)
    { dg-end-multiline-output "" } */
 }
 
+/* Braced initializers.  ***************************************/
+
+/* We can't test the ranges of these directly, since the underlying
+   tree nodes don't retain a location.  However, we can test that they
+   have ranges during parsing by building compound expressions using
+   them, and verifying the ranges of the compound expressions.  */
+
+#define vector(elcount, type)  \
+__attribute__((vector_size((elcount)*sizeof(type)))) type
+
+void test_braced_init (void)
+{
+  /* Verify start of range.  */
+  __emit_expression_range (0, (vector(4, float)){2., 2., 2., 2.} + 1); /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0, (vector(4, float)){2., 2., 2., 2.} + 1);
+                                                 ~~~~~~~~~~~~~~~~~^~~
+   { dg-end-multiline-output "" } */
+
+  /* Verify end of range.  */
+  __emit_expression_range (0, &(vector(4, float)){2., 2., 2., 2.}); /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0, &(vector(4, float)){2., 2., 2., 2.});
+                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+}
+
+/* Statement expressions.  ***************************************/
+
+void test_statement_expression (void)
+{
+  __emit_expression_range (0, ({ static int a; a; }) );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0, ({ static int a; a; }) );
+                               ~^~~~~~~~~~~~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+}
+
+/* Other expressions.  */
+
+void test_address_of_label (void)
+{
+ label:
+  __emit_expression_range (0, &&label );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0, &&label );
+                               ^~~~~~~
+   { dg-end-multiline-output "" } */
+}
+
+void test_transaction_expressions (void)
+{
+  int i;
+  i = __transaction_atomic (42); /* { dg-error "without transactional memory support enabled" } */
+/* { dg-begin-multiline-output "" }
+   i = __transaction_atomic (42);
+       ^~~~~~~~~~~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+  i = __transaction_relaxed (42); /* { dg-error "without transactional memory support enabled" } */
+/* { dg-begin-multiline-output "" }
+   i = __transaction_relaxed (42);
+       ^~~~~~~~~~~~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+}
+
+void test_keywords (int i)
+{
+  __emit_expression_range (0, __FUNCTION__[i] );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0, __FUNCTION__[i] );
+                               ~~~~~~~~~~~~^~~
+   { dg-end-multiline-output "" } */
+
+  __emit_expression_range (0, __PRETTY_FUNCTION__ );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0, __PRETTY_FUNCTION__ );
+                               ^~~~~~~~~~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+
+  __emit_expression_range (0, __func__ );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0, __func__ );
+                               ^~~~~~~~
+   { dg-end-multiline-output "" } */
+}
+
+void test_builtin_va_arg (__builtin_va_list v)
+{
+  __emit_expression_range (0,  __builtin_va_arg (v, int) );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0,  __builtin_va_arg (v, int) );
+                                ~~~~~~~~~~~~~~~~~~~~~^~~~
+   { dg-end-multiline-output "" } */
+
+  __emit_expression_range (0,  __builtin_va_arg (v, int) + 1 );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0,  __builtin_va_arg (v, int) + 1 );
+                                ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
+   { dg-end-multiline-output "" } */
+}
+
+struct s
+{
+  int f;
+};
+
+void test_builtin_offsetof (int i)
+{
+  __emit_expression_range (0,  i + __builtin_offsetof (struct s, f) );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0,  i + __builtin_offsetof (struct s, f) );
+                                ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+   { dg-end-multiline-output "" } */
+
+  __emit_expression_range (0,  __builtin_offsetof (struct s, f) + i );  /* { dg-warning "range" } */
+/* { dg-begin-multiline-output "" }
+   __emit_expression_range (0,  __builtin_offsetof (struct s, f) + i );
+                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
+   { dg-end-multiline-output "" } */
+}
+
 /* Examples of non-trivial expressions.  ****************************/
 
 extern double sqrt (double x);
diff --git a/gcc/testsuite/lib/multiline.exp b/gcc/testsuite/lib/multiline.exp
index eb72143..c3d0506 100644
--- a/gcc/testsuite/lib/multiline.exp
+++ b/gcc/testsuite/lib/multiline.exp
@@ -181,6 +181,8 @@ proc _build_multiline_regex { multiline index } {
 	                      ")" "\\)"
 	                      "[" "\\["
 	                      "]" "\\]"
+	                      "{" "\\{"
+	                      "}" "\\}"
 	                      "." "\\."
 	                      "\\" "\\\\"
 	                      "?" "\\?"
-- 
1.8.5.3

Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))

[Bernd Schmidt]

On 11/16/2015 09:50 PM, David Malcolm wrote:
> The root cause is uninitialized data.  Specifically, the C parser's
> struct c_expr gained a "src_range" field, and it turns out there are a
> few places where I wasn't initializing this when returning c_expr
> instances on the stack, and in some cases the values could get used.

> I'm working on a followup to fix the remaining places I identified via
> review of the source.

The patch is mostly OK IMO and should be installed to fix the problems, 
but I think there are a few more things to consider.

Should c_expr perhaps acquire a constructor so that this problem is 
avoided in the future? The whole thing seems somewhat error-prone.

> @@ -4278,9 +4278,11 @@ c_parser_braced_init (c_parser *parser, tree type, bool nested_p)
>         obstack_free (&braced_init_obstack, NULL);
>         return ret;
>       }
> +  location_t close_loc = c_parser_peek_token (parser)->location;

It looks like we're peeking the token twice here (via a 
c_parser_token_is_not call above the quoted code). Probably not too 
expensive but maybe we can avoid it.

>   	case RID_VA_ARG:
> -	  c_parser_consume_token (parser);
> +	  {
> +	    location_t start_loc = loc;

Does this really have to be indented in an extra braced block? Please 
fix if not.


Bernd

Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))

[David Malcolm]

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

On Mon, 2015-11-16 at 22:34 +0100, Bernd Schmidt wrote:
> On 11/16/2015 09:50 PM, David Malcolm wrote:
> > The root cause is uninitialized data.  Specifically, the C parser's
> > struct c_expr gained a "src_range" field, and it turns out there are a
> > few places where I wasn't initializing this when returning c_expr
> > instances on the stack, and in some cases the values could get used.
> 
> > I'm working on a followup to fix the remaining places I identified via
> > review of the source.
> 
> The patch is mostly OK IMO and should be installed to fix the problems, 

I'm attaching two followup patches.

The patch as is introduces some ICEs due to accessing EXPR_LOCATION ()
of a c_expr's "value" field, for some cases where value is NULL.  The
first attached patch bulletproofs both implementations of
set_c_expr_source_range for this case.

I've successfully bootstrapped and regression-tested the combination of
this plus the previous patch on x86_64-pc-linux-gnu; I've also got a
bootstrap&regrtest ongoing on powerpc-ibm-aix7.1.3.0.

> but I think there are a few more things to consider.
> 
> Should c_expr perhaps acquire a constructor so that this problem is 
> avoided in the future? The whole thing seems somewhat error-prone.

I agree that it's error prone, and the ctor approach is what I've been
trying for the C++ FE [1] but I suspect that touching that in the C FE
would be a much more invasive patch (unless we simply give it a default
ctor that makes the src_range be a pair of UNKNOWN_LOCATIONS?).  I'll
give it a go, but it feels like a separate followup.

> > @@ -4278,9 +4278,11 @@ c_parser_braced_init (c_parser *parser, tree type, bool nested_p)
> >         obstack_free (&braced_init_obstack, NULL);
> >         return ret;
> >       }
> > +  location_t close_loc = c_parser_peek_token (parser)->location;
> 
> It looks like we're peeking the token twice here (via a 
> c_parser_token_is_not call above the quoted code). Probably not too 
> expensive but maybe we can avoid it.

Thanks; I'm also attaching a patch that does so (not yet bootstrapped,
but will do so).


> >   	case RID_VA_ARG:
> > -	  c_parser_consume_token (parser);
> > +	  {
> > +	    location_t start_loc = loc;
> 
> Does this really have to be indented in an extra braced block? Please 
> fix if not.

This case gains a pair of locals: start_loc and end_loc (so that we can
track the spelling range whilst retaining the "loc" used for the caret),
and I preferred to confine their scope to within the case, hence the
extra braced block.  Omitting the braced block leads to:
../../src/gcc/c/c-parser.c:7494:7: error: jump to case label [-fpermissive]
  case RID_OFFSETOF:
       ^
../../src/gcc/c/c-parser.c:7472:17: error:   crosses initialization of ‘location_t end_loc’
      location_t end_loc = c_parser_peek_token (parser)->get_finish ();
                 ^
etc.  I could fix that by moving the locals to the top of the function,
but that seems messy, so it seemed best to add the braces (and hence
indent).
Hope that sounds like the right trade-off.

Is the combination of the 3 patches OK for trunk? (assuming
bootstrap&regrest; it's only the braced-init tweak that hasn't been).

Thanks
Dave
[1] in "[PATCH/RFC] C++ FE: expression ranges (v2)":
   https://gcc.gnu.org/ml/gcc-patches/2015-11/msg01859.html


[-- Attachment #2: 0002-Bulletproof-set_c_expr_source_range-against-NULL-exp.patch --]
[-- Type: text/x-patch, Size: 1095 bytes --]

From a19859a1ecdf850684b8d191b0ff57c8e50cc121 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Tue, 17 Nov 2015 06:09:52 -0500
Subject: [PATCH 2/3] Bulletproof set_c_expr_source_range against NULL
 expr->value

gcc/c/ChangeLog:
	* c-parser.c (set_c_expr_source_range): Bulletproof both
	overloaded implementations against NULL expr->value.
---
 gcc/c/c-parser.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index bcad80c..9ab7ceb 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -65,7 +65,8 @@ set_c_expr_source_range (c_expr *expr,
 {
   expr->src_range.m_start = start;
   expr->src_range.m_finish = finish;
-  set_source_range (expr->value, start, finish);
+  if (expr->value)
+    set_source_range (expr->value, start, finish);
 }
 
 void
@@ -73,7 +74,8 @@ set_c_expr_source_range (c_expr *expr,
 			 source_range src_range)
 {
   expr->src_range = src_range;
-  set_source_range (expr->value, src_range);
+  if (expr->value)
+    set_source_range (expr->value, src_range);
 }
 
 \f
-- 
1.8.5.3


[-- Attachment #3: 0003-Lookup-next-token-once-at-end-of-c_parser_braced_ini.patch --]
[-- Type: text/x-patch, Size: 1229 bytes --]

From 205da878acc752adb275da3ca61a342a9a124f93 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Tue, 17 Nov 2015 10:18:39 -0500
Subject: [PATCH 3/3] Lookup next token once at end of c_parser_braced_init,
 not twice

---
 gcc/c/c-parser.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 9ab7ceb..eedcaa4 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -4270,7 +4270,8 @@ c_parser_braced_init (c_parser *parser, tree type, bool nested_p)
 	    break;
 	}
     }
-  if (c_parser_next_token_is_not (parser, CPP_CLOSE_BRACE))
+  c_token *next_tok = c_parser_peek_token (parser);
+  if (next_tok->type != CPP_CLOSE_BRACE)
     {
       ret.value = error_mark_node;
       ret.original_code = ERROR_MARK;
@@ -4280,7 +4281,7 @@ c_parser_braced_init (c_parser *parser, tree type, bool nested_p)
       obstack_free (&braced_init_obstack, NULL);
       return ret;
     }
-  location_t close_loc = c_parser_peek_token (parser)->location;
+  location_t close_loc = next_tok->location;
   c_parser_consume_token (parser);
   ret = pop_init_level (brace_loc, 0, &braced_init_obstack);
   obstack_free (&braced_init_obstack, NULL);
-- 
1.8.5.3

Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))

[Bernd Schmidt]

On 11/17/2015 04:13 PM, David Malcolm wrote:
> On Mon, 2015-11-16 at 22:34 +0100, Bernd Schmidt wrote:
>>
>> Should c_expr perhaps acquire a constructor so that this problem is
>> avoided in the future? The whole thing seems somewhat error-prone.
>
> I agree that it's error prone, and the ctor approach is what I've been
> trying for the C++ FE [1] but I suspect that touching that in the C FE
> would be a much more invasive patch (unless we simply give it a default
> ctor that makes the src_range be a pair of UNKNOWN_LOCATIONS?).

The UNKNOWN_LOCATIONS pair would have been my approach, yes.

> This case gains a pair of locals: start_loc and end_loc (so that we can
> track the spelling range whilst retaining the "loc" used for the caret),
> and I preferred to confine their scope to within the case, hence the
> extra braced block.  Omitting the braced block leads to:
> ../../src/gcc/c/c-parser.c:7494:7: error: jump to case label [-fpermissive]
>    case RID_OFFSETOF:
>         ^
> ../../src/gcc/c/c-parser.c:7472:17: error:   crosses initialization of ‘location_t end_loc’
>        location_t end_loc = c_parser_peek_token (parser)->get_finish ();
>                   ^
> etc.

Hmm, odd, I tried placing just the location_t start_loc line into the 
switch and that appeared to compile fine. But I guess this is not a huge 
problem.
>
> Is the combination of the 3 patches OK for trunk? (assuming
> bootstrap&regrest; it's only the braced-init tweak that hasn't been).

Yes.


Bernd

Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))

[David Malcolm]

On Tue, 2015-11-17 at 16:24 +0100, Bernd Schmidt wrote:
> On 11/17/2015 04:13 PM, David Malcolm wrote:
> > On Mon, 2015-11-16 at 22:34 +0100, Bernd Schmidt wrote:
> >>
> >> Should c_expr perhaps acquire a constructor so that this problem is
> >> avoided in the future? The whole thing seems somewhat error-prone.
> >
> > I agree that it's error prone, and the ctor approach is what I've been
> > trying for the C++ FE [1] but I suspect that touching that in the C FE
> > would be a much more invasive patch (unless we simply give it a default
> > ctor that makes the src_range be a pair of UNKNOWN_LOCATIONS?).
> 
> The UNKNOWN_LOCATIONS pair would have been my approach, yes.
> 
> > This case gains a pair of locals: start_loc and end_loc (so that we can
> > track the spelling range whilst retaining the "loc" used for the caret),
> > and I preferred to confine their scope to within the case, hence the
> > extra braced block.  Omitting the braced block leads to:
> > ../../src/gcc/c/c-parser.c:7494:7: error: jump to case label [-fpermissive]
> >    case RID_OFFSETOF:
> >         ^
> > ../../src/gcc/c/c-parser.c:7472:17: error:   crosses initialization of ‘location_t end_loc’
> >        location_t end_loc = c_parser_peek_token (parser)->get_finish ();
> >                   ^
> > etc.
> 
> Hmm, odd, I tried placing just the location_t start_loc line into the 
> switch and that appeared to compile fine. But I guess this is not a huge 
> problem.
> >
> > Is the combination of the 3 patches OK for trunk? (assuming
> > bootstrap&regrest; it's only the braced-init tweak that hasn't been).
> 
> Yes.

Thanks.  I've committed the 3 patches to trunk as r230497, which should
fix the worst of the regressions caused by r230331 seen on AIX.  I'll
continue to investigate as per the discussion above.

Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))

[David Edelsohn]

On Tue, Nov 17, 2015 at 3:12 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Tue, 2015-11-17 at 16:24 +0100, Bernd Schmidt wrote:
>> On 11/17/2015 04:13 PM, David Malcolm wrote:
>> > On Mon, 2015-11-16 at 22:34 +0100, Bernd Schmidt wrote:
>> >>
>> >> Should c_expr perhaps acquire a constructor so that this problem is
>> >> avoided in the future? The whole thing seems somewhat error-prone.
>> >
>> > I agree that it's error prone, and the ctor approach is what I've been
>> > trying for the C++ FE [1] but I suspect that touching that in the C FE
>> > would be a much more invasive patch (unless we simply give it a default
>> > ctor that makes the src_range be a pair of UNKNOWN_LOCATIONS?).
>>
>> The UNKNOWN_LOCATIONS pair would have been my approach, yes.
>>
>> > This case gains a pair of locals: start_loc and end_loc (so that we can
>> > track the spelling range whilst retaining the "loc" used for the caret),
>> > and I preferred to confine their scope to within the case, hence the
>> > extra braced block.  Omitting the braced block leads to:
>> > ../../src/gcc/c/c-parser.c:7494:7: error: jump to case label [-fpermissive]
>> >    case RID_OFFSETOF:
>> >         ^
>> > ../../src/gcc/c/c-parser.c:7472:17: error:   crosses initialization of ‘location_t end_loc’
>> >        location_t end_loc = c_parser_peek_token (parser)->get_finish ();
>> >                   ^
>> > etc.
>>
>> Hmm, odd, I tried placing just the location_t start_loc line into the
>> switch and that appeared to compile fine. But I guess this is not a huge
>> problem.
>> >
>> > Is the combination of the 3 patches OK for trunk? (assuming
>> > bootstrap&regrest; it's only the braced-init tweak that hasn't been).
>>
>> Yes.
>
> Thanks.  I've committed the 3 patches to trunk as r230497, which should
> fix the worst of the regressions caused by r230331 seen on AIX.  I'll
> continue to investigate as per the discussion above.

Hi, David

The new stret-1.m Objective C failure on AIX shows the same symptoms.
Is there another fix needed for Objective C?

#1  0x10016794 in _Z14linemap_lookupP9line_mapsj (set=0x70000000, line=991)
    at /nasfarm/edelsohn/src/src/libcpp/line-map.c:991
991       linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set));


Thanks, David

Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))

[David Malcolm]

On Sat, 2015-11-21 at 13:54 -0500, David Edelsohn wrote:
> On Tue, Nov 17, 2015 at 3:12 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> > On Tue, 2015-11-17 at 16:24 +0100, Bernd Schmidt wrote:
> >> On 11/17/2015 04:13 PM, David Malcolm wrote:
> >> > On Mon, 2015-11-16 at 22:34 +0100, Bernd Schmidt wrote:
> >> >>
> >> >> Should c_expr perhaps acquire a constructor so that this problem is
> >> >> avoided in the future? The whole thing seems somewhat error-prone.
> >> >
> >> > I agree that it's error prone, and the ctor approach is what I've been
> >> > trying for the C++ FE [1] but I suspect that touching that in the C FE
> >> > would be a much more invasive patch (unless we simply give it a default
> >> > ctor that makes the src_range be a pair of UNKNOWN_LOCATIONS?).
> >>
> >> The UNKNOWN_LOCATIONS pair would have been my approach, yes.
> >>
> >> > This case gains a pair of locals: start_loc and end_loc (so that we can
> >> > track the spelling range whilst retaining the "loc" used for the caret),
> >> > and I preferred to confine their scope to within the case, hence the
> >> > extra braced block.  Omitting the braced block leads to:
> >> > ../../src/gcc/c/c-parser.c:7494:7: error: jump to case label [-fpermissive]
> >> >    case RID_OFFSETOF:
> >> >         ^
> >> > ../../src/gcc/c/c-parser.c:7472:17: error:   crosses initialization of ‘location_t end_loc’
> >> >        location_t end_loc = c_parser_peek_token (parser)->get_finish ();
> >> >                   ^
> >> > etc.
> >>
> >> Hmm, odd, I tried placing just the location_t start_loc line into the
> >> switch and that appeared to compile fine. But I guess this is not a huge
> >> problem.
> >> >
> >> > Is the combination of the 3 patches OK for trunk? (assuming
> >> > bootstrap&regrest; it's only the braced-init tweak that hasn't been).
> >>
> >> Yes.
> >
> > Thanks.  I've committed the 3 patches to trunk as r230497, which should
> > fix the worst of the regressions caused by r230331 seen on AIX.  I'll
> > continue to investigate as per the discussion above.
> 
> Hi, David
> 
> The new stret-1.m Objective C failure on AIX shows the same symptoms.
> Is there another fix needed for Objective C?
> 
> #1  0x10016794 in _Z14linemap_lookupP9line_mapsj (set=0x70000000, line=991)
>     at /nasfarm/edelsohn/src/src/libcpp/line-map.c:991
> 991       linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set));

I believe this one is fixed by the patch I posted here:
 "[PATCH] Fix PR objc/68438 (uninitialized source ranges)"
   https://gcc.gnu.org/ml/gcc-patches/2015-11/msg02536.html

(it runs cleanly under valgrind on x86_64 with that patch applied)

Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))

[David Edelsohn]

On Sat, Nov 21, 2015 at 3:00 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Sat, 2015-11-21 at 13:54 -0500, David Edelsohn wrote:
>> On Tue, Nov 17, 2015 at 3:12 PM, David Malcolm <dmalcolm@redhat.com> wrote:
>> > On Tue, 2015-11-17 at 16:24 +0100, Bernd Schmidt wrote:
>> >> On 11/17/2015 04:13 PM, David Malcolm wrote:
>> >> > On Mon, 2015-11-16 at 22:34 +0100, Bernd Schmidt wrote:
>> >> >>
>> >> >> Should c_expr perhaps acquire a constructor so that this problem is
>> >> >> avoided in the future? The whole thing seems somewhat error-prone.
>> >> >
>> >> > I agree that it's error prone, and the ctor approach is what I've been
>> >> > trying for the C++ FE [1] but I suspect that touching that in the C FE
>> >> > would be a much more invasive patch (unless we simply give it a default
>> >> > ctor that makes the src_range be a pair of UNKNOWN_LOCATIONS?).
>> >>
>> >> The UNKNOWN_LOCATIONS pair would have been my approach, yes.
>> >>
>> >> > This case gains a pair of locals: start_loc and end_loc (so that we can
>> >> > track the spelling range whilst retaining the "loc" used for the caret),
>> >> > and I preferred to confine their scope to within the case, hence the
>> >> > extra braced block.  Omitting the braced block leads to:
>> >> > ../../src/gcc/c/c-parser.c:7494:7: error: jump to case label [-fpermissive]
>> >> >    case RID_OFFSETOF:
>> >> >         ^
>> >> > ../../src/gcc/c/c-parser.c:7472:17: error:   crosses initialization of ‘location_t end_loc’
>> >> >        location_t end_loc = c_parser_peek_token (parser)->get_finish ();
>> >> >                   ^
>> >> > etc.
>> >>
>> >> Hmm, odd, I tried placing just the location_t start_loc line into the
>> >> switch and that appeared to compile fine. But I guess this is not a huge
>> >> problem.
>> >> >
>> >> > Is the combination of the 3 patches OK for trunk? (assuming
>> >> > bootstrap&regrest; it's only the braced-init tweak that hasn't been).
>> >>
>> >> Yes.
>> >
>> > Thanks.  I've committed the 3 patches to trunk as r230497, which should
>> > fix the worst of the regressions caused by r230331 seen on AIX.  I'll
>> > continue to investigate as per the discussion above.
>>
>> Hi, David
>>
>> The new stret-1.m Objective C failure on AIX shows the same symptoms.
>> Is there another fix needed for Objective C?
>>
>> #1  0x10016794 in _Z14linemap_lookupP9line_mapsj (set=0x70000000, line=991)
>>     at /nasfarm/edelsohn/src/src/libcpp/line-map.c:991
>> 991       linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set));
>
> I believe this one is fixed by the patch I posted here:
>  "[PATCH] Fix PR objc/68438 (uninitialized source ranges)"
>    https://gcc.gnu.org/ml/gcc-patches/2015-11/msg02536.html
>
> (it runs cleanly under valgrind on x86_64 with that patch applied)

Yep, thanks.  I missed the follow-on patch.

Thanks, David

Fwd: libcpp/C FE source range patch committed (r230331).

[Jay Foad]

(Resending as plain text. Sorry for the HTML!)

On 14 November 2015 at 14:50, David Edelsohn <dje.gcc@gmail.com> wrote:
>
> This patch causes numerous new testsuite failure on AIX caused by the
> compiler crashing during compilation, e.g.

r230331 also seems to be causing this on x86_64-pc-linux-gnu:

$ cat x.c
#define P(b) b&&4
int a[]=0;
int f() { X||P(d); }
$ ~/gcc/build/gcc/cc1 -quiet -Wall x.c
[...]
x.c:3:1: internal compiler error: in contains_point, at
diagnostic-show-locus.c:335
 int f() { X||P(d); }
 ^~~

0x1268fc9 contains_point
/home/jay/svn/gcc/trunk/gcc/diagnostic-show-locus.c:335
0x1268fc9 get_state_at_point
/home/jay/svn/gcc/trunk/gcc/diagnostic-show-locus.c:612
0x12696e2 print_source_line
/home/jay/svn/gcc/trunk/gcc/diagnostic-show-locus.c:533
0x12696e2 diagnostic_show_locus(diagnostic_context*, diagnostic_info const*)
/home/jay/svn/gcc/trunk/gcc/diagnostic-show-locus.c:710
0x69b210 c_diagnostic_finalizer
/home/jay/svn/gcc/trunk/gcc/c-family/c-opts.c:167
0x1267220 diagnostic_report_diagnostic(diagnostic_context*, diagnostic_info*)
/home/jay/svn/gcc/trunk/gcc/diagnostic.c:800
0x1267b07 warning_at(unsigned int, int, char const*, ...)
/home/jay/svn/gcc/trunk/gcc/diagnostic.c:1029
0x607e58 parser_build_binary_op(unsigned int, tree_code, c_expr, c_expr)
/home/jay/svn/gcc/trunk/gcc/c/c-typeck.c:3514
0x61855a c_parser_binary_expression
/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:6539
0x618a18 c_parser_conditional_expression
/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:6182
0x619100 c_parser_expr_no_commas
/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:6099
0x6198d2 c_parser_expression
/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:8230
0x61a3a9 c_parser_expression_conv
/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:8263
0x633431 c_parser_statement_after_labels
/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:5172
0x635065 c_parser_compound_statement_nostart
/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:4757
0x6358ae c_parser_compound_statement
/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:4594
0x6314e3 c_parser_declaration_or_fndef
/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:2015
0x63d31d c_parser_external_declaration
/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:1459
0x63dbe9 c_parser_translation_unit
/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:1346
0x63dbe9 c_parse_file()
/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:17622
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.

Jay.

Re: Fwd: libcpp/C FE source range patch committed (r230331).

[Marek Polacek]

On Tue, Nov 24, 2015 at 11:24:38AM +0000, Jay Foad wrote:
> r230331 also seems to be causing this on x86_64-pc-linux-gnu:
> 
> $ cat x.c
> #define P(b) b&&4
> int a[]=0;
> int f() { X||P(d); }
> $ ~/gcc/build/gcc/cc1 -quiet -Wall x.c
> [...]
> x.c:3:1: internal compiler error: in contains_point, at
> diagnostic-show-locus.c:335
>  int f() { X||P(d); }

Could you please open a PR?

	Marek

Re: Fwd: libcpp/C FE source range patch committed (r230331).

[Jay Foad]

On 24 November 2015 at 11:34, Marek Polacek <polacek@redhat.com> wrote:
> On Tue, Nov 24, 2015 at 11:24:38AM +0000, Jay Foad wrote:
>> r230331 also seems to be causing this on x86_64-pc-linux-gnu:
>>
>> $ cat x.c
>> #define P(b) b&&4
>> int a[]=0;
>> int f() { X||P(d); }
>> $ ~/gcc/build/gcc/cc1 -quiet -Wall x.c
>> [...]
>> x.c:3:1: internal compiler error: in contains_point, at
>> diagnostic-show-locus.c:335
>>  int f() { X||P(d); }
>
> Could you please open a PR?

I see now that it's already reported as PR c/68473.

Jay.

Jay.

Re: Fwd: libcpp/C FE source range patch committed (r230331).

[Marek Polacek]

On Tue, Nov 24, 2015 at 11:50:21AM +0000, Jay Foad wrote:
> I see now that it's already reported as PR c/68473.

Ooops, sorry, I knew about that one, but thought that's a different problem.

	Marek