This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))
- From: David Malcolm <dmalcolm at redhat dot com>
- To: David Edelsohn <dje dot gcc at gmail dot com>
- Cc: Jeffrey Law <law at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Richard Biener <richard dot guenther at gmail dot com>, Dodji Seketeli <dodji at redhat dot com>
- Date: Mon, 16 Nov 2015 15:50:00 -0500
- Subject: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))
- Authentication-results: sourceware.org; auth=none
- References: <CAGWvnynj92T8T_U8P7x1SPyQpiW=_1jtUWhMxSn+aiVo+Ffkbw at mail dot gmail dot com> <1447561939 dot 19594 dot 27 dot camel at surprise>
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®rtest 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
>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