* Re: libcpp/C FE source range patch committed (r230331). @ 2015-11-14 14:50 David Edelsohn 2015-11-15 4:32 ` David Malcolm [not found] ` <CANd1uZkKmxqX_6y0M6dxgfpdf83HuXX9+gRBiqzYECqon54NrQ@mail.gmail.com> 0 siblings, 2 replies; 14+ messages in thread From: David Edelsohn @ 2015-11-14 14:50 UTC (permalink / raw) To: David Malcolm; +Cc: Jeffrey Law, GCC Patches, Richard Biener, Dodji Seketeli 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: libcpp/C FE source range patch committed (r230331). 2015-11-14 14:50 libcpp/C FE source range patch committed (r230331) David Edelsohn @ 2015-11-15 4:32 ` David Malcolm 2015-11-16 20:50 ` [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331)) David Malcolm [not found] ` <CANd1uZkKmxqX_6y0M6dxgfpdf83HuXX9+gRBiqzYECqon54NrQ@mail.gmail.com> 1 sibling, 1 reply; 14+ messages in thread From: David Malcolm @ 2015-11-15 4:32 UTC (permalink / raw) To: David Edelsohn; +Cc: Jeffrey Law, GCC Patches, Richard Biener, Dodji Seketeli 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331)) 2015-11-15 4:32 ` David Malcolm @ 2015-11-16 20:50 ` David Malcolm 2015-11-16 21:34 ` Bernd Schmidt 0 siblings, 1 reply; 14+ messages in thread From: David Malcolm @ 2015-11-16 20:50 UTC (permalink / raw) To: David Edelsohn; +Cc: Jeffrey Law, GCC Patches, Richard Biener, Dodji Seketeli [-- 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®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 [-- 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331)) 2015-11-16 20:50 ` [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331)) David Malcolm @ 2015-11-16 21:34 ` Bernd Schmidt 2015-11-17 15:13 ` David Malcolm 0 siblings, 1 reply; 14+ messages in thread From: Bernd Schmidt @ 2015-11-16 21:34 UTC (permalink / raw) To: David Malcolm, David Edelsohn Cc: Jeffrey Law, GCC Patches, Richard Biener, Dodji Seketeli 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331)) 2015-11-16 21:34 ` Bernd Schmidt @ 2015-11-17 15:13 ` David Malcolm 2015-11-17 15:24 ` Bernd Schmidt 0 siblings, 1 reply; 14+ messages in thread From: David Malcolm @ 2015-11-17 15:13 UTC (permalink / raw) To: Bernd Schmidt Cc: David Edelsohn, Jeffrey Law, GCC Patches, Richard Biener, Dodji Seketeli [-- 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®rtest 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®rest; 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331)) 2015-11-17 15:13 ` David Malcolm @ 2015-11-17 15:24 ` Bernd Schmidt 2015-11-17 20:12 ` David Malcolm 0 siblings, 1 reply; 14+ messages in thread From: Bernd Schmidt @ 2015-11-17 15:24 UTC (permalink / raw) To: David Malcolm Cc: David Edelsohn, Jeffrey Law, GCC Patches, Richard Biener, Dodji Seketeli 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®rest; it's only the braced-init tweak that hasn't been). Yes. Bernd ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331)) 2015-11-17 15:24 ` Bernd Schmidt @ 2015-11-17 20:12 ` David Malcolm 2015-11-21 18:58 ` David Edelsohn 0 siblings, 1 reply; 14+ messages in thread From: David Malcolm @ 2015-11-17 20:12 UTC (permalink / raw) To: Bernd Schmidt Cc: David Edelsohn, Jeffrey Law, GCC Patches, Richard Biener, Dodji Seketeli 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®rest; 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331)) 2015-11-17 20:12 ` David Malcolm @ 2015-11-21 18:58 ` David Edelsohn 2015-11-21 20:07 ` David Malcolm 0 siblings, 1 reply; 14+ messages in thread From: David Edelsohn @ 2015-11-21 18:58 UTC (permalink / raw) To: David Malcolm Cc: Bernd Schmidt, Jeffrey Law, GCC Patches, Richard Biener, Dodji Seketeli 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®rest; 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331)) 2015-11-21 18:58 ` David Edelsohn @ 2015-11-21 20:07 ` David Malcolm 2015-11-21 22:03 ` David Edelsohn 0 siblings, 1 reply; 14+ messages in thread From: David Malcolm @ 2015-11-21 20:07 UTC (permalink / raw) To: David Edelsohn Cc: Bernd Schmidt, Jeffrey Law, GCC Patches, Richard Biener, Dodji Seketeli 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®rest; 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) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331)) 2015-11-21 20:07 ` David Malcolm @ 2015-11-21 22:03 ` David Edelsohn 0 siblings, 0 replies; 14+ messages in thread From: David Edelsohn @ 2015-11-21 22:03 UTC (permalink / raw) To: David Malcolm Cc: Bernd Schmidt, Jeffrey Law, GCC Patches, Richard Biener, Dodji Seketeli 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®rest; 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <CANd1uZkKmxqX_6y0M6dxgfpdf83HuXX9+gRBiqzYECqon54NrQ@mail.gmail.com>]
* Fwd: libcpp/C FE source range patch committed (r230331). [not found] ` <CANd1uZkKmxqX_6y0M6dxgfpdf83HuXX9+gRBiqzYECqon54NrQ@mail.gmail.com> @ 2015-11-24 11:27 ` Jay Foad 2015-11-24 11:37 ` Marek Polacek 0 siblings, 1 reply; 14+ messages in thread From: Jay Foad @ 2015-11-24 11:27 UTC (permalink / raw) To: David Edelsohn Cc: Cc: David Malcolm, Jeffrey Law, GCC Patches, Richard Biener, Dodji Seketeli (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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Fwd: libcpp/C FE source range patch committed (r230331). 2015-11-24 11:27 ` Fwd: libcpp/C FE source range patch committed (r230331) Jay Foad @ 2015-11-24 11:37 ` Marek Polacek 2015-11-24 11:54 ` Jay Foad 0 siblings, 1 reply; 14+ messages in thread From: Marek Polacek @ 2015-11-24 11:37 UTC (permalink / raw) To: Jay Foad Cc: David Edelsohn, Cc: David Malcolm, Jeffrey Law, GCC Patches, Richard Biener, Dodji Seketeli 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Fwd: libcpp/C FE source range patch committed (r230331). 2015-11-24 11:37 ` Marek Polacek @ 2015-11-24 11:54 ` Jay Foad 2015-11-24 11:55 ` Marek Polacek 0 siblings, 1 reply; 14+ messages in thread From: Jay Foad @ 2015-11-24 11:54 UTC (permalink / raw) To: Marek Polacek Cc: David Edelsohn, Cc: David Malcolm, Jeffrey Law, GCC Patches, Richard Biener, Dodji Seketeli 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Fwd: libcpp/C FE source range patch committed (r230331). 2015-11-24 11:54 ` Jay Foad @ 2015-11-24 11:55 ` Marek Polacek 0 siblings, 0 replies; 14+ messages in thread From: Marek Polacek @ 2015-11-24 11:55 UTC (permalink / raw) To: Jay Foad Cc: David Edelsohn, Cc: David Malcolm, Jeffrey Law, GCC Patches, Richard Biener, Dodji Seketeli 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-11-24 11:54 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-11-14 14:50 libcpp/C FE source range patch committed (r230331) David Edelsohn 2015-11-15 4:32 ` David Malcolm 2015-11-16 20:50 ` [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331)) David Malcolm 2015-11-16 21:34 ` Bernd Schmidt 2015-11-17 15:13 ` David Malcolm 2015-11-17 15:24 ` Bernd Schmidt 2015-11-17 20:12 ` David Malcolm 2015-11-21 18:58 ` David Edelsohn 2015-11-21 20:07 ` David Malcolm 2015-11-21 22:03 ` David Edelsohn [not found] ` <CANd1uZkKmxqX_6y0M6dxgfpdf83HuXX9+gRBiqzYECqon54NrQ@mail.gmail.com> 2015-11-24 11:27 ` Fwd: libcpp/C FE source range patch committed (r230331) Jay Foad 2015-11-24 11:37 ` Marek Polacek 2015-11-24 11:54 ` Jay Foad 2015-11-24 11:55 ` Marek Polacek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).