[ping*2] Fix ICE in pp_cxx_unqualified_id (PR c++/88348)
On Fri, Dec 14, 2018 at 10:59 AM Zhouyi Zhou wrote: > > Hello, > > Ping for https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00151.html > > By the way, are there any GCC open project for volunteer beginners to > > do in spare time. > > Thanks in advance for your feedback, > > Zhouyi Hello, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88348 still exists in GCC I fetched from repository today. . PING https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00151.html Thanks in advance for your feedback. Zhouyi
Re: [PATCH] avoid assuming arrays have nonzero size (PR 88956)
On 1/29/19 11:23 AM, Jakub Jelinek wrote:
--- gcc/gimple-fold.c (revision 268086)
+++ gcc/gimple-fold.c (working copy)
@@ -6715,12 +6715,14 @@ fold_array_ctor_reference (tree type, tree ctor,
elt_size = wi::to_offset (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (ctor;
/* When TYPE is non-null, verify that it specifies a constant-sized
- accessed not larger than size of array element. */
- if (type
- && (!TYPE_SIZE_UNIT (type)
- || TREE_CODE (TYPE_SIZE_UNIT (type)) != INTEGER_CST
- || elt_size < wi::to_offset (TYPE_SIZE_UNIT (type))
- || elt_size == 0))
+ accessed not larger than size of array element. Avoid using zero
+ ELT_SIZE, the result of an empty initializer for a zero-length
+ array. */
The comment is misleading, there are many reasons why you could have
zero elt_size, C struct S {}, or int x[10][0], etc. Just don't change
anything in the comment at all, or say
Punt if element size is zero to avoid division by zero.
or something similar.
Thanks. The goal of the comment is to give an example of the unusual
case when the size of an array element may be zero, not suggest there
is just one such case, or enumerate all all of them. I've adjusted
the text and added tests for the empty zero struct.
+ if (elt_size == 0
+ || (type
+ && (!TYPE_SIZE_UNIT (type)
+ || TREE_CODE (TYPE_SIZE_UNIT (type)) != INTEGER_CST
+ || elt_size < wi::to_offset (TYPE_SIZE_UNIT (type)
return NULL_TREE;
Otherwise LGTM.
If you could fix also the two comments a few lines above this:
/* Static constructors for variably sized objects makes no sense. */
to
/* Static constructors for variably sized objects make no sense. */
it would be appreciated.
Sure, I don't mind doing that.
Committed in r268378.
Martin
[C++ PATCH] PR c++/86943 - wrong code converting lambda to function pointer.
In this PR, instantiating the static thunk returned from the generic lambda
conversion function template was using normal overload resolution, which
meant calling an extra constructor when forwarding its argument. Fixed by
special-casing thunk calls significantly more.
Tested x86_64-pc-linux-gnu, applying to trunk.
* lambda.c (maybe_add_lambda_conv_op): Use a template-id in the
call. Only forward parms for decltype.
* pt.c (tsubst_copy_and_build) [CALL_EXPR]: Handle CALL_FROM_THUNK_P
specially.
* typeck.c (check_return_expr): Don't mess with a thunk call.
---
gcc/cp/lambda.c | 29 ++
gcc/cp/pt.c | 58 +++
gcc/cp/typeck.c | 5 ++
.../g++.dg/cpp0x/lambda/lambda-conv13.C | 33 +++
gcc/cp/ChangeLog | 9 +++
5 files changed, 111 insertions(+), 23 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-conv13.C
diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c
index 6e6db1fd72e..bc64a4173f9 100644
--- a/gcc/cp/lambda.c
+++ b/gcc/cp/lambda.c
@@ -1095,8 +1095,10 @@ maybe_add_lambda_conv_op (tree type)
implementation of the conversion operator. */
tree instance = cp_build_fold_indirect_ref (thisarg);
- tree objfn = build_min (COMPONENT_REF, NULL_TREE,
- instance, DECL_NAME (callop), NULL_TREE);
+ tree objfn = lookup_template_function (DECL_NAME (callop),
+DECL_TI_ARGS (callop));
+ objfn = build_min (COMPONENT_REF, NULL_TREE,
+instance, objfn, NULL_TREE);
int nargs = list_length (DECL_ARGUMENTS (callop)) - 1;
call = prepare_op_call (objfn, nargs);
@@ -1137,18 +1139,21 @@ maybe_add_lambda_conv_op (tree type)
if (generic_lambda_p)
{
- /* Avoid capturing variables in this context. */
- ++cp_unevaluated_operand;
- tree a = forward_parm (tgt);
- --cp_unevaluated_operand;
-
+ tree a = tgt;
+ if (DECL_PACK_P (tgt))
+ {
+ a = make_pack_expansion (a);
+ PACK_EXPANSION_LOCAL_P (a) = true;
+ }
CALL_EXPR_ARG (call, ix) = a;
- if (decltype_call)
- CALL_EXPR_ARG (decltype_call, ix) = unshare_expr (a);
- if (PACK_EXPANSION_P (a))
- /* Set this after unsharing so it's not in decltype_call. */
- PACK_EXPANSION_LOCAL_P (a) = true;
+ if (decltype_call)
+ {
+ /* Avoid capturing variables in this context. */
+ ++cp_unevaluated_operand;
+ CALL_EXPR_ARG (decltype_call, ix) = forward_parm (tgt);
+ --cp_unevaluated_operand;
+ }
++ix;
}
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index f8b3054533e..cb06a570d48 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -18680,6 +18680,52 @@ tsubst_copy_and_build (tree t,
}
}
+ /* Stripped-down processing for a call in a thunk. Specifically, in
+ the thunk template for a generic lambda. */
+ if (CALL_FROM_THUNK_P (t))
+ {
+ tree thisarg = NULL_TREE;
+ if (TREE_CODE (function) == COMPONENT_REF)
+ {
+ thisarg = TREE_OPERAND (function, 0);
+ if (TREE_CODE (thisarg) == INDIRECT_REF)
+ thisarg = TREE_OPERAND (thisarg, 0);
+ function = TREE_OPERAND (function, 1);
+ if (TREE_CODE (function) == BASELINK)
+ function = BASELINK_FUNCTIONS (function);
+ }
+ /* We aren't going to do normal overload resolution, so force the
+ template-id to resolve. */
+ function = resolve_nondeduced_context (function, complain);
+ for (unsigned i = 0; i < nargs; ++i)
+ {
+ /* In a thunk, pass through args directly, without any
+ conversions. */
+ tree arg = (*call_args)[i];
+ while (TREE_CODE (arg) != PARM_DECL)
+ arg = TREE_OPERAND (arg, 0);
+ (*call_args)[i] = arg;
+ }
+ if (thisarg)
+ {
+ /* Shift the other args over to make room. */
+ vec_safe_push (call_args, (*call_args)[nargs-1]);
+ for (int i = nargs-1; i > 0; --i)
+ (*call_args)[i] = (*call_args)[i-1];
+ (*call_args)[0] = thisarg;
+ }
+ ret = build_call_a (function, call_args->length (),
+ call_args->address ());
+ /* The thunk location is not interesting. */
+ SET_EXPR_LOCATION (ret, UNKNOWN_LOCATION);
+ CALL_FROM_THUNK_P (ret) = true;
+ if (CLASS_TYPE_P (TREE_TYPE (ret)))
+
Re: C++ PATCH for c++/89083, c++/80864 - ICE with list initialization in template
On Tue, Jan 29, 2019 at 6:53 PM Marek Polacek wrote:
>
> My recent patch for 88815 and 78244 caused 89083, a P1 9 regression, which
> happens to be the same problem as 80864 and its many dupes, something I'd
> been meaning to fix for a long time.
>
> Basically, the problem is repeated reshaping of a constructor, once when
> parsing, and then again when substituting. With the recent fix, we call
> reshape_init + digest_init in finish_compound_literal even in a template
> if the expression is not instantiation-dependent, and then again when
> tsubst_*.
>
> For instance, in initlist107.C, when parsing a functional cast, we call
> finish_compound_literal which calls reshape_init, which turns
>
> { NON_LVALUE_EXPR<1>, NON_LVALUE_EXPR<2> }
>
> into
>
> { { NON_LVALUE_EXPR<1>, NON_LVALUE_EXPR<2> } }
>
> and then digest_init turns that into
>
> { .x = { 1, 2 } }
>
> which is a compound literal (TREE_HAS_CONSTRUCTOR set), but the subexpression
> "{ 1, 2 }" isn't. "{ 1, 2 }" will now have the type int[3], so it's not
> BRACE_ENCLOSED_INITIALIZER_P.
>
> And then tsubst_* processes "{ .x = { 1, 2 } }". The case CONSTRUCTOR
> in tsubst_copy_and_build will call finish_compound_literal on a copy of
> "{ 1, 2 }" wrapped in a new { }, because the whole expr has
> TREE_HAS_CONSTRUCTOR.
> That crashes in reshape_init_r in the
> 6155 if (TREE_CODE (stripped_init) == CONSTRUCTOR)
> block; we have a constructor, it's not COMPOUND_LITERAL_P, and because
> digest_init had given it the type int[3], we hit
> 6172 gcc_assert (BRACE_ENCLOSED_INITIALIZER_P (stripped_init));
>
> As expand_default_init explains in a comment, a CONSTRUCTOR of the target's
> type
> is a previously digested initializer, so we should probably do a similar trick
> here. This fixes all the variants of the problem I've come up with.
>
> 80864 is a similar case, we reshape when parsing and then second time in
> fold_non_dependent_expr called from store_init_value, because of the
> 'constexpr'.
>
> Also update a stale comment.
>
> Bootstrapped/regtest running on x86_64-linux, ok for trunk and 8 after a
> while?
>
> 2019-01-29 Marek Polacek
>
> PR c++/89083, c++/80864 - ICE with list initialization in template.
> * decl.c (reshape_init_r): Don't reshape a digested initializer.
>
> * g++.dg/cpp0x/initlist107.C: New test.
> * g++.dg/cpp0x/initlist108.C: New test.
> * g++.dg/cpp0x/initlist109.C: New test.
>
> diff --git gcc/cp/decl.c gcc/cp/decl.c
> index 79eeac177b6..da08ecc21aa 100644
> --- gcc/cp/decl.c
> +++ gcc/cp/decl.c
> @@ -6161,11 +6161,17 @@ reshape_init_r (tree type, reshape_iter *d, bool
> first_initializer_p,
> ;
> else if (COMPOUND_LITERAL_P (stripped_init))
> /* For a nested compound literal, there is no need to reshape since
> -brace elision is not allowed. Even if we decided to allow it,
> -we should add a call to reshape_init in finish_compound_literal,
> -before calling digest_init, so changing this code would still
> -not be necessary. */
> +we called reshape_init in finish_compound_literal, before calling
> +digest_init. */
> gcc_assert (!BRACE_ENCLOSED_INITIALIZER_P (stripped_init));
> + /* Similarly, a CONSTRUCTOR of the target's type is a previously
> +digested initializer. */
> + else if (same_type_ignoring_top_level_qualifiers_p (type,
> + TREE_TYPE
> (init)))
Hmm, aren't both of these tests true for a dependent compound literal,
which won't have been reshaped already?
Jason
Re: [C++ PATCH] [PR87770] test partial specializations for type dependence
On Mon, Jan 28, 2019 at 11:37 PM Alexandre Oliva wrote: > On Jan 27, 2019, Jason Merrill wrote: > > >> + ??? How do we > >> + tell apart a partial from a full explicit specialization in a > >> + non-template context? */ > > > We don't need to tell them apart here, the caller checks if there are > > any dependent template arguments. > > The single caller does, indeed, but the function does not make that a > requirement, so others might call it and fail to check it. Should that > test be moved here too? Hmm, I wouldn't expect that from a function named "instantiates_primary_template_p". Perhaps another function that calls instantiates_primary_template_p and then checks for dependent innermost template args? I'm not sure what to call that function, perhaps decl_has_dependent_primary_targs? > Anyhow, the question was really about the fact that the non-template > context has no template argument depth for us to compare with. When I > wrote that comment, I was returning true for !ctinfo, unconditionally, > reasoning that if NODE's context is not a template, then NODE must > specialize some a primary template. Right. > But if we want partial but not full > explicit specializations, then just having a deeper (or nonzero) > template argument depth is not enough, is it? See above. Though we're not necessarily dealing with explicit specializations at all here, what we have is a decl produced from a template and some args, which might or might not be specialized. > >> + tree ctxt; > >> + if (!DECL_P (node)) > >> +ctxt = TYPE_CONTEXT (node); > >> + else > >> +ctxt = DECL_CONTEXT (node); > > > We know tmpl is a decl, so we can unconditionally take its DECL_CONTEXT. > > Does it hurt too much to keep it more general, so that it can deal with > template class types too? A class template also comes from a TEMPLATE_DECL; either way, tmpl is a TEMPLATE_DECL. Note that I'm talking about the "tmpl" variable, not "node". Jason
Re: C++ PATCH to fix r268343 fallout (PR testsuite/89110)
On Tue, Jan 29, 2019 at 5:54 PM Marek Polacek wrote: > > As a consequence of r268343, the following tests need to be updated to > expect the error in all modes. > > Sorry for the breakage. > > Tested on x86_64-linux, ok for trunk? OK, thanks.
Re: [backtrace] Avoid segfault
On Tue, Jan 29, 2019 at 3:17 PM Segher Boessenkool wrote: > > On Sun, Jan 27, 2019 at 01:53:18PM -0800, Ian Lance Taylor wrote: > > You need to use a temporary file, such as $@.tmp, for the final sed > > command, followed by a mv to $@. Otherwise a failure in the sed will > > leave what appears to be an up to date file. > > Or you just set .DELETE_ON_ERROR, we require GNU make after all so might > as well use it! It's useful, but it doesn't help if the file is partially written on disk and then the computer crashes. It also doesn't help if the program is not careful to check for errors on writing to standard output--many programs aren't--and the disk fills up after writing out part of the file. Yes, both cases have happened to me when I was a release engineer aeons ago, so I try to be careful. Ian
[PATCH] print correct array sizes in errors (PR 87996)
PR c++/87996 is a P2 regression reported in December about C++ error
messages for declarations of arrays larger than PTRDIFF_MAX saying
the sizes of the arrays are negative. GCC 7 says they're negative.
The attached patch corrects that by issuing messages that, besides
reflecting the correct sign, also mention the invalid size of
the array when it's available, similarly to what Clang does, and
the maximum object size GCC allows using a format already in use
in other GCC messages. This is helpful when the array bounds
involve non-trivial expressions (such as template arguments).
Tested on x86_64-linux.
Martin
PR c++/87996 - [8/9 Regression] size of array is negative error when SIZE_MAX/2 < sizeof(array) <= SIZE_MAX
gcc/ChangeLog:
PR c++/87996
* builtins.c (max_object_size): Move from here...
* builtins.h (max_object_size): ...and here...
* tree.c (max_object_size): ...to here...
* tree.h (max_object_size): ...and here.
gcc/c-family/ChangeLog:
PR c++/87996
* c-common.c (invalid_array_size_error): New function.
(valid_array_size_p): Call it. Handle size as well as type.
* c-common.h (valid_constant_size_p): New function.
(enum cst_size_error): New type.
gcc/cp/ChangeLog:
PR c++/87996
* decl.c (compute_array_index_type_loc): Preserve signed sizes
for diagnostics. Call valid_array_size_p instead of error.
* init.c (build_new_1): Compute size for diagnostic. Call
invalid_array_size_error
(build_new): Call valid_array_size_p instead of error.
gcc/testsuite/ChangeLog:
PR c++/87996
* c-c++-common/array-5.c: New test.
* c-c++-common/pr68107.c: Adjust text of diagnostics.
* g++.dg/init/new38.C: Same.
* g++.dg/init/new43.C: Same.
* g++.dg/init/new44.C: Same.
* g++.dg/init/new46.C: Same.
* g++.dg/other/large-size-array.C: Same.
* g++.dg/other/new-size-type.C: Same.
* g++.dg/template/array30.C: Same.
* g++.dg/template/array32.C: New test.
* g++.dg/template/dependent-name3.C: Adjust.
* gcc.dg/large-size-array-3.c: Same.
* gcc.dg/large-size-array-5.c: Same.
* gcc.dg/large-size-array.c: Same.
* g++.old-deja/g++.brendan/array1.C: Same.
* g++.old-deja/g++.mike/p6149.C: Same.
Index: gcc/builtins.c
===
--- gcc/builtins.c (revision 268372)
+++ gcc/builtins.c (working copy)
@@ -11210,12 +11210,3 @@ target_char_cst_p (tree t, char *p)
*p = (char)tree_to_uhwi (t);
return true;
}
-
-/* Return the maximum object size. */
-
-tree
-max_object_size (void)
-{
- /* To do: Make this a configurable parameter. */
- return TYPE_MAX_VALUE (ptrdiff_type_node);
-}
Index: gcc/builtins.h
===
--- gcc/builtins.h (revision 268372)
+++ gcc/builtins.h (working copy)
@@ -150,6 +150,5 @@ extern internal_fn replacement_internal_fn (gcall
extern void warn_string_no_nul (location_t, const char *, tree, tree);
extern tree unterminated_array (tree, tree * = NULL, bool * = NULL);
-extern tree max_object_size ();
#endif /* GCC_BUILTINS_H */
Index: gcc/c-family/c-common.c
===
--- gcc/c-family/c-common.c (revision 268372)
+++ gcc/c-family/c-common.c (working copy)
@@ -8231,29 +8231,82 @@ reject_gcc_builtin (const_tree expr, location_t lo
return false;
}
+/* Issue an ERROR for an invalid SIZE of array NAME which is null
+ for unnamed arrays. */
+
+void
+invalid_array_size_error (location_t loc, cst_size_error error,
+ const_tree size, const_tree name)
+{
+ tree maxsize = max_object_size ();
+ switch (error)
+{
+case cst_size_negative:
+ if (name)
+ error_at (loc, "size %qE of array %qE is negative",
+ size, name);
+ else
+ error_at (loc, "size %qE of array is negative",
+ size);
+ break;
+case cst_size_too_big:
+ if (name)
+ error_at (loc, "size %qE of array %qE exceeds maximum "
+ "object size %qE", size, name, maxsize);
+ else
+ error_at (loc, "size %qE of array exceeds maximum "
+ "object size %qE", size, maxsize);
+ break;
+case cst_size_overflow:
+ if (name)
+ error_at (loc, "size of array %qE exceeds maximum "
+ "object size %qE", name, maxsize);
+ else
+ error_at (loc, "size of array exceeds maximum "
+ "object size %qE", maxsize);
+ break;
+default:
+ gcc_unreachable ();
+}
+}
+
/* Check if array size calculations overflow or if the array covers more
than half of the address space. Return true if the size of the array
- is valid, false otherwise. TYPE is the type of the array and NAME is
- the name of the array, or NULL_TREE for unnamed arrays. */
+ is valid, false otherwise. T is either the type of the array or its
+ size, and NAME is the name of the array, or null for unnamed arrays. */
bool
-valid_array_size_p (location_t loc, tree type, tree name, bool complain)
+valid_array_size_p (location_t loc, const_tree t, tree name, bool complain)
{
- if (type !=
Re: [Patch][Aarch64]PR rtl-optimization/87763 - Fix lsl_asr_sbfiz.c test by checking for subregs
On Tue, Jan 29, 2019 at 02:51:30PM -0800, Andrew Pinski wrote: > On Tue, Jan 29, 2019 at 2:36 PM Steve Ellcey wrote: > > So the various tests that started failing with r265398 seem to need > > different fixes. This particular fix is for the > > gcc.target/aarch64/lsl_asr_sbfiz.c failure. The problem is that the > > instructions we are trying to match to *ashiftsi_extv_bfiz now have > > explicit subregs in them where they didn't before. The new version > > is: > > > > (set (reg:SI 93) > > (ashift:SI (subreg:SI (sign_extract:DI (subreg:DI (reg:SI 95) 0) > > (const_int 3 [0x3]) > > (const_int 0 [0])) 0) > > (const_int 19 [0x13]))) > > > > > > The subreg's were not there before. My proposed fix is to add an new > > instruction like *ashiftsi_extv_bfiz but with the subregs. This fixes > > lsl_asr_sbfiz.c. Does this seem like the right way to fix this? > > Seems to me rather this should have been simplified to just: > (set (reg:SI 93) > (ashift:SI (sign_extract:SI (reg:SI 95) > (const_int 3 [0x3]) > (const_int 0 [0])) > (const_int 19 [0x13]))) Yes. > Because the two subreg cancel each other out. Well, why did it ever think of using DI at all? > This would be a thing to add to simplify-rtx.c. This is probably specific to combine actually. Segher
C++ PATCH for c++/89083, c++/80864 - ICE with list initialization in template
My recent patch for 88815 and 78244 caused 89083, a P1 9 regression, which
happens to be the same problem as 80864 and its many dupes, something I'd
been meaning to fix for a long time.
Basically, the problem is repeated reshaping of a constructor, once when
parsing, and then again when substituting. With the recent fix, we call
reshape_init + digest_init in finish_compound_literal even in a template
if the expression is not instantiation-dependent, and then again when
tsubst_*.
For instance, in initlist107.C, when parsing a functional cast, we call
finish_compound_literal which calls reshape_init, which turns
{ NON_LVALUE_EXPR<1>, NON_LVALUE_EXPR<2> }
into
{ { NON_LVALUE_EXPR<1>, NON_LVALUE_EXPR<2> } }
and then digest_init turns that into
{ .x = { 1, 2 } }
which is a compound literal (TREE_HAS_CONSTRUCTOR set), but the subexpression
"{ 1, 2 }" isn't. "{ 1, 2 }" will now have the type int[3], so it's not
BRACE_ENCLOSED_INITIALIZER_P.
And then tsubst_* processes "{ .x = { 1, 2 } }". The case CONSTRUCTOR
in tsubst_copy_and_build will call finish_compound_literal on a copy of
"{ 1, 2 }" wrapped in a new { }, because the whole expr has
TREE_HAS_CONSTRUCTOR.
That crashes in reshape_init_r in the
6155 if (TREE_CODE (stripped_init) == CONSTRUCTOR)
block; we have a constructor, it's not COMPOUND_LITERAL_P, and because
digest_init had given it the type int[3], we hit
6172 gcc_assert (BRACE_ENCLOSED_INITIALIZER_P (stripped_init));
As expand_default_init explains in a comment, a CONSTRUCTOR of the target's type
is a previously digested initializer, so we should probably do a similar trick
here. This fixes all the variants of the problem I've come up with.
80864 is a similar case, we reshape when parsing and then second time in
fold_non_dependent_expr called from store_init_value, because of the
'constexpr'.
Also update a stale comment.
Bootstrapped/regtest running on x86_64-linux, ok for trunk and 8 after a while?
2019-01-29 Marek Polacek
PR c++/89083, c++/80864 - ICE with list initialization in template.
* decl.c (reshape_init_r): Don't reshape a digested initializer.
* g++.dg/cpp0x/initlist107.C: New test.
* g++.dg/cpp0x/initlist108.C: New test.
* g++.dg/cpp0x/initlist109.C: New test.
diff --git gcc/cp/decl.c gcc/cp/decl.c
index 79eeac177b6..da08ecc21aa 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -6161,11 +6161,17 @@ reshape_init_r (tree type, reshape_iter *d, bool
first_initializer_p,
;
else if (COMPOUND_LITERAL_P (stripped_init))
/* For a nested compound literal, there is no need to reshape since
-brace elision is not allowed. Even if we decided to allow it,
-we should add a call to reshape_init in finish_compound_literal,
-before calling digest_init, so changing this code would still
-not be necessary. */
+we called reshape_init in finish_compound_literal, before calling
+digest_init. */
gcc_assert (!BRACE_ENCLOSED_INITIALIZER_P (stripped_init));
+ /* Similarly, a CONSTRUCTOR of the target's type is a previously
+digested initializer. */
+ else if (same_type_ignoring_top_level_qualifiers_p (type,
+ TREE_TYPE (init)))
+ {
+ ++d->cur;
+ return init;
+ }
else
{
++d->cur;
diff --git gcc/testsuite/g++.dg/cpp0x/initlist107.C
gcc/testsuite/g++.dg/cpp0x/initlist107.C
new file mode 100644
index 000..9acfe7cb267
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/initlist107.C
@@ -0,0 +1,24 @@
+// PR c++/89083
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wmissing-braces" }
+
+struct A { int x[3]; };
+
+template
+decltype(A{1, 2}, T()) fn1(T t) // { dg-warning "missing braces" }
+{
+ return t;
+}
+
+template
+decltype(A{{1, 2}}, T()) fn2(T t)
+{
+ return t;
+}
+
+void
+f()
+{
+ fn1(1);
+ fn2(1);
+}
diff --git gcc/testsuite/g++.dg/cpp0x/initlist108.C
gcc/testsuite/g++.dg/cpp0x/initlist108.C
new file mode 100644
index 000..e2839787fa5
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/initlist108.C
@@ -0,0 +1,34 @@
+// PR c++/80864
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wmissing-braces" }
+
+struct S {
+ char c[1];
+};
+
+template
+void
+fn ()
+{
+ constexpr S s1 = S{};
+ constexpr S s2 = S{{}};
+ constexpr S s3 = S{{{}}};
+ constexpr S s4 = {};
+ constexpr S s5 = {{}};
+ constexpr S s6 = {{{}}};
+ constexpr S s7{{}};
+ constexpr S s8{S{}};
+ constexpr S s9{S{{}}};
+ constexpr S s10{S{{{;
+ constexpr S s11 = S();
+ constexpr S s12 = S({});
+ constexpr S s13 = S({{}});
+ constexpr S s14 = {{}};
+ constexpr S s15 = {{{}}};
+}
+
+void
+foo ()
+{
+ fn();
+}
diff --git gcc/testsuite/g++.dg/cpp0x/initlist109.C
gcc/testsuite/g++.dg/cpp0x/initlist109.C
new file mode 100644
Re: [backtrace] Avoid segfault
On Sun, Jan 27, 2019 at 01:53:18PM -0800, Ian Lance Taylor wrote: > You need to use a temporary file, such as $@.tmp, for the final sed > command, followed by a mv to $@. Otherwise a failure in the sed will > leave what appears to be an up to date file. Or you just set .DELETE_ON_ERROR, we require GNU make after all so might as well use it! Segher
Re: [C++ PATCH] [PR87770] test partial specializations for type dependence
On Jan 29, 2019, Alexandre Oliva wrote:
> The single caller does, indeed, but the function does not make that a
> requirement, so others might call it and fail to check it. Should that
> test be moved here too?
Like this... Regstrapped on x86_64-linux-gnu. Ok to install?
[PR87770] test partial specializations for type dependence
From: Alexandre Oliva
When instantiating a partial specialization of a template member
function for a full specialization of a class template, we test
whether the context of variables local to the partial specialization,
i.e., the partial specialization itself, is dependent, and this ICEs
in type_dependent_expression_p, when checking that the function type
isn't type-dependent because it is not in a type-dependent scope.
We shouldn't have got that far: the previous block in
type_dependent_expression_p catches cases in which the function itself
takes template arguments of its own, but it only did so for primary
templates, not for partial specializations. This patch fixes that.
for gcc/cp/ChangeLog
PR c++/87770
* pt.c (instantiates_primary_template_p): New.
(type_dependent_expression_p): Use it.
for gcc/testsuite/ChangeLog
PR c++/87770
* g++.dg/pr87770.C: New.
---
gcc/cp/pt.c| 46 +---
gcc/testsuite/g++.dg/pr87770.C | 11 ++
2 files changed, 54 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/pr87770.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 48c180cc13b3b..03f14a9cbe20e 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -400,6 +400,48 @@ template_class_depth (tree type)
return depth;
}
+/* Return TRUE if NODE instantiates a template that has arguments of
+ its own, be it directly a primary template or indirectly through a
+ partial specializations. */
+static inline bool
+instantiates_primary_template_p (tree node)
+{
+ tree tinfo = get_template_info (node);
+ if (!tinfo)
+return false;
+
+ tree tmpl = TI_TEMPLATE (tinfo);
+ if (!PRIMARY_TEMPLATE_P (tmpl))
+{
+ if (!DECL_TEMPLATE_SPECIALIZATION (tmpl))
+ return false;
+
+ /* So now we know we have a specialization, but it could be a full
+or a partial specialization. To tell which, compare the depth of
+its template arguments with those of its context. */
+
+ tree ctxt;
+ if (!DECL_P (node))
+ ctxt = TYPE_CONTEXT (node);
+ else
+ ctxt = DECL_CONTEXT (node);
+
+ tree ctinfo = get_template_info (ctxt);
+
+ int cdepth;
+ if (!ctinfo)
+ cdepth = 0;
+ else
+ cdepth = TMPL_ARGS_DEPTH (TI_ARGS (ctinfo));
+
+ if (TMPL_ARGS_DEPTH (TI_ARGS (tinfo)) <= cdepth)
+ return false;
+}
+
+ return (any_dependent_template_arguments_p
+ (INNERMOST_TEMPLATE_ARGS (TI_ARGS (tinfo;
+}
+
/* Subroutine of maybe_begin_member_template_processing.
Returns true if processing DECL needs us to push template parms. */
@@ -25622,9 +25664,7 @@ type_dependent_expression_p (tree expression)
that come from the template-id; the template arguments for the
enclosing class do not make it type-dependent unless they are used in
the type of the decl. */
- if (PRIMARY_TEMPLATE_P (DECL_TI_TEMPLATE (expression))
- && (any_dependent_template_arguments_p
- (INNERMOST_TEMPLATE_ARGS (DECL_TI_ARGS (expression)
+ if (instantiates_primary_template_p (expression))
return true;
}
diff --git a/gcc/testsuite/g++.dg/pr87770.C b/gcc/testsuite/g++.dg/pr87770.C
new file mode 100644
index 0..69eff4a786fef
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr87770.C
@@ -0,0 +1,11 @@
+// { dg-do compile }
+
+template struct d {
+ template d(e);
+};
+template <> template d::d(e);
+template <> template d::d(e) {
+ long g;
+ (void)g;
+}
+template d::d(char);
--
Alexandre Oliva, freedom fighter https://FSFLA.org/blogs/lxo
Be the change, be Free! FSF Latin America board member
GNU Toolchain EngineerFree Software Evangelist
Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe
C++ PATCH to fix r268343 fallout (PR testsuite/89110)
As a consequence of r268343, the following tests need to be updated to
expect the error in all modes.
Sorry for the breakage.
Tested on x86_64-linux, ok for trunk?
2019-01-29 Marek Polacek
PR testsuite/89110
* g++.dg/other/nontype-1.C: Expect error in all modes.
* g++.dg/parse/crash13.C: Likewise.
* g++.dg/parse/error36.C: Likewise.
* g++.dg/template/error29.C: Likewise.
diff --git gcc/testsuite/g++.dg/other/nontype-1.C
gcc/testsuite/g++.dg/other/nontype-1.C
index 8d90c322a7e..11bbfb82968 100644
--- gcc/testsuite/g++.dg/other/nontype-1.C
+++ gcc/testsuite/g++.dg/other/nontype-1.C
@@ -1,7 +1,7 @@
template
bool asfun(Op f,
- Op::first_argument_type a, // { dg-error "not a type" "" { target
c++17_down } }
- Op::second_argument_type b) // { dg-error "not a type" "" { target
c++17_down } }
+ Op::first_argument_type a, // { dg-error "not a type" }
+ Op::second_argument_type b) // { dg-error "not a type" }
{
return Op(a, b);
}
diff --git gcc/testsuite/g++.dg/parse/crash13.C
gcc/testsuite/g++.dg/parse/crash13.C
index 7a4939a462d..3c298ec8ede 100644
--- gcc/testsuite/g++.dg/parse/crash13.C
+++ gcc/testsuite/g++.dg/parse/crash13.C
@@ -12,11 +12,11 @@ struct A
};
template
-void func(A::B* ) // { dg-error "variable|template|expression" "" {
target c++17_down } }
+void func(A::B* ) // { dg-error "variable|template|expression" }
{
}
int main()
{
- func(0); // { dg-error "not declared|expression|;" "" { target
c++17_down } }
+ func(0); // { dg-error "not declared|expression|;" }
}
diff --git gcc/testsuite/g++.dg/parse/error36.C
gcc/testsuite/g++.dg/parse/error36.C
index 7e52d1537e0..bf57a6b68ce 100644
--- gcc/testsuite/g++.dg/parse/error36.C
+++ gcc/testsuite/g++.dg/parse/error36.C
@@ -25,7 +25,7 @@ template struct B
// PR c++/40738
template
-void g(const A::type ); // { dg-error "typename" "" { target c++17_down
} }
+void g(const A::type ); // { dg-error "typename" }
// PR c++/18451
template A::B A::b; // { dg-error "typename" "" { target
c++17_down } }
diff --git gcc/testsuite/g++.dg/template/error29.C
gcc/testsuite/g++.dg/template/error29.C
index 6e335487224..2e2291d7e87 100644
--- gcc/testsuite/g++.dg/template/error29.C
+++ gcc/testsuite/g++.dg/template/error29.C
@@ -1,5 +1,5 @@
// PR c++/33209
-template void foo(int, T::x); // { dg-error "T::x" "" { target
c++17_down } }
+template void foo(int, T::x); // { dg-error "T::x" }
-template class T> void foo2(int, T::x); // { dg-error
"T::x" "" { target c++17_down } }
+template class T> void foo2(int, T::x); // { dg-error
"T::x" }
Re: [Patch][Aarch64]PR rtl-optimization/87763 - Fix lsl_asr_sbfiz.c test by checking for subregs
On Tue, Jan 29, 2019 at 2:36 PM Steve Ellcey wrote: > > So the various tests that started failing with r265398 seem to need > different fixes. This particular fix is for the > gcc.target/aarch64/lsl_asr_sbfiz.c failure. The problem is that the > instructions we are trying to match to *ashiftsi_extv_bfiz now have > explicit subregs in them where they didn't before. The new version > is: > > (set (reg:SI 93) > (ashift:SI (subreg:SI (sign_extract:DI (subreg:DI (reg:SI 95) 0) > (const_int 3 [0x3]) > (const_int 0 [0])) 0) > (const_int 19 [0x13]))) > > > The subreg's were not there before. My proposed fix is to add an new > instruction like *ashiftsi_extv_bfiz but with the subregs. This fixes > lsl_asr_sbfiz.c. Does this seem like the right way to fix this? Seems to me rather this should have been simplified to just: (set (reg:SI 93) (ashift:SI (sign_extract:SI (reg:SI 95) (const_int 3 [0x3]) (const_int 0 [0])) (const_int 19 [0x13]))) Because the two subreg cancel each other out. This would be a thing to add to simplify-rtx.c. Thanks, Andrew > > Steve Ellcey > sell...@marvell.com > > > 2018-01-29 Steve Ellcey > > PR rtl-optimization/87763 > * config/aarch64/aarch64.md (*ashiftsi_extv_bfiz_alt): > New Instruction. > > diff --git a/gcc/config/aarch64/aarch64.md > b/gcc/config/aarch64/aarch64.md > index b7f6fe0f135..d65230c4837 100644 > --- a/gcc/config/aarch64/aarch64.md > +++ b/gcc/config/aarch64/aarch64.md > @@ -5531,6 +5531,22 @@ >[(set_attr "type" "bfx")] > ) > > +(define_insn "*ashiftsi_extv_bfiz_alt" > + [(set (match_operand:SI 0 "register_operand" "=r") > + (ashift:SI > + (subreg:SI > + (sign_extract:DI > + (subreg:DI (match_operand:SI 1 "register_operand" "r") 0) > + (match_operand 2 "aarch64_simd_shift_imm_offset_si" "n") > + (const_int 0)) > + 0) > + (match_operand 3 "aarch64_simd_shift_imm_si" "n")))] > + "IN_RANGE (INTVAL (operands[2]) + INTVAL (operands[3]), > +1, GET_MODE_BITSIZE (SImode) - 1)" > + "sbfiz\\t%w0, %w1, %3, %2" > + [(set_attr "type" "bfx")] > +) > + > ;; When the bit position and width of the equivalent extraction add up > to 32 > ;; we can use a W-reg LSL instruction taking advantage of the implicit > ;; zero-extension of the X-reg.
Re: [patch, fortran] Fix PR 57048
Hi Steve, Thanks for the patch, and OK to commit. Committed to trunk (r268372). Thanks! I will backport to the affected branches later in the week. Regards Thomas
[Patch][Aarch64]PR rtl-optimization/87763 - Fix lsl_asr_sbfiz.c test by checking for subregs
So the various tests that started failing with r265398 seem to need different fixes. This particular fix is for the gcc.target/aarch64/lsl_asr_sbfiz.c failure. The problem is that the instructions we are trying to match to *ashiftsi_extv_bfiz now have explicit subregs in them where they didn't before. The new version is: (set (reg:SI 93) (ashift:SI (subreg:SI (sign_extract:DI (subreg:DI (reg:SI 95) 0) (const_int 3 [0x3]) (const_int 0 [0])) 0) (const_int 19 [0x13]))) The subreg's were not there before. My proposed fix is to add an new instruction like *ashiftsi_extv_bfiz but with the subregs. This fixes lsl_asr_sbfiz.c. Does this seem like the right way to fix this? Steve Ellcey sell...@marvell.com 2018-01-29 Steve Ellcey PR rtl-optimization/87763 * config/aarch64/aarch64.md (*ashiftsi_extv_bfiz_alt): New Instruction. diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index b7f6fe0f135..d65230c4837 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -5531,6 +5531,22 @@ [(set_attr "type" "bfx")] ) +(define_insn "*ashiftsi_extv_bfiz_alt" + [(set (match_operand:SI 0 "register_operand" "=r") + (ashift:SI + (subreg:SI + (sign_extract:DI + (subreg:DI (match_operand:SI 1 "register_operand" "r") 0) + (match_operand 2 "aarch64_simd_shift_imm_offset_si" "n") + (const_int 0)) + 0) + (match_operand 3 "aarch64_simd_shift_imm_si" "n")))] + "IN_RANGE (INTVAL (operands[2]) + INTVAL (operands[3]), +1, GET_MODE_BITSIZE (SImode) - 1)" + "sbfiz\\t%w0, %w1, %3, %2" + [(set_attr "type" "bfx")] +) + ;; When the bit position and width of the equivalent extraction add up to 32 ;; we can use a W-reg LSL instruction taking advantage of the implicit ;; zero-extension of the X-reg.
Re: [C PATCH] Ignore compound literals in -W*misses-init warning (PR c/89061)
On Tue, 29 Jan 2019, Jakub Jelinek wrote: > Hi! > > While gotos across automatic compound literal initialization do > cross their initialization, I can't think of any way how could code after > the label to which the goto bypasses it get access to the uninitialized > compound literal. Even if the complit address is taken, it can be assigned > to some variable only if it is not bypassed by the goto, otherwise there > should be no way to refer to it. > > So, the following patch disables the warning for compound literals. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Don't emit "parameter passing ABI changes in -fabi-version=12 (GCC 8)" warnings for internal linkage functions (PR c++/89105)
On Tue, Jan 29, 2019 at 4:49 PM Jakub Jelinek wrote: > > On Tue, Jan 29, 2019 at 04:29:59PM -0500, Jason Merrill wrote: > > On Tue, Jan 29, 2019 at 1:38 PM Jakub Jelinek wrote: > > > > > > Emitting this warning for internal linkage functions makes no sense to me, > > > the ABI of those functions is solely under control of the compiler that > > > knows the callee as well as all callers and can do anything it wants. > > > I've added DECL_PRESERVE_P to the test, so if somebody uses used attribute > > > and accesses those from inline assembly, they get warning. > > > > Calling an affected function from inline assembly seems extremely > > Ok, can drop that. > > > unlikely, so I'd probably just check TREE_PUBLIC. Why do you also > > check DECL_EXTERNAL? > > I thought TREE_PUBLIC might not be set on DECL_EXTERNAL fndecls, but it > seems it is set. > > So, like this then (if it passes bootstrap/regtest)? OK. Jason
Re: [PATCH] Don't emit "parameter passing ABI changes in -fabi-version=12 (GCC 8)" warnings for internal linkage functions (PR c++/89105)
On Tue, Jan 29, 2019 at 04:29:59PM -0500, Jason Merrill wrote:
> On Tue, Jan 29, 2019 at 1:38 PM Jakub Jelinek wrote:
> >
> > Emitting this warning for internal linkage functions makes no sense to me,
> > the ABI of those functions is solely under control of the compiler that
> > knows the callee as well as all callers and can do anything it wants.
> > I've added DECL_PRESERVE_P to the test, so if somebody uses used attribute
> > and accesses those from inline assembly, they get warning.
>
> Calling an affected function from inline assembly seems extremely
Ok, can drop that.
> unlikely, so I'd probably just check TREE_PUBLIC. Why do you also
> check DECL_EXTERNAL?
I thought TREE_PUBLIC might not be set on DECL_EXTERNAL fndecls, but it
seems it is set.
So, like this then (if it passes bootstrap/regtest)?
2019-01-29 Jakub Jelinek
PR c++/89105
* config/i386/i386.c (ix86_warn_parameter_passing_abi): Don't warn
for arguments to functions that are TU-local and shouldn't be
referenced by assembly.
* g++.target/i386/pr89105.C: New test.
--- gcc/config/i386/i386.c.jj 2019-01-24 21:20:06.902275003 +0100
+++ gcc/config/i386/i386.c 2019-01-29 16:50:20.157550206 +0100
@@ -29562,6 +29562,10 @@ ix86_warn_parameter_passing_abi (cumulat
if (!TYPE_EMPTY_P (type))
return;
+ /* Don't warn if the function isn't visible outside of the TU. */
+ if (cum->decl && !TREE_PUBLIC (cum->decl))
+return;
+
const_tree ctx = get_ultimate_context (cum->decl);
if (ctx != NULL_TREE
&& !TRANSLATION_UNIT_WARN_EMPTY_P (ctx))
--- gcc/testsuite/g++.target/i386/pr89105.C.jj 2019-01-29 16:53:23.692535030
+0100
+++ gcc/testsuite/g++.target/i386/pr89105.C 2019-01-29 16:53:59.952939332
+0100
@@ -0,0 +1,16 @@
+// PR c++/89105
+// { dg-do compile { target c++11 } }
+// { dg-options "-fabi-version=12 -Wabi=11" }
+
+namespace {
+ template
+void run(F f, int i) // { dg-bogus "parameter passing ABI changes in
-fabi-version=12" }
+{
+ f(i);
+}
+}
+
+void f()
+{
+ run([](int) { }, 1); // { dg-bogus "parameter passing ABI changes in
-fabi-version=12" }
+}
Jakub
Re: [patch, fortran] Fix PR 57048
On Tue, Jan 29, 2019 at 10:12:49PM +0100, Thomas König wrote: > Hi Steve, > > >>PR fortran/57048 > >>* interface.c (gfc_compare_types): If a derived type and an > >>integer both have a derived type, and they are identical, > >>this is a C binding type and compares equal. > > > > I don't understand this sentence. How can an INTEGER have a > > derived type? > > ISO C handling is a bit of a mess. Of course. > The original tree has > >symtree: 'C_funptr'|| symbol: 'c_funptr' > type spec : (DERIVED c_funptr C_INTEROP ISO_C) > attributes: (DERIVED BIND(C) USE-ASSOC(__iso_c_binding)) > components: > (c_address (INTEGER 8 C_INTEROP) () PRIVATE) > > and we somehow later lose the derived type stuff and use a naked > integer. The problem appears to be that the module is written out > when that conversion has not yet happened. > > So, the C_funptr (capitalized because it is a derived type) is > written to the module file, and later compared to c_funptr upon > module reading. Hilarity ensues, and we cannot use a C function > pointer from a module, the topic of the PR. Ugh. Thanks for the explanation. > >> 2019-01-28 Thomas Koenig > >> > >>PR fortran/57048 > >>* gfortran.dg/c_funptr_1.f90: New file. > >>* gfortran.dg/c_funptr_1_mod.f90: New file. > > > > Yep, this is a rather messy fix to a messy situation. I think the > C interop stuff could do with a thorough redesign, but that's > not going to happen for gcc 9. > > So, I think this is about the best we can do - at least it gets the > bug fixed. If you want me to put in a FIXME, I'll gladly do this. > Perhaps, add the PR number to the comment in code so history of why this is a special case can be found. I'll leave up to you on adding a "FIXME: If ISO C Binding support is ever rewritten, this should be revisted" or some such phrasing. Thanks for the patch, and OK to commit. -- Steve 20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4 20161221 https://www.youtube.com/watch?v=IbCHE-hONow
Re: [PATCH] Don't emit "parameter passing ABI changes in -fabi-version=12 (GCC 8)" warnings for internal linkage functions (PR c++/89105)
On Tue, Jan 29, 2019 at 1:38 PM Jakub Jelinek wrote: > > Emitting this warning for internal linkage functions makes no sense to me, > the ABI of those functions is solely under control of the compiler that > knows the callee as well as all callers and can do anything it wants. > I've added DECL_PRESERVE_P to the test, so if somebody uses used attribute > and accesses those from inline assembly, they get warning. Calling an affected function from inline assembly seems extremely unlikely, so I'd probably just check TREE_PUBLIC. Why do you also check DECL_EXTERNAL? Jason
Re: [patch, fortran] Fix PR 57048
Hi Steve, PR fortran/57048 * interface.c (gfc_compare_types): If a derived type and an integer both have a derived type, and they are identical, this is a C binding type and compares equal. I don't understand this sentence. How can an INTEGER have a derived type? ISO C handling is a bit of a mess. Of course. The original tree has symtree: 'C_funptr'|| symbol: 'c_funptr' type spec : (DERIVED c_funptr C_INTEROP ISO_C) attributes: (DERIVED BIND(C) USE-ASSOC(__iso_c_binding)) components: (c_address (INTEGER 8 C_INTEROP) () PRIVATE) and we somehow later lose the derived type stuff and use a naked integer. The problem appears to be that the module is written out when that conversion has not yet happened. So, the C_funptr (capitalized because it is a derived type) is written to the module file, and later compared to c_funptr upon module reading. Hilarity ensues, and we cannot use a C function pointer from a module, the topic of the PR. 2019-01-28 Thomas Koenig PR fortran/57048 * gfortran.dg/c_funptr_1.f90: New file. * gfortran.dg/c_funptr_1_mod.f90: New file. Index: interface.c === --- interface.c (Revision 268104) +++ interface.c (Arbeitskopie) @@ -692,6 +692,15 @@ gfc_compare_types (gfc_typespec *ts1, gfc_typespec if (ts1->type == BT_VOID || ts2->type == BT_VOID) return true; + /* Special case for our C interop types. There should be a better + way of doing this... */ + + if (((ts1->type == BT_INTEGER && ts2->type == BT_DERIVED) + || (ts1->type == BT_DERIVED && ts2->type == BT_INTEGER)) + && ts1->u.derived && ts2->u.derived + && ts1->u.derived == ts2->u.derived) +return true; If the above is a test for ISO C binding, shouldn't the test also check (ts1->is_c_interop || ts1->is_iso_c) and similar for ts2? Unfortunately, these are both set to zero. Yep, this is a rather messy fix to a messy situation. I think the C interop stuff could do with a thorough redesign, but that's not going to happen for gcc 9. So, I think this is about the best we can do - at least it gets the bug fixed. If you want me to put in a FIXME, I'll gladly do this. Regards Thomas
Re: [PATCH, fortran ieee]: Clear stalled interrupt flags in glibc set_fpu_trap_exceptions
On Tue, Jan 29, 2019 at 08:46:40PM +0100, Uros Bizjak wrote: > > When changing trap masks, it is necessary to clear pending traps to > prevent firing spurious interrupts. Attached patch also optimizes > set_fpu_trap_exceptions function considerably to only call > feenableexcept and fedisableexcept functions each once. > > 2019-01-29 Uroš Bizjak s/Uro/Uros ? > * config/fpu-glibc.h (set_fpu_trap_exceptions): Clear stalled > exception flags before changing trap mode. Optimize to call > feenableexcept and fedisableexcept only once. > > Patch was bootstrapped and regression tested on alphaev68-linux-gnu, > where it fixes gfortran.dg/ieee/ieee_10.f90 failures. > > OK for mainline? > Uros, Your decription suggests that this fixes PR fortran/88678. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88678 I managed to identify that the issue was traceable to fpu-glibc.h, but I got stuck going any further. In any event, the patch looks good to me. Thanks. -- steve
Re: [EXT] Re: [Driver] Add support for -fuse-ld=lld
On 29/01/19 14:12 -0500, Marek Polacek wrote: On Tue, Jan 29, 2019 at 07:07:31PM +, Romain GEISSLER wrote: > Le 29 janv. 2019 à 20:03, Marek Polacek a écrit : > > On Fri, Jan 25, 2019 at 03:06:41PM +, Jonathan Wakely wrote: > > Indeed. Romain, are you going to submit a followup patch to address this? > > Marek Hi, I submitted the original patch because it was not written by me but by Davide, and I thought that modifying myself would require me to sign a FSF copyright assignment (which is Ok for me, but my employer needs to sign too and right now it is not done). So if the followup patch can be committed without assignment, I can do it right now. If not, I will need to get my company legal team to figure out the assignment and it can take weeks. Do you think it would be obvious enough not to require an assignment ? Thanks for the quick response. I believe a formatting fix and a trivial 1- or 2-line change can be committed without that assignment. Agreed.
[PATCH, fortran ieee]: Clear stalled interrupt flags in glibc set_fpu_trap_exceptions
Hello!
When changing trap masks, it is necessary to clear pending traps to
prevent firing spurious interrupts. Attached patch also optimizes
set_fpu_trap_exceptions function considerably to only call
feenableexcept and fedisableexcept functions each once.
2019-01-29 Uroš Bizjak
* config/fpu-glibc.h (set_fpu_trap_exceptions): Clear stalled
exception flags before changing trap mode. Optimize to call
feenableexcept and fedisableexcept only once.
Patch was bootstrapped and regression tested on alphaev68-linux-gnu,
where it fixes gfortran.dg/ieee/ieee_10.f90 failures.
OK for mainline?
Uros.
Index: config/fpu-glibc.h
===
--- config/fpu-glibc.h (revision 268248)
+++ config/fpu-glibc.h (working copy)
@@ -39,48 +39,56 @@
void set_fpu_trap_exceptions (int trap, int notrap)
{
+ int mode_set = 0, mode_clr = 0;
+
#ifdef FE_INVALID
if (trap & GFC_FPE_INVALID)
-feenableexcept (FE_INVALID);
+mode_set |= FE_INVALID;
if (notrap & GFC_FPE_INVALID)
-fedisableexcept (FE_INVALID);
+mode_clr |= FE_INVALID;
#endif
/* Some glibc targets (like alpha) have FE_DENORMAL, but not many. */
#ifdef FE_DENORMAL
if (trap & GFC_FPE_DENORMAL)
-feenableexcept (FE_DENORMAL);
+mode_set |= FE_DENORMAL;
if (notrap & GFC_FPE_DENORMAL)
-fedisableexcept (FE_DENORMAL);
+mode_clr |= FE_DENORMAL;
#endif
#ifdef FE_DIVBYZERO
if (trap & GFC_FPE_ZERO)
-feenableexcept (FE_DIVBYZERO);
+mode_set |= FE_DIVBYZERO;
if (notrap & GFC_FPE_ZERO)
-fedisableexcept (FE_DIVBYZERO);
+mode_clr |= FE_DIVBYZERO;
#endif
#ifdef FE_OVERFLOW
if (trap & GFC_FPE_OVERFLOW)
-feenableexcept (FE_OVERFLOW);
+mode_set |= FE_OVERFLOW;
if (notrap & GFC_FPE_OVERFLOW)
-fedisableexcept (FE_OVERFLOW);
+mode_clr |= FE_OVERFLOW;
#endif
#ifdef FE_UNDERFLOW
if (trap & GFC_FPE_UNDERFLOW)
-feenableexcept (FE_UNDERFLOW);
+mode_set |= FE_UNDERFLOW;
if (notrap & GFC_FPE_UNDERFLOW)
-fedisableexcept (FE_UNDERFLOW);
+mode_clr |= FE_UNDERFLOW;
#endif
#ifdef FE_INEXACT
if (trap & GFC_FPE_INEXACT)
-feenableexcept (FE_INEXACT);
+mode_set |= FE_INEXACT;
if (notrap & GFC_FPE_INEXACT)
-fedisableexcept (FE_INEXACT);
+mode_clr |= FE_INEXACT;
#endif
+
+ /* Clear stalled exception flags. */
+ feclearexcept (FE_ALL_EXCEPT);
+
+ feenableexcept (mode_set);
+ fedisableexcept (mode_clr);
}
Re: [patch, fortran] Fix PR 57048
On Mon, Jan 28, 2019 at 08:49:52PM +0100, Thomas König wrote: > > the attached patch fixes a long-time regression where a c_funptr from a > module could not be found. > > The solution is a bit of a hack, but so is our whole implementation of > the C interop stuff. > > Regression-tested. OK for trunk? > > Regards > > Thomas > > 2019-01-28 Thomas Koenig > > PR fortran/57048 > * interface.c (gfc_compare_types): If a derived type and an > integer both have a derived type, and they are identical, > this is a C binding type and compares equal. I don't understand this sentence. How can an INTEGER have a derived type? > > 2019-01-28 Thomas Koenig > > PR fortran/57048 > * gfortran.dg/c_funptr_1.f90: New file. > * gfortran.dg/c_funptr_1_mod.f90: New file. > Index: interface.c > === > --- interface.c (Revision 268104) > +++ interface.c (Arbeitskopie) > @@ -692,6 +692,15 @@ gfc_compare_types (gfc_typespec *ts1, gfc_typespec >if (ts1->type == BT_VOID || ts2->type == BT_VOID) > return true; > > + /* Special case for our C interop types. There should be a better > + way of doing this... */ > + > + if (((ts1->type == BT_INTEGER && ts2->type == BT_DERIVED) > + || (ts1->type == BT_DERIVED && ts2->type == BT_INTEGER)) > + && ts1->u.derived && ts2->u.derived > + && ts1->u.derived == ts2->u.derived) > +return true; If the above is a test for ISO C binding, shouldn't the test also check (ts1->is_c_interop || ts1->is_iso_c) and similar for ts2? -- Steve
Re: [EXT] Re: [Driver] Add support for -fuse-ld=lld
On Tue, Jan 29, 2019 at 07:07:31PM +, Romain GEISSLER wrote: > > Le 29 janv. 2019 à 20:03, Marek Polacek a écrit : > > > > On Fri, Jan 25, 2019 at 03:06:41PM +, Jonathan Wakely wrote: > > > > Indeed. Romain, are you going to submit a followup patch to address this? > > > > Marek > > Hi, > > I submitted the original patch because it was not written by me but by > Davide, and I thought that modifying myself would require me to sign a FSF > copyright assignment (which is Ok for me, but my employer needs to sign too > and right now it is not done). > > So if the followup patch can be committed without assignment, I can do it > right now. If not, I will need to get my company legal team to figure out the > assignment and it can take weeks. Do you think it would be obvious enough not > to require an assignment ? Thanks for the quick response. I believe a formatting fix and a trivial 1- or 2-line change can be committed without that assignment. Marek
Re: [EXT] Re: [Driver] Add support for -fuse-ld=lld
> Le 29 janv. 2019 à 20:03, Marek Polacek a écrit : > > On Fri, Jan 25, 2019 at 03:06:41PM +, Jonathan Wakely wrote: > > Indeed. Romain, are you going to submit a followup patch to address this? > > Marek Hi, I submitted the original patch because it was not written by me but by Davide, and I thought that modifying myself would require me to sign a FSF copyright assignment (which is Ok for me, but my employer needs to sign too and right now it is not done). So if the followup patch can be committed without assignment, I can do it right now. If not, I will need to get my company legal team to figure out the assignment and it can take weeks. Do you think it would be obvious enough not to require an assignment ? Cheers, Romain
Re: [Driver] Add support for -fuse-ld=lld
On Fri, Jan 25, 2019 at 03:06:41PM +, Jonathan Wakely wrote: > On 20/10/18 12:18 +0200, Romain Geissler wrote: > > Hi, > > > > I would like to raise again the question of supporting -fuse-ld=ldd. A > > patch implementing it was already submitted in > > https://gcc.gnu.org/ml/gcc-patches/2016-06/msg01722.html by Davide > > Italiano. This patch still applies correctly to current trunk. I am CC-ing > > the original author and re-posting it here unchanged for reference. > > > > I think we can consider this patch as relevant despite the goals and > > licence difference of LLVM vs GNU, based on what was written by Mike Stump > > in https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00157.html > > > > Back then, the technical problem raised by lld was reported as > > https://bugs.llvm.org/show_bug.cgi?id=28414 now closed. In this bug, every > > reported problems have been fixed except the last one. H.J. Lu mentions > > this last problem (lld does not produces symbol versions predecessor > > relationship while ld.bfd and ld.gold do, which seems to be a decision > > taken on purpose and advertised as a harmless change) as being one reason > > against supporting in -fuse-ld=ldd in gcc. Is it still the case today, and > > if yes, why ? > > > > Is there any other reason why -fuse-ld=ldd shall not be supported by gcc ? > > This patch was committed to trunk (r265940), but HJ's review comments > were never addressed (and look correct to me): > https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00146.html > > The multi-line condition should be split before the || operator not > after it, and the negation of -fuse-ld=lld is not -fuse-ld-lld. Indeed. Romain, are you going to submit a followup patch to address this? Marek
[PATCH] Don't emit "parameter passing ABI changes in -fabi-version=12 (GCC 8)" warnings for internal linkage functions (PR c++/89105)
Hi!
Emitting this warning for internal linkage functions makes no sense to me,
the ABI of those functions is solely under control of the compiler that
knows the callee as well as all callers and can do anything it wants.
I've added DECL_PRESERVE_P to the test, so if somebody uses used attribute
and accesses those from inline assembly, they get warning.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-01-29 Jakub Jelinek
PR c++/89105
* config/i386/i386.c (ix86_warn_parameter_passing_abi): Don't warn
for arguments to functions that are TU-local and shouldn't be
referenced by assembly.
* g++.target/i386/pr89105.C: New test.
--- gcc/config/i386/i386.c.jj 2019-01-24 21:20:06.902275003 +0100
+++ gcc/config/i386/i386.c 2019-01-29 16:50:20.157550206 +0100
@@ -29562,6 +29562,14 @@ ix86_warn_parameter_passing_abi (cumulat
if (!TYPE_EMPTY_P (type))
return;
+ /* Don't warn if the function isn't visible outside of the
+ TU and can't be accessed by assembly either. */
+ if (cum->decl
+ && !(TREE_PUBLIC (cum->decl)
+ || DECL_EXTERNAL (cum->decl)
+ || DECL_PRESERVE_P (cum->decl)))
+return;
+
const_tree ctx = get_ultimate_context (cum->decl);
if (ctx != NULL_TREE
&& !TRANSLATION_UNIT_WARN_EMPTY_P (ctx))
--- gcc/testsuite/g++.target/i386/pr89105.C.jj 2019-01-29 16:53:23.692535030
+0100
+++ gcc/testsuite/g++.target/i386/pr89105.C 2019-01-29 16:53:59.952939332
+0100
@@ -0,0 +1,16 @@
+// PR c++/89105
+// { dg-do compile { target c++11 } }
+// { dg-options "-fabi-version=12 -Wabi=11" }
+
+namespace {
+ template
+void run(F f, int i) // { dg-bogus "parameter passing ABI changes in
-fabi-version=12" }
+{
+ f(i);
+}
+}
+
+void f()
+{
+ run([](int) { }, 1); // { dg-bogus "parameter passing ABI changes in
-fabi-version=12" }
+}
Jakub
[C PATCH] Ignore compound literals in -W*misses-init warning (PR c/89061)
Hi!
While gotos across automatic compound literal initialization do
cross their initialization, I can't think of any way how could code after
the label to which the goto bypasses it get access to the uninitialized
compound literal. Even if the complit address is taken, it can be assigned
to some variable only if it is not bypassed by the goto, otherwise there
should be no way to refer to it.
So, the following patch disables the warning for compound literals.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-01-29 Jakub Jelinek
PR c/89061
* c-tree.h (C_DECL_COMPOUND_LITERAL_P): Define.
* c-decl.c (decl_jump_unsafe): Return false for
C_DECL_COMPOUND_LITERAL_P decls.
(build_compound_literal): Set C_DECL_COMPOUND_LITERAL_P.
* gcc.dg/pr89061.c: New test.
--- gcc/c/c-tree.h.jj 2019-01-01 12:37:48.637458450 +0100
+++ gcc/c/c-tree.h 2019-01-29 16:14:25.310950693 +0100
@@ -96,6 +96,10 @@ along with GCC; see the file COPYING3.
#pragma omp threadprivate. */
#define C_DECL_THREADPRIVATE_P(DECL) DECL_LANG_FLAG_3 (VAR_DECL_CHECK (DECL))
+/* Set on VAR_DECLs for compound literals. */
+#define C_DECL_COMPOUND_LITERAL_P(DECL) \
+ DECL_LANG_FLAG_5 (VAR_DECL_CHECK (DECL))
+
/* Nonzero for a decl which either doesn't exist or isn't a prototype.
N.B. Could be simplified if all built-in decls had complete prototypes
(but this is presently difficult because some of them need FILE*). */
--- gcc/c/c-decl.c.jj 2019-01-29 00:48:57.361683401 +0100
+++ gcc/c/c-decl.c 2019-01-29 16:12:54.383444385 +0100
@@ -659,6 +659,14 @@ decl_jump_unsafe (tree decl)
if (error_operand_p (decl))
return false;
+ /* Don't warn for compound literals. If a goto statement crosses
+ their initialization, it should cross also all the places where
+ the complit is used or where the complit address might be saved
+ into some variable, so code after the label to which goto jumps
+ should not be able to refer to the compound literal. */
+ if (VAR_P (decl) && C_DECL_COMPOUND_LITERAL_P (decl))
+return false;
+
/* Always warn about crossing variably modified types. */
if ((VAR_P (decl) || TREE_CODE (decl) == TYPE_DECL)
&& variably_modified_type_p (TREE_TYPE (decl), NULL_TREE))
@@ -5486,6 +5494,7 @@ build_compound_literal (location_t loc,
DECL_READ_P (decl) = 1;
DECL_ARTIFICIAL (decl) = 1;
DECL_IGNORED_P (decl) = 1;
+ C_DECL_COMPOUND_LITERAL_P (decl) = 1;
TREE_TYPE (decl) = type;
c_apply_type_quals_to_decl (TYPE_QUALS (strip_array_types (type)), decl);
if (alignas_align)
--- gcc/testsuite/gcc.dg/pr89061.c.jj 2019-01-29 16:15:22.220015753 +0100
+++ gcc/testsuite/gcc.dg/pr89061.c 2019-01-29 16:08:50.862444669 +0100
@@ -0,0 +1,27 @@
+/* PR c/89061 */
+/* { dg-do compile } */
+/* { dg-options "-Wjump-misses-init" } */
+
+struct S { int s; };
+
+int
+foo (int x)
+{
+ struct S s = { 0 };
+ if ((s.s = x) == 0)
+goto cleanup; /* { dg-bogus "jump skips variable
initialization" } */
+ s = (struct S) { .s = 42 };
+ cleanup:
+ return s.s;
+}
+
+int
+bar (int x)
+{
+ struct S *s = &(struct S) { 0 };
+ if ((s->s = x) == 0)
+goto cleanup; /* { dg-bogus "jump skips variable
initialization" } */
+ s = &(struct S) { .s = 42 };
+ cleanup:
+ return s->s;
+}
Jakub
Re: [PATCH] avoid assuming arrays have nonzero size (PR 88956)
On Tue, Jan 22, 2019 at 06:18:28PM -0700, Martin Sebor wrote:
> PS In GCC 10, unless there is an important use case that escapes
> me, I think GCC should warn for zero-length non-member array
> objects, or perhaps even for internal struct members (those followed
> by another member). Not to avoid these sorts of bugs but because
> they seem too dangerous to use safely.
No, we shouldn't warn.
> --- gcc/gimple-fold.c (revision 268086)
> +++ gcc/gimple-fold.c (working copy)
> @@ -6715,12 +6715,14 @@ fold_array_ctor_reference (tree type, tree ctor,
>elt_size = wi::to_offset (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (ctor;
>
>/* When TYPE is non-null, verify that it specifies a constant-sized
> - accessed not larger than size of array element. */
> - if (type
> - && (!TYPE_SIZE_UNIT (type)
> - || TREE_CODE (TYPE_SIZE_UNIT (type)) != INTEGER_CST
> - || elt_size < wi::to_offset (TYPE_SIZE_UNIT (type))
> - || elt_size == 0))
> + accessed not larger than size of array element. Avoid using zero
> + ELT_SIZE, the result of an empty initializer for a zero-length
> + array. */
The comment is misleading, there are many reasons why you could have
zero elt_size, C struct S {}, or int x[10][0], etc. Just don't change
anything in the comment at all, or say
Punt if element size is zero to avoid division by zero.
or something similar.
> + if (elt_size == 0
> + || (type
> + && (!TYPE_SIZE_UNIT (type)
> + || TREE_CODE (TYPE_SIZE_UNIT (type)) != INTEGER_CST
> + || elt_size < wi::to_offset (TYPE_SIZE_UNIT (type)
> return NULL_TREE;
Otherwise LGTM.
If you could fix also the two comments a few lines above this:
/* Static constructors for variably sized objects makes no sense. */
to
/* Static constructors for variably sized objects make no sense. */
it would be appreciated.
Jakub
Re: [PATCH, OpenACC] Rework computation of default OpenACC mapping clauses
On 28/01/2019 18:00, Thomas Schwinge wrote: On Fri, 25 Jan 2019 15:09:49 +0100, Gergö Barany wrote: OK for openacc-gcc-8-branch? Yes. Thanks, committed along with the other patches I posted at the same time (Rework OpenACC Fortran DO loop initialization, Remove spurious OpenACC error on combining "auto" with gang/worker/vector), and with Reviewed-by: notes added to the commit messages. Thanks, Gergö
Re: [PATCH] avoid assuming arrays have nonzero size (PR 88956)
On 1/29/19 5:44 AM, Richard Sandiford wrote: Martin Sebor writes: Ping: https://gcc.gnu.org/ml/gcc-patches/2019-01/msg01340.html This is a straightforward fix for an ICE. I will commit it tomorrow unless there are suggestions for other changes. That's not how it works. Please wait for the patch to be approved by someone. The rules are explicit that "Obvious fixes can be committed without prior approval." I consider moving an equality test three lines up an obvious fix. Martin
[C++ PATCH] PR c++/89089 - ICE with [[no_unique_address]].
In 89089, we were never actually setting DECL_SIZE on an empty data member,
because its type is a POD, so we didn't set it in the maybe-overlapping
section. Fixed by also handling empty types there.
In 88865, we were failing to consider empty data members in
include_empty_classes. Fixed by making end_of_class always include them.
While looking at these I noticed that the ABI says that a
potentially-overlapping data member makes its class non-layout-POD, and that
an empty data member doesn't prevent its class from being empty, so I've
implemented those points as well.
Tested x86_64-pc-linux-gnu, applying to trunk.
PR c++/88865 - wrong layout with [[no_unique_address]].
* class.c (check_field_decls): A potentially-overlapping field makes
the class non-layout-POD, but not non-empty.
(end_of_class): Always consider empty data members.
(layout_class_type): Set DECL_SIZE for empty fields.
---
gcc/cp/class.c| 48 ---
gcc/testsuite/g++.dg/abi/no_unique_address4.C | 25 ++
gcc/testsuite/g++.dg/abi/no_unique_address5.C | 18 +++
.../g++.dg/cpp2a/no_unique_address2.C | 12 +
gcc/cp/ChangeLog | 9
5 files changed, 96 insertions(+), 16 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/abi/no_unique_address4.C
create mode 100644 gcc/testsuite/g++.dg/abi/no_unique_address5.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/no_unique_address2.C
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index e8773c2a0b0..48da081212a 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -206,6 +206,7 @@ static int empty_base_at_nonzero_offset_p (tree, tree,
splay_tree);
static tree end_of_base (tree);
static tree get_vcall_index (tree, tree);
static bool type_maybe_constexpr_default_constructor (tree);
+static bool field_poverlapping_p (tree);
/* Return a COND_EXPR that executes TRUE_STMT if this execution of the
'structor is in charge of 'structing virtual bases, or FALSE_STMT
@@ -3556,6 +3557,11 @@ check_field_decls (tree t, tree *access_decls,
/* We don't treat zero-width bitfields as making a class
non-empty. */
;
+ else if (field_poverlapping_p (x) && is_empty_class (type))
+ {
+ /* Empty data members also don't make a class non-empty. */
+ CLASSTYPE_CONTAINS_EMPTY_CLASS_P (t) = 1;
+ }
else
{
/* The class is non-empty. */
@@ -3608,6 +3614,11 @@ check_field_decls (tree t, tree *access_decls,
to be allowed in POD structs. */
CLASSTYPE_NON_LAYOUT_POD_P (t) = 1;
+ if (field_poverlapping_p (x))
+ /* A potentially-overlapping non-static data member makes the class
+ non-layout-POD. */
+ CLASSTYPE_NON_LAYOUT_POD_P (t) = 1;
+
if (!std_layout_type_p (type))
CLASSTYPE_NON_STD_LAYOUT (t) = 1;
@@ -5926,13 +5937,12 @@ end_of_base (tree binfo)
return size_binop (PLUS_EXPR, BINFO_OFFSET (binfo), size);
}
-/* Returns the offset of the byte just past the end of the base class
- with the highest offset in T. If INCLUDE_VIRTUALS_P is zero, then
- only non-virtual bases are included. If INCLUDE_FIELDS_P is true,
- then also consider non-static data members. */
+/* Returns the offset of the byte just past the end of the base class or empty
+ data member with the highest offset in T. If INCLUDE_VIRTUALS_P is zero,
+ then only non-virtual bases are included. */
static tree
-end_of_class (tree t, bool include_virtuals_p, bool include_fields_p = false)
+end_of_class (tree t, bool include_virtuals_p)
{
tree result = size_zero_node;
vec *vbases;
@@ -5955,15 +5965,19 @@ end_of_class (tree t, bool include_virtuals_p, bool
include_fields_p = false)
result = offset;
}
- if (include_fields_p)
-for (tree field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field))
- if (TREE_CODE (field) == FIELD_DECL)
- {
- offset = size_binop (PLUS_EXPR, DECL_FIELD_OFFSET (field),
- DECL_SIZE_UNIT (field));
- if (tree_int_cst_lt (result, offset))
- result = offset;
- }
+ /* Also consider empty data members. */
+ for (tree field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field))
+if (TREE_CODE (field) == FIELD_DECL
+ && !DECL_ARTIFICIAL (field)
+ && field_poverlapping_p (field)
+ && is_empty_class (TREE_TYPE (field)))
+ {
+ /* Update sizeof(C) to max (sizeof(C), offset(D)+sizeof(D)) */
+ offset = size_binop (PLUS_EXPR, DECL_FIELD_OFFSET (field),
+TYPE_SIZE_UNIT (TREE_TYPE (field)));
+ if (tree_int_cst_lt (result, offset))
+ result = offset;
+ }
if (include_virtuals_p)
for (vbases = CLASSTYPE_VBASECLASSES (t), i = 0;
@@ -6154,12 +6168,14 @@ layout_class_type (tree t, tree *virtuals_p)
bool might_overlap = field_poverlapping_p (field);
libgo patch committed: Fix sigprof frame counting
This libgo patch by Cherry Zhang fixes sigprof frame counting. If
sigtramp and sigtrampgo are both on stack, n -= framesToDiscard is
executed twice, which should actually run only once. Bootstrapped and
ran Go testsuite on x86_64-pc-linux-gnu. Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 268358)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-c2cac0ba0a92e74d5675c3c9f4e53d2567dbc903
+5af8ee0693944c280b1f529450dbfd4ec1ee451d
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: libgo/go/runtime/proc.go
===
--- libgo/go/runtime/proc.go(revision 268158)
+++ libgo/go/runtime/proc.go(working copy)
@@ -3542,14 +3542,13 @@ func sigprof(pc uintptr, gp *g, mp *m) {
for i := 0; i < n; i++ {
if stklocs[i].function == "runtime.sigtrampgo" && i+2 <
n {
framesToDiscard = i + 2
- n -= framesToDiscard
}
if stklocs[i].function == "runtime.sigtramp" && i+2 < n
{
framesToDiscard = i + 2
- n -= framesToDiscard
break
}
}
+ n -= framesToDiscard
for i := 0; i < n; i++ {
stk[i] = stklocs[i+framesToDiscard].pc
}
Re: [PATCH 9/9] [libbacktrace] Add printdwarftest_dwz_cmp.sh test-case
On Tue, Jan 22, 2019 at 1:43 PM Tom de Vries wrote: > > What is an acceptable way to proceed here? I could add a > libbacktrace_nodwarf.la, and have the test-case add a -DFOR_TESTING or > some such when compiling dwarf.c, and add the necessary handling in > dwarf.c conditional on FOR_TESTING. WDYT? I guess we could try it. I'm very seriously concerned with the additional complexity being added to this package. It has to live in a very constrained space and it's already complex. Sacrificing maintainability for testing is a bad tradeoff in the long run. But we can try it. Ian
Re: [backtrace] Avoid segfault
On Tue, Jan 29, 2019 at 12:24 AM Tom de Vries wrote: > > On 27-01-19 22:53, Ian Lance Taylor wrote: > > On Sun, Jan 27, 2019 at 1:16 PM Tom de Vries wrote: > >> > >> On 25-01-19 18:15, Nathan Sidwell wrote: > >>> On 1/25/19 5:28 AM, Tom de Vries wrote: > > This patch fixes it by passing "" instead of NULL, in the call to > elf_add at line 3083 (for .gnu_debugaltlink), not the call to elf_add at > line 3044 (for .gnu_debuglink) mentioned above. > > Nathan, does this fix the problem for you? If not, can you provide a > reproducer, or give a hint on how one could be constructed? > >>> > >>> I still hit the problem, and am installing this as sufficiently obvious. > >>> I'm on a fedora system debugging pr88995. The debuglink_name is > >>> "../../.dwz/isl-0.16.1-7.fc29.x86_64" > >>> > >> > >> I've managed to reproduce this segfault instance by adding a test-case > >> that uses both build-id and dwz. > >> > >> OK for trunk? > > > >> +elf_for_test.c: elf.c > >> + PWD=$$(pwd -P); \ > >> + BUILD_ID_DIR="usr/lib/debug/.build-id/"; \ > >> + SEARCH='#define SYSTEM_BUILD_ID_DIR'; \ > >> + REPLACE="#define SYSTEM_BUILD_ID_DIR \"$$PWD/$$BUILD_ID_DIR\""; \ > >> + $(SED) "s%^$$SEARCH.*\$$%$$REPLACE%" \ > >> + $< \ > >> + > $@ > > > > You need to use a temporary file, such as $@.tmp, for the final sed > > command, followed by a mv to $@. Otherwise a failure in the sed will > > leave what appears to be an up to date file. > > > > Done. > > Also, I've factored out a script install-debuginfo-for-buildid.sh, > hoping this will make things more readable/maintainable. > > > Honestly I'm not sure this patch is worth doing. It adds a lot of > > complex mechanism in order to test a patch that is fairly obvious. > > Agreed, the patch is fairly obvious. > > But at the moment, there's no test-case that exercises the build-id > support in libbacktrace. IMO, that alone would be a good reason to add > this test-case. > > > While it's good practice to add a test for every change, it's not good > > practice for the testsuite to become so complex that it becomes in > > itself difficult to maintain. > > Understood. > > Please let me know if this is acceptable, or if I can do anything that > would make this easier to maintain. I guess this is OK. Please add a copyright header to the new shell script. Also please add a comment explaining what it does. Thanks. Ian
Fix libphobos testsuite failures on Solaris
Yet another trivial fix for a Solaris libphobos testsuite failure: FAIL: libphobos.shared/load.d -shared-libphobos -ldl (test for excess errors) Excess errors: /vol/gcc/src/hg/trunk/local/libphobos/testsuite/libphobos.shared/load.d:9: error: static assert "unimplemented" I guess this is obvious? Tested on i386-pc-solaris2.11. Ok for mainline? Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University 2018-12-11 Rainer Orth * testsuite/libphobos.shared/load.d [Solaris]: Import core.sys.solaris.dlfcn. # HG changeset patch # Parent 72693fde3d76ea72811c57d822fd624c0dc3594f Fix libphobos testsuite failures on Solaris diff --git a/libphobos/testsuite/libphobos.shared/load.d b/libphobos/testsuite/libphobos.shared/load.d --- a/libphobos/testsuite/libphobos.shared/load.d +++ b/libphobos/testsuite/libphobos.shared/load.d @@ -6,6 +6,7 @@ import core.thread; version (linux) import core.sys.linux.dlfcn; else version (FreeBSD) import core.sys.freebsd.dlfcn; else version (NetBSD) import core.sys.netbsd.dlfcn; +else version (Solaris) import core.sys.solaris.dlfcn; else static assert(0, "unimplemented"); void loadSym(T)(void* handle, ref T val, const char* mangle)
Fix gdc testsuite failures on Solaris
This patch fixes two gdc testsuite failures on Solaris:
UNRESOLVED: gdc.test/runnable/dhry.d compilation failed to produce executable
UNRESOLVED: gdc.test/runnable/dhry.d -shared-libphobos compilation failed to
produce executable
gdc.test/runnable/dhry.d:489:16: error: undefined identifier 'dtime'
gdc.test/runnable/dhry.d:543:14: error: undefined identifier 'dtime'
In line with the existing code, I've added yet another (identical)
implementation of dtime. However, to avoid further duplication, it's
probably way better to introduce (say) version (Have_Gettimeofday)
instead.
The other failures is
UNRESOLVED: gdc.test/runnable/cppa.d compilation failed to produce executable
UNRESOLVED: gdc.test/runnable/cppa.d -g compilation failed to produce
executable
UNRESOLVED: gdc.test/runnable/cppa.d -g -shared-libphobos compilation failed
to produce executable
UNRESOLVED: gdc.test/runnable/cppa.d -shared-libphobos compilation failed to
produce executable
In file included from gdc.test/runnable/extra-files/cppb.cpp:36:
/vol/gcc/src/hg/trunk/local/gcc/testsuite/../../libstdc++-v3/libsupc++/exception:37:10:
fatal error: bits/c++config.h: No such file or directory
compilation terminated.
It turns out that one the -I flags added by is wrong for the non-default
multilib:
$objdir/gcc/testsuite/gdc1/../../gdc -B$objdir/gcc/testsuite/gdc1/../../
gdc.test/runnable/cppa.d -m64 -fno-diagnostics-show-caret
-fno-diagnostics-show-line-numbers -fdiagnostics-color=never
-I$objdir/i386-pc-solaris2.11/amd64/libphobos/libdruntime
-I$srcdir/gcc/testsuite/../../libphobos/libdruntime
-I$srcdir/gcc/testsuite/../../libphobos/src
-I$objdir/i386-pc-solaris2.11/amd64/libstdc++-v3/include
-I$objdir/i386-pc-solaris2.11/amd64/libstdc++-v3/include/amd64
-I$srcdir/gcc/testsuite/../../libstdc++-v3/libsupc++
-I$srcdir/gcc/testsuite/gdc.test/runnable
gdc.test/runnable/extra-files/cppb.cpp
-B$objdir/i386-pc-solaris2.11/amd64/libphobos/src
-L$objdir/i386-pc-solaris2.11/amd64/libphobos/src/.libs
-L$objdir/i386-pc-solaris2.11/amd64/libphobos/libdruntime/.libs
-L$objdir/i386-pc-solaris2.11/amd64/libstdc++-v3/src/.libs -lm -o ./cppa.exe
While bits/c++config.h is in
i386-pc-solaris2.11/amd64/libstdc++-v3/include/i386-pc-solaris2.11
gdc searches .../libstdc++-v3/include/amd64 instead which doesn't
exist. This needs to be $target_triplet instead, which lets the test
PASS for both the default and the 64-bit multilib. The same issue
exists with multilib testing on Linux, btw.
Tested on i386-pc-solaris2.11 and x86_64-pc-linux-gnu. Ok for mainline?
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
2018-12-12 Rainer Orth
* gdc.test/runnable/dhry.d [Solaris]: Import
core.sys.posix.sys.time.
(dtime): Define.
* lib/gdc.exp (gdc_include_flags): Declare target_triplet.
Remove target.
Use $target_triplet in C++ include path.
# HG changeset patch
# Parent c9fb71dd0383141099bbe01ff36128303e774f7a
Fix gdc testsuite failures on Solaris
diff --git a/gcc/testsuite/gdc.test/runnable/dhry.d b/gcc/testsuite/gdc.test/runnable/dhry.d
--- a/gcc/testsuite/gdc.test/runnable/dhry.d
+++ b/gcc/testsuite/gdc.test/runnable/dhry.d
@@ -925,3 +925,19 @@ version (NetBSD)
return q;
}
}
+
+version (Solaris)
+{
+import core.sys.posix.sys.time;
+
+double dtime()
+{
+ double q;
+ timeval tv;
+
+ gettimeofday(,null);
+ q = cast(double)tv.tv_sec + cast(double)tv.tv_usec * 1.0e-6;
+
+ return q;
+}
+}
diff --git a/gcc/testsuite/lib/gdc.exp b/gcc/testsuite/lib/gdc.exp
--- a/gcc/testsuite/lib/gdc.exp
+++ b/gcc/testsuite/lib/gdc.exp
@@ -68,7 +68,7 @@ proc gdc_version { } {
#
proc gdc_include_flags { paths } {
-global srcdir
+global srcdir target_triplet
global TESTING_IN_BUILD_TREE
set flags ""
@@ -78,7 +78,6 @@ proc gdc_include_flags { paths } {
}
set gccpath ${paths}
-set target [file tail [file normalize ${paths}]]
if { $gccpath != "" } {
if [file exists "${gccpath}/libphobos/libdruntime"] {
@@ -92,7 +91,7 @@ proc gdc_include_flags { paths } {
if { $gccpath != "" } {
if [file exists "${gccpath}/libstdc++-v3/include"] {
append flags "-I${gccpath}/libstdc++-v3/include "
-append flags "-I${gccpath}/libstdc++-v3/include/$target "
+append flags "-I${gccpath}/libstdc++-v3/include/$target_triplet "
}
}
append flags "-I${srcdir}/../../libstdc++-v3/libsupc++"
RE: [PATCH] [ARC]: Enable init_array support
2019-xx-xx Vineet Gupta * gcc/config.gcc: Force .init_array for ARC Signed-off-by: Vineet Gupta --- Hi, Thank you for your contribution. I'll push it asap. Cheers, Claudiu
Re: [PATCH] avoid assuming arrays have nonzero size (PR 88956)
Martin Sebor writes: > Ping: https://gcc.gnu.org/ml/gcc-patches/2019-01/msg01340.html > > This is a straightforward fix for an ICE. I will commit it tomorrow > unless there are suggestions for other changes. That's not how it works. Please wait for the patch to be approved by someone. Thanks, Richard
[libphobos, build] Enable libphobos on Solaris 11/x86
With the set of libphobos Solaris patches just posted, it would become
possible to enable libphobos on Solaris 11/x86 by default.
This is what this patch does.
* It uses a LIBPHOBOS_SUPPORTED variable both in toplevel configure and
libphobos/configure.tgt, following what libvtv does.
* It's necessary to disable libphobos when Solaris as is in use: it has
a relatively low line length limit of 10240 which is exceeded in a few
libphobos files.
Bootstrapped without regressions on i386-pc-solaris2.11 (as and gas, gas
and gld, Solaris 11.3/11.4/11.5) on top of the previous set of patches.
Also tested manually that explicit
--enable-libphobos/--disable-libphobos give the desired results
(i.e. override the defaults).
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
2019-01-23 Rainer Orth
toplevel:
* configure.ac (enable_libphobos): Check LIBPHOBOS_SUPPORTED.
* configure: Regenerate.
libphobos:
* configure.tgt (LIBPHOBOS_SUPPORTED): Default to no.
Set to yes explicitly.
(x86_64-*-solaris2.11* | i?86-*-solaris2.11*): Mark supported.
* configure.ac (x86_64-*-solaris2.* | i?86-*-solaris2.*): Only
mark supported with gas.
(ENABLE_LIBPHOBOS): New conditional.
* configure: Regenerate.
* Makefile.am (SUBDIRS): Only set if ENABLE_LIBPHOBOS.
* Makefile.in: Regenerate.
# HG changeset patch
# Parent 7943668e276d15b949fddd7723cb5213fb465977
Enable libphobos on Solaris 11/x86
diff --git a/configure.ac b/configure.ac
--- a/configure.ac
+++ b/configure.ac
@@ -694,7 +694,7 @@ if test -d ${srcdir}/libphobos; then
AC_MSG_CHECKING([for libphobos support])
if (srcdir=${srcdir}/libphobos; \
. ${srcdir}/configure.tgt; \
- test -n "$UNSUPPORTED")
+ test "$LIBPHOBOS_SUPPORTED" != "yes")
then
AC_MSG_RESULT([no])
noconfigdirs="$noconfigdirs target-libphobos"
diff --git a/libphobos/Makefile.am b/libphobos/Makefile.am
--- a/libphobos/Makefile.am
+++ b/libphobos/Makefile.am
@@ -15,7 +15,11 @@
# along with GCC; see the file COPYING3. If not see
#
Provide __start_minfo/__stop_minfo for linkers that don't (PR d/87864)
Solaris ld only gained support for section bracketing in Solaris 11.4.
Fortunately, in gdc it is only used for the minfo section, so it's easy
to provide a workaround by adding two additional startup files
drt{begin,end}.o which define __start_minfo and __stop_minfo.
This patch does just that.
I've raised a couple of questions in the PR already:
* I've introduced a new -dstartfiles option which triggers the use of
libgphobos.spec even with -nophoboslib. Since it's effectively
internal to the build system, I'm not currently documenting it.
* I'm reading the spec file addition from a file: keeping it in a make
variable would be extremely messy due to the necessary quoting.
* I've chosen to use -Wc instead of -Xcompiler throughout: it's way
shorter when more options need to be passed and it can take several
comma-separated options at once.
* libdruntime/gcc/drtstuff.c needs a copyright notice unless one wants
to keep it in the public domain (also plausible). Effectively
something for Iain to decide.
Bootstrapped without regressions on i386-pc-solaris2.11 (Solaris 11.3),
no regressions compared to Solaris 11.4 test results.
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
2018-11-20 Rainer Orth
libphobos:
PR d/87864
* configure.ac [!DCFG_MINFO_BRACKETING] (DRTSTUFF_SPEC): New variable.
Substitute it.
* libdruntime/m4/druntime/os.m4 (DRUNTIME_OS_MINFO_BRACKETING):
New automake conditional.
* configure: Regenerate.
* libdruntime/gcc/drtstuff.c: New file.
* libdruntime/Makefile.am [!DRUNTIME_OS_MINFO_BRACKETING]
(DRTSTUFF, toolexeclib_DATA): New variables.
(gcc/drtbegin.lo, gcc/drtend.lo): New rules.
(libgdruntime_la_LDFLAGS): Add -dstartfiles -Bgcc -B../src.
(libgdruntime_la_DEPENDENCIES): New variable.
* src/Makefile.am (libgphobos_la_LDFLAGS): Add -dstartfiles
-B../libdruntime/gcc.
* libdruntime/Makefile.in, src/Makefile.in: Regenerate.
* Makefile.in, testsuite/Makefile.in: Regenerate.
* libdruntime/rt/sections_elf_shared.d (Minfo_Bracketing): Don't
assert.
* src/drtstuff.spec: New file.
* src/libgphobos.spec.in (DRTSTUFF_SPEC): Substitute.
(*lib): Only pass SPEC_PHOBOS_DEPS without -debuglib, -defaultlib,
-nophoboslib.
* testsuite/testsuite_flags.in <--gdcldflags> (GDCLDFLAGS): Add
-B${BUILD_DIR}/libdruntime/gcc.
* libdruntime/Makefile.am (unittest_static_LDFLAGS): Use -Wc
instead of -Xcompiler.
(libgdruntime_t_la_LDFLAGS): Likewise.
(unittest_LDFLAGS): Likewise.
* src/Makefile.am (unittest_static_LDFLAGS): Likewise.
(libgphobos_t_la_LDFLAGS): Likewise.
(unittest_LDFLAGS): Likewise.
gcc/d:
PR d/87864
* lang.opt (dstartfiles): New option.
* d-spec.cc (need_spec): New variable.
(lang_specific_driver) : Enable need_spec.
(lang_specific_pre_link): Also load libgphobos.spec if need_spec.
gcc/testsuite:
PR d/87864
* lib/gdc.exp (gdc_link_flags): Add path to drtbegin.o/drtend.o if
present.
# HG changeset patch
# Parent 2b02744cb551e91a3c5dc300f12ae168581adc34
Provide __start_minfo/__stop_minfo for linkers that don't (PR d/87864)
diff --git a/gcc/d/d-spec.cc b/gcc/d/d-spec.cc
--- a/gcc/d/d-spec.cc
+++ b/gcc/d/d-spec.cc
@@ -72,6 +72,9 @@ static phobos_action phobos_library = PH
standard libraries. */
static bool need_phobos = true;
+/* If true, do load libgphobos.spec even if not needed otherwise. */
+static bool need_spec = false;
+
void
lang_specific_driver (cl_decoded_option **in_decoded_options,
unsigned int *in_decoded_options_count,
@@ -144,6 +147,10 @@ lang_specific_driver (cl_decoded_option
switch (decoded_options[i].opt_index)
{
+ case OPT_dstartfiles:
+ need_spec = true;
+ break;
+
case OPT_nostdlib:
case OPT_nodefaultlibs:
phobos_library = PHOBOS_NOLINK;
@@ -491,7 +498,7 @@ lang_specific_driver (cl_decoded_option
int
lang_specific_pre_link (void)
{
- if (phobos_library != PHOBOS_NOLINK && need_phobos)
+ if ((phobos_library != PHOBOS_NOLINK && need_phobos) || need_spec)
do_spec ("%:include(libgphobos.spec)");
return 0;
diff --git a/gcc/d/lang.opt b/gcc/d/lang.opt
--- a/gcc/d/lang.opt
+++ b/gcc/d/lang.opt
@@ -162,6 +162,10 @@ defaultlib=
Driver Joined
Default library to use instead of phobos.
+dstartfiles
+Driver
+Do link the standard D startup files in the compilation.
+
-verbose
D Alias(v)
diff --git a/gcc/testsuite/lib/gdc.exp b/gcc/testsuite/lib/gdc.exp
--- a/gcc/testsuite/lib/gdc.exp
+++ b/gcc/testsuite/lib/gdc.exp
@@ -119,6 +119,10 @@ proc gdc_link_flags { paths } {
if { $gccpath != "" } {
# Path to libgphobos.spec.
append flags
Re: [patch][pr88920] Fix noisy check_effective_target_offload_gcn
On Tue, Jan 29, 2019 at 11:37 AM Andrew Stubbs wrote: > > My recent patch to permit GCN testing using an LLVM assembler and linker > has caused some noise in the log files for libgomp testing. > > There are lots of messages like this: > >fatal error: GCC is not configured to support amdgcn-unknown-amdhsa as >offload target > > These messages are harmless; they're caused by > check_effective_target_offload_gcn discovering the answer is "no". > However, there's more noise than I intended. > > This patch adds caching so that the message will only occur once per > test run. > > OK to commit? OK. > Also, is there a way to redirect the output so that the message does not > show up at all, unless the verbosity level is raised? > > Thanks > > Andrew
[libphobos] Work around lack of dlpi_tls_modid before Solaris 11.5
Before Solaris 11.5, struct dl_phdr_info lacked the dlpi_tls_modid
member. While the support might be backported to Solaris 11.4, it
certainly won't to previous Solaris releases. To work around this, I've
used the following patch. Again, it's pretty straightforward.
Point of note:
* On Solaris, FreeBSD and NetBSD, dl_phdr_info is always visible in
, while on Linux one needs to define _GNU_SOURCE.
AC_USE_SYSTEM_EXTENSION takes care of this, but needs to be called
early (i.e. not in DRUNTIME_OS_DLPI_TLS_MODID) to avoid an autoconf
warning:
configure.ac:129: warning: AC_COMPILE_IFELSE was called before
AC_USE_SYSTEM_EXTENSIONS
m4/druntime/os.m4:190: DRUNTIME_OS_DLPI_TLS_MODID is expanded from...
configure.ac:129: the top level
Tested on i386-pc-solaris2.11 (Solaris 11.4) and x86_64-pc-linux-gnu.
Not unexpectedly, there are a couple of regressions compared to the
Solaris 11.5/x86 results:
+FAIL: gdc.test/runnable/testaa.d execution test
+FAIL: gdc.test/runnable/testaa.d -fPIC execution test
+FAIL: gdc.test/runnable/testaa.d -fPIC -shared-libphobos execution test
+FAIL: gdc.test/runnable/testaa.d -shared-libphobos execution test
32 and 64-bit
+FAIL: gdc.test/runnable/xtest55.d execution test
+FAIL: gdc.test/runnable/xtest55.d -shared-libphobos execution test
64-bit only
+FAIL: libphobos.shared/link_linkdep.d -I/vol/gcc/src/hg/trunk/local/libphobos/t
estsuite/libphobos.shared liblinkdep.so lib.so -shared-libphobos execution test
+FAIL: libphobos.shared/load_linkdep.d -shared-libphobos -ldl execution test
+FAIL: libphobos.shared/load_loaddep.d -shared-libphobos -ldl execution test
+FAIL: libphobos.shared/linkDR.c -shared-libphobos -ldl -pthread execution test
+FAIL: libphobos.shared/host.c -ldl -pthread execution test
+FAIL: libphobos.shared/loadDR.c -ldl -pthread -g execution test
32 and 64-bit
+FAIL: libphobos.shared/link.d
-I/vol/gcc/src/hg/trunk/local/libphobos/testsuite/libphobos.shared lib.so
-shared-libphobos execution test
64-bit only
I haven't looked much closer yet, just posting this to see if anything
along those lines could be acceptable at all.
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
2019-01-22 Rainer Orth
* m4/druntime/os.m4 (DRUNTIME_OS_DLPI_TLS_MODID): New macro.
* configure.ac: Use it.
Call AC_USE_SYSTEM_EXTENSIONS.
* configure: Regenerate.
* Makefile.in, libdruntime/Makefile.in, src/Makefile.in,
testsuite/Makefile.in: Regenerate.
* libdruntime/gcc/config.d.in (OS_Have_Dlpi_Tls_Modid): Define.
* libdruntime/rt/sections_elf_shared.d (scanSegments) :
Only set pdso._tlsMod, pdso._tlsSize if OS_Have_Dlpi_Tls_Modid.
# HG changeset patch
# Parent 517609f2fc6f40a6cbf15addd7f1006864256064
Work around lack of dlpi_tls_modid before Solaris 11.5
diff --git a/libphobos/configure.ac b/libphobos/configure.ac
--- a/libphobos/configure.ac
+++ b/libphobos/configure.ac
@@ -32,6 +32,7 @@ AC_CONFIG_HEADERS(config.h)
AM_ENABLE_MULTILIB(, ..)
AC_CANONICAL_SYSTEM
+AC_USE_SYSTEM_EXTENSIONS
target_alias=${target_alias-$target}
AC_SUBST(target_alias)
@@ -126,6 +127,7 @@ DRUNTIME_OS_SOURCES
DRUNTIME_OS_THREAD_MODEL
DRUNTIME_OS_ARM_EABI_UNWINDER
DRUNTIME_OS_MINFO_BRACKETING
+DRUNTIME_OS_DLPI_TLS_MODID
DRUNTIME_OS_LINK_SPEC
WITH_LOCAL_DRUNTIME([
diff --git a/libphobos/libdruntime/gcc/config.d.in b/libphobos/libdruntime/gcc/config.d.in
--- a/libphobos/libdruntime/gcc/config.d.in
+++ b/libphobos/libdruntime/gcc/config.d.in
@@ -38,6 +38,9 @@ enum ThreadModel GNU_Thread_Model = Thre
// Whether the linker provides __start_minfo and __stop_minfo symbols
enum Minfo_Bracketing = @DCFG_MINFO_BRACKETING@;
+// Whether struct dl_phdr_info has dlpi_tls_modid member.
+enum OS_Have_Dlpi_Tls_Modid = @DCFG_DLPI_TLS_MODID@;
+
// Whether target has support for builtin atomics.
enum GNU_Have_Atomics = @DCFG_HAVE_ATOMIC_BUILTINS@;
diff --git a/libphobos/libdruntime/rt/sections_elf_shared.d b/libphobos/libdruntime/rt/sections_elf_shared.d
--- a/libphobos/libdruntime/rt/sections_elf_shared.d
+++ b/libphobos/libdruntime/rt/sections_elf_shared.d
@@ -751,8 +751,16 @@ void scanSegments(in ref dl_phdr_info in
case PT_TLS: // TLS segment
assert(!pdso._tlsSize); // is unique per DSO
-pdso._tlsMod = info.dlpi_tls_modid;
-pdso._tlsSize = phdr.p_memsz;
+static if (OS_Have_Dlpi_Tls_Modid)
+{
+pdso._tlsMod = info.dlpi_tls_modid;
+pdso._tlsSize = phdr.p_memsz;
+}
+else
+{
+pdso._tlsMod = 0;
+pdso._tlsSize = 0;
+}
break;
default:
diff --git a/libphobos/m4/druntime/os.m4 b/libphobos/m4/druntime/os.m4
--- a/libphobos/m4/druntime/os.m4
+++ b/libphobos/m4/druntime/os.m4
@@ -183,6 +183,20 @@
[libphobos] Work around Solaris ld bug linking __tls_get_addr on 64-bit x86
Here's the patch I mentioned in https://gcc.gnu.org/ml/gcc-patches/2019-01/msg01661.html to work around an amd64 Solaris ld bug. I'm just posting it for reference now: until it's clear if a fix will make it into Solaris 11.5 or not, there's no point in applying it yet. Still, review comments are appreciated. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University 2019-01-28 Rainer Orth * m4/druntime/os.m4 (DRUNTIME_OS_LINK_SPEC): New macro. * configure.ac: Call it. * configure: Regenerate. * src/libgphobos.spec.in (*link): Append OS_LINK_SPEC. # HG changeset patch # Parent 4dadb62112e1e735535bc37fe31e14c82340a1b4 Work around Solaris ld bug linking __tls_get_addr on 64-bit x86 diff --git a/libphobos/configure.ac b/libphobos/configure.ac --- a/libphobos/configure.ac +++ b/libphobos/configure.ac @@ -126,6 +126,7 @@ DRUNTIME_OS_SOURCES DRUNTIME_OS_THREAD_MODEL DRUNTIME_OS_ARM_EABI_UNWINDER DRUNTIME_OS_MINFO_BRACKETING +DRUNTIME_OS_LINK_SPEC WITH_LOCAL_DRUNTIME([ AC_LANG_PUSH([D]) diff --git a/libphobos/m4/druntime/os.m4 b/libphobos/m4/druntime/os.m4 --- a/libphobos/m4/druntime/os.m4 +++ b/libphobos/m4/druntime/os.m4 @@ -182,3 +182,27 @@ AC_DEFUN([DRUNTIME_OS_MINFO_BRACKETING], AC_SUBST(DCFG_MINFO_BRACKETING) AC_LANG_POP([C]) ]) + +# DRUNTIME_OS_LINK_SPEC +# - +# Add target-specific link options to link_spec. +AC_DEFUN([DRUNTIME_OS_LINK_SPEC], +[ + case $target in +i?86-*-solaris2.* | x86_64-*-solaris2.*) + # 64-bit Solaris/x86 ld breaks calls to __tls_get_addr with non-TLS + # relocs. Work around by disabling TLS transitions. Not necessary + # on 32-bit x86, but cannot be distinguished reliably in specs. + if test $DCFG_DLPI_TLS_MODID = true; then +druntime_ld_prog=`$CC -print-prog-name=ld` +if test -n "$druntime_ld_prog" \ + && $druntime_ld_prog -v 2>&1 | grep GNU > /dev/null 2>&1; then + : +else + OS_LINK_SPEC='-z relax=transtls' +fi + fi + ;; + esac + AC_SUBST(OS_LINK_SPEC) +]) diff --git a/libphobos/src/libgphobos.spec.in b/libphobos/src/libgphobos.spec.in --- a/libphobos/src/libgphobos.spec.in +++ b/libphobos/src/libgphobos.spec.in @@ -4,5 +4,8 @@ # order. # +%rename link linkorig_gdc_renamed +*link: %(linkorig_gdc_renamed) @OS_LINK_SPEC@ + %rename lib liborig_gdc_renamed *lib: @SPEC_PHOBOS_DEPS@ %(liborig_gdc_renamed)
[patch][pr88920] Fix noisy check_effective_target_offload_gcn
My recent patch to permit GCN testing using an LLVM assembler and linker
has caused some noise in the log files for libgomp testing.
There are lots of messages like this:
fatal error: GCC is not configured to support amdgcn-unknown-amdhsa as
offload target
These messages are harmless; they're caused by
check_effective_target_offload_gcn discovering the answer is "no".
However, there's more noise than I intended.
This patch adds caching so that the message will only occur once per
test run.
OK to commit?
Also, is there a way to redirect the output so that the message does not
show up at all, unless the verbosity level is raised?
Thanks
Andrew
Cache effective-target llvm_binutils result.
2019-01-21 Andrew Stubbs
gcc/testsuite/
* lib/target-supports.exp: Cache result.
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index c0df467..f233f10 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -9255,6 +9255,7 @@ foreach N {df} {
# Return 1 if this target uses an LLVM assembler and/or linker
proc check_effective_target_llvm_binutils { } {
-return [expr { [istarget amdgcn*-*-*]
- || [check_effective_target_offload_gcn] } ]
+return [check_cached_effective_target llvm_binutils {
+ expr { [istarget amdgcn*-*-*]
+ || [check_effective_target_offload_gcn] }}]
}
[libphobos] Use sections_elf_shared.d on Solaris 11.5 (PR d/88150)
I've successfully been using a late prototype of an implementation of the dlpi_tls_modid field of struct dl_phdr_info on Solaris 11.5 Beta. This allowed me to get pretty reasonable test results using sections_elf_shared.d on Solaris. This pretty straightforward patch implements this. Only a few points are worth mentioning: * libdruntime/rt/bss_sections.c refers to __bss_start, which only gld defines. Unfortunately, it's marked weak, so the absence with Solaris ld went unnoticed at first. Lacking an exact equivalent, I'm using _edata instead, which is pretty close modulo section alignment. * As detailed in the PR, not dlclose()ing the handle in handleForName is necessary to avoid an assertion failure. * I'm removing sections_solaris.d since it wouldn't even compile and seems pretty useless. This patch gave me the following testsuite results on Solaris 11.5/x86 (a few more minor testsuite fixes were included, too): === gdc tests === Running target unix FAIL: gdc.test/runnable/nulltype.d execution test FAIL: gdc.test/runnable/nulltype.d -O2 execution test FAIL: gdc.test/runnable/nulltype.d -O2 -shared-libphobos execution test FAIL: gdc.test/runnable/nulltype.d -g execution test FAIL: gdc.test/runnable/nulltype.d -g -O2 execution test FAIL: gdc.test/runnable/nulltype.d -g -O2 -shared-libphobos execution test FAIL: gdc.test/runnable/nulltype.d -g -shared-libphobos execution test FAIL: gdc.test/runnable/nulltype.d -shared-libphobos execution test === gdc Summary for unix === # of expected passes30785 # of unexpected failures8 Running target unix/-m64 === gdc Summary for unix/-m64 === # of expected passes30793 === gdc Summary === # of expected passes61578 # of unexpected failures8 /var/gcc/gcc-9.0.1-20190128/11.5-gcc-gas-libphobos/gcc/gdc version 9.0.1 20190128 (experimental) [trunk revision 268335] (GCC) === libphobos tests === Running target unix FAIL: libphobos.shared/load.d -shared-libphobos -ldl execution test FAIL: libphobos.shared/load_13414.d -shared-libphobos -ldl execution test FAIL: libphobos.shared/finalize.d -shared-libphobos -ldl execution test FAIL: libphobos.shared/linkD.c lib.so -ldl -pthread execution test FAIL: libphobos.unittests/druntime/shared/core.sync.mutex FAIL: libphobos.unittests/druntime/shared/core.time === libphobos Summary for unix === # of expected passes119 # of unexpected failures6 Running target unix/-m64 FAIL: libphobos.shared/load.d -shared-libphobos -ldl execution test FAIL: libphobos.shared/load_13414.d -shared-libphobos -ldl execution test FAIL: libphobos.shared/finalize.d -shared-libphobos -ldl execution test FAIL: libphobos.shared/linkD.c lib.so -ldl -pthread execution test FAIL: libphobos.unittests/druntime/shared/ld.so.1: FAIL: libphobos.unittests/druntime/shared/unittest: FAIL: libphobos.unittests/druntime/shared/fatal: FAIL: libphobos.unittests/druntime/shared/libgdruntime_t.so.0: FAIL: libphobos.unittests/druntime/shared/open FAIL: libphobos.unittests/druntime/shared/failed: FAIL: libphobos.unittests/druntime/shared/No FAIL: libphobos.unittests/druntime/shared/such FAIL: libphobos.unittests/druntime/shared/file FAIL: libphobos.unittests/druntime/shared/or FAIL: libphobos.unittests/druntime/shared/directory === libphobos Summary for unix/-m64 === # of expected passes78 # of unexpected failures15 === libphobos Summary === # of expected passes197 # of unexpected failures21 The 32-bit nulltype.d failures occur on Linux, too (PR d/87824), and the 64-bit libphobos.unittests/druntime/shared failures happen because libgdruntime_t.so.0 is only built for the default multilib. The libphobos.shared failures clearly bear investigating. For the amd64 results, I needed a separate patch to workaround an ld bug, to be submitted shortly. While I did run a sparc-sun-solaris2.11 bootstrap, too, results are pretty useless due to PR d/88462 (the minfo alignment issue). Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University 2018-11-21 Rainer Orth libphobos: PR d/88150 * libdruntime/core/sys/solaris/dlfcn.d: Mark @nogc. * libdruntime/core/sys/solaris/link.d (struct dl_phdr_info): Declare dlpi_tls_modid, dlpi_tls_data members. * libdruntime/core/sys/solaris/sys/link.d (Elf32_Dyn, Elf64_Dyn): Fix syntax. * libdruntime/rt/sections_elf_shared.d [Solaris] (SharedELF): Set to true. Import core.sys.solaris.dlfcn, core.sys.solaris.link, core.sys.solaris.sys.elf, core.sys.solaris.sys.link. (dummy_ref): Declare. (initSections): Initialize dummy_ref. (_Dmodule_ref): Declare.
Re: [PATCH] Handle timeout warnings in dg-extract-results
On Tue, 29 Jan 2019 at 11:12, Iain Sandoe wrote: > > Hi Christophe, > > > On 29 Jan 2019, at 09:58, Christophe Lyon > > wrote: > > > > On Sat, 26 Jan 2019 at 17:43, Iain Sandoe wrote: > >> > >> Hi Christophe, > >> > >>> On 23 Jan 2019, at 13:16, Christophe Lyon > >>> wrote: > >> > >>> dg-extract-results currently moves lines like > >>> WARNING: program timed out > >>> at the end of each .exp section when it generates .sum files. > >>> > >>> This is because it sorts its output based on the 2nd field, which is > >>> normally the testname as in: > >>> FAIL: gcc.c-torture/execute/20020129-1.c -O2 -flto > >>> -fno-use-linker-plugin -flto-partition=none execution test > >>> > >>> As you can notice 'program' comes after > >>> gcc.c-torture/execute/20020129-1.c alphabetically, and generally after > >>> most (all?) GCC testnames. > >>> > >>> This is a bit of a pain when trying to handle transient test failures > >>> because you can no longer match such a WARNING line to its FAIL > >>> counterpart. > >>> > >>> The attached patch changes this behavior by replacing the line > >>> WARNING: program timed out > >>> with > >>> WARNING: gcc.c-torture/execute/20020129-1.c -O2 -flto > >>> -fno-use-linker-plugin -flto-partition=none execution test program > >>> timed out > >>> > >>> The effect is that this line will now appear immediately above the > >>> FAIL: gcc.c-torture/execute/20020129-1.c -O2 -flto > >>> -fno-use-linker-plugin -flto-partition=none execution test > >>> so that it's easier to match them. > >>> > >>> > >>> I'm not sure how much people depend on the .sum format, I also > >>> considered emitting > >>> WARNING: program timed out gcc.c-torture/execute/20020129-1.c -O2 > >>> -flto -fno-use-linker-plugin -flto-partition=none execution test > >>> > >>> I also restricted the patch to handling only 'program timed out' > >>> cases, to avoid breaking other things. > >>> > >>> I considered fixing this in Dejagnu, but it seemed more complicated, > >>> and would delay adoption in GCC anyway. > >>> > >>> What do people think about this? > >> > >> It seems a good idea (for those of us with targets prone to timeout > >> issues). > >> I’m going to give it a try on Darwin. > >> > > > > Thanks for taking a look and confirming I'm not the only one having to face > > random timeouts :-) > > My worst-cases are a couple of PCH-related that are still unanalysed.. but > those are not the only ones. > + when I use VMs to cover some of the earlier versions of the system, they > are much more prone to occasional timeouts. > > > Did it work for you? > > So far it’s looking nice… (anything that makes the dg output more stable is > Good in my book) > > … I am not sure (not checked thoroughly) but I think I saw one case in > libgomp testing where it didn’t pick up the timeout, will poke some more at > that if i see it again. If you still have the logs, you can run the script manually. > > thanks for the patch! > Iain >
Re: [PATCH] Handle timeout warnings in dg-extract-results
Hi Christophe, > On 29 Jan 2019, at 09:58, Christophe Lyon wrote: > > On Sat, 26 Jan 2019 at 17:43, Iain Sandoe wrote: >> >> Hi Christophe, >> >>> On 23 Jan 2019, at 13:16, Christophe Lyon >>> wrote: >> >>> dg-extract-results currently moves lines like >>> WARNING: program timed out >>> at the end of each .exp section when it generates .sum files. >>> >>> This is because it sorts its output based on the 2nd field, which is >>> normally the testname as in: >>> FAIL: gcc.c-torture/execute/20020129-1.c -O2 -flto >>> -fno-use-linker-plugin -flto-partition=none execution test >>> >>> As you can notice 'program' comes after >>> gcc.c-torture/execute/20020129-1.c alphabetically, and generally after >>> most (all?) GCC testnames. >>> >>> This is a bit of a pain when trying to handle transient test failures >>> because you can no longer match such a WARNING line to its FAIL >>> counterpart. >>> >>> The attached patch changes this behavior by replacing the line >>> WARNING: program timed out >>> with >>> WARNING: gcc.c-torture/execute/20020129-1.c -O2 -flto >>> -fno-use-linker-plugin -flto-partition=none execution test program >>> timed out >>> >>> The effect is that this line will now appear immediately above the >>> FAIL: gcc.c-torture/execute/20020129-1.c -O2 -flto >>> -fno-use-linker-plugin -flto-partition=none execution test >>> so that it's easier to match them. >>> >>> >>> I'm not sure how much people depend on the .sum format, I also >>> considered emitting >>> WARNING: program timed out gcc.c-torture/execute/20020129-1.c -O2 >>> -flto -fno-use-linker-plugin -flto-partition=none execution test >>> >>> I also restricted the patch to handling only 'program timed out' >>> cases, to avoid breaking other things. >>> >>> I considered fixing this in Dejagnu, but it seemed more complicated, >>> and would delay adoption in GCC anyway. >>> >>> What do people think about this? >> >> It seems a good idea (for those of us with targets prone to timeout issues). >> I’m going to give it a try on Darwin. >> > > Thanks for taking a look and confirming I'm not the only one having to face > random timeouts :-) My worst-cases are a couple of PCH-related that are still unanalysed.. but those are not the only ones. + when I use VMs to cover some of the earlier versions of the system, they are much more prone to occasional timeouts. > Did it work for you? So far it’s looking nice… (anything that makes the dg output more stable is Good in my book) … I am not sure (not checked thoroughly) but I think I saw one case in libgomp testing where it didn’t pick up the timeout, will poke some more at that if i see it again. thanks for the patch! Iain
Re: [testsuite] Mark gdc.dg/pr89042?.d as compile tests
On Tue, 29 Jan 2019 at 09:52, Rainer Orth wrote: > > I've seen the new gdc.dg/pr89042?.d tests FAIL in a parallel multilib > bootstrap on i386-pc-solaris2.11: > > +FAIL: gdc.dg/pr89042a.d -O0 (test for excess errors) > +UNRESOLVED: gdc.dg/pr89042a.d -O0 compilation failed to produce executable > +FAIL: gdc.dg/pr89042a.d -O0 -frelease (test for excess errors) > +UNRESOLVED: gdc.dg/pr89042a.d -O0 -frelease compilation failed to produce > executable > +FAIL: gdc.dg/pr89042a.d -O0 -frelease -g (test for excess errors) > +UNRESOLVED: gdc.dg/pr89042a.d -O0 -frelease -g compilation failed to > produce executable > +FAIL: gdc.dg/pr89042a.d -O0 -g (test for excess errors) > +UNRESOLVED: gdc.dg/pr89042a.d -O0 -g compilation failed to produce > executable > +FAIL: gdc.dg/pr89042a.d -O1 (test for excess errors) > +UNRESOLVED: gdc.dg/pr89042a.d -O1 compilation failed to produce executable > +FAIL: gdc.dg/pr89042a.d -O1 -frelease (test for excess errors) > [...] > +FAIL: gdc.dg/pr89042b.d -O0 (test for excess errors) > +UNRESOLVED: gdc.dg/pr89042b.d -O0 compilation failed to produce executable > +FAIL: gdc.dg/pr89042b.d -O0 -frelease (test for excess errors) > [...] > > Undefined first referenced > symbol in file > main/usr/lib/amd64/crt1.o > ld: fatal: symbol referencing errors > collect2: error: ld returned 1 exit status > compiler exited with status 1 > FAIL: gdc.dg/pr89042a.d -O0 (test for excess errors) > > Although they are obviouly compile tests, an attempt is made to link > them which fails. > > As an immediate fix, I've marked them as compile tests explicitly. > However, the underlying problem is that the gdc testsuite drivers > (gdc-test.exp in particular) don't save and restore dg-do-what-default > as they should. So depending on what driver is run before in a parallel > make check, it can happen that dg-do-what-default is set to link or run > from a previous test. > > Tested on i386-pc-solaris2.11, installed on mainline. > I'll have a look into that, this is OK in the meantime. Thanks -- Iain
Re: [PATCH] Handle timeout warnings in dg-extract-results
On Sat, 26 Jan 2019 at 17:43, Iain Sandoe wrote: > > Hi Christophe, > > > On 23 Jan 2019, at 13:16, Christophe Lyon > > wrote: > > > dg-extract-results currently moves lines like > > WARNING: program timed out > > at the end of each .exp section when it generates .sum files. > > > > This is because it sorts its output based on the 2nd field, which is > > normally the testname as in: > > FAIL: gcc.c-torture/execute/20020129-1.c -O2 -flto > > -fno-use-linker-plugin -flto-partition=none execution test > > > > As you can notice 'program' comes after > > gcc.c-torture/execute/20020129-1.c alphabetically, and generally after > > most (all?) GCC testnames. > > > > This is a bit of a pain when trying to handle transient test failures > > because you can no longer match such a WARNING line to its FAIL > > counterpart. > > > > The attached patch changes this behavior by replacing the line > > WARNING: program timed out > > with > > WARNING: gcc.c-torture/execute/20020129-1.c -O2 -flto > > -fno-use-linker-plugin -flto-partition=none execution test program > > timed out > > > > The effect is that this line will now appear immediately above the > > FAIL: gcc.c-torture/execute/20020129-1.c -O2 -flto > > -fno-use-linker-plugin -flto-partition=none execution test > > so that it's easier to match them. > > > > > > I'm not sure how much people depend on the .sum format, I also > > considered emitting > > WARNING: program timed out gcc.c-torture/execute/20020129-1.c -O2 > > -flto -fno-use-linker-plugin -flto-partition=none execution test > > > > I also restricted the patch to handling only 'program timed out' > > cases, to avoid breaking other things. > > > > I considered fixing this in Dejagnu, but it seemed more complicated, > > and would delay adoption in GCC anyway. > > > > What do people think about this? > > It seems a good idea (for those of us with targets prone to timeout issues). > I’m going to give it a try on Darwin. > Thanks for taking a look and confirming I'm not the only one having to face random timeouts :-) Did it work for you? > Iain > > > > Thanks, > > > > Christophe > > >
[testsuite] Mark gdc.dg/pr89042?.d as compile tests
I've seen the new gdc.dg/pr89042?.d tests FAIL in a parallel multilib
bootstrap on i386-pc-solaris2.11:
+FAIL: gdc.dg/pr89042a.d -O0 (test for excess errors)
+UNRESOLVED: gdc.dg/pr89042a.d -O0 compilation failed to produce executable
+FAIL: gdc.dg/pr89042a.d -O0 -frelease (test for excess errors)
+UNRESOLVED: gdc.dg/pr89042a.d -O0 -frelease compilation failed to produce
executable
+FAIL: gdc.dg/pr89042a.d -O0 -frelease -g (test for excess errors)
+UNRESOLVED: gdc.dg/pr89042a.d -O0 -frelease -g compilation failed to
produce executable
+FAIL: gdc.dg/pr89042a.d -O0 -g (test for excess errors)
+UNRESOLVED: gdc.dg/pr89042a.d -O0 -g compilation failed to produce
executable
+FAIL: gdc.dg/pr89042a.d -O1 (test for excess errors)
+UNRESOLVED: gdc.dg/pr89042a.d -O1 compilation failed to produce executable
+FAIL: gdc.dg/pr89042a.d -O1 -frelease (test for excess errors)
[...]
+FAIL: gdc.dg/pr89042b.d -O0 (test for excess errors)
+UNRESOLVED: gdc.dg/pr89042b.d -O0 compilation failed to produce executable
+FAIL: gdc.dg/pr89042b.d -O0 -frelease (test for excess errors)
[...]
Undefined first referenced
symbol in file
main/usr/lib/amd64/crt1.o
ld: fatal: symbol referencing errors
collect2: error: ld returned 1 exit status
compiler exited with status 1
FAIL: gdc.dg/pr89042a.d -O0 (test for excess errors)
Although they are obviouly compile tests, an attempt is made to link
them which fails.
As an immediate fix, I've marked them as compile tests explicitly.
However, the underlying problem is that the gdc testsuite drivers
(gdc-test.exp in particular) don't save and restore dg-do-what-default
as they should. So depending on what driver is run before in a parallel
make check, it can happen that dg-do-what-default is set to link or run
from a previous test.
Tested on i386-pc-solaris2.11, installed on mainline.
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
2019-01-28 Rainer Orth
* gdc.dg/pr89042a.d: Mark as compile test.
* gdc.dg/pr89042b.d: Likewise.
# HG changeset patch
# Parent 4b1525dfaa492568a95a76642dff2640b0c188df
Mark gdc.dg/pr89042?.d as compile tests
diff --git a/gcc/testsuite/gdc.dg/pr89042a.d b/gcc/testsuite/gdc.dg/pr89042a.d
--- a/gcc/testsuite/gdc.dg/pr89042a.d
+++ b/gcc/testsuite/gdc.dg/pr89042a.d
@@ -1,2 +1,3 @@
// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89042
+// { dg-do compile }
enum void[] a = void;
diff --git a/gcc/testsuite/gdc.dg/pr89042b.d b/gcc/testsuite/gdc.dg/pr89042b.d
--- a/gcc/testsuite/gdc.dg/pr89042b.d
+++ b/gcc/testsuite/gdc.dg/pr89042b.d
@@ -1,2 +1,3 @@
// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89042
+// { dg-do compile }
enum void[2] a = void;
Re: [backtrace] Avoid segfault
On 27-01-19 22:53, Ian Lance Taylor wrote: > On Sun, Jan 27, 2019 at 1:16 PM Tom de Vries wrote: >> >> On 25-01-19 18:15, Nathan Sidwell wrote: >>> On 1/25/19 5:28 AM, Tom de Vries wrote: This patch fixes it by passing "" instead of NULL, in the call to elf_add at line 3083 (for .gnu_debugaltlink), not the call to elf_add at line 3044 (for .gnu_debuglink) mentioned above. Nathan, does this fix the problem for you? If not, can you provide a reproducer, or give a hint on how one could be constructed? >>> >>> I still hit the problem, and am installing this as sufficiently obvious. >>> I'm on a fedora system debugging pr88995. The debuglink_name is >>> "../../.dwz/isl-0.16.1-7.fc29.x86_64" >>> >> >> I've managed to reproduce this segfault instance by adding a test-case >> that uses both build-id and dwz. >> >> OK for trunk? > >> +elf_for_test.c: elf.c >> + PWD=$$(pwd -P); \ >> + BUILD_ID_DIR="usr/lib/debug/.build-id/"; \ >> + SEARCH='#define SYSTEM_BUILD_ID_DIR'; \ >> + REPLACE="#define SYSTEM_BUILD_ID_DIR \"$$PWD/$$BUILD_ID_DIR\""; \ >> + $(SED) "s%^$$SEARCH.*\$$%$$REPLACE%" \ >> + $< \ >> + > $@ > > You need to use a temporary file, such as $@.tmp, for the final sed > command, followed by a mv to $@. Otherwise a failure in the sed will > leave what appears to be an up to date file. > Done. Also, I've factored out a script install-debuginfo-for-buildid.sh, hoping this will make things more readable/maintainable. > Honestly I'm not sure this patch is worth doing. It adds a lot of > complex mechanism in order to test a patch that is fairly obvious. Agreed, the patch is fairly obvious. But at the moment, there's no test-case that exercises the build-id support in libbacktrace. IMO, that alone would be a good reason to add this test-case. > While it's good practice to add a test for every change, it's not good > practice for the testsuite to become so complex that it becomes in > itself difficult to maintain. Understood. Please let me know if this is acceptable, or if I can do anything that would make this easier to maintain. Thanks, - Tom [libbacktrace] Add test-cases exercising build-id and dwz Add test-cases b2test_buildid and b3test_dwz_buildid. The last one triggers the segfault fixed by "[backtrace] Avoid segfault" ( r268275 ). 2019-01-27 Tom de Vries * install-debuginfo-for-buildid.sh.in: New script. * Makefile.am (check_PROGRAMS): Add b2test and b3test. (TESTS): Add b2test_buildid and b3test_dwz_buildid. * Makefile.in: Regenerate. * configure.ac (HAVE_ELF): Set with AM_CONDITIONAL. (READELF): Set with AC_CHECK_PROG. (install-debuginfo-for-buildid.sh): Generate with AC_CONFIG_FILES. * configure: Regenerate. * elf.c (SYSTEM_BUILD_ID_DIR): Factor out of ... (elf_open_debugfile_by_buildid): ... here. --- libbacktrace/Makefile.am | 50 ++ libbacktrace/Makefile.in | 193 ++- libbacktrace/configure | 60 ++- libbacktrace/configure.ac| 3 + libbacktrace/elf.c | 4 +- libbacktrace/install-debuginfo-for-buildid.sh.in | 27 6 files changed, 294 insertions(+), 43 deletions(-) diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am index 1c4ab07aa19..71a2ed478cc 100644 --- a/libbacktrace/Makefile.am +++ b/libbacktrace/Makefile.am @@ -108,6 +108,28 @@ libbacktrace_noformat_la_LIBADD = $(BACKTRACE_FILE) $(VIEW_FILE) $(ALLOC_FILE) libbacktrace_noformat_la_DEPENDENCIES = $(libbacktrace_noformat_la_LIBADD) +if HAVE_ELF +if HAVE_OBJCOPY_DEBUGLINK + +TEST_BUILD_ID_DIR=$(abs_builddir)/usr/lib/debug/.build-id/ + +check_LTLIBRARIES += libbacktrace_elf_for_test.la + +libbacktrace_elf_for_test_la_SOURCES = $(libbacktrace_la_SOURCES) +libbacktrace_elf_for_test_la_LIBADD = $(BACKTRACE_FILE) elf_for_test.lo \ + $(VIEW_FILE) $(ALLOC_FILE) + +elf_for_test.c: elf.c + SEARCH='^#define SYSTEM_BUILD_ID_DIR.*$$'; \ + REPLACE="#define SYSTEM_BUILD_ID_DIR \"$(TEST_BUILD_ID_DIR)\""; \ + $(SED) "s%$$SEARCH%$$REPLACE%" \ + $< \ + > $@.tmp + mv $@.tmp $@ + +endif HAVE_OBJCOPY_DEBUGLINK +endif HAVE_ELF + xcoff_%.c: xcoff.c SEARCH='#error "Unknown BACKTRACE_XCOFF_SIZE"'; \ REPLACE='#undef BACKTRACE_XCOFF_SIZE\ @@ -172,6 +194,28 @@ allocfail.sh: allocfail TESTS += allocfail.sh +if HAVE_ELF +if HAVE_OBJCOPY_DEBUGLINK + +b2test_SOURCES = $(btest_SOURCES) +b2test_CFLAGS = $(btest_CFLAGS) +b2test_LDFLAGS = -Wl,--build-id +b2test_LDADD = libbacktrace_elf_for_test.la + +check_PROGRAMS += b2test +TESTS += b2test_buildid + +b3test_SOURCES = $(btest_SOURCES) +b3test_CFLAGS = $(btest_CFLAGS) +b3test_LDFLAGS = -Wl,--build-id +b3test_LDADD = libbacktrace_elf_for_test.la + +check_PROGRAMS += b3test +TESTS += b3test_dwz_buildid + +endif HAVE_OBJCOPY_DEBUGLINK +endif HAVE_ELF + btest_SOURCES = btest.c testlib.c btest_CFLAGS = $(AM_CFLAGS) -g -O btest_LDADD = libbacktrace.la @@ -275,6 +319,12 @@
