Re: [Patch, fortran] PR61831 side-effect deallocation of variable components
Le 16/05/2015 18:43, Mikael Morin a écrit :
> Hello,
>
> this is about PR61831 where in code like:
>
> type :: string_t
> character(LEN=1), dimension(:), allocatable :: chars
> end type string_t
> type(string_t) :: prt_in
> (...)
> tmp = new_prt_spec ([prt_in])
>
> the deallocation of the argument's allocatable components after the
> procedure call (to new_prt_spec) has the side effect of freeing prt_in's
> allocatable components, as the array constructor temporary for [prt_in]
> is a shallow copy of prt_in.
>
> This bug is a regression caused by the Dominique's PR41936 memory leak
> fix, itself based on a patch originally from me.
>
> The attached patch is basically a revert of that fix. It avoids the
> problem by not deallocating allocatable components in the problematic
> case, at the price of a (possible) memory leak. A new function is
> introduced telling whether there is aliasing, so that we don't regress
> on PR41936's memory leak when there is no aliasing, and we don't free
> components when there is aliasing.
> The possible remaining memory leak case is the case of a "mixed" array
> constructor with some parts aliasing variables, and some non-aliasing parts.
>
> The patch takes also the opportunity to reassemble the scattered
> procedure argument deallocation code into a single place.
>
> The test needs pr65792's fix (thanks Paul), so for the 4.9 branch I
> propose commenting the parts that depend on PR65792 in the test.
>
> Regression tested on x86_64-linux. OK for 6/5/4.9 ?
Hello,
I would like to come back to the patch:
https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01491.html
PR66082 made me notice one bug in that patch:
for descriptorless arrays, the patch was deallocating only the first
element's allocatable components.
As gfc_conv_array_parameter returns the array data only, the bounds are
lost and it is not possible to loop through all the elements.
With the attached patch, the deallocation code is kept in
gfc_conv_array_parameter where the bounds of descriptorless arrays are
known.
To test this fixes the bug, I have added a count of while (1) loops in
the dump of pr66082's test. I'm open to better ideas to properly test this.
For arrays with descriptors, I have decided to not handle them in
gfc_conv_array_parameter, because in some cases gfc_conv_expr_descriptor
is called directly from gfc_conv_procedure_call, without passing through
gfc_conv_array_parameter.
To be able to select everything but descriptorless arrays for
allocatable component deallocation, the flags are moved around somewhat.
The rest is as in the previous patch.
The test provided is basically unchanged, thus not suitable for the
branches without pr65792.
We can decide what to do with it later (backport pr65792 or disable
parts of the test), I would like to have the fix in mainline first.
It has been fortran-tested on x86_64-linux, OK for trunk?
Mikael
2015-06-20 Mikael Morin
Dominique d'Humieres
PR fortran/61831
* trans-array.c (gfc_conv_array_parameter): Guard allocatable
component deallocation code generation with descriptorless
calling convention flag.
* trans-expr.c (gfc_conv_expr_reference): Remove allocatable
component deallocation code generation from revision 212329.
(expr_may_alias_variables): New function.
(gfc_conv_procedure_call): New boolean elemental_proc to factor
check for procedure elemental-ness. Rename boolean f to nodesc_arg
and declare it in the outer scope. Use expr_may_alias_variables,
elemental_proc and nodesc_arg to decide whether generate allocatable
component deallocation code.
(gfc_trans_subarray_assign): Set deep copy flag.
2015-06-20 Mikael Morin
PR fortran/61831
* gfortran.dg/derived_constructor_components_6.f90: New.
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index fece3ab..bda887c 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -7386,10 +7386,11 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * expr, bool g77,
}
/* Deallocate the allocatable components of structures that are
- not variable. */
- if ((expr->ts.type == BT_DERIVED || expr->ts.type == BT_CLASS)
- && expr->ts.u.derived->attr.alloc_comp
- && expr->expr_type != EXPR_VARIABLE)
+ not variable, for descriptorless arguments.
+ Arguments with a descriptor are handled in gfc_conv_procedure_call. */
+ if (g77 && (expr->ts.type == BT_DERIVED || expr->ts.type == BT_CLASS)
+ && expr->ts.u.derived->attr.alloc_comp
+ && expr->expr_type != EXPR_VARIABLE)
{
tmp = build_fold_indirect_ref_loc (input_location, se->expr);
tmp = gfc_deallocate_alloc_comp (expr->ts.u.derived, tmp, expr->rank);
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 5d6555b..583e692 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/tr
Re: [PATCH, rs6000] Fix PR65914
On Sat, 2015-06-20 at 13:37 -0700, Mike Stump wrote: > On Jun 19, 2015, at 5:36 PM, David Edelsohn wrote: > > Maybe you should ask Richi or Jakub about the testcase because you are > > placing it in a non-target-specific location. It should succeed on > > all targets, but it may expose latent bugs on other targets. > > A latent bug is one that is broken, but appears to work, and then a change in > the compiler is made to expose the bug that was always there but didn’t show > a failure. It only applies to the compiler proper. > > In this case, since the change is to add a test case, the test case can only > show bugs that aren’t latent (or failures in the test case). Yes, I felt that in this case we seem to have C++14 adding some new code paths that could well cause surprises on other targets besides Power. It seems like the sort of thing that language-specific torture tests are for, but please let me know if that's wrong and I'll move it to a target directory. Thanks, Bill
*Ping* patch, fortran] Warn about constant integer divisions
*ping* https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00966.html > Hello world, > > the attached patch emits a warning for constant integer division. > While correct according to the standard, I cannot really think > of a legitimate reason why people would want to write 3/5 where > they could have written 0 , so my preference would be to put > this under -Wconversion (like in the attached patch). > > However, I am open to discussion on that. It is easy enough to > change. > > Regression-tested. Opinions? Comments? Would somebody rather > have -Wconversion-extra? OK for trunk?
New Swedish PO file for 'gcc' (version 5.1.0)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the Swedish team of translators. The file is available at: http://translationproject.org/latest/gcc/sv.po (This file, 'gcc-5.1.0.sv.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: http://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: http://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
Re: [C++/58583] ICE instantiating NSDMIs
On 06/20/15 02:09, Andreas Schwab wrote: This also fails on powerpc. what is the build compiler? James provided a clue that 4.9.2 is ok but 4.8.1 is bad. nathan
[PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
Attached is a patch to reject C and C++ constructs that result
in obtaining a pointer (or a reference in C++) to a builtin
function. These constructs are currently silently accepted by
GCC and, in most cases(*), result in a linker error. The patch
brings GCC on par with Clang by rejecting all such constructs.
Bootstrapped and tested on x86_64-unknown-linux-gnu.
Okay to commit to trunk?
Martin
[*] Besides rejecting constructs that lead to linker errors
the patch leads to rejecting also some benign constructs (that
are also rejected by Clang). For example, the program below
currently compiles and links successfully with GCC 5.1 is
rejected with the patch applied. That's intended (but I'm
open to arguments against it).
$ cat /build/tmp/u.cpp && /build/gcc-66516/gcc/xg++
-B/build/gcc-66516/gcc -Wall -c -std=c++11 /build/tmp/u.cpp
static constexpr int (&r)(int) = __builtin_ffs;
int main () { return r (0); }
/build/tmp/u.cpp:1:34: error: invalid initialization of a reference with
a builtin function
static constexpr int (&r)(int) = __builtin_ffs;
^
2015-06-21 Martin Sebor
PR c/66516
* c/c-typeck.c (default_function_array_conversion): Reject
converting a builtin function to a pointer.
(parser_build_unary_op): Reject taking the address of a builtin
function.
* cp/call.c (convert_like_real): Reject converting a builtin function
to a pointer.
(initialize_reference): Reject initializing a reference with a builtin
function.
* cp/typeck.c (cp_build_addr_expr_strict): Reject taking the address
of a builtin function.
(build_reinterpret_cast_1): Reject casting a builtin function to
a pointer.
(convert_for_initialization): Reject initializing a pointer with
the a builtin function.
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 636e0bb..637a292 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -58,6 +58,8 @@ along with GCC; see the file COPYING3. If not see
#include "cilk.h"
#include "gomp-constants.h"
+#include
+
/* Possible cases of implicit bad conversions. Used to select
diagnostic messages in convert_for_assignment. */
enum impl_conv {
@@ -1940,6 +1942,12 @@ default_function_array_conversion (location_t loc, struct c_expr exp)
}
break;
case FUNCTION_TYPE:
+ if (DECL_IS_BUILTIN (exp.value))
+ {
+ error_at (loc, "converting builtin function to a pointer");
+ exp.value = error_mark_node;
+ }
+ else
exp.value = function_to_pointer_conversion (loc, exp.value);
break;
default:
@@ -3384,7 +3392,14 @@ parser_build_unary_op (location_t loc, enum tree_code code, struct c_expr arg)
result.original_code = code;
result.original_type = NULL;
- if (TREE_OVERFLOW_P (result.value) && !TREE_OVERFLOW_P (arg.value))
+ if (code == ADDR_EXPR
+ && TREE_CODE (TREE_TYPE (arg.value)) == FUNCTION_TYPE
+ && DECL_IS_BUILTIN (arg.value))
+{
+ error_at (loc, "taking address of a builtin function");
+ result.value = error_mark_node;
+}
+ else if (TREE_OVERFLOW_P (result.value) && !TREE_OVERFLOW_P (arg.value))
overflow_warning (loc, result.value);
return result;
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 2d90ed9..df4cc77 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6570,6 +6570,13 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
expr = build_target_expr_with_type (expr, type, complain);
}
+ if (TREE_CODE (TREE_TYPE (expr)) == FUNCTION_TYPE
+ && DECL_P (expr) && DECL_IS_BUILTIN (expr))
+ {
+ error_at (input_location, "taking address of a builtin function");
+ return error_mark_node;
+ }
+
/* Take the address of the thing to which we will bind the
reference. */
expr = cp_build_addr_expr (expr, complain);
@@ -9768,8 +9775,18 @@ initialize_reference (tree type, tree expr,
}
if (conv->kind == ck_ref_bind)
+{
+ if (TREE_CODE (TREE_TYPE (expr)) == FUNCTION_TYPE
+ && DECL_P (expr) && DECL_IS_BUILTIN (expr))
+ {
+ error_at (input_location, "invalid initialization of a reference "
+ "with a builtin function");
+ return error_mark_node;
+ }
+
/* Perform the conversion. */
expr = convert_like (conv, expr, complain);
+}
else if (conv->kind == ck_ambig)
/* We gave an error in build_user_type_conversion_1. */
expr = error_mark_node;
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 5b09b73..fbea052 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -5621,6 +5621,13 @@ cp_build_addr_expr (tree arg, tsubst_flags_t complain)
static tree
cp_build_addr_expr_strict (tree arg, tsubst_flags_t complain)
{
+ if (TREE_CODE (TREE_TYPE (arg)) == FUNCTION_TYPE
+ && DECL_P (arg) && DECL_IS_BUILTIN (arg))
+{
+ error_at (input_location, "taking address of a builtin function");
+ return error_mark_node;
+}
+
return cp_build_addr_expr_1 (arg, 1, complain);
}
@@ -6860,6 +6867,14 @@ build_reinterpret_cast_1 (tree type, tree expr,
Re: [PATCH] Combine related fail of gcc.target/powerpc/ti_math1.c
On Fri, May 22, 2015 at 04:04:19PM +0930, Alan Modra wrote: https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02055.html > * rtlanal.c (commutative_operand_precedence): Correct comments. > * simplify-rtx.c (simplify_plus_minus_op_data_cmp): Delete forward > declaration. Return an int. Distinguish REG,REG return from > others. > (struct simplify_plus_minus_op_data): Make local to function. > (simplify_plus_minus): Rename canonicalized to not_canonical. > Don't set not_canonical if merely sorting registers. Avoid > packing ops if nothing changes. White space fixes. Ping? -- Alan Modra Australia Development Lab, IBM
Re: [C++/58583] ICE instantiating NSDMIs
On 06/21/15 19:00, Nathan Sidwell wrote: On 06/20/15 02:09, Andreas Schwab wrote: This also fails on powerpc. what is the build compiler? James provided a clue that 4.9.2 is ok but 4.8.1 is bad. FWIW, I can't reproduce the failure with using a stock 4.8.2 build compiler for an x86_64-linux target. nathan
Re: [PATCH, rs6000] Fix PR65914
On Fri, 2015-06-19 at 20:36 -0400, David Edelsohn wrote: > On Fri, Jun 19, 2015 at 6:35 PM, Bill Schmidt > wrote: > > Hi, > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65914 demonstrates that we > > fail to match vector predicates with pseudos that are in the virtual > > stack register range. The reduced test case provided with the bug > > report, when compiled for the C++14 standard, demonstrates that we need > > to be able to do this. This patch loosens the restriction for the > > vector predicates so that all pseudos are accepted. Thanks to Uli > > Weigand for his insights on this bug. > > > > As a side benefit, applying this patch fixes a number of libgomp tests > > that have been failing recently. > > > > Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no > > regressions. Is this ok for trunk? After it burns in, I would propose > > to backport it to GCC 5.1 and GCC 4.9 (when the tree is open). > > > > Thanks, > > Bill > > > > > > [gcc] > > > > 2015-05-19 Bill Schmidt > > > > * config/rs6000/predicates.md (altivec_register_operand): Permit > > virtual stack registers. > > (vsx_register_operand): Likewise. > > (vfloat_operand): Likewise. > > (vint_operand): Likewise. > > (vlogical_operand): Likewise. > > Okay. > > > [gcc/testsuite] > > > > 2015-05-19 Bill Schmidt > > > > * g++.dg/torture/pr65419.C: New. > > Maybe you should ask Richi or Jakub about the testcase because you are > placing it in a non-target-specific location. It should succeed on > all targets, but it may expose latent bugs on other targets. OK, thanks. Richi, Jakub, is this ok as a general C++ torture test? Thanks, Bill > > Thanks, David >
Re: [PATCH] Refactoring: use std::swap instead of manual swaps (part 2)
On 16.06.2015 9:09, Uros Bizjak wrote: > According to [1], patches that convert to std::swap are considered as > obvious and don't need approval. If there is a non-trivial part, > please split it to a separate patch for an eventual discussion. > > [1] https://gcc.gnu.org/ml/gcc-patches/2015-04/msg01620.html > > Uros. > OK. I think that the whole patch is rather obvious, thus committing. It applied smoothly, so I hope that the previous test result is still valid (x86_64-linux bootstrap/regtest + build for full target list). And just in case, I also tested it on arm-eabi simulator today. The applied patch is identical to the one I have posted in my original message, so I'm not repeating it here. -- Regards, Mikhail Maltsev
Re: [PATCH, rs6000] Fix PR65914
On Sun, Jun 21, 2015 at 08:47:10PM -0500, Bill Schmidt wrote: > > > * g++.dg/torture/pr65419.C: New. > > > > Maybe you should ask Richi or Jakub about the testcase because you are > > placing it in a non-target-specific location. It should succeed on > > all targets, but it may expose latent bugs on other targets. > > OK, thanks. Richi, Jakub, is this ok as a general C++ torture test? Yes, of course, there is nothing rs6000 specific on the testcase. And, if it exposes latent bugs, at least we'll know it, that is what we have tests for, don't we? Jakub
Re: [PATCH] Fix PR ipa/65908.
Hi,
this patch extends earlier Martin's patch. I completely removed
parse_tree_args because it assumed that every function has only one set
of parm types and one return value type. We really want to compare two
sets of parameter types (DECL_ARGUMENTS and TYPE_PARM_TYPES) as they do not
need to 100% match and similarly for the return value.
I also added check that skips unnecesary matching (in addition to the
signature) for parameters that are not used.
The patch bootstrapped/regtested x86_64-linux, and I checked it behaves
sane with Firefox. I will commit it to mainline and will follow with branch
in couple days.
Honza
PR ipa/65908
* ipa-icf.c (sem_item::target_supports_symbol_aliases): Remove
construction of arg_types.
(sem_function::sem_function): Likewise.
(sem_function::~sem_function): Remove destruction of arg_types.
(sem_function::compatible_parm_types_p): New function.
(sem_function::equals_wpa): Reorg matching of return values
and parameter types.
(sem_function::equals_private): Reorg mathcing of argument types.
(sem_function::parse_tree_args): Remove.
* ipa-icf.h (init_wpa): Do not call it.
(parse_tree_args): Remove.
(compatible_parm_types_p): Declare.
(result_type): Remove.
(arg_types): Remove.
* testsuite/g++.dg/ipa/pr65908.C: New testcase.
Index: ipa-icf.h
===
--- ipa-icf.h (revision 224713)
+++ ipa-icf.h (working copy)
@@ -292,7 +292,6 @@ public:
inline virtual void init_wpa (void)
{
-parse_tree_args ();
}
virtual void init (void);
@@ -310,9 +309,6 @@ public:
dump_function_to_file (decl, file, TDF_DETAILS);
}
- /* Parses function arguments and result type. */
- void parse_tree_args (void);
-
/* Returns cgraph_node. */
inline cgraph_node *get_node (void)
{
@@ -329,15 +325,13 @@ public:
semantic function item. */
static sem_function *parse (cgraph_node *node, bitmap_obstack *stack);
+ /* Perform additional checks needed to match types of used function
+ paramters. */
+ bool compatible_parm_types_p (tree, tree);
+
/* Exception handling region tree. */
eh_region region_tree;
- /* Result type tree node. */
- tree result_type;
-
- /* Array of argument tree types. */
- vec arg_types;
-
/* Number of function arguments. */
unsigned int arg_count;
Index: testsuite/g++.dg/ipa/pr65908.C
===
--- testsuite/g++.dg/ipa/pr65908.C (revision 0)
+++ testsuite/g++.dg/ipa/pr65908.C (revision 0)
@@ -0,0 +1,27 @@
+// PR ipa/65908
+// { dg-do compile }
+// { dg-options "-O2" }
+// { dg-additional-options "-fPIC" { target fpic } }
+
+class A
+{
+ A (A &);
+};
+class B
+{
+ const A &m_fn1 () const;
+};
+class C
+{
+ A m_fn2 () const;
+};
+A
+C::m_fn2 () const
+{
+ throw 0;
+}
+const A &
+B::m_fn1 () const
+{
+ throw 0;
+}
Index: ipa-icf.c
===
--- ipa-icf.c (revision 224713)
+++ ipa-icf.c (working copy)
@@ -259,7 +259,6 @@ sem_item::target_supports_symbol_aliases
sem_function::sem_function (bitmap_obstack *stack): sem_item (FUNC, stack),
m_checker (NULL), m_compared_func (NULL)
{
- arg_types.create (0);
bb_sizes.create (0);
bb_sorted.create (0);
}
@@ -271,7 +270,6 @@ sem_function::sem_function (cgraph_node
sem_item (FUNC, node, hash, stack),
m_checker (NULL), m_compared_func (NULL)
{
- arg_types.create (0);
bb_sizes.create (0);
bb_sorted.create (0);
}
@@ -281,7 +279,6 @@ sem_function::~sem_function ()
for (unsigned i = 0; i < bb_sorted.length (); i++)
delete (bb_sorted[i]);
- arg_types.release ();
bb_sizes.release ();
bb_sorted.release ();
}
@@ -581,6 +578,30 @@ sem_function::param_used_p (unsigned int
return ipa_is_param_used (IPA_NODE_REF (get_node ()), i);
}
+/* Perform additional check needed to match types function parameters that are
+ used. Unlike for normal decls it matters if type is TYPE_RESTRICT and we
+ make an assumption that REFERENCE_TYPE parameters are always non-NULL. */
+
+bool
+sem_function::compatible_parm_types_p (tree parm1, tree parm2)
+{
+ /* Be sure that parameters are TBAA compatible. */
+ if (!func_checker::compatible_types_p (parm1, parm2))
+return return_false_with_msg ("parameter type is not compatible");
+
+ if (POINTER_TYPE_P (parm1)
+ && (TYPE_RESTRICT (parm1) != TYPE_RESTRICT (parm2)))
+return return_false_with_msg ("argument restrict flag mismatch");
+
+ /* nonnull_arg_p implies non-zero range to REFERENCE types. */
+ if (POINTER_TYPE_P (parm1)
+ && TREE_CODE (parm1) != TREE_CODE (parm2)
+ && opt_for_fn (decl, flag_delete_null_pointer_checks))
+return return_false_with_msg ("pointer wrt reference mismatch");
+
+ return true;
+}
+
/* Fast equality function based on kn
