Re: [PING] [PATCH] c/71392 - SEGV calling integer overflow built-ins with a null pointer
> Attached is an updated version of the original patch described > below to annotate with the nonnull attribute the Built-In Functions > to Perform Arithmetic with Overflow Checking. 3 out of 5 ChangeLog entries are incorrect: gcc/ada/ChangeLog: PR c/71392 * gcc/ada/gcc-interface/utils.c (handle_nonnull_attribute): Accept the nonnull attribute in type-generic builtins. gcc/c-family/ChangeLog: PR c/71392 * gcc/c-family/c-common.c (handle_nonnull_attribute): Accept the nonnull attribute in type-generic builtins. gcc/lto/ChangeLog: PR c/71392 * gcc/lto/lto-lang.c (handle_nonnull_attribute): Accept the nonnull attribute in type-generic builtins. Prefixes of files must always be relative to the ChangeLog's directory: gcc/ada/ChangeLog: PR c/71392 * gcc-interface/utils.c (handle_nonnull_attribute): ... gcc/c-family/ChangeLog: PR c/71392 * c-common.c (handle_nonnull_attribute):... gcc/lto/ChangeLog: PR c/71392 * lto-lang.c (handle_nonnull_attribute): ... -- Eric Botcazou
Re: [PATCH, i386]: Implement PR 71246, Missing built-in functions for float128 NaNs
On Fri, Jun 10, 2016 at 11:55 PM, Joseph Myers wrote: > On Fri, 10 Jun 2016, Uros Bizjak wrote: > >> Joseph, does it look OK to you? Richi, I hope I got tree stuff >> implemented correctly. > > It's plausible, but really needs testcases (which could examine the > bit-patterns of __float128 objects initialized using these built-in > functions, to make sure those are as expected). Yes, I plan to add these testcases as a follow-up (including __builtin_infq) for all __float128 targets. Uros.
[regex, libstdc++/71500, patch] Fix icase on bracket expression
Bootstrapped and tested on x86_64-pc-linux-gnu with debug macro. Thanks! -- Regards, Tim Shen commit b8e06b00162b9396e639f517d5cbde0cbd5932fc Author: Tim Shen Date: Sat Jun 11 00:41:09 2016 -0700 2016-06-11 Tim Shen PR libstdc++/71500 * include/bits/regex_compiler.h (_RegexTranslator<>::_M_transform): Take case into consideration when looking at range expression. * include/bits/regex_compiler.tcc (_Compiler<>::_M_insert_bracket_matcher, _Compiler<>::_M_expression_term): Re-define __last_char to buffer the last character, which may be flushed to _M_add_char or be used as the beginning of a range. * testsuite/28_regex/regression.cc: Add new testcase. diff --git a/libstdc++-v3/include/bits/regex_compiler.h b/libstdc++-v3/include/bits/regex_compiler.h index 410d61b..d545c04 100644 --- a/libstdc++-v3/include/bits/regex_compiler.h +++ b/libstdc++-v3/include/bits/regex_compiler.h @@ -235,8 +235,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _StrTransT _M_transform(_CharT __ch) const { - return _M_transform_impl(__ch, typename integral_constant::type()); + return _M_transform_impl( + _M_translate(__ch), + typename integral_constant::type()); } private: diff --git a/libstdc++-v3/include/bits/regex_compiler.tcc b/libstdc++-v3/include/bits/regex_compiler.tcc index ff69e16..3513e50 100644 --- a/libstdc++-v3/include/bits/regex_compiler.tcc +++ b/libstdc++-v3/include/bits/regex_compiler.tcc @@ -428,11 +428,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION if (!(_M_flags & regex_constants::ECMAScript)) if (_M_try_char()) { - __matcher._M_add_char(_M_value[0]); __last_char.first = true; __last_char.second = _M_value[0]; } while (_M_expression_term(__last_char, __matcher)); + if (__last_char.first) + __matcher._M_add_char(__last_char.second); + __matcher._M_ready(); _M_stack.push(_StateSeqT( *_M_nfa, @@ -449,8 +451,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION if (_M_match_token(_ScannerT::_S_token_bracket_end)) return false; + const auto __flush = [&] + { + if (__last_char.first) + { + __matcher._M_add_char(__last_char.second); + __last_char.first = false; + } + }; if (_M_match_token(_ScannerT::_S_token_collsymbol)) { + __flush(); auto __symbol = __matcher._M_add_collate_element(_M_value); if (__symbol.size() == 1) { @@ -459,9 +470,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } } else if (_M_match_token(_ScannerT::_S_token_equiv_class_name)) - __matcher._M_add_equivalence_class(_M_value); + { + __flush(); + __matcher._M_add_equivalence_class(_M_value); + } else if (_M_match_token(_ScannerT::_S_token_char_class_name)) - __matcher._M_add_character_class(_M_value, false); + { + __flush(); + __matcher._M_add_character_class(_M_value, false); + } // POSIX doesn't allow '-' as a start-range char (say [a-z--0]), // except when the '-' is the first or last character in the bracket // expression ([--0]). ECMAScript treats all '-' after a range as a @@ -476,7 +493,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { if (!__last_char.first) { - __matcher._M_add_char(_M_value[0]); + __last_char.first = true; + __last_char.second = _M_value[0]; if (_M_value[0] == '-' && !(_M_flags & regex_constants::ECMAScript)) { @@ -488,8 +506,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION "a dash is not treated literally only when it is at " "beginning or end."); } - __last_char.first = true; - __last_char.second = _M_value[0]; } else { @@ -499,22 +515,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { __matcher._M_make_range(__last_char.second , _M_value[0]); __last_char.first = false; + return true; } - else - { - if (_M_scanner._M_get_token() - != _ScannerT::_S_token_bracket_end) - __throw_regex_error( - regex_constants::error_range, - "Unexpected end of bracket expression."); - __matcher._M_add_char(_M_value[0]); - } - } - else - { - __matcher._M_add_char(_M_value[0]); - __last_char.second = _M_value[0]; + if (_M_scanner._M_get_token() + != _ScannerT::_S_token_bracket_end
[Ada] Fix small oversight for unconstrained array types
This fixes a small oversight introduced in gigi some time ago, whereby the UNCONSTRAINED_ARRAY_TYPE object created for unconstrained array types gets the XUT suffix like the associate RECORD_TYPE. Tested on x86_64-suse-linux, applied on the mainline and 6 branch. 2016-06-11 Pierre-Marie de Rodat * gcc-interface/decl.c (gnat_to_gnu_entity): Do not clobber gnat_entity_name with temporary names for XUP and XUT types. -- Eric BotcazouIndex: gcc-interface/decl.c === --- gcc-interface/decl.c (revision 237323) +++ gcc-interface/decl.c (working copy) @@ -2335,10 +2335,12 @@ gnat_to_gnu_entity (Entity_Id gnat_entit gnat_name = Packed_Array_Impl_Type (gnat_entity); else gnat_name = gnat_entity; - if (gnat_encodings != DWARF_GNAT_ENCODINGS_MINIMAL) - gnu_entity_name = create_concat_name (gnat_name, "XUP"); - create_type_decl (gnu_entity_name, gnu_fat_type, artificial_p, - debug_info_p, gnat_entity); + tree xup_name + = (gnat_encodings == DWARF_GNAT_ENCODINGS_MINIMAL) + ? get_entity_name (gnat_name) + : create_concat_name (gnat_name, "XUP"); + create_type_decl (xup_name, gnu_fat_type, artificial_p, debug_info_p, + gnat_entity); /* Create the type to be designated by thin pointers: a record type for the array and its template. We used to shift the fields to have the @@ -2348,11 +2350,11 @@ gnat_to_gnu_entity (Entity_Id gnat_entit Note that GDB can handle standard DWARF information for them, so we don't have to name them as a GNAT encoding, except if specifically asked to. */ - if (gnat_encodings != DWARF_GNAT_ENCODINGS_MINIMAL) - gnu_entity_name = create_concat_name (gnat_name, "XUT"); - else - gnu_entity_name = get_entity_name (gnat_name); - tem = build_unc_object_type (gnu_template_type, tem, gnu_entity_name, + tree xut_name + = (gnat_encodings == DWARF_GNAT_ENCODINGS_MINIMAL) + ? get_entity_name (gnat_name) + : create_concat_name (gnat_name, "XUT"); + tem = build_unc_object_type (gnu_template_type, tem, xut_name, debug_info_p); SET_TYPE_UNCONSTRAINED_ARRAY (tem, gnu_type);
[Ada] Fix wrong code for case statement on character
This fixes a regression present on the mainline and 6 branch, whereby the compiler generates wrong code for a case statement whose expression is of the standard Character type and which contains a large range of values among other things. Tested on x86_64-suse-linux, applied on the mainline and 6 branch. 2016-06-11 Eric Botcazou * gcc-interface/trans.c (Case_Statement_to_gnu): Deal with characters. 2016-06-11 Eric Botcazou * gnat.dg/case_character.adb: New test. -- Eric BotcazouIndex: gcc-interface/trans.c === --- gcc-interface/trans.c (revision 237323) +++ gcc-interface/trans.c (working copy) @@ -2472,13 +2472,15 @@ Attribute_to_gnu (Node_Id gnat_node, tre static tree Case_Statement_to_gnu (Node_Id gnat_node) { - tree gnu_result, gnu_expr, gnu_label; + tree gnu_result, gnu_expr, gnu_type, gnu_label; Node_Id gnat_when; location_t end_locus; bool may_fallthru = false; gnu_expr = gnat_to_gnu (Expression (gnat_node)); gnu_expr = convert (get_base_type (TREE_TYPE (gnu_expr)), gnu_expr); + gnu_expr = maybe_character_value (gnu_expr); + gnu_type = TREE_TYPE (gnu_expr); /* We build a SWITCH_EXPR that contains the code with interspersed CASE_LABEL_EXPRs for each label. */ @@ -2548,6 +2550,11 @@ Case_Statement_to_gnu (Node_Id gnat_node gcc_assert (!gnu_low || TREE_CODE (gnu_low) == INTEGER_CST); gcc_assert (!gnu_high || TREE_CODE (gnu_high) == INTEGER_CST); + if (gnu_low && TREE_TYPE (gnu_low) != gnu_type) + gnu_low = convert (gnu_type, gnu_low); + if (gnu_high && TREE_TYPE (gnu_high) != gnu_type) + gnu_high = convert (gnu_type, gnu_high); + add_stmt_with_node (build_case_label (gnu_low, gnu_high, label), gnat_choice); choices_added_p = true; @@ -2579,8 +2586,8 @@ Case_Statement_to_gnu (Node_Id gnat_node /* Now emit a definition of the label the cases branch to, if any. */ if (may_fallthru) add_stmt (build1 (LABEL_EXPR, void_type_node, gnu_label)); - gnu_result = build3 (SWITCH_EXPR, TREE_TYPE (gnu_expr), gnu_expr, - end_stmt_group (), NULL_TREE); + gnu_result += build3 (SWITCH_EXPR, gnu_type, gnu_expr, end_stmt_group (), NULL_TREE); return gnu_result; } -- { dg-do run } procedure Case_Character is function Test (C : Character) return Integer is begin case C is when ASCII.HT | ' ' .. Character'Last => return 1; when others => return 0; end case; end; begin if Test ('A') /= 1 then raise Program_Error; end if; end;
Re: [regex, libstdc++/71500, patch] Fix icase on bracket expression
On 11/06/16 00:53 -0700, Tim Shen wrote: diff --git a/libstdc++-v3/include/bits/regex_compiler.h b/libstdc++-v3/include/bits/regex_compiler.h index 410d61b..d545c04 100644 --- a/libstdc++-v3/include/bits/regex_compiler.h +++ b/libstdc++-v3/include/bits/regex_compiler.h @@ -235,8 +235,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _StrTransT _M_transform(_CharT __ch) const { - return _M_transform_impl(__ch, typename integral_constant::type()); + return _M_transform_impl( + _M_translate(__ch), + typename integral_constant::type()); N.B. The "typename" and "::type" are redundant here, because it names the same type as the integral_constant itself, and you could use __bool_constant<__collate> instead: return _M_transform_impl(_M_translate(__ch), __bool_constant<__collate>()); OK for trunk without the redundant typename ...::type, your choice whether to use __bool_constant or not. Will this fix apply cleanly to the branches too?
[Ada] Use new middle-end support for overflow checking
This replaces the manual implementation of overflow checking in gigi by the use of new middle-end support, at least in the generic case of two values not known at compile time. This will generate better code than the current implementation, provided that the back-end implements the associated support. Tested on x86_64-suse-linux, applied on the mainline. 2016-06-11 Eric Botcazou * gcc-interface/trans.c (build_binary_op_trapv): If no operand is a constant, use the generic implementation of the middle-end; otherwise turn the dynamic conditions into static conditions and simplify. -- Eric BotcazouIndex: gcc-interface/trans.c === --- gcc-interface/trans.c (revision 237326) +++ gcc-interface/trans.c (working copy) @@ -8875,19 +8875,16 @@ build_binary_op_trapv (enum tree_code co tree rhs = gnat_protect_expr (right); tree type_max = TYPE_MAX_VALUE (gnu_type); tree type_min = TYPE_MIN_VALUE (gnu_type); - tree zero = build_int_cst (gnu_type, 0); - tree gnu_expr, rhs_lt_zero, tmp1, tmp2; - tree check_pos, check_neg, check; + tree gnu_expr, check; + int sgn; /* Assert that the precision is a power of 2. */ gcc_assert ((precision & (precision - 1)) == 0); - /* Prefer a constant or known-positive rhs to simplify checks. */ - if (!TREE_CONSTANT (rhs) - && commutative_tree_code (code) - && (TREE_CONSTANT (lhs) - || (!tree_expr_nonnegative_p (rhs) - && tree_expr_nonnegative_p (lhs + /* Prefer a constant on the RHS to simplify checks. */ + if (TREE_CODE (rhs) != INTEGER_CST + && TREE_CODE (lhs) == INTEGER_CST + && (code == PLUS_EXPR || code == MULT_EXPR)) { tree tmp = lhs; lhs = rhs; @@ -8898,151 +8895,149 @@ build_binary_op_trapv (enum tree_code co /* If we can fold the expression to a constant, just return it. The caller will deal with overflow, no need to generate a check. */ - if (TREE_CONSTANT (gnu_expr)) + if (TREE_CODE (gnu_expr) == INTEGER_CST) return gnu_expr; - rhs_lt_zero = tree_expr_nonnegative_p (rhs) - ? boolean_false_node - : build_binary_op (LT_EXPR, boolean_type_node, rhs, zero); - - /* ??? Should use more efficient check for operand_equal_p (lhs, rhs, 0) */ - - /* Try a few strategies that may be cheaper than the general - code at the end of the function, if the rhs is not known. - The strategies are: - - Call library function for 64-bit multiplication (complex) - - Widen, if input arguments are sufficiently small - - Determine overflow using wrapped result for addition/subtraction. */ - - if (!TREE_CONSTANT (rhs)) + /* If no operand is a constant, we use the generic implementation. */ + if (TREE_CODE (lhs) != INTEGER_CST && TREE_CODE (rhs) != INTEGER_CST) { - /* Even for add/subtract double size to get another base type. */ - const unsigned int needed_precision = precision * 2; - - if (code == MULT_EXPR && precision == 64) + /* Never inline a 64-bit mult for a 32-bit target, it's way too long. */ + if (code == MULT_EXPR && precision == 64 && BITS_PER_WORD < 64) { - tree int_64 = gnat_type_for_size (64, 0); - + tree int64 = gnat_type_for_size (64, 0); return convert (gnu_type, build_call_n_expr (mulv64_decl, 2, - convert (int_64, lhs), - convert (int_64, rhs))); + convert (int64, lhs), + convert (int64, rhs))); } - if (needed_precision <= BITS_PER_WORD - || (code == MULT_EXPR && needed_precision <= LONG_LONG_TYPE_SIZE)) - { - tree wide_type = gnat_type_for_size (needed_precision, 0); - tree wide_result = build_binary_op (code, wide_type, - convert (wide_type, lhs), - convert (wide_type, rhs)); - - check = build_binary_op - (TRUTH_ORIF_EXPR, boolean_type_node, - build_binary_op (LT_EXPR, boolean_type_node, wide_result, - convert (wide_type, type_min)), - build_binary_op (GT_EXPR, boolean_type_node, wide_result, - convert (wide_type, type_max))); - - return - emit_check (check, gnu_expr, CE_Overflow_Check_Failed, gnat_node); - } + enum internal_fn icode; - if (code == PLUS_EXPR || code == MINUS_EXPR) + switch (code) { - tree unsigned_type = gnat_type_for_size (precision, 1); - tree wrapped_expr - = convert (gnu_type, - build_binary_op (code, unsigned_type, - convert (unsigned_type, lhs), - convert (unsigned_type, rhs))); - - /* Overflow when (rhs < 0) ^ (wrapped_expr < lhs)), for addition - or when (rhs < 0) ^ (wrapped_expr > lhs) for subtraction. */ - check - = build_binary_op (TRUTH_XOR_EXPR, boolean_type_node, rhs_lt_zero, - build_binary_op (code == PLUS_EXPR - ? LT_EXPR : GT_EXPR, - boolean_type_node, - wrapped_expr, lhs)); - - return - emit_check (check, gnu_expr, CE_Overflow_Check_Failed, gnat_node); + case PLUS_E
[PATCH] Fix ICE with invalid use of flags output operand
Hi, this fixes an ICE that happens when an asm statement tries to print the flags output operand. Boot-strapped and reg-tested on x86_64-linux-gnu. OK for trunk? Thanks Bernd.gcc: 2016-06-11 Bernd Edlinger * config/i386/i386.c (print_reg): Emit an error message on attempt to print FLAGS_REG. testsuite: 2016-06-11 Bernd Edlinger * gcc.target/i386/asm-flag-7.c: New test. Index: gcc/config/i386/i386.c === --- gcc/config/i386/i386.c (revision 237323) +++ gcc/config/i386/i386.c (working copy) @@ -17180,10 +17180,15 @@ print_reg (rtx x, int code, FILE *file) gcc_assert (regno != ARG_POINTER_REGNUM && regno != FRAME_POINTER_REGNUM - && regno != FLAGS_REG && regno != FPSR_REG && regno != FPCR_REG); + if (regno == FLAGS_REG) +{ + output_operand_lossage ("invalid use of asm flag output"); + return; +} + duplicated = code == 'd' && TARGET_AVX; switch (msize) Index: gcc/testsuite/gcc.target/i386/asm-flag-7.c === --- gcc/testsuite/gcc.target/i386/asm-flag-7.c (revision 0) +++ gcc/testsuite/gcc.target/i386/asm-flag-7.c (working copy) @@ -0,0 +1,9 @@ +/* Test error conditions of asm flag outputs. */ +/* { dg-do compile } */ +/* { dg-options "" } */ + +void test(void) +{ + char x; + asm("# %0" : "=@ccz"(x)); /* { dg-error "invalid use of asm flag output" } */ +}
[C++ Patch] PR 70202: avoid creating incomplete types (was: "[C++ RFC / Patch] Again about PR 70202")
Hi again, hi Jason, I can't help further investigating this issue and I have a completely different proposal which pursues a different hint of Jason: why the TREE_TYPE of such DECL is error_mark_node in the first place? As we understand now that is just something that start_decl_1 does when it sees an incomplete type. Then why we have that kind of type? The answer is this change, back in 2006 (!), for c++/27952: https://gcc.gnu.org/ml/gcc-patches/2006-10/msg00862.html which changed the loop in xref_basetypes to early return false after error. That implies that the caller skips the body, the type remains incomplete. That has non trivial implications: we avoid ICEs but we also end up with redundant error messages about incomplete types during error recovery. And the inconsistencies shown by the present issue. Thus my idea: let's fix that old bug in a different, lighter way and revert the bulk of the old fix, just disregard and continue on the erroneous duplicate bases in the main xref_basetpes loop. I think at some point Jason hinted at that as a general error recovery strategy: we have been doing exactly that until 2006!! In a sense we are also back to my original idea of not zeroing by hand the type in cp_parser_class_head. Anyway, things appear to work fine: no regressions, more terse diagnostic, see inherit/crash6.C but also the tweaked g++.dg/inherit/virtual1.C (now the diagnostic is *very* similar to that produced by clang, by the way, eh, eh) and the new g++.dg/inherit/union2.C, no redundant error messages on the declaration of u during error recovery. The remaining issue to be discussed is what to do about that old bug, c++/27952, a crash in decay_conversion during error recovery, when vtt is found null. I propose to simply check for it in build_special_member_call, avoid passing an unusable null argument to decay_conversion and return back an error_mark_node. We could also do gcc_assert (seen_error()) before returning error_mark_node? Ah, there is also another detail: around line 12930 of the decl.c diff, I'm also reverting my own 2010 changes to fix c++/30298 (r159637) which really had to do with the 2006 change as well: we can have the gcc_assert back and still handle correctly inherit/crash1 and crash2 which I added back then. This is nice, I think. In conclusion, IMHO this approach is way better than all the other I tried so far, modulo the possible need of additional / better error recovery measures in build_special_member_call, etc. Thanks, Paolo. Index: cp/call.c === --- cp/call.c (revision 237318) +++ cp/call.c (working copy) @@ -8022,6 +8022,8 @@ build_special_member_call (tree instance, tree nam or destructor, then we fetch the VTT directly. Otherwise, we look it up using the VTT we were given. */ vtt = DECL_CHAIN (CLASSTYPE_VTABLES (current_class_type)); + if (!vtt) + return error_mark_node; vtt = decay_conversion (vtt, complain); if (vtt == error_mark_node) return error_mark_node; Index: cp/cp-tree.h === --- cp/cp-tree.h(revision 237318) +++ cp/cp-tree.h(working copy) @@ -5771,7 +5771,7 @@ extern int grok_ctor_properties (const_tree, con extern bool grok_op_properties (tree, bool); extern tree xref_tag (enum tag_types, tree, tag_scope, bool); extern tree xref_tag_from_type (tree, tree, tag_scope); -extern bool xref_basetypes (tree, tree); +extern void xref_basetypes (tree, tree); extern tree start_enum (tree, tree, tree, tree, bool, bool *); extern void finish_enum_value_list (tree); extern void finish_enum(tree); Index: cp/decl.c === --- cp/decl.c (revision 237318) +++ cp/decl.c (working copy) @@ -12871,12 +12871,9 @@ xref_tag_from_type (tree old, tree id, tag_scope s /* Create the binfo hierarchy for REF with (possibly NULL) base list BASE_LIST. For each element on BASE_LIST the TREE_PURPOSE is an access_* node, and the TREE_VALUE is the type of the base-class. - Non-NULL TREE_TYPE indicates virtual inheritance. - - Returns true if the binfo hierarchy was successfully created, - false if an error was detected. */ + Non-NULL TREE_TYPE indicates virtual inheritance. */ -bool +void xref_basetypes (tree ref, tree base_list) { tree *basep; @@ -12889,7 +12886,7 @@ xref_basetypes (tree ref, tree base_list) tree igo_prev; /* Track Inheritance Graph Order. */ if (ref == error_mark_node) -return false; +return; /* The base of a derived class is private by default, all others are public. */ @@ -12933,11
Re: [PATCH gfortran] PR 60751 - Extra comma in WRITE statement not diagnosed
Dear Dominique, That looks OK to me for 5 through 7-branches. Thanks Paul On 10 June 2016 at 19:05, Dominique d'Humières wrote: > The attached patch fixes pr60751 by allowing extra comma in WRITE statement > for std=legacy only, tested on x86_64-apple-darwin15. Is it OK for trunk, and > the gcc-5 and 6 branches? > > TIA > > Dominique > -- The difference between genius and stupidity is; genius has its limits. Albert Einstein
Re: RFC (gimplify, openmp): PATCH to is_gimple_reg to check DECL_HAS_VALUE_EXPR_P
On June 10, 2016 9:48:45 PM GMT+02:00, Jason Merrill wrote: >While working on another issue I noticed that is_gimple_reg was happily > >accepting VAR_DECLs with DECL_VALUE_EXPR even when later gimplification > >would replace them with something that is_gimple_reg doesn't like, >leading to trouble. So I've modified is_gimple_reg to check the >VALUE_EXPR. Can you instead try rejecting them? I've run into similar issues lately with is_gimple_val. Richard. >But this breaks a couple of libgomp testcases, namely >libgomp.c++/member-[45].C. This seems to be a latent bug in >gimplify_omp_for: > >> /* If DECL is not a gimple register, create a temporary >variable to act >> as an iteration counter. This is valid, since DECL cannot >be >> modified in the body of the loop. Similarly for any >iteration vars >> in simd with collapse > 1 where the iterator vars must be >> lastprivate. */ >> if (orig_for_stmt != for_stmt) >> var = decl; >> else if (!is_gimple_reg (decl) >>|| (ort == ORT_SIMD >>&& TREE_VEC_LENGTH (OMP_FOR_INIT (for_stmt)) > 1)) >> { >> struct gimplify_omp_ctx *ctx = gimplify_omp_ctxp; >> /* Make sure omp_add_variable is not called on it >prematurely. >> We call it ourselves a few lines later. */ >> gimplify_omp_ctxp = NULL; >> var = create_tmp_var (TREE_TYPE (decl), get_name (decl)); >> gimplify_omp_ctxp = ctx; >> TREE_OPERAND (t, 0) = var; >> >> gimplify_seq_add_stmt (&for_body, gimple_build_assign >(decl, var)); > >So we update DECL from VAR in the loop body, but we don't update it >after the last iteration, when we do the increment but then don't enter > >the body. As a result, this test fails: > >> #pragma omp parallel for lastprivate (T::t) >> for (T::t = 0; T::t < 32; T::t += 3) >> f[T::t + 2] |= 2; >> if (T::t != 33) >> __builtin_abort (); > >Here T::t ends up with the value 30 because it doesn't get the last >update. > >Thoughts? > >Jason
Re: [regex, libstdc++/71500, patch] Fix icase on bracket expression
On Sat, Jun 11, 2016 at 5:01 AM, Jonathan Wakely wrote: > N.B. The "typename" and "::type" are redundant here, because it names > the same type as the integral_constant itself, and you could > use __bool_constant<__collate> instead: > > return _M_transform_impl(_M_translate(__ch), > __bool_constant<__collate>()); > > OK for trunk without the redundant typename ...::type, your choice > whether to use __bool_constant or not. Thanks! I was looking at std::bool_constant but that's in C++17. __bool_constant is even better. :) > > Will this fix apply cleanly to the branches too? > For gcc6 yes; for gcc5 there needs more work. I guess it's OK for backporting to gcc6? Updated the patch according to the discussion in the libstdc++/71500 bug. -- Regards, Tim Shen commit 46d269bcfeebc497af4b3dc427d857f10f0ab931 Author: Tim Shen Date: Sat Jun 11 00:41:09 2016 -0700 2016-06-11 Tim Shen PR libstdc++/71500 * include/bits/regex_compiler.h (_RegexTranslator<>::_M_transform): Take case into consideration when looking at range expression. * include/bits/regex_compiler.tcc (_Compiler<>::_M_insert_bracket_matcher, _Compiler<>::_M_expression_term): Re-define __last_char to buffer the last character, which may be flushed to _M_add_char or be used as the beginning of a range. * testsuite/28_regex/regression.cc: Add new testcase. diff --git a/libstdc++-v3/include/bits/regex_compiler.h b/libstdc++-v3/include/bits/regex_compiler.h index 410d61b..73c0af4 100644 --- a/libstdc++-v3/include/bits/regex_compiler.h +++ b/libstdc++-v3/include/bits/regex_compiler.h @@ -226,18 +226,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { if (__icase) return _M_traits.translate_nocase(__ch); - else if (__collate) - return _M_traits.translate(__ch); else - return __ch; + return _M_traits.translate(__ch); } _StrTransT _M_transform(_CharT __ch) const - { - return _M_transform_impl(__ch, typename integral_constant::type()); - } + { return _M_transform_impl(__ch, __bool_constant<__collate>()); } private: _StrTransT @@ -247,7 +242,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _StrTransT _M_transform_impl(_CharT __ch, true_type) const { - _StrTransT __str = _StrTransT(1, _M_translate(__ch)); + _StrTransT __str = _StrTransT(1, __ch); return _M_traits.transform(__str.begin(), __str.end()); } @@ -433,6 +428,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION void _M_make_range(_CharT __l, _CharT __r) { + __l = _M_translator._M_translate(__l); + __r = _M_translator._M_translate(__r); if (__l > __r) __throw_regex_error(regex_constants::error_range, "Invalid range in bracket expression."); diff --git a/libstdc++-v3/include/bits/regex_compiler.tcc b/libstdc++-v3/include/bits/regex_compiler.tcc index ff69e16..050435d 100644 --- a/libstdc++-v3/include/bits/regex_compiler.tcc +++ b/libstdc++-v3/include/bits/regex_compiler.tcc @@ -428,11 +428,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION if (!(_M_flags & regex_constants::ECMAScript)) if (_M_try_char()) { - __matcher._M_add_char(_M_value[0]); __last_char.first = true; __last_char.second = _M_value[0]; } while (_M_expression_term(__last_char, __matcher)); + if (__last_char.first) + __matcher._M_add_char(__last_char.second); + __matcher._M_ready(); _M_stack.push(_StateSeqT( *_M_nfa, @@ -449,8 +451,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION if (_M_match_token(_ScannerT::_S_token_bracket_end)) return false; + const auto __flush = [&] + { + if (__last_char.first) + { + __matcher._M_add_char(__last_char.second); + __last_char.first = false; + } + }; if (_M_match_token(_ScannerT::_S_token_collsymbol)) { + __flush(); auto __symbol = __matcher._M_add_collate_element(_M_value); if (__symbol.size() == 1) { @@ -459,9 +470,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } } else if (_M_match_token(_ScannerT::_S_token_equiv_class_name)) - __matcher._M_add_equivalence_class(_M_value); + { + __flush(); + __matcher._M_add_equivalence_class(_M_value); + } else if (_M_match_token(_ScannerT::_S_token_char_class_name)) - __matcher._M_add_character_class(_M_value, false); + { + __flush(); + __matcher._M_add_character_class(_M_value, false); + } // POSIX doesn't allow '-' as a start-range char (say [a-z--0]), // except when the '-' is the first or last character in the bracket // expression ([--0]). ECMAScript treats all '-' after a range as
[Patch, fotran] PR70673 - [5/6/7 Regression] ICE with module containing functions with allocatable character scalars
Dear All, The fix to eliminate this ICE is trivial. Bootstrapped and regtested on FC21/x86_64 - OK for 5 to 7 branches? Cheers Paul 2016-06-11 Paul Thomas PR fortran/70673 * frontend-passes.c (realloc_string_callback): Add a call to gfc_dep_compare_expr. 2016-06-11 Paul Thomas PR fortran/70673 * gfortran.dg/pr70673.f90: New test. Index: gcc/fortran/frontend-passes.c === *** gcc/fortran/frontend-passes.c (revision 237168) --- gcc/fortran/frontend-passes.c (working copy) *** realloc_string_callback (gfc_code **c, i *** 175,180 --- 175,187 if (!gfc_check_dependency (expr1, expr2, true)) return 0; + /* gfc_check_dependency doesn't always pick up identical expressions. + However, eliminating the above sends the compiler into an infinite + loop on valid expressions. Without this check, the gimplifier emits + an ICE for a = a, where a is deferred character length. */ + if (!gfc_dep_compare_expr (expr1, expr2)) + return 0; + current_code = c; inserted_block = NULL; changed_statement = NULL; Index: gcc/testsuite/gfortran.dg/pr70673.f90 === *** gcc/testsuite/gfortran.dg/pr70673.f90 (revision 0) --- gcc/testsuite/gfortran.dg/pr70673.f90 (working copy) *** *** 0 --- 1,25 + ! { dg-do run } + ! + ! Test the fix for PR70673 + ! + ! Contributed by David Kinniburgh + ! + module m + contains + subroutine s(inp) + character(*), intent(in) :: inp + character(:), allocatable :: a + a = a ! This used to ICE. + a = inp + a = a ! This used to ICE too + if ((len (a) .ne. 5) .or. (a .ne. "hello")) call abort + a = a(2:3) ! Make sure that temporary creation is not broken. + if ((len (a) .ne. 2) .or. (a .ne. "el")) call abort + deallocate (a) + a = a ! This would ICE too. + end subroutine s + end module m + + use m + call s("hello") + end
Re: RFC (gimplify, openmp): PATCH to is_gimple_reg to check DECL_HAS_VALUE_EXPR_P
On Sat, Jun 11, 2016 at 08:43:06PM +0200, Richard Biener wrote: > On June 10, 2016 9:48:45 PM GMT+02:00, Jason Merrill wrote: > >While working on another issue I noticed that is_gimple_reg was happily > > > >accepting VAR_DECLs with DECL_VALUE_EXPR even when later gimplification > > > >would replace them with something that is_gimple_reg doesn't like, > >leading to trouble. So I've modified is_gimple_reg to check the > >VALUE_EXPR. > > Can you instead try rejecting them? I've run into similar issues lately with > is_gimple_val. I'm afraid that would break OpenMP badly. During gimplification, outside of OpenMP contexts we always replace decls for their DECL_VALUE_EXPR, but inside of OpenMP contexts we do it only for some decls. In particular, omp_notice_variable returns whether the DECL_VALUE_EXPR should be temporarily ignored (if it returns true) or not. If DECL_VALUE_EXPR is temporarily ignored, it is only for a short time, in particular until the omplower pass, which makes sure that the right thing is done with it and everything is regimplified. Anyway, looking at Jason's patch, I'm really surprised it didn't break far more, it is fine if such an ignored DECL_VALUE_EXPR is considered is_gimple_reg. And I have no idea how else to express this in the IL, the DECL_VALUE_EXPR is often something already the FEs set, and we really want to replace it with the values in most uses, just can't allow it if we want to replace it by something different instead (e.g. privatize in some OpenMP/OpenACC region). Jakub
Re: [PATCH][PR sanitizer/71480] Make ASan align string constants to shadow granularity.
On Fri, 10 Jun 2016, Jakub Jelinek wrote: > On Fri, Jun 10, 2016 at 03:13:32PM +0300, Maxim Ostapenko wrote: > > gcc/ChangeLog: > > > > 2016-06-10 Maxim Ostapenko > > > > PR sanitizer/71480 > > * varasm.c (place_block_symbol): Adjust alignment for asan protected > > STRING_CSTs even if TREE_CONSTANT_POOL_ADDRESS_P. > > > > gcc/testsuite/ChangeLog: > > > > 2016-06-10 Maxim Ostapenko > > > > PR sanitizer/71480 > > * c-c++-common/asan/pr71480.c: New test. > > > > diff --git a/gcc/testsuite/c-c++-common/asan/pr71480.c > > b/gcc/testsuite/c-c++-common/asan/pr71480.c > > new file mode 100644 > > index 000..3cf2c05 > > --- /dev/null > > +++ b/gcc/testsuite/c-c++-common/asan/pr71480.c > > @@ -0,0 +1,41 @@ > > +/* { dg-do run } */ > > + > > +__attribute__ ((noinline, noclone)) int > > +foo (char *c) > > +{ > > I'd add asm volatile ("" : : "r" (c) : "memory"); here I'd say asm("") is sufficient. > > + return 1; > > +} > > + > > +__attribute__ ((noinline, noclone)) void > > +bar (char *c) > > +{ > > and here (so that gcc doesn't try to figure out the functions are const or > pure. This is exactly what the special-case-empty-asm is for, unless I miss something extra-special about this case. Maybe to avoid a warning for the otherwise unused c? > No need for return; in function returning void. > Ok with that change, thanks. > > Jakub >