public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
@ 2017-04-25 11:58 Martin Liška
  2017-04-25 12:08 ` Jakub Jelinek
  0 siblings, 1 reply; 24+ messages in thread
From: Martin Liška @ 2017-04-25 11:58 UTC (permalink / raw)
  To: GCC Patches; +Cc: Jason Merrill, Segher Boessenkool

[-- Attachment #1: Type: text/plain, Size: 356 bytes --]

Hello.

This is patch that was originally installed by Jason and later reverted due to PR70422.
In the later PR Richi suggested a fix for that and Segher verified that it helped him
to survive regression tests. That's reason why I'm resending that.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

[-- Attachment #2: 0001-Make-__FUNCTION__-a-mergeable-string-and-do-not-gene.patch --]
[-- Type: text/x-patch, Size: 8492 bytes --]

From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 20 Apr 2017 14:56:30 +0200
Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
 symbol entry.

gcc/cp/ChangeLog:

2017-04-20  Jason Merrill  <jason@redhat.com>
	    Martin Liska  <mliska@suse.cz>
	    Segher Boessenkool  <segher@kernel.crashing.org>

	PR c++/64266
	PR c++/70353
	PR bootstrap/70422
	Core issue 1962
	* decl.c (cp_fname_init): Decay the initializer to pointer.
	(cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
	* pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
	DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
	DECL_IGNORED_P.  Don't call cp_finish_decl.

gcc/testsuite/ChangeLog:

2017-04-20  Jason Merrill  <jason@redhat.com>
	    Segher Boessenkool  <segher@kernel.crashing.org>

	PR c++/64266
	PR c++/70353
	PR bootstrap/70422
	Core issue 1962
	* g++.dg/cpp0x/constexpr-__func__2.C: Add static assert test.
	* g++.dg/ext/fnname5.C: New test.
	* g++.old-deja/g++.ext/pretty4.C: Remove.
---
 gcc/cp/decl.c                                    | 20 ++++--
 gcc/cp/pt.c                                      | 26 +++++---
 gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C |  7 +-
 gcc/testsuite/g++.dg/ext/fnname5.C               | 33 +++++++++
 gcc/testsuite/g++.old-deja/g++.ext/pretty4.C     | 85 ------------------------
 5 files changed, 66 insertions(+), 105 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/ext/fnname5.C
 delete mode 100644 gcc/testsuite/g++.old-deja/g++.ext/pretty4.C

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 8e9a466afa0..c418fa4ce89 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -4348,13 +4348,15 @@ cp_fname_init (const char* name, tree *type_p)
   type = cp_build_qualified_type (char_type_node, TYPE_QUAL_CONST);
   type = build_cplus_array_type (type, domain);
 
-  *type_p = type;
+  *type_p = type_decays_to (type);
 
   if (init)
     TREE_TYPE (init) = type;
   else
     init = error_mark_node;
 
+  init = decay_conversion (init, tf_warning_or_error);
+
   return init;
 }
 
@@ -4380,12 +4382,21 @@ cp_make_fname_decl (location_t loc, tree id, int type_dep)
   /* As we're using pushdecl_with_scope, we must set the context.  */
   DECL_CONTEXT (decl) = current_function_decl;
 
-  TREE_STATIC (decl) = 1;
   TREE_READONLY (decl) = 1;
   DECL_ARTIFICIAL (decl) = 1;
+  DECL_IGNORED_P (decl) = 1;
+  DECL_DECLARED_CONSTEXPR_P (decl) = 1;
 
   TREE_USED (decl) = 1;
 
+  if (init)
+    {
+      SET_DECL_VALUE_EXPR (decl, init);
+      DECL_HAS_VALUE_EXPR_P (decl) = 1;
+      /* For decl_constant_var_p.  */
+      DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = 1;
+    }
+
   if (current_function_decl)
     {
       cp_binding_level *b = current_binding_level;
@@ -4394,13 +4405,12 @@ cp_make_fname_decl (location_t loc, tree id, int type_dep)
       while (b->level_chain->kind != sk_function_parms)
 	b = b->level_chain;
       pushdecl_with_scope (decl, b, /*is_friend=*/false);
-      cp_finish_decl (decl, init, /*init_const_expr_p=*/false, NULL_TREE,
-		      LOOKUP_ONLYCONVERTING);
+      add_decl_expr (decl);
     }
   else
     {
       DECL_THIS_STATIC (decl) = true;
-      pushdecl_top_level_and_finish (decl, init);
+      pushdecl_top_level_and_finish (decl, NULL_TREE);
     }
 
   return decl;
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index f8436b30b37..8d8cd0c5861 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -15808,21 +15808,25 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 		    DECL_CONTEXT (decl) = current_function_decl;
 		    cp_check_omp_declare_reduction (decl);
 		  }
+		else if (VAR_P (decl)
+			 && DECL_PRETTY_FUNCTION_P (decl))
+		  {
+		    /* For __PRETTY_FUNCTION__ we have to adjust the
+		       initializer.  */
+		    const char *const name
+		      = cxx_printable_name (current_function_decl, 2);
+		    init = cp_fname_init (name, &TREE_TYPE (decl));
+		    SET_DECL_VALUE_EXPR (decl, init);
+		    DECL_HAS_VALUE_EXPR_P (decl) = 1;
+		    DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = 1;
+		    maybe_push_decl (decl);
+		  }
 		else
 		  {
 		    int const_init = false;
 		    maybe_push_decl (decl);
-		    if (VAR_P (decl)
-			&& DECL_PRETTY_FUNCTION_P (decl))
-		      {
-			/* For __PRETTY_FUNCTION__ we have to adjust the
-			   initializer.  */
-			const char *const name
-			  = cxx_printable_name (current_function_decl, 2);
-			init = cp_fname_init (name, &TREE_TYPE (decl));
-		      }
-		    else
-		      init = tsubst_init (init, decl, args, complain, in_decl);
+
+		    init = tsubst_init (init, decl, args, complain, in_decl);
 
 		    if (VAR_P (decl))
 		      const_init = (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C
index e6782905423..673fb4f3a93 100644
--- a/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C
@@ -1,5 +1,5 @@
 // PR c++/70353
-// { dg-do link { target c++11 } }
+// { dg-do compile { target c++11 } }
 
 constexpr const char* ce ()
 {
@@ -8,6 +8,5 @@ constexpr const char* ce ()
 
 const char *c = ce();
 
-int main()
-{
-}
+#define SA(X) static_assert((X),#X)
+SA(ce()[0] == 'c');
diff --git a/gcc/testsuite/g++.dg/ext/fnname5.C b/gcc/testsuite/g++.dg/ext/fnname5.C
new file mode 100644
index 00000000000..04fa7d3f92d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/fnname5.C
@@ -0,0 +1,33 @@
+// PR c++/64266
+/* { dg-do compile } */
+
+extern "C" int printf (const char *, ...);
+
+struct A
+{
+  void foo(int i)
+  {
+    printf ("__FUNCTION__ = %s\n", __FUNCTION__);
+    printf ("__PRETTY_FUNCTION__ = %s\n", __PRETTY_FUNCTION__);
+  }
+
+  void foo()
+  {
+     printf ("__FUNCTION__ = %s\n", __FUNCTION__);
+  }
+};
+
+int
+main ()
+{
+  A a;
+  a.foo (0);
+  a.foo ();
+  return 0;
+}
+
+/* { dg-final { scan-assembler-not "_ZZN1A3fooEvE12__FUNCTION__" } } */
+/* { dg-final { scan-assembler-not "_ZZN1A3fooEiE12__FUNCTION__" } } */
+/* { dg-final { scan-assembler-not "_ZZN1A3fooEiE19__PRETTY_FUNCTION__" } } */
+/* { dg-final { scan-assembler ".(string|ascii)\\s+\"void A::foo\\(int\\)(.0)?\"" } } */
+/* { dg-final { scan-assembler ".(string|ascii)\\s+\"foo(.0)?\"" } } */
diff --git a/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C b/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C
deleted file mode 100644
index 9017d567132..00000000000
--- a/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C
+++ /dev/null
@@ -1,85 +0,0 @@
-// { dg-do run  }
-// Copyright (C) 2000 Free Software Foundation, Inc.
-// Contributed by Nathan Sidwell 3 Mar 2000 <nathan@codesourcery.com>
-
-// __PRETTY_FUNCTION__, __FUNCTION__ and __function__ should have the
-// type char const [X], where X is the right value for that particular function
-
-static void const *strings[4];
-static void const *tpls[4];
-static unsigned pos = 0;
-static int fail;
-static void const *ptr = 0;
-
-void unover (char const (*)[5]) {}
-void foo (char const (*)[5]) {}
-void foo (void *) {fail = 1;}
-void foo (void const *) {fail = 1;}
-void baz (char const (&)[5]) {}
-
-template<unsigned I> void PV (char const (&objRef)[I])
-{
-  strings[pos] = objRef;
-  tpls[pos] = __PRETTY_FUNCTION__;
-  pos++;
-}
-
-void fn ()
-{
-  PV (__FUNCTION__);
-  PV (__func__);
-  PV (__PRETTY_FUNCTION__);
-  PV ("wibble");
-}
-
-void baz ()
-{
-  ptr = __FUNCTION__;
-  // there should be no string const merging
-  if (ptr == "baz")
-    fail = 1;
-  // but all uses should be the same.
-  if (ptr != __FUNCTION__)
-    fail = 1;
-}
-int baz (int)
-{
-  return ptr == __FUNCTION__;
-}
-
-int main ()
-{
-  // make sure we actually emit the VAR_DECL when needed, and things have the
-  // expected type.
-  foo (&__FUNCTION__);
-  baz (__FUNCTION__);
-  unover (&__FUNCTION__);
-  if (fail)
-    return 1;
-  
-  // __FUNCTION__ should be unique across functions with the same base name
-  // (it's a local static, _not_ a string).
-  baz ();
-  if (fail)
-    return 1;
-  if (baz (1))
-    return 1;
-  fn ();
-  
-  // Check the names of fn. They should all be distinct strings (though two
-  // will have the same value).
-  if (strings[0] == strings[1])
-    return 1;
-  if (strings[0] == strings[2])
-    return 1;
-  if (strings[1] == strings[2])
-    return 1;
-
-  // check the names of the template functions so invoked
-  if (tpls[0] != tpls[1])
-    return 1;
-  if (tpls[0] == tpls[2])
-    return 1;
-  
-  return 0;
-}
-- 
2.12.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
  2017-04-25 11:58 [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry Martin Liška
@ 2017-04-25 12:08 ` Jakub Jelinek
  2017-04-26 11:48   ` Martin Liška
  0 siblings, 1 reply; 24+ messages in thread
From: Jakub Jelinek @ 2017-04-25 12:08 UTC (permalink / raw)
  To: Martin Liška, Pedro Alves, Jan Kratochvil
  Cc: GCC Patches, Jason Merrill, Segher Boessenkool

On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote:
> Hello.
> 
> This is patch that was originally installed by Jason and later reverted due to PR70422.
> In the later PR Richi suggested a fix for that and Segher verified that it helped him
> to survive regression tests. That's reason why I'm resending that.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> Ready to be installed?
> Martin

> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Thu, 20 Apr 2017 14:56:30 +0200
> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
>  symbol entry.
> 
> gcc/cp/ChangeLog:
> 
> 2017-04-20  Jason Merrill  <jason@redhat.com>
> 	    Martin Liska  <mliska@suse.cz>
> 	    Segher Boessenkool  <segher@kernel.crashing.org>
> 
> 	PR c++/64266
> 	PR c++/70353
> 	PR bootstrap/70422
> 	Core issue 1962
> 	* decl.c (cp_fname_init): Decay the initializer to pointer.
> 	(cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
> 	* pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
> 	DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
> 	DECL_IGNORED_P.  Don't call cp_finish_decl.

If we don't emit those into the debug info, will the debugger be
able to handle __FUNCTION__ etc. properly?
Admittedly, right now we emit it into debug info only if those decls
are actually used, say on:
const char * foo () { return __FUNCTION__; }
const char * bar () { return ""; }
we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger
has to have some handling of it anyway.  But while in functions
that don't refer to __FUNCTION__ it is always the debugger that needs
to synthetize those and thus they will be always pointer-equal,
if there are some uses of it and for other uses the debugger would
synthetize it, there is the possibility that the debugger synthetized
string will not be the same object as actually used in the function.

	Jakub

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
  2017-04-25 12:08 ` Jakub Jelinek
@ 2017-04-26 11:48   ` Martin Liška
  2017-05-01 19:13     ` Jason Merrill
  0 siblings, 1 reply; 24+ messages in thread
From: Martin Liška @ 2017-04-26 11:48 UTC (permalink / raw)
  To: Jakub Jelinek, Pedro Alves, Jan Kratochvil
  Cc: GCC Patches, Jason Merrill, Segher Boessenkool

On 04/25/2017 01:58 PM, Jakub Jelinek wrote:
> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote:
>> Hello.
>>
>> This is patch that was originally installed by Jason and later reverted due to PR70422.
>> In the later PR Richi suggested a fix for that and Segher verified that it helped him
>> to survive regression tests. That's reason why I'm resending that.
>>
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>
>> Ready to be installed?
>> Martin
> 
>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
>> From: marxin <mliska@suse.cz>
>> Date: Thu, 20 Apr 2017 14:56:30 +0200
>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
>>  symbol entry.
>>
>> gcc/cp/ChangeLog:
>>
>> 2017-04-20  Jason Merrill  <jason@redhat.com>
>> 	    Martin Liska  <mliska@suse.cz>
>> 	    Segher Boessenkool  <segher@kernel.crashing.org>
>>
>> 	PR c++/64266
>> 	PR c++/70353
>> 	PR bootstrap/70422
>> 	Core issue 1962
>> 	* decl.c (cp_fname_init): Decay the initializer to pointer.
>> 	(cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
>> 	* pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
>> 	DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
>> 	DECL_IGNORED_P.  Don't call cp_finish_decl.
> 
> If we don't emit those into the debug info, will the debugger be
> able to handle __FUNCTION__ etc. properly?

No, debugger with the patch can't handled these. Similar to how clang
behaves currently. Maybe it can be conditionally enabled with -g3, or -g?

> Admittedly, right now we emit it into debug info only if those decls
> are actually used, say on:
> const char * foo () { return __FUNCTION__; }
> const char * bar () { return ""; }
> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger
> has to have some handling of it anyway.  But while in functions
> that don't refer to __FUNCTION__ it is always the debugger that needs
> to synthetize those and thus they will be always pointer-equal,
> if there are some uses of it and for other uses the debugger would
> synthetize it, there is the possibility that the debugger synthetized
> string will not be the same object as actually used in the function.

You're right, currently one has to use a special function to be able to
print it in debugger. I believe we've already discussed that, according
to spec, the strings don't have to point to a same string.

Suggestions what we should do with the patch?
Thanks,
Martin

> 
> 	Jakub
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
  2017-04-26 11:48   ` Martin Liška
@ 2017-05-01 19:13     ` Jason Merrill
  2017-07-14  8:35       ` Martin Liška
  0 siblings, 1 reply; 24+ messages in thread
From: Jason Merrill @ 2017-05-01 19:13 UTC (permalink / raw)
  To: Martin Liška
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, GCC Patches,
	Segher Boessenkool, wilson

On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote:
> On 04/25/2017 01:58 PM, Jakub Jelinek wrote:
>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote:
>>> Hello.
>>>
>>> This is patch that was originally installed by Jason and later reverted due to PR70422.
>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him
>>> to survive regression tests. That's reason why I'm resending that.
>>>
>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>
>>> Ready to be installed?
>>> Martin
>>
>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
>>> From: marxin <mliska@suse.cz>
>>> Date: Thu, 20 Apr 2017 14:56:30 +0200
>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
>>>  symbol entry.
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 2017-04-20  Jason Merrill  <jason@redhat.com>
>>>          Martin Liska  <mliska@suse.cz>
>>>          Segher Boessenkool  <segher@kernel.crashing.org>
>>>
>>>      PR c++/64266
>>>      PR c++/70353
>>>      PR bootstrap/70422
>>>      Core issue 1962
>>>      * decl.c (cp_fname_init): Decay the initializer to pointer.
>>>      (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
>>>      * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
>>>      DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
>>>      DECL_IGNORED_P.  Don't call cp_finish_decl.
>>
>> If we don't emit those into the debug info, will the debugger be
>> able to handle __FUNCTION__ etc. properly?
>
> No, debugger with the patch can't handled these. Similar to how clang
> behaves currently. Maybe it can be conditionally enabled with -g3, or -g?
>
>> Admittedly, right now we emit it into debug info only if those decls
>> are actually used, say on:
>> const char * foo () { return __FUNCTION__; }
>> const char * bar () { return ""; }
>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger
>> has to have some handling of it anyway.  But while in functions
>> that don't refer to __FUNCTION__ it is always the debugger that needs
>> to synthetize those and thus they will be always pointer-equal,
>> if there are some uses of it and for other uses the debugger would
>> synthetize it, there is the possibility that the debugger synthetized
>> string will not be the same object as actually used in the function.
>
> You're right, currently one has to use a special function to be able to
> print it in debugger. I believe we've already discussed that, according
> to spec, the strings don't have to point to a same string.
>
> Suggestions what we should do with the patch?

We need to emit debug information for these variables.  From Jim's
description in 70422 it seems that the problem is that the reference
to the string from the debug information is breaking
function_mergeable_rodata_prefix, which relies on
current_function_decl.  It seems to me that its callers should pass
along their decl parameter so that f_m_r_p can use the decl's
DECL_CONTEXT rather than rely on current_function_decl being set
properly.

Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
  2017-05-01 19:13     ` Jason Merrill
@ 2017-07-14  8:35       ` Martin Liška
  2017-07-14 20:00         ` Jim Wilson
                           ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Martin Liška @ 2017-07-14  8:35 UTC (permalink / raw)
  To: Jason Merrill
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, GCC Patches,
	Segher Boessenkool, wilson

[-- Attachment #1: Type: text/plain, Size: 3500 bytes --]

On 05/01/2017 09:13 PM, Jason Merrill wrote:
> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote:
>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote:
>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote:
>>>> Hello.
>>>>
>>>> This is patch that was originally installed by Jason and later reverted due to PR70422.
>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him
>>>> to survive regression tests. That's reason why I'm resending that.
>>>>
>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>>
>>>> Ready to be installed?
>>>> Martin
>>>
>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
>>>> From: marxin <mliska@suse.cz>
>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200
>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
>>>>  symbol entry.
>>>>
>>>> gcc/cp/ChangeLog:
>>>>
>>>> 2017-04-20  Jason Merrill  <jason@redhat.com>
>>>>          Martin Liska  <mliska@suse.cz>
>>>>          Segher Boessenkool  <segher@kernel.crashing.org>
>>>>
>>>>      PR c++/64266
>>>>      PR c++/70353
>>>>      PR bootstrap/70422
>>>>      Core issue 1962
>>>>      * decl.c (cp_fname_init): Decay the initializer to pointer.
>>>>      (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
>>>>      * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
>>>>      DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
>>>>      DECL_IGNORED_P.  Don't call cp_finish_decl.
>>>
>>> If we don't emit those into the debug info, will the debugger be
>>> able to handle __FUNCTION__ etc. properly?
>>
>> No, debugger with the patch can't handled these. Similar to how clang
>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g?
>>
>>> Admittedly, right now we emit it into debug info only if those decls
>>> are actually used, say on:
>>> const char * foo () { return __FUNCTION__; }
>>> const char * bar () { return ""; }
>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger
>>> has to have some handling of it anyway.  But while in functions
>>> that don't refer to __FUNCTION__ it is always the debugger that needs
>>> to synthetize those and thus they will be always pointer-equal,
>>> if there are some uses of it and for other uses the debugger would
>>> synthetize it, there is the possibility that the debugger synthetized
>>> string will not be the same object as actually used in the function.
>>
>> You're right, currently one has to use a special function to be able to
>> print it in debugger. I believe we've already discussed that, according
>> to spec, the strings don't have to point to a same string.
>>
>> Suggestions what we should do with the patch?
> 
> We need to emit debug information for these variables.  From Jim's
> description in 70422 it seems that the problem is that the reference
> to the string from the debug information is breaking
> function_mergeable_rodata_prefix, which relies on
> current_function_decl.  It seems to me that its callers should pass
> along their decl parameter so that f_m_r_p can use the decl's
> DECL_CONTEXT rather than rely on current_function_decl being set
> properly> 
> Jason
> 

Ok, after some time I returned back to it. I followed your advises and
changed the function function_mergeable_rodata_prefix. Apart from a small
rebase was needed.

May I ask Jim to test the patch?
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Martin

[-- Attachment #2: 0001-Make-__FUNCTION__-a-mergeable-string-and-do-not-gene.patch --]
[-- Type: text/x-patch, Size: 9348 bytes --]

From 8a7a8b421d81c47cf686175f309f79c0b7a8ce76 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 20 Apr 2017 14:56:30 +0200
Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
 symbol entry.

gcc/cp/ChangeLog:

2017-04-20  Jason Merrill  <jason@redhat.com>
	    Martin Liska  <mliska@suse.cz>
	    Segher Boessenkool  <segher@kernel.crashing.org>

	PR c++/64266
	PR c++/70353
	PR bootstrap/70422
	Core issue 1962
	* decl.c (cp_fname_init): Decay the initializer to pointer.
	(cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
	* pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
	DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
	DECL_IGNORED_P.  Don't call cp_finish_decl.
	* decl.c (cp_make_fname_decl): Add only non error mark
	expression.

gcc/testsuite/ChangeLog:

2017-04-20  Jason Merrill  <jason@redhat.com>
	    Segher Boessenkool  <segher@kernel.crashing.org>

	PR c++/64266
	PR c++/70353
	PR bootstrap/70422
	Core issue 1962
	* g++.dg/cpp0x/constexpr-__func__2.C: Add static assert test.
	* g++.dg/ext/fnname5.C: New test.
	* g++.old-deja/g++.ext/pretty4.C: Remove.

gcc/ChangeLog:

2017-07-14  Martin Liska  <mliska@suse.cz>

	* varasm.c (function_mergeable_rodata_prefix): Use DECL_CONTEXT
	for function declarations.
---
 gcc/cp/decl.c                                    | 24 +++++--
 gcc/cp/pt.c                                      | 26 +++++---
 gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C |  7 +-
 gcc/testsuite/g++.dg/ext/fnname5.C               | 33 +++++++++
 gcc/testsuite/g++.old-deja/g++.ext/pretty4.C     | 85 ------------------------
 gcc/varasm.c                                     |  7 +-
 6 files changed, 76 insertions(+), 106 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/ext/fnname5.C
 delete mode 100644 gcc/testsuite/g++.old-deja/g++.ext/pretty4.C

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 5b8e6a22b01..3f27b72052a 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -4294,13 +4294,15 @@ cp_fname_init (const char* name, tree *type_p)
   type = cp_build_qualified_type (char_type_node, TYPE_QUAL_CONST);
   type = build_cplus_array_type (type, domain);
 
-  *type_p = type;
+  *type_p = type_decays_to (type);
 
   if (init)
     TREE_TYPE (init) = type;
   else
     init = error_mark_node;
 
+  init = decay_conversion (init, tf_warning_or_error);
+
   return init;
 }
 
@@ -4323,23 +4325,35 @@ cp_make_fname_decl (location_t loc, tree id, int type_dep)
   if (name)
     free (CONST_CAST (char *, name));
 
-  TREE_STATIC (decl) = 1;
+  /* As we're using pushdecl_with_scope, we must set the context.  */
+  DECL_CONTEXT (decl) = current_function_decl;
+
   TREE_READONLY (decl) = 1;
   DECL_ARTIFICIAL (decl) = 1;
+  DECL_IGNORED_P (decl) = 1;
+  DECL_DECLARED_CONSTEXPR_P (decl) = 1;
 
   TREE_USED (decl) = 1;
 
+  if (init)
+    {
+      SET_DECL_VALUE_EXPR (decl, init);
+      DECL_HAS_VALUE_EXPR_P (decl) = 1;
+      /* For decl_constant_var_p.  */
+      DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = 1;
+    }
+
   if (current_function_decl)
     {
       DECL_CONTEXT (decl) = current_function_decl;
       decl = pushdecl_outermost_localscope (decl);
-      cp_finish_decl (decl, init, /*init_const_expr_p=*/false, NULL_TREE,
-		      LOOKUP_ONLYCONVERTING);
+      if (decl != error_mark_node)
+	add_decl_expr (decl);
     }
   else
     {
       DECL_THIS_STATIC (decl) = true;
-      pushdecl_top_level_and_finish (decl, init);
+      pushdecl_top_level_and_finish (decl, NULL_TREE);
     }
 
   return decl;
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index bd02951cb85..f8e096b9b52 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -15862,21 +15862,25 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 		    DECL_CONTEXT (decl) = current_function_decl;
 		    cp_check_omp_declare_reduction (decl);
 		  }
+		else if (VAR_P (decl)
+			 && DECL_PRETTY_FUNCTION_P (decl))
+		  {
+		    /* For __PRETTY_FUNCTION__ we have to adjust the
+		       initializer.  */
+		    const char *const name
+		      = cxx_printable_name (current_function_decl, 2);
+		    init = cp_fname_init (name, &TREE_TYPE (decl));
+		    SET_DECL_VALUE_EXPR (decl, init);
+		    DECL_HAS_VALUE_EXPR_P (decl) = 1;
+		    DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = 1;
+		    maybe_push_decl (decl);
+		  }
 		else
 		  {
 		    int const_init = false;
 		    maybe_push_decl (decl);
-		    if (VAR_P (decl)
-			&& DECL_PRETTY_FUNCTION_P (decl))
-		      {
-			/* For __PRETTY_FUNCTION__ we have to adjust the
-			   initializer.  */
-			const char *const name
-			  = cxx_printable_name (current_function_decl, 2);
-			init = cp_fname_init (name, &TREE_TYPE (decl));
-		      }
-		    else
-		      init = tsubst_init (init, decl, args, complain, in_decl);
+
+		    init = tsubst_init (init, decl, args, complain, in_decl);
 
 		    if (VAR_P (decl))
 		      const_init = (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C
index e6782905423..673fb4f3a93 100644
--- a/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C
@@ -1,5 +1,5 @@
 // PR c++/70353
-// { dg-do link { target c++11 } }
+// { dg-do compile { target c++11 } }
 
 constexpr const char* ce ()
 {
@@ -8,6 +8,5 @@ constexpr const char* ce ()
 
 const char *c = ce();
 
-int main()
-{
-}
+#define SA(X) static_assert((X),#X)
+SA(ce()[0] == 'c');
diff --git a/gcc/testsuite/g++.dg/ext/fnname5.C b/gcc/testsuite/g++.dg/ext/fnname5.C
new file mode 100644
index 00000000000..04fa7d3f92d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/fnname5.C
@@ -0,0 +1,33 @@
+// PR c++/64266
+/* { dg-do compile } */
+
+extern "C" int printf (const char *, ...);
+
+struct A
+{
+  void foo(int i)
+  {
+    printf ("__FUNCTION__ = %s\n", __FUNCTION__);
+    printf ("__PRETTY_FUNCTION__ = %s\n", __PRETTY_FUNCTION__);
+  }
+
+  void foo()
+  {
+     printf ("__FUNCTION__ = %s\n", __FUNCTION__);
+  }
+};
+
+int
+main ()
+{
+  A a;
+  a.foo (0);
+  a.foo ();
+  return 0;
+}
+
+/* { dg-final { scan-assembler-not "_ZZN1A3fooEvE12__FUNCTION__" } } */
+/* { dg-final { scan-assembler-not "_ZZN1A3fooEiE12__FUNCTION__" } } */
+/* { dg-final { scan-assembler-not "_ZZN1A3fooEiE19__PRETTY_FUNCTION__" } } */
+/* { dg-final { scan-assembler ".(string|ascii)\\s+\"void A::foo\\(int\\)(.0)?\"" } } */
+/* { dg-final { scan-assembler ".(string|ascii)\\s+\"foo(.0)?\"" } } */
diff --git a/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C b/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C
deleted file mode 100644
index 9017d567132..00000000000
--- a/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C
+++ /dev/null
@@ -1,85 +0,0 @@
-// { dg-do run  }
-// Copyright (C) 2000 Free Software Foundation, Inc.
-// Contributed by Nathan Sidwell 3 Mar 2000 <nathan@codesourcery.com>
-
-// __PRETTY_FUNCTION__, __FUNCTION__ and __function__ should have the
-// type char const [X], where X is the right value for that particular function
-
-static void const *strings[4];
-static void const *tpls[4];
-static unsigned pos = 0;
-static int fail;
-static void const *ptr = 0;
-
-void unover (char const (*)[5]) {}
-void foo (char const (*)[5]) {}
-void foo (void *) {fail = 1;}
-void foo (void const *) {fail = 1;}
-void baz (char const (&)[5]) {}
-
-template<unsigned I> void PV (char const (&objRef)[I])
-{
-  strings[pos] = objRef;
-  tpls[pos] = __PRETTY_FUNCTION__;
-  pos++;
-}
-
-void fn ()
-{
-  PV (__FUNCTION__);
-  PV (__func__);
-  PV (__PRETTY_FUNCTION__);
-  PV ("wibble");
-}
-
-void baz ()
-{
-  ptr = __FUNCTION__;
-  // there should be no string const merging
-  if (ptr == "baz")
-    fail = 1;
-  // but all uses should be the same.
-  if (ptr != __FUNCTION__)
-    fail = 1;
-}
-int baz (int)
-{
-  return ptr == __FUNCTION__;
-}
-
-int main ()
-{
-  // make sure we actually emit the VAR_DECL when needed, and things have the
-  // expected type.
-  foo (&__FUNCTION__);
-  baz (__FUNCTION__);
-  unover (&__FUNCTION__);
-  if (fail)
-    return 1;
-  
-  // __FUNCTION__ should be unique across functions with the same base name
-  // (it's a local static, _not_ a string).
-  baz ();
-  if (fail)
-    return 1;
-  if (baz (1))
-    return 1;
-  fn ();
-  
-  // Check the names of fn. They should all be distinct strings (though two
-  // will have the same value).
-  if (strings[0] == strings[1])
-    return 1;
-  if (strings[0] == strings[2])
-    return 1;
-  if (strings[1] == strings[2])
-    return 1;
-
-  // check the names of the template functions so invoked
-  if (tpls[0] != tpls[1])
-    return 1;
-  if (tpls[0] == tpls[2])
-    return 1;
-  
-  return 0;
-}
diff --git a/gcc/varasm.c b/gcc/varasm.c
index fbaebc1b5c0..0b56898d8f3 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -760,7 +760,12 @@ default_no_function_rodata_section (tree decl ATTRIBUTE_UNUSED)
 static const char *
 function_mergeable_rodata_prefix (void)
 {
-  section *s = targetm.asm_out.function_rodata_section (current_function_decl);
+  tree decl = current_function_decl;
+  if (decl && DECL_CONTEXT (decl)
+      && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL)
+    decl = DECL_CONTEXT (decl);
+
+  section *s = targetm.asm_out.function_rodata_section (decl);
   if (SECTION_STYLE (s) == SECTION_NAMED)
     return s->named.name;
   else
-- 
2.13.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
  2017-07-14  8:35       ` Martin Liška
@ 2017-07-14 20:00         ` Jim Wilson
  2017-07-15  6:11           ` Jim Wilson
  2017-07-27 12:56         ` Martin Liška
  2017-08-10 20:23         ` Jason Merrill
  2 siblings, 1 reply; 24+ messages in thread
From: Jim Wilson @ 2017-07-14 20:00 UTC (permalink / raw)
  To: Martin Liška
  Cc: Jason Merrill, Jakub Jelinek, Pedro Alves, Jan Kratochvil,
	GCC Patches, Segher Boessenkool, wilson

On Fri, Jul 14, 2017 at 1:35 AM, Martin Liška <mliska@suse.cz> wrote:
> May I ask Jim to test the patch?
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

I started an aarch64 bootstrap to test.  My fast machine is busy with
work tasks, so I have to use a slower machine, and hence this will
take some time.

Jim

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
  2017-07-14 20:00         ` Jim Wilson
@ 2017-07-15  6:11           ` Jim Wilson
  0 siblings, 0 replies; 24+ messages in thread
From: Jim Wilson @ 2017-07-15  6:11 UTC (permalink / raw)
  To: Martin Liška
  Cc: Jason Merrill, Jakub Jelinek, Pedro Alves, Jan Kratochvil,
	GCC Patches, Segher Boessenkool, wilson

On Fri, Jul 14, 2017 at 12:59 PM, Jim Wilson <jim.wilson@linaro.org> wrote:
> On Fri, Jul 14, 2017 at 1:35 AM, Martin Liška <mliska@suse.cz> wrote:
>> May I ask Jim to test the patch?
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>
> I started an aarch64 bootstrap to test.  My fast machine is busy with
> work tasks, so I have to use a slower machine, and hence this will
> take some time.

The aarch64-linux bootstrap with the patch succeeds.  I also did a
make check, and see no regressions.

Jim

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
  2017-07-14  8:35       ` Martin Liška
  2017-07-14 20:00         ` Jim Wilson
@ 2017-07-27 12:56         ` Martin Liška
  2017-08-10  8:57           ` Martin Liška
  2017-08-10 20:23         ` Jason Merrill
  2 siblings, 1 reply; 24+ messages in thread
From: Martin Liška @ 2017-07-27 12:56 UTC (permalink / raw)
  To: Jason Merrill
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, GCC Patches,
	Segher Boessenkool, wilson

PING^1

On 07/14/2017 10:35 AM, Martin Liška wrote:
> On 05/01/2017 09:13 PM, Jason Merrill wrote:
>> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote:
>>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote:
>>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote:
>>>>> Hello.
>>>>>
>>>>> This is patch that was originally installed by Jason and later reverted due to PR70422.
>>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him
>>>>> to survive regression tests. That's reason why I'm resending that.
>>>>>
>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>>>
>>>>> Ready to be installed?
>>>>> Martin
>>>>
>>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
>>>>> From: marxin <mliska@suse.cz>
>>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200
>>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
>>>>>  symbol entry.
>>>>>
>>>>> gcc/cp/ChangeLog:
>>>>>
>>>>> 2017-04-20  Jason Merrill  <jason@redhat.com>
>>>>>          Martin Liska  <mliska@suse.cz>
>>>>>          Segher Boessenkool  <segher@kernel.crashing.org>
>>>>>
>>>>>      PR c++/64266
>>>>>      PR c++/70353
>>>>>      PR bootstrap/70422
>>>>>      Core issue 1962
>>>>>      * decl.c (cp_fname_init): Decay the initializer to pointer.
>>>>>      (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
>>>>>      * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
>>>>>      DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
>>>>>      DECL_IGNORED_P.  Don't call cp_finish_decl.
>>>>
>>>> If we don't emit those into the debug info, will the debugger be
>>>> able to handle __FUNCTION__ etc. properly?
>>>
>>> No, debugger with the patch can't handled these. Similar to how clang
>>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g?
>>>
>>>> Admittedly, right now we emit it into debug info only if those decls
>>>> are actually used, say on:
>>>> const char * foo () { return __FUNCTION__; }
>>>> const char * bar () { return ""; }
>>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger
>>>> has to have some handling of it anyway.  But while in functions
>>>> that don't refer to __FUNCTION__ it is always the debugger that needs
>>>> to synthetize those and thus they will be always pointer-equal,
>>>> if there are some uses of it and for other uses the debugger would
>>>> synthetize it, there is the possibility that the debugger synthetized
>>>> string will not be the same object as actually used in the function.
>>>
>>> You're right, currently one has to use a special function to be able to
>>> print it in debugger. I believe we've already discussed that, according
>>> to spec, the strings don't have to point to a same string.
>>>
>>> Suggestions what we should do with the patch?
>>
>> We need to emit debug information for these variables.  From Jim's
>> description in 70422 it seems that the problem is that the reference
>> to the string from the debug information is breaking
>> function_mergeable_rodata_prefix, which relies on
>> current_function_decl.  It seems to me that its callers should pass
>> along their decl parameter so that f_m_r_p can use the decl's
>> DECL_CONTEXT rather than rely on current_function_decl being set
>> properly> 
>> Jason
>>
> 
> Ok, after some time I returned back to it. I followed your advises and
> changed the function function_mergeable_rodata_prefix. Apart from a small
> rebase was needed.
> 
> May I ask Jim to test the patch?
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> Martin
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
  2017-07-27 12:56         ` Martin Liška
@ 2017-08-10  8:57           ` Martin Liška
  0 siblings, 0 replies; 24+ messages in thread
From: Martin Liška @ 2017-08-10  8:57 UTC (permalink / raw)
  To: Jason Merrill
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, GCC Patches,
	Segher Boessenkool, wilson

PING^2

On 07/27/2017 02:56 PM, Martin Liška wrote:
> PING^1
> 
> On 07/14/2017 10:35 AM, Martin Liška wrote:
>> On 05/01/2017 09:13 PM, Jason Merrill wrote:
>>> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote:
>>>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote:
>>>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote:
>>>>>> Hello.
>>>>>>
>>>>>> This is patch that was originally installed by Jason and later reverted due to PR70422.
>>>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him
>>>>>> to survive regression tests. That's reason why I'm resending that.
>>>>>>
>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>>>>
>>>>>> Ready to be installed?
>>>>>> Martin
>>>>>
>>>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
>>>>>> From: marxin <mliska@suse.cz>
>>>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200
>>>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
>>>>>>  symbol entry.
>>>>>>
>>>>>> gcc/cp/ChangeLog:
>>>>>>
>>>>>> 2017-04-20  Jason Merrill  <jason@redhat.com>
>>>>>>          Martin Liska  <mliska@suse.cz>
>>>>>>          Segher Boessenkool  <segher@kernel.crashing.org>
>>>>>>
>>>>>>      PR c++/64266
>>>>>>      PR c++/70353
>>>>>>      PR bootstrap/70422
>>>>>>      Core issue 1962
>>>>>>      * decl.c (cp_fname_init): Decay the initializer to pointer.
>>>>>>      (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
>>>>>>      * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
>>>>>>      DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
>>>>>>      DECL_IGNORED_P.  Don't call cp_finish_decl.
>>>>>
>>>>> If we don't emit those into the debug info, will the debugger be
>>>>> able to handle __FUNCTION__ etc. properly?
>>>>
>>>> No, debugger with the patch can't handled these. Similar to how clang
>>>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g?
>>>>
>>>>> Admittedly, right now we emit it into debug info only if those decls
>>>>> are actually used, say on:
>>>>> const char * foo () { return __FUNCTION__; }
>>>>> const char * bar () { return ""; }
>>>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger
>>>>> has to have some handling of it anyway.  But while in functions
>>>>> that don't refer to __FUNCTION__ it is always the debugger that needs
>>>>> to synthetize those and thus they will be always pointer-equal,
>>>>> if there are some uses of it and for other uses the debugger would
>>>>> synthetize it, there is the possibility that the debugger synthetized
>>>>> string will not be the same object as actually used in the function.
>>>>
>>>> You're right, currently one has to use a special function to be able to
>>>> print it in debugger. I believe we've already discussed that, according
>>>> to spec, the strings don't have to point to a same string.
>>>>
>>>> Suggestions what we should do with the patch?
>>>
>>> We need to emit debug information for these variables.  From Jim's
>>> description in 70422 it seems that the problem is that the reference
>>> to the string from the debug information is breaking
>>> function_mergeable_rodata_prefix, which relies on
>>> current_function_decl.  It seems to me that its callers should pass
>>> along their decl parameter so that f_m_r_p can use the decl's
>>> DECL_CONTEXT rather than rely on current_function_decl being set
>>> properly> 
>>> Jason
>>>
>>
>> Ok, after some time I returned back to it. I followed your advises and
>> changed the function function_mergeable_rodata_prefix. Apart from a small
>> rebase was needed.
>>
>> May I ask Jim to test the patch?
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>
>> Martin
>>
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
  2017-07-14  8:35       ` Martin Liška
  2017-07-14 20:00         ` Jim Wilson
  2017-07-27 12:56         ` Martin Liška
@ 2017-08-10 20:23         ` Jason Merrill
  2017-09-14  9:22           ` Martin Liška
  2 siblings, 1 reply; 24+ messages in thread
From: Jason Merrill @ 2017-08-10 20:23 UTC (permalink / raw)
  To: Martin Liška
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, GCC Patches,
	Segher Boessenkool, wilson

On 07/14/2017 01:35 AM, Martin Liška wrote:
> On 05/01/2017 09:13 PM, Jason Merrill wrote:
>> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote:
>>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote:
>>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote:
>>>>> Hello.
>>>>>
>>>>> This is patch that was originally installed by Jason and later reverted due to PR70422.
>>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him
>>>>> to survive regression tests. That's reason why I'm resending that.
>>>>>
>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>>>
>>>>> Ready to be installed?
>>>>> Martin
>>>>
>>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
>>>>> From: marxin <mliska@suse.cz>
>>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200
>>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
>>>>>   symbol entry.
>>>>>
>>>>> gcc/cp/ChangeLog:
>>>>>
>>>>> 2017-04-20  Jason Merrill  <jason@redhat.com>
>>>>>           Martin Liska  <mliska@suse.cz>
>>>>>           Segher Boessenkool  <segher@kernel.crashing.org>
>>>>>
>>>>>       PR c++/64266
>>>>>       PR c++/70353
>>>>>       PR bootstrap/70422
>>>>>       Core issue 1962
>>>>>       * decl.c (cp_fname_init): Decay the initializer to pointer.
>>>>>       (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
>>>>>       * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
>>>>>       DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
>>>>>       DECL_IGNORED_P.  Don't call cp_finish_decl.
>>>>
>>>> If we don't emit those into the debug info, will the debugger be
>>>> able to handle __FUNCTION__ etc. properly?
>>>
>>> No, debugger with the patch can't handled these. Similar to how clang
>>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g?
>>>
>>>> Admittedly, right now we emit it into debug info only if those decls
>>>> are actually used, say on:
>>>> const char * foo () { return __FUNCTION__; }
>>>> const char * bar () { return ""; }
>>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger
>>>> has to have some handling of it anyway.  But while in functions
>>>> that don't refer to __FUNCTION__ it is always the debugger that needs
>>>> to synthetize those and thus they will be always pointer-equal,
>>>> if there are some uses of it and for other uses the debugger would
>>>> synthetize it, there is the possibility that the debugger synthetized
>>>> string will not be the same object as actually used in the function.
>>>
>>> You're right, currently one has to use a special function to be able to
>>> print it in debugger. I believe we've already discussed that, according
>>> to spec, the strings don't have to point to a same string.
>>>
>>> Suggestions what we should do with the patch?
>>
>> We need to emit debug information for these variables.  From Jim's
>> description in 70422 it seems that the problem is that the reference
>> to the string from the debug information is breaking
>> function_mergeable_rodata_prefix, which relies on
>> current_function_decl.  It seems to me that its callers should pass
>> along their decl parameter so that f_m_r_p can use the decl's
>> DECL_CONTEXT rather than rely on current_function_decl being set
>> properly>
>> Jason
>>
> 
> Ok, after some time I returned back to it. I followed your advises and
> changed the function function_mergeable_rodata_prefix. Apart from a small
> rebase was needed.
> 
> May I ask Jim to test the patch?
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

> +  DECL_IGNORED_P (decl) = 1;

As I said before, we do need to emit debug information for these 
variables, so this is wrong.

> -  section *s = targetm.asm_out.function_rodata_section (current_function_decl);
> +  tree decl = current_function_decl;
> +  if (decl && DECL_CONTEXT (decl)
> +      && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL)
> +    decl = DECL_CONTEXT (decl);

I don't see how this would help; it still relies on 
current_function_decl being set correctly, which was the problem we were 
running into before.

Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
  2017-08-10 20:23         ` Jason Merrill
@ 2017-09-14  9:22           ` Martin Liška
  2017-10-24 20:26             ` Jason Merrill
  0 siblings, 1 reply; 24+ messages in thread
From: Martin Liška @ 2017-09-14  9:22 UTC (permalink / raw)
  To: Jason Merrill
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, GCC Patches,
	Segher Boessenkool, wilson

On 08/10/2017 09:43 PM, Jason Merrill wrote:
> On 07/14/2017 01:35 AM, Martin Liška wrote:
>> On 05/01/2017 09:13 PM, Jason Merrill wrote:
>>> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote:
>>>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote:
>>>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote:
>>>>>> Hello.
>>>>>>
>>>>>> This is patch that was originally installed by Jason and later reverted due to PR70422.
>>>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him
>>>>>> to survive regression tests. That's reason why I'm resending that.
>>>>>>
>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>>>>
>>>>>> Ready to be installed?
>>>>>> Martin
>>>>>
>>>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
>>>>>> From: marxin <mliska@suse.cz>
>>>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200
>>>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
>>>>>>   symbol entry.
>>>>>>
>>>>>> gcc/cp/ChangeLog:
>>>>>>
>>>>>> 2017-04-20  Jason Merrill  <jason@redhat.com>
>>>>>>           Martin Liska  <mliska@suse.cz>
>>>>>>           Segher Boessenkool  <segher@kernel.crashing.org>
>>>>>>
>>>>>>       PR c++/64266
>>>>>>       PR c++/70353
>>>>>>       PR bootstrap/70422
>>>>>>       Core issue 1962
>>>>>>       * decl.c (cp_fname_init): Decay the initializer to pointer.
>>>>>>       (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
>>>>>>       * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
>>>>>>       DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
>>>>>>       DECL_IGNORED_P.  Don't call cp_finish_decl.
>>>>>
>>>>> If we don't emit those into the debug info, will the debugger be
>>>>> able to handle __FUNCTION__ etc. properly?
>>>>
>>>> No, debugger with the patch can't handled these. Similar to how clang
>>>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g?
>>>>
>>>>> Admittedly, right now we emit it into debug info only if those decls
>>>>> are actually used, say on:
>>>>> const char * foo () { return __FUNCTION__; }
>>>>> const char * bar () { return ""; }
>>>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger
>>>>> has to have some handling of it anyway.  But while in functions
>>>>> that don't refer to __FUNCTION__ it is always the debugger that needs
>>>>> to synthetize those and thus they will be always pointer-equal,
>>>>> if there are some uses of it and for other uses the debugger would
>>>>> synthetize it, there is the possibility that the debugger synthetized
>>>>> string will not be the same object as actually used in the function.
>>>>
>>>> You're right, currently one has to use a special function to be able to
>>>> print it in debugger. I believe we've already discussed that, according
>>>> to spec, the strings don't have to point to a same string.
>>>>
>>>> Suggestions what we should do with the patch?
>>>
>>> We need to emit debug information for these variables.  From Jim's
>>> description in 70422 it seems that the problem is that the reference
>>> to the string from the debug information is breaking
>>> function_mergeable_rodata_prefix, which relies on
>>> current_function_decl.  It seems to me that its callers should pass
>>> along their decl parameter so that f_m_r_p can use the decl's
>>> DECL_CONTEXT rather than rely on current_function_decl being set
>>> properly>
>>> Jason
>>>
>>
>> Ok, after some time I returned back to it. I followed your advises and
>> changed the function function_mergeable_rodata_prefix. Apart from a small
>> rebase was needed.
>>
>> May I ask Jim to test the patch?
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
>> +  DECL_IGNORED_P (decl) = 1;
> 
> As I said before, we do need to emit debug information for these variables, so this is wrong.

Hello.

Sorry for overlooking that.

> 
>> -  section *s = targetm.asm_out.function_rodata_section (current_function_decl);
>> +  tree decl = current_function_decl;
>> +  if (decl && DECL_CONTEXT (decl)
>> +      && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL)
>> +    decl = DECL_CONTEXT (decl);
> 
> I don't see how this would help; it still relies on current_function_decl being set correctly, which was the problem we were running into before.

I see, that's what I wanted to discuss on Cauldron with you, but eventually I did not find time.
Well problem that I see is that we want to make decision based on DECL_CONTEXT(fname_decl), but mergeable_string_section
is called with STRING_CST (which is VALUE_EXPR of created fname_decl):

#0  mergeable_string_section (decl=0x2aaaac2bf1a0, align=64, flags=0) at ../../gcc/varasm.c:796
#1  0x0000000001594ce3 in default_elf_select_section (decl=0x2aaaac2bf1a0, reloc=0, align=64) at ../../gcc/varasm.c:6641
#2  0x000000000158b649 in get_constant_section (exp=0x2aaaac2bf1a0, align=64) at ../../gcc/varasm.c:3284
#3  0x000000000158bb31 in build_constant_desc (exp=0x2aaaac2bf1a0) at ../../gcc/varasm.c:3352
#4  0x000000000158bebc in output_constant_def (exp=0x2aaaac2bf1a0, defer=0) at ../../gcc/varasm.c:3415
#5  0x0000000000d68264 in expand_expr_constant (exp=0x2aaaac2bf1a0, defer=0, modifier=EXPAND_NORMAL) at ../../gcc/expr.c:7701
#6  0x0000000000d682e2 in expand_expr_addr_expr_1 (exp=0x2aaaac2bf1a0, target=0x0, tmode=..., modifier=EXPAND_NORMAL, as=0 '\000') at ../../gcc/expr.c:7728
#7  0x0000000000d690c7 in expand_expr_addr_expr (exp=0x2aaaac2bff00, target=0x0, tmode=E_DImode, modifier=EXPAND_NORMAL) at ../../gcc/expr.c:7919
#8  0x0000000000d76d42 in expand_expr_real_1 (exp=0x2aaaac2bff00, target=0x0, tmode=E_VOIDmode, modifier=EXPAND_NORMAL, alt_rtl=0x0, inner_reference_p=false) at ../../gcc/expr.c:11084
#9  0x0000000000d69543 in expand_expr_real (exp=0x2aaaac2bff00, target=0x0, tmode=E_VOIDmode, modifier=EXPAND_NORMAL, alt_rtl=0x0, inner_reference_p=false) at ../../gcc/expr.c:8087
#10 0x0000000000be75fe in expand_normal (exp=0x2aaaac2bff00) at ../../gcc/expr.h:282
#11 0x0000000000be9626 in precompute_register_parameters (num_actuals=2, args=0x26c1550, reg_parm_seen=0x7fffffffc73c) at ../../gcc/calls.c:958
#12 0x0000000000bf2770 in expand_call (exp=0x2aaaac2d6040, target=0x2aaaac0a1480, ignore=1) at ../../gcc/calls.c:3677
#13 0x0000000000bd667d in expand_builtin (exp=0x2aaaac2d6040, target=0x2aaaac0a1480, subtarget=0x0, mode=E_VOIDmode, ignore=1) at ../../gcc/builtins.c:7591
#14 0x0000000000d75c07 in expand_expr_real_1 (exp=0x2aaaac2d6040, target=0x0, tmode=E_VOIDmode, modifier=EXPAND_NORMAL, alt_rtl=0x0, inner_reference_p=false) at ../../gcc/expr.c:10858
#15 0x0000000000d69543 in expand_expr_real (exp=0x2aaaac2d6040, target=0x2aaaac0a1480, tmode=E_VOIDmode, modifier=EXPAND_NORMAL, alt_rtl=0x0, inner_reference_p=false) at ../../gcc/expr.c:8087
#16 0x0000000000c02f4f in expand_expr (exp=0x2aaaac2d6040, target=0x2aaaac0a1480, mode=E_VOIDmode, modifier=EXPAND_NORMAL) at ../../gcc/expr.h:276
#17 0x0000000000c0b144 in expand_call_stmt (stmt=0x2aaaac2d02d0) at ../../gcc/cfgexpand.c:2666
#18 0x0000000000c0e3a6 in expand_gimple_stmt_1 (stmt=0x2aaaac2d02d0) at ../../gcc/cfgexpand.c:3585
#19 0x0000000000c0ea9c in expand_gimple_stmt (stmt=0x2aaaac2d02d0) at ../../gcc/cfgexpand.c:3751
#20 0x0000000000c1665e in expand_gimple_basic_block (bb=0x2aaaac0be2d8, disable_tail_calls=false) at ../../gcc/cfgexpand.c:5750

(gdb) p debug_tree(decl)
 <string_cst 0x2aaaac2bf1a0
    type <array_type 0x2aaaac2c4348
        type <integer_type 0x2aaaac0c15e8 char readonly unsigned string-flag type_6 QI
            size <integer_cst 0x2aaaac099d80 constant 8>
            unit-size <integer_cst 0x2aaaac099d98 constant 1>
            align:8 warn_if_not_align:0 symtab:-1408509840 alias-set -1 canonical-type 0x2aaaac0c15e8 precision:8 min <integer_cst 0x2aaaac099de0 0> max <integer_cst 0x2aaaac099dc8 255>
            pointer_to_this <pointer_type 0x2aaaac0c1690>>
        SI
        size <integer_cst 0x2aaaac099ed0 constant 32>
        unit-size <integer_cst 0x2aaaac099ee8 constant 4>
        align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x2aaaac2c4348
        domain <integer_type 0x2aaaac2c41f8 type <integer_type 0x2aaaac0b0000 sizetype>
            type_6 DI
            size <integer_cst 0x2aaaac099c90 constant 64>
            unit-size <integer_cst 0x2aaaac099ca8 constant 8>
            align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x2aaaac2c41f8 precision:64 min <integer_cst 0x2aaaac099cc0 0> max <integer_cst 0x2aaaac2bd888 3>>
        pointer_to_this <pointer_type 0x2aaaac2c43f0>>
    constant "foo\000">

And idea how to resolve this?

Thank you,
Martin

> 
> Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
  2017-09-14  9:22           ` Martin Liška
@ 2017-10-24 20:26             ` Jason Merrill
  2018-05-21 13:49               ` Martin Liška
  0 siblings, 1 reply; 24+ messages in thread
From: Jason Merrill @ 2017-10-24 20:26 UTC (permalink / raw)
  To: Martin Liška
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, GCC Patches,
	Segher Boessenkool, wilson

On Thu, Sep 14, 2017 at 5:22 AM, Martin Liška <mliska@suse.cz> wrote:
> On 08/10/2017 09:43 PM, Jason Merrill wrote:
>> On 07/14/2017 01:35 AM, Martin Liška wrote:
>>> On 05/01/2017 09:13 PM, Jason Merrill wrote:
>>>> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote:
>>>>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote:
>>>>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote:
>>>>>>> Hello.
>>>>>>>
>>>>>>> This is patch that was originally installed by Jason and later reverted due to PR70422.
>>>>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him
>>>>>>> to survive regression tests. That's reason why I'm resending that.
>>>>>>>
>>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>>>>>
>>>>>>> Ready to be installed?
>>>>>>> Martin
>>>>>>
>>>>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
>>>>>>> From: marxin <mliska@suse.cz>
>>>>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200
>>>>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
>>>>>>>   symbol entry.
>>>>>>>
>>>>>>> gcc/cp/ChangeLog:
>>>>>>>
>>>>>>> 2017-04-20  Jason Merrill  <jason@redhat.com>
>>>>>>>           Martin Liska  <mliska@suse.cz>
>>>>>>>           Segher Boessenkool  <segher@kernel.crashing.org>
>>>>>>>
>>>>>>>       PR c++/64266
>>>>>>>       PR c++/70353
>>>>>>>       PR bootstrap/70422
>>>>>>>       Core issue 1962
>>>>>>>       * decl.c (cp_fname_init): Decay the initializer to pointer.
>>>>>>>       (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
>>>>>>>       * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
>>>>>>>       DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
>>>>>>>       DECL_IGNORED_P.  Don't call cp_finish_decl.
>>>>>>
>>>>>> If we don't emit those into the debug info, will the debugger be
>>>>>> able to handle __FUNCTION__ etc. properly?
>>>>>
>>>>> No, debugger with the patch can't handled these. Similar to how clang
>>>>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g?
>>>>>
>>>>>> Admittedly, right now we emit it into debug info only if those decls
>>>>>> are actually used, say on:
>>>>>> const char * foo () { return __FUNCTION__; }
>>>>>> const char * bar () { return ""; }
>>>>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger
>>>>>> has to have some handling of it anyway.  But while in functions
>>>>>> that don't refer to __FUNCTION__ it is always the debugger that needs
>>>>>> to synthetize those and thus they will be always pointer-equal,
>>>>>> if there are some uses of it and for other uses the debugger would
>>>>>> synthetize it, there is the possibility that the debugger synthetized
>>>>>> string will not be the same object as actually used in the function.
>>>>>
>>>>> You're right, currently one has to use a special function to be able to
>>>>> print it in debugger. I believe we've already discussed that, according
>>>>> to spec, the strings don't have to point to a same string.
>>>>>
>>>>> Suggestions what we should do with the patch?
>>>>
>>>> We need to emit debug information for these variables.  From Jim's
>>>> description in 70422 it seems that the problem is that the reference
>>>> to the string from the debug information is breaking
>>>> function_mergeable_rodata_prefix, which relies on
>>>> current_function_decl.  It seems to me that its callers should pass
>>>> along their decl parameter so that f_m_r_p can use the decl's
>>>> DECL_CONTEXT rather than rely on current_function_decl being set
>>>> properly>
>>>> Jason
>>>>
>>>
>>> Ok, after some time I returned back to it. I followed your advises and
>>> changed the function function_mergeable_rodata_prefix. Apart from a small
>>> rebase was needed.
>>>
>>> May I ask Jim to test the patch?
>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>
>>> +  DECL_IGNORED_P (decl) = 1;
>>
>> As I said before, we do need to emit debug information for these variables, so this is wrong.
>
> Hello.
>
> Sorry for overlooking that.
>
>>
>>> -  section *s = targetm.asm_out.function_rodata_section (current_function_decl);
>>> +  tree decl = current_function_decl;
>>> +  if (decl && DECL_CONTEXT (decl)
>>> +      && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL)
>>> +    decl = DECL_CONTEXT (decl);
>>
>> I don't see how this would help; it still relies on current_function_decl being set correctly, which was the problem we were running into before.
>
> I see, that's what I wanted to discuss on Cauldron with you, but eventually I did not find time.
> Well problem that I see is that we want to make decision based on DECL_CONTEXT(fname_decl), but mergeable_string_section
> is called with STRING_CST (which is VALUE_EXPR of created fname_decl):
[...]
> And idea how to resolve this?

Well, when we're being called from the expander, current_function_decl is fine.

Hmm, why would current_function_decl be wrong in dwarf2out?  Can we fix that?

Why does your change to function_mergeable_rodata_prefix make any difference?

Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
  2017-10-24 20:26             ` Jason Merrill
@ 2018-05-21 13:49               ` Martin Liška
  2018-05-21 17:59                 ` Jason Merrill
  0 siblings, 1 reply; 24+ messages in thread
From: Martin Liška @ 2018-05-21 13:49 UTC (permalink / raw)
  To: Jason Merrill
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, GCC Patches,
	Segher Boessenkool, wilson

On 10/24/2017 10:24 PM, Jason Merrill wrote:
> On Thu, Sep 14, 2017 at 5:22 AM, Martin Liška <mliska@suse.cz> wrote:
>> On 08/10/2017 09:43 PM, Jason Merrill wrote:
>>> On 07/14/2017 01:35 AM, Martin Liška wrote:
>>>> On 05/01/2017 09:13 PM, Jason Merrill wrote:
>>>>> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote:
>>>>>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote:
>>>>>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote:
>>>>>>>> Hello.
>>>>>>>>
>>>>>>>> This is patch that was originally installed by Jason and later reverted due to PR70422.
>>>>>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him
>>>>>>>> to survive regression tests. That's reason why I'm resending that.
>>>>>>>>
>>>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>>>>>>
>>>>>>>> Ready to be installed?
>>>>>>>> Martin
>>>>>>>
>>>>>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
>>>>>>>> From: marxin <mliska@suse.cz>
>>>>>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200
>>>>>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
>>>>>>>>   symbol entry.
>>>>>>>>
>>>>>>>> gcc/cp/ChangeLog:
>>>>>>>>
>>>>>>>> 2017-04-20  Jason Merrill  <jason@redhat.com>
>>>>>>>>           Martin Liska  <mliska@suse.cz>
>>>>>>>>           Segher Boessenkool  <segher@kernel.crashing.org>
>>>>>>>>
>>>>>>>>       PR c++/64266
>>>>>>>>       PR c++/70353
>>>>>>>>       PR bootstrap/70422
>>>>>>>>       Core issue 1962
>>>>>>>>       * decl.c (cp_fname_init): Decay the initializer to pointer.
>>>>>>>>       (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
>>>>>>>>       * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
>>>>>>>>       DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
>>>>>>>>       DECL_IGNORED_P.  Don't call cp_finish_decl.
>>>>>>>
>>>>>>> If we don't emit those into the debug info, will the debugger be
>>>>>>> able to handle __FUNCTION__ etc. properly?
>>>>>>
>>>>>> No, debugger with the patch can't handled these. Similar to how clang
>>>>>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g?
>>>>>>
>>>>>>> Admittedly, right now we emit it into debug info only if those decls
>>>>>>> are actually used, say on:
>>>>>>> const char * foo () { return __FUNCTION__; }
>>>>>>> const char * bar () { return ""; }
>>>>>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger
>>>>>>> has to have some handling of it anyway.  But while in functions
>>>>>>> that don't refer to __FUNCTION__ it is always the debugger that needs
>>>>>>> to synthetize those and thus they will be always pointer-equal,
>>>>>>> if there are some uses of it and for other uses the debugger would
>>>>>>> synthetize it, there is the possibility that the debugger synthetized
>>>>>>> string will not be the same object as actually used in the function.
>>>>>>
>>>>>> You're right, currently one has to use a special function to be able to
>>>>>> print it in debugger. I believe we've already discussed that, according
>>>>>> to spec, the strings don't have to point to a same string.
>>>>>>
>>>>>> Suggestions what we should do with the patch?
>>>>>
>>>>> We need to emit debug information for these variables.  From Jim's
>>>>> description in 70422 it seems that the problem is that the reference
>>>>> to the string from the debug information is breaking
>>>>> function_mergeable_rodata_prefix, which relies on
>>>>> current_function_decl.  It seems to me that its callers should pass
>>>>> along their decl parameter so that f_m_r_p can use the decl's
>>>>> DECL_CONTEXT rather than rely on current_function_decl being set
>>>>> properly>
>>>>> Jason
>>>>>
>>>>
>>>> Ok, after some time I returned back to it. I followed your advises and
>>>> changed the function function_mergeable_rodata_prefix. Apart from a small
>>>> rebase was needed.
>>>>
>>>> May I ask Jim to test the patch?
>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>
>>>> +  DECL_IGNORED_P (decl) = 1;
>>>
>>> As I said before, we do need to emit debug information for these variables, so this is wrong.
>>
>> Hello.
>>
>> Sorry for overlooking that.
>>
>>>
>>>> -  section *s = targetm.asm_out.function_rodata_section (current_function_decl);
>>>> +  tree decl = current_function_decl;
>>>> +  if (decl && DECL_CONTEXT (decl)
>>>> +      && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL)
>>>> +    decl = DECL_CONTEXT (decl);
>>>
>>> I don't see how this would help; it still relies on current_function_decl being set correctly, which was the problem we were running into before.
>>
>> I see, that's what I wanted to discuss on Cauldron with you, but eventually I did not find time.
>> Well problem that I see is that we want to make decision based on DECL_CONTEXT(fname_decl), but mergeable_string_section
>> is called with STRING_CST (which is VALUE_EXPR of created fname_decl):
> [...]
>> And idea how to resolve this?
> 
> Well, when we're being called from the expander, current_function_decl is fine.
> 
> Hmm, why would current_function_decl be wrong in dwarf2out?  Can we fix that?
> 
> Why does your change to function_mergeable_rodata_prefix make any difference?
> 
> Jason
> 

Hi Jason.

Going through unresolved issues I still have in ML, I noticed this one. As perfectly analyzed here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70422#c7

The issue is that we can't find proper function decl in dwarf2out.c because it's function declaration
that was inlined. And such references are only seen in debug mode.

The only solution I see is DECL_IGNORED_P, which was also suggested by Richi in:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70422#c9

Note that both LLVM and ICC don't allow:
$ 3	  __builtin_printf ("%s\n", __PRETTY_FUNCTION__);
(gdb) p __PRETTY_FUNCTION__
No symbol "__PRETTY_FUNCTION__" in current context.

Would it be feasible solution to ignore the declaration?

Thanks,
Martin

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
  2018-05-21 13:49               ` Martin Liška
@ 2018-05-21 17:59                 ` Jason Merrill
  2018-06-04 12:11                   ` Martin Liška
  0 siblings, 1 reply; 24+ messages in thread
From: Jason Merrill @ 2018-05-21 17:59 UTC (permalink / raw)
  To: Martin Liška
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, GCC Patches,
	Segher Boessenkool, wilson

On Mon, May 21, 2018 at 9:33 AM, Martin Liška <mliska@suse.cz> wrote:
> On 10/24/2017 10:24 PM, Jason Merrill wrote:
>> On Thu, Sep 14, 2017 at 5:22 AM, Martin Liška <mliska@suse.cz> wrote:
>>> On 08/10/2017 09:43 PM, Jason Merrill wrote:
>>>> On 07/14/2017 01:35 AM, Martin Liška wrote:
>>>>> On 05/01/2017 09:13 PM, Jason Merrill wrote:
>>>>>> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote:
>>>>>>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote:
>>>>>>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote:
>>>>>>>>> Hello.
>>>>>>>>>
>>>>>>>>> This is patch that was originally installed by Jason and later reverted due to PR70422.
>>>>>>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him
>>>>>>>>> to survive regression tests. That's reason why I'm resending that.
>>>>>>>>>
>>>>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>>>>>>>
>>>>>>>>> Ready to be installed?
>>>>>>>>> Martin
>>>>>>>>
>>>>>>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
>>>>>>>>> From: marxin <mliska@suse.cz>
>>>>>>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200
>>>>>>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
>>>>>>>>>   symbol entry.
>>>>>>>>>
>>>>>>>>> gcc/cp/ChangeLog:
>>>>>>>>>
>>>>>>>>> 2017-04-20  Jason Merrill  <jason@redhat.com>
>>>>>>>>>           Martin Liska  <mliska@suse.cz>
>>>>>>>>>           Segher Boessenkool  <segher@kernel.crashing.org>
>>>>>>>>>
>>>>>>>>>       PR c++/64266
>>>>>>>>>       PR c++/70353
>>>>>>>>>       PR bootstrap/70422
>>>>>>>>>       Core issue 1962
>>>>>>>>>       * decl.c (cp_fname_init): Decay the initializer to pointer.
>>>>>>>>>       (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
>>>>>>>>>       * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
>>>>>>>>>       DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
>>>>>>>>>       DECL_IGNORED_P.  Don't call cp_finish_decl.
>>>>>>>>
>>>>>>>> If we don't emit those into the debug info, will the debugger be
>>>>>>>> able to handle __FUNCTION__ etc. properly?
>>>>>>>
>>>>>>> No, debugger with the patch can't handled these. Similar to how clang
>>>>>>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g?
>>>>>>>
>>>>>>>> Admittedly, right now we emit it into debug info only if those decls
>>>>>>>> are actually used, say on:
>>>>>>>> const char * foo () { return __FUNCTION__; }
>>>>>>>> const char * bar () { return ""; }
>>>>>>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger
>>>>>>>> has to have some handling of it anyway.  But while in functions
>>>>>>>> that don't refer to __FUNCTION__ it is always the debugger that needs
>>>>>>>> to synthetize those and thus they will be always pointer-equal,
>>>>>>>> if there are some uses of it and for other uses the debugger would
>>>>>>>> synthetize it, there is the possibility that the debugger synthetized
>>>>>>>> string will not be the same object as actually used in the function.
>>>>>>>
>>>>>>> You're right, currently one has to use a special function to be able to
>>>>>>> print it in debugger. I believe we've already discussed that, according
>>>>>>> to spec, the strings don't have to point to a same string.
>>>>>>>
>>>>>>> Suggestions what we should do with the patch?
>>>>>>
>>>>>> We need to emit debug information for these variables.  From Jim's
>>>>>> description in 70422 it seems that the problem is that the reference
>>>>>> to the string from the debug information is breaking
>>>>>> function_mergeable_rodata_prefix, which relies on
>>>>>> current_function_decl.  It seems to me that its callers should pass
>>>>>> along their decl parameter so that f_m_r_p can use the decl's
>>>>>> DECL_CONTEXT rather than rely on current_function_decl being set
>>>>>> properly>
>>>>>> Jason
>>>>>>
>>>>>
>>>>> Ok, after some time I returned back to it. I followed your advises and
>>>>> changed the function function_mergeable_rodata_prefix. Apart from a small
>>>>> rebase was needed.
>>>>>
>>>>> May I ask Jim to test the patch?
>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>>
>>>>> +  DECL_IGNORED_P (decl) = 1;
>>>>
>>>> As I said before, we do need to emit debug information for these variables, so this is wrong.
>>>
>>> Hello.
>>>
>>> Sorry for overlooking that.
>>>
>>>>
>>>>> -  section *s = targetm.asm_out.function_rodata_section (current_function_decl);
>>>>> +  tree decl = current_function_decl;
>>>>> +  if (decl && DECL_CONTEXT (decl)
>>>>> +      && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL)
>>>>> +    decl = DECL_CONTEXT (decl);
>>>>
>>>> I don't see how this would help; it still relies on current_function_decl being set correctly, which was the problem we were running into before.
>>>
>>> I see, that's what I wanted to discuss on Cauldron with you, but eventually I did not find time.
>>> Well problem that I see is that we want to make decision based on DECL_CONTEXT(fname_decl), but mergeable_string_section
>>> is called with STRING_CST (which is VALUE_EXPR of created fname_decl):
>> [...]
>>> And idea how to resolve this?
>>
>> Well, when we're being called from the expander, current_function_decl is fine.
>>
>> Hmm, why would current_function_decl be wrong in dwarf2out?  Can we fix that?
>>
>> Why does your change to function_mergeable_rodata_prefix make any difference?
>
> Going through unresolved issues I still have in ML, I noticed this one. As perfectly analyzed here:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70422#c7
>
> The issue is that we can't find proper function decl in dwarf2out.c because it's function declaration
> that was inlined. And such references are only seen in debug mode.
>
> The only solution I see is DECL_IGNORED_P, which was also suggested by Richi in:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70422#c9
>
> Note that both LLVM and ICC don't allow:
> $ 3       __builtin_printf ("%s\n", __PRETTY_FUNCTION__);
> (gdb) p __PRETTY_FUNCTION__
> No symbol "__PRETTY_FUNCTION__" in current context.
>
> Would it be feasible solution to ignore the declaration?

It seems a rather user-unfriendly solution; being able to look at the
value of an identifier used in the program is a pretty basic
expectation for the debugging experience.

In Jim's comment you mention, he says, "It seems like there are some
conflicting goals here. We want to share the string across functions,
but we also want to put it in function specific comdat sections."

The recent discussion thread at
https://gcc.gnu.org/ml/gcc/2018-05/threads.html#00109 deals with a
very similar issue, of wanting to both merge constant data and
associate it with a function.  The discussion there seems to be
tending toward merging rather than function-grouping, which seems to
match the desire in this PR.

Why do __*FUNCTION__ cause problems that

constexpr const char *p = "function name";

doesn't?

Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
  2018-05-21 17:59                 ` Jason Merrill
@ 2018-06-04 12:11                   ` Martin Liška
  2018-10-23  9:11                     ` [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts (PR c++/64266) Martin Liška
  0 siblings, 1 reply; 24+ messages in thread
From: Martin Liška @ 2018-06-04 12:11 UTC (permalink / raw)
  To: Jason Merrill
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, GCC Patches,
	Segher Boessenkool, wilson

On 05/21/2018 07:19 PM, Jason Merrill wrote:
> On Mon, May 21, 2018 at 9:33 AM, Martin Liška <mliska@suse.cz> wrote:
>> On 10/24/2017 10:24 PM, Jason Merrill wrote:
>>> On Thu, Sep 14, 2017 at 5:22 AM, Martin Liška <mliska@suse.cz> wrote:
>>>> On 08/10/2017 09:43 PM, Jason Merrill wrote:
>>>>> On 07/14/2017 01:35 AM, Martin Liška wrote:
>>>>>> On 05/01/2017 09:13 PM, Jason Merrill wrote:
>>>>>>> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote:
>>>>>>>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote:
>>>>>>>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote:
>>>>>>>>>> Hello.
>>>>>>>>>>
>>>>>>>>>> This is patch that was originally installed by Jason and later reverted due to PR70422.
>>>>>>>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him
>>>>>>>>>> to survive regression tests. That's reason why I'm resending that.
>>>>>>>>>>
>>>>>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>>>>>>>>
>>>>>>>>>> Ready to be installed?
>>>>>>>>>> Martin
>>>>>>>>>
>>>>>>>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
>>>>>>>>>> From: marxin <mliska@suse.cz>
>>>>>>>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200
>>>>>>>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
>>>>>>>>>>   symbol entry.
>>>>>>>>>>
>>>>>>>>>> gcc/cp/ChangeLog:
>>>>>>>>>>
>>>>>>>>>> 2017-04-20  Jason Merrill  <jason@redhat.com>
>>>>>>>>>>           Martin Liska  <mliska@suse.cz>
>>>>>>>>>>           Segher Boessenkool  <segher@kernel.crashing.org>
>>>>>>>>>>
>>>>>>>>>>       PR c++/64266
>>>>>>>>>>       PR c++/70353
>>>>>>>>>>       PR bootstrap/70422
>>>>>>>>>>       Core issue 1962
>>>>>>>>>>       * decl.c (cp_fname_init): Decay the initializer to pointer.
>>>>>>>>>>       (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
>>>>>>>>>>       * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
>>>>>>>>>>       DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
>>>>>>>>>>       DECL_IGNORED_P.  Don't call cp_finish_decl.
>>>>>>>>>
>>>>>>>>> If we don't emit those into the debug info, will the debugger be
>>>>>>>>> able to handle __FUNCTION__ etc. properly?
>>>>>>>>
>>>>>>>> No, debugger with the patch can't handled these. Similar to how clang
>>>>>>>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g?
>>>>>>>>
>>>>>>>>> Admittedly, right now we emit it into debug info only if those decls
>>>>>>>>> are actually used, say on:
>>>>>>>>> const char * foo () { return __FUNCTION__; }
>>>>>>>>> const char * bar () { return ""; }
>>>>>>>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger
>>>>>>>>> has to have some handling of it anyway.  But while in functions
>>>>>>>>> that don't refer to __FUNCTION__ it is always the debugger that needs
>>>>>>>>> to synthetize those and thus they will be always pointer-equal,
>>>>>>>>> if there are some uses of it and for other uses the debugger would
>>>>>>>>> synthetize it, there is the possibility that the debugger synthetized
>>>>>>>>> string will not be the same object as actually used in the function.
>>>>>>>>
>>>>>>>> You're right, currently one has to use a special function to be able to
>>>>>>>> print it in debugger. I believe we've already discussed that, according
>>>>>>>> to spec, the strings don't have to point to a same string.
>>>>>>>>
>>>>>>>> Suggestions what we should do with the patch?
>>>>>>>
>>>>>>> We need to emit debug information for these variables.  From Jim's
>>>>>>> description in 70422 it seems that the problem is that the reference
>>>>>>> to the string from the debug information is breaking
>>>>>>> function_mergeable_rodata_prefix, which relies on
>>>>>>> current_function_decl.  It seems to me that its callers should pass
>>>>>>> along their decl parameter so that f_m_r_p can use the decl's
>>>>>>> DECL_CONTEXT rather than rely on current_function_decl being set
>>>>>>> properly>
>>>>>>> Jason
>>>>>>>
>>>>>>
>>>>>> Ok, after some time I returned back to it. I followed your advises and
>>>>>> changed the function function_mergeable_rodata_prefix. Apart from a small
>>>>>> rebase was needed.
>>>>>>
>>>>>> May I ask Jim to test the patch?
>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>>>
>>>>>> +  DECL_IGNORED_P (decl) = 1;
>>>>>
>>>>> As I said before, we do need to emit debug information for these variables, so this is wrong.
>>>>
>>>> Hello.
>>>>
>>>> Sorry for overlooking that.
>>>>
>>>>>
>>>>>> -  section *s = targetm.asm_out.function_rodata_section (current_function_decl);
>>>>>> +  tree decl = current_function_decl;
>>>>>> +  if (decl && DECL_CONTEXT (decl)
>>>>>> +      && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL)
>>>>>> +    decl = DECL_CONTEXT (decl);
>>>>>
>>>>> I don't see how this would help; it still relies on current_function_decl being set correctly, which was the problem we were running into before.
>>>>
>>>> I see, that's what I wanted to discuss on Cauldron with you, but eventually I did not find time.
>>>> Well problem that I see is that we want to make decision based on DECL_CONTEXT(fname_decl), but mergeable_string_section
>>>> is called with STRING_CST (which is VALUE_EXPR of created fname_decl):
>>> [...]
>>>> And idea how to resolve this?
>>>
>>> Well, when we're being called from the expander, current_function_decl is fine.
>>>
>>> Hmm, why would current_function_decl be wrong in dwarf2out?  Can we fix that?
>>>
>>> Why does your change to function_mergeable_rodata_prefix make any difference?
>>
>> Going through unresolved issues I still have in ML, I noticed this one. As perfectly analyzed here:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70422#c7
>>
>> The issue is that we can't find proper function decl in dwarf2out.c because it's function declaration
>> that was inlined. And such references are only seen in debug mode.
>>
>> The only solution I see is DECL_IGNORED_P, which was also suggested by Richi in:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70422#c9
>>
>> Note that both LLVM and ICC don't allow:
>> $ 3       __builtin_printf ("%s\n", __PRETTY_FUNCTION__);
>> (gdb) p __PRETTY_FUNCTION__
>> No symbol "__PRETTY_FUNCTION__" in current context.
>>
>> Would it be feasible solution to ignore the declaration?
> 
> It seems a rather user-unfriendly solution; being able to look at the
> value of an identifier used in the program is a pretty basic
> expectation for the debugging experience.

Hello.

I see, not that clang++ also does not emit debug info for a constexprs:

(gdb) list
1	constexpr const char *myconstexpr = "function name";
2	
3	int main()
4	{
5	  __builtin_printf (__PRETTY_FUNCTION__);
6	  __builtin_printf (myconstexpr);
7	}
(gdb) p myconstexpr
$1 = <optimized out>


> 
> In Jim's comment you mention, he says, "It seems like there are some
> conflicting goals here. We want to share the string across functions,
> but we also want to put it in function specific comdat sections."
> 
> The recent discussion thread at
> https://gcc.gnu.org/ml/gcc/2018-05/threads.html#00109 deals with a
> very similar issue, of wanting to both merge constant data and
> associate it with a function.  The discussion there seems to be
> tending toward merging rather than function-grouping, which seems to
> match the desire in this PR.

That's promising.

> 
> Why do __*FUNCTION__ cause problems that
> 
> constexpr const char *p = "function name";
> 
> doesn't?

Difference is that for 'p' we do @object in assembly:

	.section	.rodata
.LC0:
	.string	"function name"
	.align 8
	.type	_ZL11myconstexpr, @object
	.size	_ZL11myconstexpr, 8
_ZL11myconstexpr:
	.quad	.LC0

Motivation for the original patch was to remove need of doing a symbol.
The issues I still see is how (and where) to assign to a string constant (of a __PRETTY_FUNCTION name)
a section?

Martin

> 
> Jason
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts (PR c++/64266).
  2018-06-04 12:11                   ` Martin Liška
@ 2018-10-23  9:11                     ` Martin Liška
  2018-10-23 12:20                       ` Richard Biener
  2018-10-24 18:55                       ` Jason Merrill
  0 siblings, 2 replies; 24+ messages in thread
From: Martin Liška @ 2018-10-23  9:11 UTC (permalink / raw)
  To: Jason Merrill
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, GCC Patches,
	Segher Boessenkool, wilson, Nathan Sidwell

[-- Attachment #1: Type: text/plain, Size: 6732 bytes --]

Hi.

I've returned to this long-lasting issue after quite some time. Thanks to Honza I hope
I can now address the root cause which caused output of a string constant when debug info
was emitted. The problematic situation happened with following back-trace:

#0  mergeable_string_section (decl=<string_cst 0x7ffff67be210>, align=64, flags=0) at /home/marxin/Programming/gcc/gcc/varasm.c:808
#1  0x0000000001779bf3 in default_elf_select_section (decl=<string_cst 0x7ffff67be210>, reloc=0, align=64) at /home/marxin/Programming/gcc/gcc/varasm.c:6739
#2  0x000000000176efb6 in get_constant_section (exp=<string_cst 0x7ffff67be210>, align=64) at /home/marxin/Programming/gcc/gcc/varasm.c:3302
#3  0x000000000176f468 in build_constant_desc (exp=<string_cst 0x7ffff67be210>) at /home/marxin/Programming/gcc/gcc/varasm.c:3371
#4  0x000000000176f81c in output_constant_def (exp=<string_cst 0x7ffff67be210>, defer=1) at /home/marxin/Programming/gcc/gcc/varasm.c:3434
#5  0x000000000176d406 in decode_addr_const (exp=<addr_expr 0x7ffff682f8c0>, value=0x7fffffffc540) at /home/marxin/Programming/gcc/gcc/varasm.c:2951
#6  0x000000000176d93f in const_hash_1 (exp=<addr_expr 0x7ffff682f8c0>) at /home/marxin/Programming/gcc/gcc/varasm.c:3054
#7  0x000000000176fdc2 in lookup_constant_def (exp=<addr_expr 0x7ffff682f8c0>) at /home/marxin/Programming/gcc/gcc/varasm.c:3557
#8  0x0000000000dd5778 in cst_pool_loc_descr (loc=<addr_expr 0x7ffff682f8c0>) at /home/marxin/Programming/gcc/gcc/dwarf2out.c:17288

That was in situation where we emit debug info of a function that has an inlined __PRETTY_FUNCTION__ from
a different function. As seen, the constant is output due to const_hash_1 function call. Proper fix would
be to not emit these string constants for purpose of hash function.

However, I still see some minor ICEs, it's probably related to decay_conversion in cp_fname_init:

1) ./xg++ -B. /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C

/home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C:6:17: internal compiler error: Segmentation fault
6 |     [] { return __func__; }();
  |                 ^~~~~~~~
0x1344568 crash_signal
	/home/marxin/Programming/gcc/gcc/toplev.c:325
0x7ffff6bc310f ???
	/usr/src/debug/glibc-2.27-6.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
0x9db134 is_capture_proxy(tree_node*)
	/home/marxin/Programming/gcc/gcc/cp/lambda.c:261
0xaeecb7 tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool)
	/home/marxin/Programming/gcc/gcc/cp/pt.c:16700
0xaee5fd tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool)
	/home/marxin/Programming/gcc/gcc/cp/pt.c:16636
0xaf0ffb tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool)
	/home/marxin/Programming/gcc/gcc/cp/pt.c:16942

where
(gdb) p debug_tree(decl)
 <var_decl 0x7ffff7fee480 __func__
    type <pointer_type 0x7ffff69b5540
        type <integer_type 0x7ffff69b5498 char readonly unsigned string-flag type_6 QI
            size <integer_cst 0x7ffff698df78 constant 8>
            unit-size <integer_cst 0x7ffff698df90 constant 1>
            align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b5498 precision:8 min <integer_cst 0x7ffff698dfd8 0> max <integer_cst 0x7ffff698dfc0 255>
            pointer_to_this <pointer_type 0x7ffff69b5540>>
        unsigned DI
        size <integer_cst 0x7ffff698de88 constant 64>
        unit-size <integer_cst 0x7ffff698dea0 constant 8>
        align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b5540>
    readonly used tree_2 unsigned DI /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C:6:17 size <integer_cst 0x7ffff698de88 64> unit-size <integer_cst 0x7ffff698dea0 8>
    align:64 warn_if_not_align:0 context <function_decl 0x7ffff67abd00 operator()>
    value-expr <nop_expr 0x7ffff67b7660 type <pointer_type 0x7ffff69b5540>
        constant
        arg:0 <addr_expr 0x7ffff67b7640 type <pointer_type 0x7ffff67bd3f0>
            constant
            arg:0 <string_cst 0x7ffff67b7620 type <array_type 0x7ffff67bd348>
                constant "operator()\000">>>>

and
#0  0x00000000009db134 in is_capture_proxy (decl=<tree 0x0>) at /home/marxin/Programming/gcc/gcc/cp/lambda.c:261

2 ) ./xg++ -B. /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C -c
/home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C:16:24: internal compiler error: Segmentation fault
16 |     __PRETTY_FUNCTION__), 0))
   |                        ^
/home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C:36:10: note: in expansion of macro ‘assert’
36 | int a = (assert (foo ()), 1);
   |          ^~~~~~
0x1344568 crash_signal
	/home/marxin/Programming/gcc/gcc/toplev.c:325
0x7ffff6bc310f ???
	/usr/src/debug/glibc-2.27-6.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
0x9db270 is_capture_proxy(tree_node*)
	/home/marxin/Programming/gcc/gcc/cp/lambda.c:265
0x9dbad9 is_normal_capture_proxy(tree_node*)
	/home/marxin/Programming/gcc/gcc/cp/lambda.c:274
0x9c1fe6 mark_use(tree_node*, bool, bool, unsigned int, bool)
	/home/marxin/Programming/gcc/gcc/cp/expr.c:114
0x89d9ab convert_like_real
	/home/marxin/Programming/gcc/gcc/cp/call.c:6905

where:

(gdb) p debug_tree(decl)
 <var_decl 0x7ffff7fee870 __PRETTY_FUNCTION__
    type <pointer_type 0x7ffff69b4540
        type <integer_type 0x7ffff69b4498 char readonly unsigned string-flag type_6 QI
            size <integer_cst 0x7ffff698df78 constant 8>
            unit-size <integer_cst 0x7ffff698df90 constant 1>
            align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b4498 precision:8 min <integer_cst 0x7ffff698dfd8 0> max <integer_cst 0x7ffff698dfc0 255>
            pointer_to_this <pointer_type 0x7ffff69b4540>>
        unsigned type_6 DI
        size <integer_cst 0x7ffff698de88 constant 64>
        unit-size <integer_cst 0x7ffff698dea0 constant 8>
        align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b4540>
    readonly used tree_1 tree_2 unsigned read decl_6 DI /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C:36:10 size <integer_cst 0x7ffff698de88 64> unit-size <integer_cst 0x7ffff698dea0 8>
    align:64 warn_if_not_align:0
    value-expr <nop_expr 0x7ffff67b69a0 type <pointer_type 0x7ffff69b4540>
        constant
        arg:0 <addr_expr 0x7ffff67b6980 type <pointer_type 0x7ffff67c3738>
            constant
            arg:0 <string_cst 0x7ffff67b6960 type <array_type 0x7ffff67c3690>
                constant "top level\000">>> chain <var_decl 0x7ffff7fee7e0 a>>

Can please a C++ maintainer help me with that? Having that I hope I can finish the patch.

Thank you,
Martin

[-- Attachment #2: 0001-Make-__PRETTY_FUNCTION__-like-functions-mergeable-st.patch --]
[-- Type: text/x-patch, Size: 11765 bytes --]

From 19db91a83ef40cbe8ca598fb397ee9a9fd8155eb Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 4 Oct 2018 15:34:51 +0200
Subject: [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts
 (PR c++/64266).

gcc/ChangeLog:

2018-10-22  Martin Liska  <mliska@suse.cz>

	PR c++/64266
	PR bootstrap/70422
	* varasm.c (decode_addr_const): Add new argument build.
	(const_hash_1): Set build to false by default.
	(lookup_constant_def): Do not build by default.

gcc/cp/ChangeLog:

2018-10-22  Martin Liska  <mliska@suse.cz>

	PR c++/64266
	PR bootstrap/70422
	* decl.c (cp_fname_init): Decay the initializer to pointer.
	(cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
	DECL_VALUE_EXPR, DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P.
	Don't call cp_finish_decl.
	* pt.c (tsubst_expr):  Set DECL_VALUE_EXPR,
	DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P.  Don't call cp_finish_decl.

gcc/testsuite/ChangeLog:

2018-10-22  Martin Liska  <mliska@suse.cz>

	PR c++/64266
	PR bootstrap/70422
	* g++.dg/cpp0x/constexpr-__func__2.C (main):
	(SA): Make it static assert.
	* g++.old-deja/g++.ext/pretty4.C: Remove.
---
 gcc/cp/decl.c                                 | 23 +++--
 gcc/cp/pt.c                                   | 25 +++---
 .../g++.dg/cpp0x/constexpr-__func__2.C        |  7 +-
 gcc/testsuite/g++.old-deja/g++.ext/pretty4.C  | 85 -------------------
 gcc/varasm.c                                  | 43 ++++++----
 5 files changed, 62 insertions(+), 121 deletions(-)
 delete mode 100644 gcc/testsuite/g++.old-deja/g++.ext/pretty4.C

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 2d9d56ef6e1..13167582266 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -4445,13 +4445,15 @@ cp_fname_init (const char* name, tree *type_p)
   type = cp_build_qualified_type (char_type_node, TYPE_QUAL_CONST);
   type = build_cplus_array_type (type, domain);
 
-  *type_p = type;
+  *type_p = type_decays_to (type);
 
   if (init)
     TREE_TYPE (init) = type;
   else
     init = error_mark_node;
 
+  init = decay_conversion (init, tf_warning_or_error);
+
   return init;
 }
 
@@ -4474,23 +4476,34 @@ cp_make_fname_decl (location_t loc, tree id, int type_dep)
   if (name)
     free (CONST_CAST (char *, name));
 
-  TREE_STATIC (decl) = 1;
+  /* As we're using pushdecl_with_scope, we must set the context.  */
+  DECL_CONTEXT (decl) = current_function_decl;
+
   TREE_READONLY (decl) = 1;
   DECL_ARTIFICIAL (decl) = 1;
+  DECL_DECLARED_CONSTEXPR_P (decl) = 1;
 
   TREE_USED (decl) = 1;
 
+  if (init)
+    {
+      SET_DECL_VALUE_EXPR (decl, init);
+      DECL_HAS_VALUE_EXPR_P (decl) = 1;
+      /* For decl_constant_var_p.  */
+      DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = 1;
+    }
+
   if (current_function_decl)
     {
       DECL_CONTEXT (decl) = current_function_decl;
       decl = pushdecl_outermost_localscope (decl);
-      cp_finish_decl (decl, init, /*init_const_expr_p=*/false, NULL_TREE,
-		      LOOKUP_ONLYCONVERTING);
+      if (decl != error_mark_node)
+	add_decl_expr (decl);
     }
   else
     {
       DECL_THIS_STATIC (decl) = true;
-      pushdecl_top_level_and_finish (decl, init);
+      pushdecl_top_level_and_finish (decl, NULL_TREE);
     }
 
   return decl;
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index b8b6545434b..0269fa5129a 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -16746,6 +16746,19 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 		    DECL_CONTEXT (decl) = current_function_decl;
 		    cp_check_omp_declare_reduction (decl);
 		  }
+		else if (VAR_P (decl)
+			 && DECL_PRETTY_FUNCTION_P (decl))
+		  {
+		    /* For __PRETTY_FUNCTION__ we have to adjust the
+		       initializer.  */
+		    const char *const name
+		      = cxx_printable_name (current_function_decl, 2);
+		    init = cp_fname_init (name, &TREE_TYPE (decl));
+		    SET_DECL_VALUE_EXPR (decl, init);
+		    DECL_HAS_VALUE_EXPR_P (decl) = 1;
+		    DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = 1;
+		    maybe_push_decl (decl);
+		  }
 		else
 		  {
 		    int const_init = false;
@@ -16760,17 +16773,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 						   complain, in_decl, &first,
 						   &cnt);
 
-		    if (VAR_P (decl)
-			&& DECL_PRETTY_FUNCTION_P (decl))
-		      {
-			/* For __PRETTY_FUNCTION__ we have to adjust the
-			   initializer.  */
-			const char *const name
-			  = cxx_printable_name (current_function_decl, 2);
-			init = cp_fname_init (name, &TREE_TYPE (decl));
-		      }
-		    else
-		      init = tsubst_init (init, decl, args, complain, in_decl);
+		    init = tsubst_init (init, decl, args, complain, in_decl);
 
 		    if (VAR_P (decl))
 		      const_init = (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C
index e6782905423..673fb4f3a93 100644
--- a/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C
@@ -1,5 +1,5 @@
 // PR c++/70353
-// { dg-do link { target c++11 } }
+// { dg-do compile { target c++11 } }
 
 constexpr const char* ce ()
 {
@@ -8,6 +8,5 @@ constexpr const char* ce ()
 
 const char *c = ce();
 
-int main()
-{
-}
+#define SA(X) static_assert((X),#X)
+SA(ce()[0] == 'c');
diff --git a/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C b/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C
deleted file mode 100644
index 9017d567132..00000000000
--- a/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C
+++ /dev/null
@@ -1,85 +0,0 @@
-// { dg-do run  }
-// Copyright (C) 2000 Free Software Foundation, Inc.
-// Contributed by Nathan Sidwell 3 Mar 2000 <nathan@codesourcery.com>
-
-// __PRETTY_FUNCTION__, __FUNCTION__ and __function__ should have the
-// type char const [X], where X is the right value for that particular function
-
-static void const *strings[4];
-static void const *tpls[4];
-static unsigned pos = 0;
-static int fail;
-static void const *ptr = 0;
-
-void unover (char const (*)[5]) {}
-void foo (char const (*)[5]) {}
-void foo (void *) {fail = 1;}
-void foo (void const *) {fail = 1;}
-void baz (char const (&)[5]) {}
-
-template<unsigned I> void PV (char const (&objRef)[I])
-{
-  strings[pos] = objRef;
-  tpls[pos] = __PRETTY_FUNCTION__;
-  pos++;
-}
-
-void fn ()
-{
-  PV (__FUNCTION__);
-  PV (__func__);
-  PV (__PRETTY_FUNCTION__);
-  PV ("wibble");
-}
-
-void baz ()
-{
-  ptr = __FUNCTION__;
-  // there should be no string const merging
-  if (ptr == "baz")
-    fail = 1;
-  // but all uses should be the same.
-  if (ptr != __FUNCTION__)
-    fail = 1;
-}
-int baz (int)
-{
-  return ptr == __FUNCTION__;
-}
-
-int main ()
-{
-  // make sure we actually emit the VAR_DECL when needed, and things have the
-  // expected type.
-  foo (&__FUNCTION__);
-  baz (__FUNCTION__);
-  unover (&__FUNCTION__);
-  if (fail)
-    return 1;
-  
-  // __FUNCTION__ should be unique across functions with the same base name
-  // (it's a local static, _not_ a string).
-  baz ();
-  if (fail)
-    return 1;
-  if (baz (1))
-    return 1;
-  fn ();
-  
-  // Check the names of fn. They should all be distinct strings (though two
-  // will have the same value).
-  if (strings[0] == strings[1])
-    return 1;
-  if (strings[0] == strings[2])
-    return 1;
-  if (strings[1] == strings[2])
-    return 1;
-
-  // check the names of the template functions so invoked
-  if (tpls[0] != tpls[1])
-    return 1;
-  if (tpls[0] == tpls[2])
-    return 1;
-  
-  return 0;
-}
diff --git a/gcc/varasm.c b/gcc/varasm.c
index c789a03e1f3..0040f07c0e4 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -105,8 +105,8 @@ static int contains_pointers_p (tree);
 #ifdef ASM_OUTPUT_EXTERNAL
 static bool incorporeal_function_p (tree);
 #endif
-static void decode_addr_const (tree, struct addr_const *);
-static hashval_t const_hash_1 (const tree);
+static bool decode_addr_const (tree, struct addr_const *, bool build = true);
+static hashval_t const_hash_1 (const tree, bool build = true);
 static int compare_constant (const tree, const tree);
 static void output_constant_def_contents (rtx);
 static void output_addressed_constants (tree);
@@ -2877,15 +2877,17 @@ assemble_real (REAL_VALUE_TYPE d, scalar_float_mode mode, unsigned int align,
 /* Given an expression EXP with a constant value,
    reduce it to the sum of an assembler symbol and an integer.
    Store them both in the structure *VALUE.
-   EXP must be reducible.  */
+   EXP must be reducible.
+   If BUILD is false the function may fail if it is address expr of a constant
+   that is not in the constant pool.  */
 
 struct addr_const {
   rtx base;
   poly_int64 offset;
 };
 
-static void
-decode_addr_const (tree exp, struct addr_const *value)
+static bool
+decode_addr_const (tree exp, struct addr_const *value, bool build)
 {
   tree target = TREE_OPERAND (exp, 0);
   poly_int64 offset = 0;
@@ -2943,7 +2945,14 @@ decode_addr_const (tree exp, struct addr_const *value)
     case COMPLEX_CST:
     case CONSTRUCTOR:
     case INTEGER_CST:
-      x = output_constant_def (target, 1);
+      if (!build)
+	{
+	  x = lookup_constant_def (target);
+	  if (!x)
+	    return false;
+	}
+      else
+	x = output_constant_def (target, 1);
       break;
 
     case INDIRECT_REF:
@@ -2962,6 +2971,7 @@ decode_addr_const (tree exp, struct addr_const *value)
 
   value->base = x;
   value->offset = offset;
+  return true;
 }
 \f
 static GTY(()) hash_table<tree_descriptor_hasher> *const_desc_htab;
@@ -2985,7 +2995,7 @@ tree_descriptor_hasher::hash (constant_descriptor_tree *ptr)
 }
 
 static hashval_t
-const_hash_1 (const tree exp)
+const_hash_1 (const tree exp, bool build)
 {
   const char *p;
   hashval_t hi;
@@ -3014,8 +3024,8 @@ const_hash_1 (const tree exp)
       break;
 
     case COMPLEX_CST:
-      return (const_hash_1 (TREE_REALPART (exp)) * 5
-	      + const_hash_1 (TREE_IMAGPART (exp)));
+      return (const_hash_1 (TREE_REALPART (exp), build) * 5
+	      + const_hash_1 (TREE_IMAGPART (exp), build));
 
     case VECTOR_CST:
       {
@@ -3023,7 +3033,7 @@ const_hash_1 (const tree exp)
 	hi = hi * 563 + VECTOR_CST_NELTS_PER_PATTERN (exp);
 	unsigned int count = vector_cst_encoded_nelts (exp);
 	for (unsigned int i = 0; i < count; ++i)
-	  hi = hi * 563 + const_hash_1 (VECTOR_CST_ENCODED_ELT (exp, i));
+	  hi = hi * 563 + const_hash_1 (VECTOR_CST_ENCODED_ELT (exp, i), build);
 	return hi;
       }
 
@@ -3036,7 +3046,7 @@ const_hash_1 (const tree exp)
 
 	FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (exp), idx, value)
 	  if (value)
-	    hi = hi * 603 + const_hash_1 (value);
+	    hi = hi * 603 + const_hash_1 (value, build);
 
 	return hi;
       }
@@ -3046,7 +3056,8 @@ const_hash_1 (const tree exp)
       {
 	struct addr_const value;
 
-	decode_addr_const (exp, &value);
+	if (!decode_addr_const (exp, &value, build))
+	  return 0;
 	switch (GET_CODE (value.base))
 	  {
 	  case SYMBOL_REF:
@@ -3072,11 +3083,11 @@ const_hash_1 (const tree exp)
     case PLUS_EXPR:
     case POINTER_PLUS_EXPR:
     case MINUS_EXPR:
-      return (const_hash_1 (TREE_OPERAND (exp, 0)) * 9
-	      + const_hash_1 (TREE_OPERAND (exp, 1)));
+      return (const_hash_1 (TREE_OPERAND (exp, 0), build) * 9
+	      + const_hash_1 (TREE_OPERAND (exp, 1), build));
 
     CASE_CONVERT:
-      return const_hash_1 (TREE_OPERAND (exp, 0)) * 7 + 2;
+      return const_hash_1 (TREE_OPERAND (exp, 0), build) * 7 + 2;
 
     default:
       /* A language specific constant. Just hash the code.  */
@@ -3549,7 +3560,7 @@ lookup_constant_def (tree exp)
   struct constant_descriptor_tree key;
 
   key.value = exp;
-  key.hash = const_hash_1 (exp);
+  key.hash = const_hash_1 (exp, false);
   constant_descriptor_tree *desc
     = const_desc_htab->find_with_hash (&key, key.hash);
 
-- 
2.19.0


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts (PR c++/64266).
  2018-10-23  9:11                     ` [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts (PR c++/64266) Martin Liška
@ 2018-10-23 12:20                       ` Richard Biener
  2018-10-24 14:19                         ` Martin Liška
  2018-10-24 18:55                       ` Jason Merrill
  1 sibling, 1 reply; 24+ messages in thread
From: Richard Biener @ 2018-10-23 12:20 UTC (permalink / raw)
  To: Martin Liška
  Cc: Jason Merrill, Jakub Jelinek, Pedro Alves, Jan Kratochvil,
	GCC Patches, Segher Boessenkool, wilson, Nathan Sidwell

On Tue, Oct 23, 2018 at 10:59 AM Martin Liška <mliska@suse.cz> wrote:
>
> Hi.
>
> I've returned to this long-lasting issue after quite some time. Thanks to Honza I hope
> I can now address the root cause which caused output of a string constant when debug info
> was emitted. The problematic situation happened with following back-trace:
>
> #0  mergeable_string_section (decl=<string_cst 0x7ffff67be210>, align=64, flags=0) at /home/marxin/Programming/gcc/gcc/varasm.c:808
> #1  0x0000000001779bf3 in default_elf_select_section (decl=<string_cst 0x7ffff67be210>, reloc=0, align=64) at /home/marxin/Programming/gcc/gcc/varasm.c:6739
> #2  0x000000000176efb6 in get_constant_section (exp=<string_cst 0x7ffff67be210>, align=64) at /home/marxin/Programming/gcc/gcc/varasm.c:3302
> #3  0x000000000176f468 in build_constant_desc (exp=<string_cst 0x7ffff67be210>) at /home/marxin/Programming/gcc/gcc/varasm.c:3371
> #4  0x000000000176f81c in output_constant_def (exp=<string_cst 0x7ffff67be210>, defer=1) at /home/marxin/Programming/gcc/gcc/varasm.c:3434
> #5  0x000000000176d406 in decode_addr_const (exp=<addr_expr 0x7ffff682f8c0>, value=0x7fffffffc540) at /home/marxin/Programming/gcc/gcc/varasm.c:2951
> #6  0x000000000176d93f in const_hash_1 (exp=<addr_expr 0x7ffff682f8c0>) at /home/marxin/Programming/gcc/gcc/varasm.c:3054
> #7  0x000000000176fdc2 in lookup_constant_def (exp=<addr_expr 0x7ffff682f8c0>) at /home/marxin/Programming/gcc/gcc/varasm.c:3557
> #8  0x0000000000dd5778 in cst_pool_loc_descr (loc=<addr_expr 0x7ffff682f8c0>) at /home/marxin/Programming/gcc/gcc/dwarf2out.c:17288
>
> That was in situation where we emit debug info of a function that has an inlined __PRETTY_FUNCTION__ from
> a different function. As seen, the constant is output due to const_hash_1 function call. Proper fix would
> be to not emit these string constants for purpose of hash function.

possibly sth like the following - that avoids all cases of calling
output_constant_def.  Probably worth testing
separately.

diff --git a/gcc/varasm.c b/gcc/varasm.c
index 91650eea9f7..9121dbd2c84 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -3047,6 +3047,10 @@ const_hash_1 (const tree exp)
       }

     case ADDR_EXPR:
+      if (CONSTANT_CLASS_P (TREE_OPERAND (exp, 0)))
+       return const_hash_1 (TREE_OPERAND (exp, 0));
+
+      /* Fallthru.  */
     case FDESC_EXPR:
       {
        struct addr_const value;

> However, I still see some minor ICEs, it's probably related to decay_conversion in cp_fname_init:
>
> 1) ./xg++ -B. /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C
>
> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C:6:17: internal compiler error: Segmentation fault
> 6 |     [] { return __func__; }();
>   |                 ^~~~~~~~
> 0x1344568 crash_signal
>         /home/marxin/Programming/gcc/gcc/toplev.c:325
> 0x7ffff6bc310f ???
>         /usr/src/debug/glibc-2.27-6.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
> 0x9db134 is_capture_proxy(tree_node*)
>         /home/marxin/Programming/gcc/gcc/cp/lambda.c:261
> 0xaeecb7 tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool)
>         /home/marxin/Programming/gcc/gcc/cp/pt.c:16700
> 0xaee5fd tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool)
>         /home/marxin/Programming/gcc/gcc/cp/pt.c:16636
> 0xaf0ffb tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool)
>         /home/marxin/Programming/gcc/gcc/cp/pt.c:16942
>
> where
> (gdb) p debug_tree(decl)
>  <var_decl 0x7ffff7fee480 __func__
>     type <pointer_type 0x7ffff69b5540
>         type <integer_type 0x7ffff69b5498 char readonly unsigned string-flag type_6 QI
>             size <integer_cst 0x7ffff698df78 constant 8>
>             unit-size <integer_cst 0x7ffff698df90 constant 1>
>             align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b5498 precision:8 min <integer_cst 0x7ffff698dfd8 0> max <integer_cst 0x7ffff698dfc0 255>
>             pointer_to_this <pointer_type 0x7ffff69b5540>>
>         unsigned DI
>         size <integer_cst 0x7ffff698de88 constant 64>
>         unit-size <integer_cst 0x7ffff698dea0 constant 8>
>         align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b5540>
>     readonly used tree_2 unsigned DI /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C:6:17 size <integer_cst 0x7ffff698de88 64> unit-size <integer_cst 0x7ffff698dea0 8>
>     align:64 warn_if_not_align:0 context <function_decl 0x7ffff67abd00 operator()>
>     value-expr <nop_expr 0x7ffff67b7660 type <pointer_type 0x7ffff69b5540>
>         constant
>         arg:0 <addr_expr 0x7ffff67b7640 type <pointer_type 0x7ffff67bd3f0>
>             constant
>             arg:0 <string_cst 0x7ffff67b7620 type <array_type 0x7ffff67bd348>
>                 constant "operator()\000">>>>
>
> and
> #0  0x00000000009db134 in is_capture_proxy (decl=<tree 0x0>) at /home/marxin/Programming/gcc/gcc/cp/lambda.c:261
>
> 2 ) ./xg++ -B. /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C -c
> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C:16:24: internal compiler error: Segmentation fault
> 16 |     __PRETTY_FUNCTION__), 0))
>    |                        ^
> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C:36:10: note: in expansion of macro ‘assert’
> 36 | int a = (assert (foo ()), 1);
>    |          ^~~~~~
> 0x1344568 crash_signal
>         /home/marxin/Programming/gcc/gcc/toplev.c:325
> 0x7ffff6bc310f ???
>         /usr/src/debug/glibc-2.27-6.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
> 0x9db270 is_capture_proxy(tree_node*)
>         /home/marxin/Programming/gcc/gcc/cp/lambda.c:265
> 0x9dbad9 is_normal_capture_proxy(tree_node*)
>         /home/marxin/Programming/gcc/gcc/cp/lambda.c:274
> 0x9c1fe6 mark_use(tree_node*, bool, bool, unsigned int, bool)
>         /home/marxin/Programming/gcc/gcc/cp/expr.c:114
> 0x89d9ab convert_like_real
>         /home/marxin/Programming/gcc/gcc/cp/call.c:6905
>
> where:
>
> (gdb) p debug_tree(decl)
>  <var_decl 0x7ffff7fee870 __PRETTY_FUNCTION__
>     type <pointer_type 0x7ffff69b4540
>         type <integer_type 0x7ffff69b4498 char readonly unsigned string-flag type_6 QI
>             size <integer_cst 0x7ffff698df78 constant 8>
>             unit-size <integer_cst 0x7ffff698df90 constant 1>
>             align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b4498 precision:8 min <integer_cst 0x7ffff698dfd8 0> max <integer_cst 0x7ffff698dfc0 255>
>             pointer_to_this <pointer_type 0x7ffff69b4540>>
>         unsigned type_6 DI
>         size <integer_cst 0x7ffff698de88 constant 64>
>         unit-size <integer_cst 0x7ffff698dea0 constant 8>
>         align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b4540>
>     readonly used tree_1 tree_2 unsigned read decl_6 DI /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C:36:10 size <integer_cst 0x7ffff698de88 64> unit-size <integer_cst 0x7ffff698dea0 8>
>     align:64 warn_if_not_align:0
>     value-expr <nop_expr 0x7ffff67b69a0 type <pointer_type 0x7ffff69b4540>
>         constant
>         arg:0 <addr_expr 0x7ffff67b6980 type <pointer_type 0x7ffff67c3738>
>             constant
>             arg:0 <string_cst 0x7ffff67b6960 type <array_type 0x7ffff67c3690>
>                 constant "top level\000">>> chain <var_decl 0x7ffff7fee7e0 a>>
>
> Can please a C++ maintainer help me with that? Having that I hope I can finish the patch.
>
> Thank you,
> Martin

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts (PR c++/64266).
  2018-10-23 12:20                       ` Richard Biener
@ 2018-10-24 14:19                         ` Martin Liška
  2018-10-24 14:26                           ` Richard Biener
  0 siblings, 1 reply; 24+ messages in thread
From: Martin Liška @ 2018-10-24 14:19 UTC (permalink / raw)
  To: Richard Biener
  Cc: Jason Merrill, Jakub Jelinek, Pedro Alves, Jan Kratochvil,
	GCC Patches, Segher Boessenkool, wilson, Nathan Sidwell

On 10/23/18 1:19 PM, Richard Biener wrote:
> On Tue, Oct 23, 2018 at 10:59 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> Hi.
>>
>> I've returned to this long-lasting issue after quite some time. Thanks to Honza I hope
>> I can now address the root cause which caused output of a string constant when debug info
>> was emitted. The problematic situation happened with following back-trace:
>>
>> #0  mergeable_string_section (decl=<string_cst 0x7ffff67be210>, align=64, flags=0) at /home/marxin/Programming/gcc/gcc/varasm.c:808
>> #1  0x0000000001779bf3 in default_elf_select_section (decl=<string_cst 0x7ffff67be210>, reloc=0, align=64) at /home/marxin/Programming/gcc/gcc/varasm.c:6739
>> #2  0x000000000176efb6 in get_constant_section (exp=<string_cst 0x7ffff67be210>, align=64) at /home/marxin/Programming/gcc/gcc/varasm.c:3302
>> #3  0x000000000176f468 in build_constant_desc (exp=<string_cst 0x7ffff67be210>) at /home/marxin/Programming/gcc/gcc/varasm.c:3371
>> #4  0x000000000176f81c in output_constant_def (exp=<string_cst 0x7ffff67be210>, defer=1) at /home/marxin/Programming/gcc/gcc/varasm.c:3434
>> #5  0x000000000176d406 in decode_addr_const (exp=<addr_expr 0x7ffff682f8c0>, value=0x7fffffffc540) at /home/marxin/Programming/gcc/gcc/varasm.c:2951
>> #6  0x000000000176d93f in const_hash_1 (exp=<addr_expr 0x7ffff682f8c0>) at /home/marxin/Programming/gcc/gcc/varasm.c:3054
>> #7  0x000000000176fdc2 in lookup_constant_def (exp=<addr_expr 0x7ffff682f8c0>) at /home/marxin/Programming/gcc/gcc/varasm.c:3557
>> #8  0x0000000000dd5778 in cst_pool_loc_descr (loc=<addr_expr 0x7ffff682f8c0>) at /home/marxin/Programming/gcc/gcc/dwarf2out.c:17288
>>
>> That was in situation where we emit debug info of a function that has an inlined __PRETTY_FUNCTION__ from
>> a different function. As seen, the constant is output due to const_hash_1 function call. Proper fix would
>> be to not emit these string constants for purpose of hash function.
> 
> possibly sth like the following - that avoids all cases of calling
> output_constant_def.  Probably worth testing
> separately.

Thanks for the hint. I can confirm it works for the pretty function patch.
I also tested the patch separately on current trunk. May I install the hunk
right now, or should I wait?

Thanks,
Martin

> 
> diff --git a/gcc/varasm.c b/gcc/varasm.c
> index 91650eea9f7..9121dbd2c84 100644
> --- a/gcc/varasm.c
> +++ b/gcc/varasm.c
> @@ -3047,6 +3047,10 @@ const_hash_1 (const tree exp)
>        }
> 
>      case ADDR_EXPR:
> +      if (CONSTANT_CLASS_P (TREE_OPERAND (exp, 0)))
> +       return const_hash_1 (TREE_OPERAND (exp, 0));
> +
> +      /* Fallthru.  */
>      case FDESC_EXPR:
>        {
>         struct addr_const value;
> 
>> However, I still see some minor ICEs, it's probably related to decay_conversion in cp_fname_init:
>>
>> 1) ./xg++ -B. /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C
>>
>> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C:6:17: internal compiler error: Segmentation fault
>> 6 |     [] { return __func__; }();
>>   |                 ^~~~~~~~
>> 0x1344568 crash_signal
>>         /home/marxin/Programming/gcc/gcc/toplev.c:325
>> 0x7ffff6bc310f ???
>>         /usr/src/debug/glibc-2.27-6.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
>> 0x9db134 is_capture_proxy(tree_node*)
>>         /home/marxin/Programming/gcc/gcc/cp/lambda.c:261
>> 0xaeecb7 tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool)
>>         /home/marxin/Programming/gcc/gcc/cp/pt.c:16700
>> 0xaee5fd tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool)
>>         /home/marxin/Programming/gcc/gcc/cp/pt.c:16636
>> 0xaf0ffb tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool)
>>         /home/marxin/Programming/gcc/gcc/cp/pt.c:16942
>>
>> where
>> (gdb) p debug_tree(decl)
>>  <var_decl 0x7ffff7fee480 __func__
>>     type <pointer_type 0x7ffff69b5540
>>         type <integer_type 0x7ffff69b5498 char readonly unsigned string-flag type_6 QI
>>             size <integer_cst 0x7ffff698df78 constant 8>
>>             unit-size <integer_cst 0x7ffff698df90 constant 1>
>>             align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b5498 precision:8 min <integer_cst 0x7ffff698dfd8 0> max <integer_cst 0x7ffff698dfc0 255>
>>             pointer_to_this <pointer_type 0x7ffff69b5540>>
>>         unsigned DI
>>         size <integer_cst 0x7ffff698de88 constant 64>
>>         unit-size <integer_cst 0x7ffff698dea0 constant 8>
>>         align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b5540>
>>     readonly used tree_2 unsigned DI /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C:6:17 size <integer_cst 0x7ffff698de88 64> unit-size <integer_cst 0x7ffff698dea0 8>
>>     align:64 warn_if_not_align:0 context <function_decl 0x7ffff67abd00 operator()>
>>     value-expr <nop_expr 0x7ffff67b7660 type <pointer_type 0x7ffff69b5540>
>>         constant
>>         arg:0 <addr_expr 0x7ffff67b7640 type <pointer_type 0x7ffff67bd3f0>
>>             constant
>>             arg:0 <string_cst 0x7ffff67b7620 type <array_type 0x7ffff67bd348>
>>                 constant "operator()\000">>>>
>>
>> and
>> #0  0x00000000009db134 in is_capture_proxy (decl=<tree 0x0>) at /home/marxin/Programming/gcc/gcc/cp/lambda.c:261
>>
>> 2 ) ./xg++ -B. /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C -c
>> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C:16:24: internal compiler error: Segmentation fault
>> 16 |     __PRETTY_FUNCTION__), 0))
>>    |                        ^
>> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C:36:10: note: in expansion of macro ‘assert’
>> 36 | int a = (assert (foo ()), 1);
>>    |          ^~~~~~
>> 0x1344568 crash_signal
>>         /home/marxin/Programming/gcc/gcc/toplev.c:325
>> 0x7ffff6bc310f ???
>>         /usr/src/debug/glibc-2.27-6.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
>> 0x9db270 is_capture_proxy(tree_node*)
>>         /home/marxin/Programming/gcc/gcc/cp/lambda.c:265
>> 0x9dbad9 is_normal_capture_proxy(tree_node*)
>>         /home/marxin/Programming/gcc/gcc/cp/lambda.c:274
>> 0x9c1fe6 mark_use(tree_node*, bool, bool, unsigned int, bool)
>>         /home/marxin/Programming/gcc/gcc/cp/expr.c:114
>> 0x89d9ab convert_like_real
>>         /home/marxin/Programming/gcc/gcc/cp/call.c:6905
>>
>> where:
>>
>> (gdb) p debug_tree(decl)
>>  <var_decl 0x7ffff7fee870 __PRETTY_FUNCTION__
>>     type <pointer_type 0x7ffff69b4540
>>         type <integer_type 0x7ffff69b4498 char readonly unsigned string-flag type_6 QI
>>             size <integer_cst 0x7ffff698df78 constant 8>
>>             unit-size <integer_cst 0x7ffff698df90 constant 1>
>>             align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b4498 precision:8 min <integer_cst 0x7ffff698dfd8 0> max <integer_cst 0x7ffff698dfc0 255>
>>             pointer_to_this <pointer_type 0x7ffff69b4540>>
>>         unsigned type_6 DI
>>         size <integer_cst 0x7ffff698de88 constant 64>
>>         unit-size <integer_cst 0x7ffff698dea0 constant 8>
>>         align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b4540>
>>     readonly used tree_1 tree_2 unsigned read decl_6 DI /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C:36:10 size <integer_cst 0x7ffff698de88 64> unit-size <integer_cst 0x7ffff698dea0 8>
>>     align:64 warn_if_not_align:0
>>     value-expr <nop_expr 0x7ffff67b69a0 type <pointer_type 0x7ffff69b4540>
>>         constant
>>         arg:0 <addr_expr 0x7ffff67b6980 type <pointer_type 0x7ffff67c3738>
>>             constant
>>             arg:0 <string_cst 0x7ffff67b6960 type <array_type 0x7ffff67c3690>
>>                 constant "top level\000">>> chain <var_decl 0x7ffff7fee7e0 a>>
>>
>> Can please a C++ maintainer help me with that? Having that I hope I can finish the patch.
>>
>> Thank you,
>> Martin

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts (PR c++/64266).
  2018-10-24 14:19                         ` Martin Liška
@ 2018-10-24 14:26                           ` Richard Biener
  0 siblings, 0 replies; 24+ messages in thread
From: Richard Biener @ 2018-10-24 14:26 UTC (permalink / raw)
  To: Martin Liška
  Cc: Jason Merrill, Jakub Jelinek, Pedro Alves, Jan Kratochvil,
	GCC Patches, Segher Boessenkool, wilson, Nathan Sidwell

On Wed, Oct 24, 2018 at 3:34 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 10/23/18 1:19 PM, Richard Biener wrote:
> > On Tue, Oct 23, 2018 at 10:59 AM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> Hi.
> >>
> >> I've returned to this long-lasting issue after quite some time. Thanks to Honza I hope
> >> I can now address the root cause which caused output of a string constant when debug info
> >> was emitted. The problematic situation happened with following back-trace:
> >>
> >> #0  mergeable_string_section (decl=<string_cst 0x7ffff67be210>, align=64, flags=0) at /home/marxin/Programming/gcc/gcc/varasm.c:808
> >> #1  0x0000000001779bf3 in default_elf_select_section (decl=<string_cst 0x7ffff67be210>, reloc=0, align=64) at /home/marxin/Programming/gcc/gcc/varasm.c:6739
> >> #2  0x000000000176efb6 in get_constant_section (exp=<string_cst 0x7ffff67be210>, align=64) at /home/marxin/Programming/gcc/gcc/varasm.c:3302
> >> #3  0x000000000176f468 in build_constant_desc (exp=<string_cst 0x7ffff67be210>) at /home/marxin/Programming/gcc/gcc/varasm.c:3371
> >> #4  0x000000000176f81c in output_constant_def (exp=<string_cst 0x7ffff67be210>, defer=1) at /home/marxin/Programming/gcc/gcc/varasm.c:3434
> >> #5  0x000000000176d406 in decode_addr_const (exp=<addr_expr 0x7ffff682f8c0>, value=0x7fffffffc540) at /home/marxin/Programming/gcc/gcc/varasm.c:2951
> >> #6  0x000000000176d93f in const_hash_1 (exp=<addr_expr 0x7ffff682f8c0>) at /home/marxin/Programming/gcc/gcc/varasm.c:3054
> >> #7  0x000000000176fdc2 in lookup_constant_def (exp=<addr_expr 0x7ffff682f8c0>) at /home/marxin/Programming/gcc/gcc/varasm.c:3557
> >> #8  0x0000000000dd5778 in cst_pool_loc_descr (loc=<addr_expr 0x7ffff682f8c0>) at /home/marxin/Programming/gcc/gcc/dwarf2out.c:17288
> >>
> >> That was in situation where we emit debug info of a function that has an inlined __PRETTY_FUNCTION__ from
> >> a different function. As seen, the constant is output due to const_hash_1 function call. Proper fix would
> >> be to not emit these string constants for purpose of hash function.
> >
> > possibly sth like the following - that avoids all cases of calling
> > output_constant_def.  Probably worth testing
> > separately.
>
> Thanks for the hint. I can confirm it works for the pretty function patch.
> I also tested the patch separately on current trunk. May I install the hunk
> right now, or should I wait?

You can install it right now I think.

Richard.

> Thanks,
> Martin
>
> >
> > diff --git a/gcc/varasm.c b/gcc/varasm.c
> > index 91650eea9f7..9121dbd2c84 100644
> > --- a/gcc/varasm.c
> > +++ b/gcc/varasm.c
> > @@ -3047,6 +3047,10 @@ const_hash_1 (const tree exp)
> >        }
> >
> >      case ADDR_EXPR:
> > +      if (CONSTANT_CLASS_P (TREE_OPERAND (exp, 0)))
> > +       return const_hash_1 (TREE_OPERAND (exp, 0));
> > +
> > +      /* Fallthru.  */
> >      case FDESC_EXPR:
> >        {
> >         struct addr_const value;
> >
> >> However, I still see some minor ICEs, it's probably related to decay_conversion in cp_fname_init:
> >>
> >> 1) ./xg++ -B. /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C
> >>
> >> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C:6:17: internal compiler error: Segmentation fault
> >> 6 |     [] { return __func__; }();
> >>   |                 ^~~~~~~~
> >> 0x1344568 crash_signal
> >>         /home/marxin/Programming/gcc/gcc/toplev.c:325
> >> 0x7ffff6bc310f ???
> >>         /usr/src/debug/glibc-2.27-6.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
> >> 0x9db134 is_capture_proxy(tree_node*)
> >>         /home/marxin/Programming/gcc/gcc/cp/lambda.c:261
> >> 0xaeecb7 tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool)
> >>         /home/marxin/Programming/gcc/gcc/cp/pt.c:16700
> >> 0xaee5fd tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool)
> >>         /home/marxin/Programming/gcc/gcc/cp/pt.c:16636
> >> 0xaf0ffb tsubst_expr(tree_node*, tree_node*, int, tree_node*, bool)
> >>         /home/marxin/Programming/gcc/gcc/cp/pt.c:16942
> >>
> >> where
> >> (gdb) p debug_tree(decl)
> >>  <var_decl 0x7ffff7fee480 __func__
> >>     type <pointer_type 0x7ffff69b5540
> >>         type <integer_type 0x7ffff69b5498 char readonly unsigned string-flag type_6 QI
> >>             size <integer_cst 0x7ffff698df78 constant 8>
> >>             unit-size <integer_cst 0x7ffff698df90 constant 1>
> >>             align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b5498 precision:8 min <integer_cst 0x7ffff698dfd8 0> max <integer_cst 0x7ffff698dfc0 255>
> >>             pointer_to_this <pointer_type 0x7ffff69b5540>>
> >>         unsigned DI
> >>         size <integer_cst 0x7ffff698de88 constant 64>
> >>         unit-size <integer_cst 0x7ffff698dea0 constant 8>
> >>         align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b5540>
> >>     readonly used tree_2 unsigned DI /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C:6:17 size <integer_cst 0x7ffff698de88 64> unit-size <integer_cst 0x7ffff698dea0 8>
> >>     align:64 warn_if_not_align:0 context <function_decl 0x7ffff67abd00 operator()>
> >>     value-expr <nop_expr 0x7ffff67b7660 type <pointer_type 0x7ffff69b5540>
> >>         constant
> >>         arg:0 <addr_expr 0x7ffff67b7640 type <pointer_type 0x7ffff67bd3f0>
> >>             constant
> >>             arg:0 <string_cst 0x7ffff67b7620 type <array_type 0x7ffff67bd348>
> >>                 constant "operator()\000">>>>
> >>
> >> and
> >> #0  0x00000000009db134 in is_capture_proxy (decl=<tree 0x0>) at /home/marxin/Programming/gcc/gcc/cp/lambda.c:261
> >>
> >> 2 ) ./xg++ -B. /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C -c
> >> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C:16:24: internal compiler error: Segmentation fault
> >> 16 |     __PRETTY_FUNCTION__), 0))
> >>    |                        ^
> >> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C:36:10: note: in expansion of macro ‘assert’
> >> 36 | int a = (assert (foo ()), 1);
> >>    |          ^~~~~~
> >> 0x1344568 crash_signal
> >>         /home/marxin/Programming/gcc/gcc/toplev.c:325
> >> 0x7ffff6bc310f ???
> >>         /usr/src/debug/glibc-2.27-6.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
> >> 0x9db270 is_capture_proxy(tree_node*)
> >>         /home/marxin/Programming/gcc/gcc/cp/lambda.c:265
> >> 0x9dbad9 is_normal_capture_proxy(tree_node*)
> >>         /home/marxin/Programming/gcc/gcc/cp/lambda.c:274
> >> 0x9c1fe6 mark_use(tree_node*, bool, bool, unsigned int, bool)
> >>         /home/marxin/Programming/gcc/gcc/cp/expr.c:114
> >> 0x89d9ab convert_like_real
> >>         /home/marxin/Programming/gcc/gcc/cp/call.c:6905
> >>
> >> where:
> >>
> >> (gdb) p debug_tree(decl)
> >>  <var_decl 0x7ffff7fee870 __PRETTY_FUNCTION__
> >>     type <pointer_type 0x7ffff69b4540
> >>         type <integer_type 0x7ffff69b4498 char readonly unsigned string-flag type_6 QI
> >>             size <integer_cst 0x7ffff698df78 constant 8>
> >>             unit-size <integer_cst 0x7ffff698df90 constant 1>
> >>             align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b4498 precision:8 min <integer_cst 0x7ffff698dfd8 0> max <integer_cst 0x7ffff698dfc0 255>
> >>             pointer_to_this <pointer_type 0x7ffff69b4540>>
> >>         unsigned type_6 DI
> >>         size <integer_cst 0x7ffff698de88 constant 64>
> >>         unit-size <integer_cst 0x7ffff698dea0 constant 8>
> >>         align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7ffff69b4540>
> >>     readonly used tree_1 tree_2 unsigned read decl_6 DI /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/ext/pretty2.C:36:10 size <integer_cst 0x7ffff698de88 64> unit-size <integer_cst 0x7ffff698dea0 8>
> >>     align:64 warn_if_not_align:0
> >>     value-expr <nop_expr 0x7ffff67b69a0 type <pointer_type 0x7ffff69b4540>
> >>         constant
> >>         arg:0 <addr_expr 0x7ffff67b6980 type <pointer_type 0x7ffff67c3738>
> >>             constant
> >>             arg:0 <string_cst 0x7ffff67b6960 type <array_type 0x7ffff67c3690>
> >>                 constant "top level\000">>> chain <var_decl 0x7ffff7fee7e0 a>>
> >>
> >> Can please a C++ maintainer help me with that? Having that I hope I can finish the patch.
> >>
> >> Thank you,
> >> Martin
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts (PR c++/64266).
  2018-10-23  9:11                     ` [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts (PR c++/64266) Martin Liška
  2018-10-23 12:20                       ` Richard Biener
@ 2018-10-24 18:55                       ` Jason Merrill
  2018-10-26  7:36                         ` Martin Liška
  1 sibling, 1 reply; 24+ messages in thread
From: Jason Merrill @ 2018-10-24 18:55 UTC (permalink / raw)
  To: Martin Liška
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, gcc-patches List,
	Segher Boessenkool, wilson, Nathan Sidwell

On Tue, Oct 23, 2018 at 4:59 AM Martin Liška <mliska@suse.cz> wrote:
> However, I still see some minor ICEs, it's probably related to decay_conversion in cp_fname_init:
>
> 1) ./xg++ -B. /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C
>
> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C:6:17: internal compiler error: Segmentation fault
> 6 |     [] { return __func__; }();
>   |                 ^~~~~~~~
> 0x1344568 crash_signal
>         /home/marxin/Programming/gcc/gcc/toplev.c:325
> 0x7ffff6bc310f ???
>         /usr/src/debug/glibc-2.27-6.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
> 0x9db134 is_capture_proxy(tree_node*)

The problem in both tests is that is_capture_proxy thinks your
__func__ VAR_DECL with DECL_VALUE_EXPR is a capture proxy, since it is
neither an anonymous union proxy nor a structured binding.

The standard says,

The function-local predefined variable __func__ is defined as if a
definition of the form
   static const char __func__[] = "function-name ";
had been provided, where function-name is an implementation-defined
string. It is unspecified whether such a variable has an address
distinct from that of any other object in the program.

So changing the type of __func__ (from array to pointer) still breaks
conformance.  And we need to keep the type checks from pretty4.C, even
though the checks for strings being distinct need to go.

Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts (PR c++/64266).
  2018-10-24 18:55                       ` Jason Merrill
@ 2018-10-26  7:36                         ` Martin Liška
  2018-10-29 14:22                           ` Jason Merrill
  0 siblings, 1 reply; 24+ messages in thread
From: Martin Liška @ 2018-10-26  7:36 UTC (permalink / raw)
  To: Jason Merrill
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, gcc-patches List,
	Segher Boessenkool, wilson, Nathan Sidwell

[-- Attachment #1: Type: text/plain, Size: 1932 bytes --]

On 10/24/18 7:24 PM, Jason Merrill wrote:
> On Tue, Oct 23, 2018 at 4:59 AM Martin Liška <mliska@suse.cz> wrote:
>> However, I still see some minor ICEs, it's probably related to decay_conversion in cp_fname_init:
>>
>> 1) ./xg++ -B. /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C
>>
>> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C:6:17: internal compiler error: Segmentation fault
>> 6 |     [] { return __func__; }();
>>   |                 ^~~~~~~~
>> 0x1344568 crash_signal
>>         /home/marxin/Programming/gcc/gcc/toplev.c:325
>> 0x7ffff6bc310f ???
>>         /usr/src/debug/glibc-2.27-6.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
>> 0x9db134 is_capture_proxy(tree_node*)

Hi.

> 
> The problem in both tests is that is_capture_proxy thinks your
> __func__ VAR_DECL with DECL_VALUE_EXPR is a capture proxy, since it is
> neither an anonymous union proxy nor a structured binding.

I see, however I'm a rookie in area of C++ FE. Would it be solvable this problem
with lambdas?

> 
> The standard says,
> 
> The function-local predefined variable __func__ is defined as if a
> definition of the form
>    static const char __func__[] = "function-name ";
> had been provided, where function-name is an implementation-defined
> string. It is unspecified whether such a variable has an address
> distinct from that of any other object in the program.
> 
> So changing the type of __func__ (from array to pointer) still breaks
> conformance.  And we need to keep the type checks from pretty4.C, even
> though the checks for strings being distinct need to go.

I added following patch which puts back type to const char[] (instead of char *)
and I made the variable static. Now I see pretty4.C testcase passing again.
To be honest I'm not convinced about the FE changes, so a help would
be appreciated.

Thanks,
Martin

> 
> Jason
> 


[-- Attachment #2: 0001-Fix-back.patch --]
[-- Type: text/x-patch, Size: 1026 bytes --]

From fd2e13b23e0bbbb7a7b02025432843782e1ab579 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 25 Oct 2018 18:12:10 +0200
Subject: [PATCH] Fix back.

---
 gcc/cp/decl.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 9624df081e4..74ad871b3f4 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -4445,15 +4445,13 @@ cp_fname_init (const char* name, tree *type_p)
   type = cp_build_qualified_type (char_type_node, TYPE_QUAL_CONST);
   type = build_cplus_array_type (type, domain);
 
-  *type_p = type_decays_to (type);
+  *type_p = type;
 
   if (init)
     TREE_TYPE (init) = type;
   else
     init = error_mark_node;
 
-  init = decay_conversion (init, tf_warning_or_error);
-
   return init;
 }
 
@@ -4482,6 +4480,7 @@ cp_make_fname_decl (location_t loc, tree id, int type_dep)
   TREE_READONLY (decl) = 1;
   DECL_ARTIFICIAL (decl) = 1;
   DECL_DECLARED_CONSTEXPR_P (decl) = 1;
+  TREE_STATIC (decl) = 1;
 
   TREE_USED (decl) = 1;
 
-- 
2.19.0


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts (PR c++/64266).
  2018-10-26  7:36                         ` Martin Liška
@ 2018-10-29 14:22                           ` Jason Merrill
  2018-10-31  6:27                             ` Jason Merrill
  0 siblings, 1 reply; 24+ messages in thread
From: Jason Merrill @ 2018-10-29 14:22 UTC (permalink / raw)
  To: Martin Liška
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, gcc-patches List,
	Segher Boessenkool, wilson, Nathan Sidwell

On Fri, Oct 26, 2018 at 3:14 AM Martin Liška <mliska@suse.cz> wrote:
> On 10/24/18 7:24 PM, Jason Merrill wrote:
> > On Tue, Oct 23, 2018 at 4:59 AM Martin Liška <mliska@suse.cz> wrote:
> >> However, I still see some minor ICEs, it's probably related to decay_conversion in cp_fname_init:
> >>
> >> 1) ./xg++ -B. /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C
> >>
> >> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C:6:17: internal compiler error: Segmentation fault
> >> 6 |     [] { return __func__; }();
> >>   |                 ^~~~~~~~
> >> 0x1344568 crash_signal
> >>         /home/marxin/Programming/gcc/gcc/toplev.c:325
> >> 0x7ffff6bc310f ???
> >>         /usr/src/debug/glibc-2.27-6.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
> >> 0x9db134 is_capture_proxy(tree_node*)
>
> Hi.
>
> >
> > The problem in both tests is that is_capture_proxy thinks your
> > __func__ VAR_DECL with DECL_VALUE_EXPR is a capture proxy, since it is
> > neither an anonymous union proxy nor a structured binding.
>
> I see, however I'm a rookie in area of C++ FE. Would it be solvable this problem
> with lambdas?
>
> >
> > The standard says,
> >
> > The function-local predefined variable __func__ is defined as if a
> > definition of the form
> >    static const char __func__[] = "function-name ";
> > had been provided, where function-name is an implementation-defined
> > string. It is unspecified whether such a variable has an address
> > distinct from that of any other object in the program.
> >
> > So changing the type of __func__ (from array to pointer) still breaks
> > conformance.  And we need to keep the type checks from pretty4.C, even
> > though the checks for strings being distinct need to go.
>
> I added following patch which puts back type to const char[] (instead of char *)
> and I made the variable static. Now I see pretty4.C testcase passing again.
> To be honest I'm not convinced about the FE changes, so a help would
> be appreciated.

OK, I'll poke at it.

Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts (PR c++/64266).
  2018-10-29 14:22                           ` Jason Merrill
@ 2018-10-31  6:27                             ` Jason Merrill
  2018-11-01  9:16                               ` Martin Liška
  0 siblings, 1 reply; 24+ messages in thread
From: Jason Merrill @ 2018-10-31  6:27 UTC (permalink / raw)
  To: Martin Liška
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, gcc-patches List,
	Segher Boessenkool, wilson, Nathan Sidwell

[-- Attachment #1: Type: text/plain, Size: 2377 bytes --]

On 10/29/18 9:37 AM, Jason Merrill wrote:
> On Fri, Oct 26, 2018 at 3:14 AM Martin Liška <mliska@suse.cz> wrote:
>> On 10/24/18 7:24 PM, Jason Merrill wrote:
>>> On Tue, Oct 23, 2018 at 4:59 AM Martin Liška <mliska@suse.cz> wrote:
>>>> However, I still see some minor ICEs, it's probably related to decay_conversion in cp_fname_init:
>>>>
>>>> 1) ./xg++ -B. /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C
>>>>
>>>> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C:6:17: internal compiler error: Segmentation fault
>>>> 6 |     [] { return __func__; }();
>>>>    |                 ^~~~~~~~
>>>> 0x1344568 crash_signal
>>>>          /home/marxin/Programming/gcc/gcc/toplev.c:325
>>>> 0x7ffff6bc310f ???
>>>>          /usr/src/debug/glibc-2.27-6.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
>>>> 0x9db134 is_capture_proxy(tree_node*)
>>
>> Hi.
>>
>>>
>>> The problem in both tests is that is_capture_proxy thinks your
>>> __func__ VAR_DECL with DECL_VALUE_EXPR is a capture proxy, since it is
>>> neither an anonymous union proxy nor a structured binding.
>>
>> I see, however I'm a rookie in area of C++ FE. Would it be solvable this problem
>> with lambdas?
>>
>>>
>>> The standard says,
>>>
>>> The function-local predefined variable __func__ is defined as if a
>>> definition of the form
>>>     static const char __func__[] = "function-name ";
>>> had been provided, where function-name is an implementation-defined
>>> string. It is unspecified whether such a variable has an address
>>> distinct from that of any other object in the program.
>>>
>>> So changing the type of __func__ (from array to pointer) still breaks
>>> conformance.  And we need to keep the type checks from pretty4.C, even
>>> though the checks for strings being distinct need to go.
>>
>> I added following patch which puts back type to const char[] (instead of char *)
>> and I made the variable static. Now I see pretty4.C testcase passing again.
>> To be honest I'm not convinced about the FE changes, so a help would
>> be appreciated.
> 
> OK, I'll poke at it.

This further patch teaches is_capture_proxy about function-name 
variables, and changes the handling of __PRETTY_FUNCTION__ in template 
instantiations to create new variables rather than instantiations.

The C++ changes are OK with this.


[-- Attachment #2: fname.diff --]
[-- Type: text/x-patch, Size: 4747 bytes --]

commit 100f6f38f9d8bbb5d77ba7a4ccb86a4c85aa2cd9
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Oct 29 22:35:59 2018 -0400

    fname-p

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 8454cb4e178..f1a10297e79 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -3122,6 +3122,14 @@ struct GTY(()) lang_decl {
   (DECL_NAME (NODE) \
    && id_equal (DECL_NAME (NODE), "__PRETTY_FUNCTION__"))
 
+/* For a DECL, true if it is __func__ or similar.  */
+#define DECL_FNAME_P(NODE)					\
+  (VAR_P (NODE) && DECL_NAME (NODE) && DECL_ARTIFICIAL (NODE)	\
+   && DECL_HAS_VALUE_EXPR_P (NODE)				\
+   && (id_equal (DECL_NAME (NODE), "__PRETTY_FUNCTION__")	\
+       || id_equal (DECL_NAME (NODE), "__FUNCTION__")		\
+       || id_equal (DECL_NAME (NODE), "__func__")))
+
 /* Nonzero if the variable was declared to be thread-local.
    We need a special C++ version of this test because the middle-end
    DECL_THREAD_LOCAL_P uses the symtab, so we can't use it for
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index cce9cc99ff1..bde8023ce3b 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -4465,7 +4465,7 @@ cp_fname_init (const char* name, tree *type_p)
 static tree
 cp_make_fname_decl (location_t loc, tree id, int type_dep)
 {
-  const char *const name = (type_dep && processing_template_decl
+  const char *const name = (type_dep && in_template_function ()
 			    ? NULL : fname_as_string (type_dep));
   tree type;
   tree init = cp_fname_init (name, &type);
@@ -7064,8 +7064,9 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
 	    init = NULL_TREE;
 	  release_tree_vector (cleanups);
 	}
-      else if (!DECL_PRETTY_FUNCTION_P (decl))
+      else
 	{
+	  gcc_assert (!DECL_PRETTY_FUNCTION_P (decl));
 	  /* Deduce array size even if the initializer is dependent.  */
 	  maybe_deduce_size_from_array_init (decl, init);
 	  /* And complain about multiple initializers.  */
diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c
index 297327f1ab6..318671bbcd0 100644
--- a/gcc/cp/lambda.c
+++ b/gcc/cp/lambda.c
@@ -262,6 +262,7 @@ is_capture_proxy (tree decl)
 	  && DECL_HAS_VALUE_EXPR_P (decl)
 	  && !DECL_ANON_UNION_VAR_P (decl)
 	  && !DECL_DECOMPOSITION_P (decl)
+	  && !DECL_FNAME_P (decl)
 	  && LAMBDA_FUNCTION_P (DECL_CONTEXT (decl)));
 }
 
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 6975027076e..510264d38a0 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -16702,6 +16702,10 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 	    register_local_specialization (inst, decl);
 	    break;
 	  }
+	else if (DECL_PRETTY_FUNCTION_P (decl))
+	  decl = make_fname_decl (DECL_SOURCE_LOCATION (decl),
+				  DECL_NAME (decl),
+				  true/*DECL_PRETTY_FUNCTION_P (decl)*/);
 	else if (DECL_IMPLICIT_TYPEDEF_P (decl)
 		 && LAMBDA_TYPE_P (TREE_TYPE (decl)))
 	  /* Don't copy the old closure; we'll create a new one in
@@ -16746,19 +16750,6 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 		    DECL_CONTEXT (decl) = current_function_decl;
 		    cp_check_omp_declare_reduction (decl);
 		  }
-		else if (VAR_P (decl)
-			 && DECL_PRETTY_FUNCTION_P (decl))
-		  {
-		    /* For __PRETTY_FUNCTION__ we have to adjust the
-		       initializer.  */
-		    const char *const name
-		      = cxx_printable_name (current_function_decl, 2);
-		    init = cp_fname_init (name, &TREE_TYPE (decl));
-		    SET_DECL_VALUE_EXPR (decl, init);
-		    DECL_HAS_VALUE_EXPR_P (decl) = 1;
-		    DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = 1;
-		    maybe_push_decl (decl);
-		  }
 		else
 		  {
 		    int const_init = false;
diff --git a/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C b/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C
new file mode 100644
index 00000000000..eb5d0de5cc6
--- /dev/null
+++ b/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C
@@ -0,0 +1,39 @@
+// { dg-do run  }
+// Copyright (C) 2000 Free Software Foundation, Inc.
+// Contributed by Nathan Sidwell 3 Mar 2000 <nathan@codesourcery.com>
+
+// __PRETTY_FUNCTION__, __FUNCTION__ and __function__ should have the
+// type char const [X], where X is the right value for that particular function
+
+static int fail;
+
+void unover (char const (*)[5]) {}
+void foo (char const (*)[5]) {}
+void foo (void *) {fail = 1;}
+void foo (void const *) {fail = 1;}
+void baz (char const (&)[5]) {}
+
+void baz ()
+{
+  static void const *ptr = __FUNCTION__;
+  // all uses should be the same.
+  if (ptr != __FUNCTION__)
+    fail = 1;
+}
+
+int main ()
+{
+  // make sure we actually emit the VAR_DECL when needed, and things have the
+  // expected type.
+  foo (&__FUNCTION__);
+  baz (__FUNCTION__);
+  unover (&__FUNCTION__);
+  if (fail)
+    return 1;
+  
+  baz ();
+  if (fail)
+    return 1;
+  
+  return 0;
+}

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts (PR c++/64266).
  2018-10-31  6:27                             ` Jason Merrill
@ 2018-11-01  9:16                               ` Martin Liška
  0 siblings, 0 replies; 24+ messages in thread
From: Martin Liška @ 2018-11-01  9:16 UTC (permalink / raw)
  To: Jason Merrill
  Cc: Jakub Jelinek, Pedro Alves, Jan Kratochvil, gcc-patches List,
	Segher Boessenkool, wilson, Nathan Sidwell

[-- Attachment #1: Type: text/plain, Size: 2698 bytes --]

On 10/31/18 2:42 AM, Jason Merrill wrote:
> On 10/29/18 9:37 AM, Jason Merrill wrote:
>> On Fri, Oct 26, 2018 at 3:14 AM Martin Liška <mliska@suse.cz> wrote:
>>> On 10/24/18 7:24 PM, Jason Merrill wrote:
>>>> On Tue, Oct 23, 2018 at 4:59 AM Martin Liška <mliska@suse.cz> wrote:
>>>>> However, I still see some minor ICEs, it's probably related to decay_conversion in cp_fname_init:
>>>>>
>>>>> 1) ./xg++ -B. /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C
>>>>>
>>>>> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-__func__2.C:6:17: internal compiler error: Segmentation fault
>>>>> 6 |     [] { return __func__; }();
>>>>>    |                 ^~~~~~~~
>>>>> 0x1344568 crash_signal
>>>>>          /home/marxin/Programming/gcc/gcc/toplev.c:325
>>>>> 0x7ffff6bc310f ???
>>>>>          /usr/src/debug/glibc-2.27-6.1.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
>>>>> 0x9db134 is_capture_proxy(tree_node*)
>>>
>>> Hi.
>>>
>>>>
>>>> The problem in both tests is that is_capture_proxy thinks your
>>>> __func__ VAR_DECL with DECL_VALUE_EXPR is a capture proxy, since it is
>>>> neither an anonymous union proxy nor a structured binding.
>>>
>>> I see, however I'm a rookie in area of C++ FE. Would it be solvable this problem
>>> with lambdas?
>>>
>>>>
>>>> The standard says,
>>>>
>>>> The function-local predefined variable __func__ is defined as if a
>>>> definition of the form
>>>>     static const char __func__[] = "function-name ";
>>>> had been provided, where function-name is an implementation-defined
>>>> string. It is unspecified whether such a variable has an address
>>>> distinct from that of any other object in the program.
>>>>
>>>> So changing the type of __func__ (from array to pointer) still breaks
>>>> conformance.  And we need to keep the type checks from pretty4.C, even
>>>> though the checks for strings being distinct need to go.
>>>
>>> I added following patch which puts back type to const char[] (instead of char *)
>>> and I made the variable static. Now I see pretty4.C testcase passing again.
>>> To be honest I'm not convinced about the FE changes, so a help would
>>> be appreciated.
>>
>> OK, I'll poke at it.
> 
> This further patch teaches is_capture_proxy about function-name variables, and changes the handling of __PRETTY_FUNCTION__ in template instantiations to create new variables rather than instantiations.
> 
> The C++ changes are OK with this.
> 

Thank you very much Jason!
There's final version of the patch that I'm going to install.

Martin

[-- Attachment #2: 0001-Make-__PRETTY_FUNCTION__-like-functions-mergeable-st.patch --]
[-- Type: text/x-patch, Size: 8801 bytes --]

From 077608ee88e277d569851969babe0f2fcf20fbf2 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 4 Oct 2018 15:34:51 +0200
Subject: [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts
 (PR c++/64266).

gcc/cp/ChangeLog:

2018-10-31  Martin Liska  <mliska@suse.cz>
	    Jason Merrill  <jason@redhat.com>

	PR c++/64266
	PR bootstrap/70422
	PR ipa/81277
	* cp-tree.h (DECL_FNAME_P): New macro.
	* decl.c (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
        DECL_VALUE_EXPR, DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P.
	(cp_finish_decl):
	* lambda.c (is_capture_proxy): Use DECL_FNAME_P.
	* pt.c (tsubst_expr): Handle DECL_PRETTY_FUNCTION_P.

gcc/testsuite/ChangeLog:

2018-10-31  Martin Liska  <mliska@suse.cz>
	    Jason Merrill  <jason@redhat.com>

	PR c++/64266
	PR bootstrap/70422
	PR ipa/81277
	* g++.dg/cpp0x/constexpr-__func__2.C: Make it a compilation
	test.
	* g++.old-deja/g++.ext/pretty4.C: Remove as the run-time
	assumptions are not longer valid.
---
 gcc/cp/cp-tree.h                              |  8 ++
 gcc/cp/decl.c                                 | 25 ++++--
 gcc/cp/lambda.c                               |  1 +
 gcc/cp/pt.c                                   | 16 ++--
 .../g++.dg/cpp0x/constexpr-__func__2.C        |  7 +-
 gcc/testsuite/g++.old-deja/g++.ext/pretty4.C  | 85 -------------------
 6 files changed, 36 insertions(+), 106 deletions(-)
 delete mode 100644 gcc/testsuite/g++.old-deja/g++.ext/pretty4.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 03e88838cbe..42449f10a48 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -3132,6 +3132,14 @@ struct GTY(()) lang_decl {
   (DECL_NAME (NODE) \
    && id_equal (DECL_NAME (NODE), "__PRETTY_FUNCTION__"))
 
+/* For a DECL, true if it is __func__ or similar.  */
+#define DECL_FNAME_P(NODE)					\
+  (VAR_P (NODE) && DECL_NAME (NODE) && DECL_ARTIFICIAL (NODE)	\
+   && DECL_HAS_VALUE_EXPR_P (NODE)				\
+   && (id_equal (DECL_NAME (NODE), "__PRETTY_FUNCTION__")	\
+       || id_equal (DECL_NAME (NODE), "__FUNCTION__")		\
+       || id_equal (DECL_NAME (NODE), "__func__")))
+
 /* Nonzero if the variable was declared to be thread-local.
    We need a special C++ version of this test because the middle-end
    DECL_THREAD_LOCAL_P uses the symtab, so we can't use it for
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 11320b65e71..1cea5262b62 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -4465,7 +4465,7 @@ cp_fname_init (const char* name, tree *type_p)
 static tree
 cp_make_fname_decl (location_t loc, tree id, int type_dep)
 {
-  const char *const name = (type_dep && processing_template_decl
+  const char *const name = (type_dep && in_template_function ()
 			    ? NULL : fname_as_string (type_dep));
   tree type;
   tree init = cp_fname_init (name, &type);
@@ -4474,23 +4474,35 @@ cp_make_fname_decl (location_t loc, tree id, int type_dep)
   if (name)
     free (CONST_CAST (char *, name));
 
-  TREE_STATIC (decl) = 1;
+  /* As we're using pushdecl_with_scope, we must set the context.  */
+  DECL_CONTEXT (decl) = current_function_decl;
+
   TREE_READONLY (decl) = 1;
   DECL_ARTIFICIAL (decl) = 1;
+  DECL_DECLARED_CONSTEXPR_P (decl) = 1;
+  TREE_STATIC (decl) = 1;
 
   TREE_USED (decl) = 1;
 
+  if (init)
+    {
+      SET_DECL_VALUE_EXPR (decl, init);
+      DECL_HAS_VALUE_EXPR_P (decl) = 1;
+      /* For decl_constant_var_p.  */
+      DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = 1;
+    }
+
   if (current_function_decl)
     {
       DECL_CONTEXT (decl) = current_function_decl;
       decl = pushdecl_outermost_localscope (decl);
-      cp_finish_decl (decl, init, /*init_const_expr_p=*/false, NULL_TREE,
-		      LOOKUP_ONLYCONVERTING);
+      if (decl != error_mark_node)
+	add_decl_expr (decl);
     }
   else
     {
       DECL_THIS_STATIC (decl) = true;
-      pushdecl_top_level_and_finish (decl, init);
+      pushdecl_top_level_and_finish (decl, NULL_TREE);
     }
 
   return decl;
@@ -7052,8 +7064,9 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
 	    init = NULL_TREE;
 	  release_tree_vector (cleanups);
 	}
-      else if (!DECL_PRETTY_FUNCTION_P (decl))
+      else
 	{
+	  gcc_assert (!DECL_PRETTY_FUNCTION_P (decl));
 	  /* Deduce array size even if the initializer is dependent.  */
 	  maybe_deduce_size_from_array_init (decl, init);
 	  /* And complain about multiple initializers.  */
diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c
index 297327f1ab6..318671bbcd0 100644
--- a/gcc/cp/lambda.c
+++ b/gcc/cp/lambda.c
@@ -262,6 +262,7 @@ is_capture_proxy (tree decl)
 	  && DECL_HAS_VALUE_EXPR_P (decl)
 	  && !DECL_ANON_UNION_VAR_P (decl)
 	  && !DECL_DECOMPOSITION_P (decl)
+	  && !DECL_FNAME_P (decl)
 	  && LAMBDA_FUNCTION_P (DECL_CONTEXT (decl)));
 }
 
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index fc6cf989501..2dc0cb1629c 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -16735,6 +16735,10 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 	    register_local_specialization (inst, decl);
 	    break;
 	  }
+	else if (DECL_PRETTY_FUNCTION_P (decl))
+	  decl = make_fname_decl (DECL_SOURCE_LOCATION (decl),
+				  DECL_NAME (decl),
+				  true/*DECL_PRETTY_FUNCTION_P (decl)*/);
 	else if (DECL_IMPLICIT_TYPEDEF_P (decl)
 		 && LAMBDA_TYPE_P (TREE_TYPE (decl)))
 	  /* Don't copy the old closure; we'll create a new one in
@@ -16793,17 +16797,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 						   complain, in_decl, &first,
 						   &cnt);
 
-		    if (VAR_P (decl)
-			&& DECL_PRETTY_FUNCTION_P (decl))
-		      {
-			/* For __PRETTY_FUNCTION__ we have to adjust the
-			   initializer.  */
-			const char *const name
-			  = cxx_printable_name (current_function_decl, 2);
-			init = cp_fname_init (name, &TREE_TYPE (decl));
-		      }
-		    else
-		      init = tsubst_init (init, decl, args, complain, in_decl);
+		    init = tsubst_init (init, decl, args, complain, in_decl);
 
 		    if (VAR_P (decl))
 		      const_init = (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C
index e6782905423..673fb4f3a93 100644
--- a/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C
@@ -1,5 +1,5 @@
 // PR c++/70353
-// { dg-do link { target c++11 } }
+// { dg-do compile { target c++11 } }
 
 constexpr const char* ce ()
 {
@@ -8,6 +8,5 @@ constexpr const char* ce ()
 
 const char *c = ce();
 
-int main()
-{
-}
+#define SA(X) static_assert((X),#X)
+SA(ce()[0] == 'c');
diff --git a/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C b/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C
deleted file mode 100644
index 9017d567132..00000000000
--- a/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C
+++ /dev/null
@@ -1,85 +0,0 @@
-// { dg-do run  }
-// Copyright (C) 2000 Free Software Foundation, Inc.
-// Contributed by Nathan Sidwell 3 Mar 2000 <nathan@codesourcery.com>
-
-// __PRETTY_FUNCTION__, __FUNCTION__ and __function__ should have the
-// type char const [X], where X is the right value for that particular function
-
-static void const *strings[4];
-static void const *tpls[4];
-static unsigned pos = 0;
-static int fail;
-static void const *ptr = 0;
-
-void unover (char const (*)[5]) {}
-void foo (char const (*)[5]) {}
-void foo (void *) {fail = 1;}
-void foo (void const *) {fail = 1;}
-void baz (char const (&)[5]) {}
-
-template<unsigned I> void PV (char const (&objRef)[I])
-{
-  strings[pos] = objRef;
-  tpls[pos] = __PRETTY_FUNCTION__;
-  pos++;
-}
-
-void fn ()
-{
-  PV (__FUNCTION__);
-  PV (__func__);
-  PV (__PRETTY_FUNCTION__);
-  PV ("wibble");
-}
-
-void baz ()
-{
-  ptr = __FUNCTION__;
-  // there should be no string const merging
-  if (ptr == "baz")
-    fail = 1;
-  // but all uses should be the same.
-  if (ptr != __FUNCTION__)
-    fail = 1;
-}
-int baz (int)
-{
-  return ptr == __FUNCTION__;
-}
-
-int main ()
-{
-  // make sure we actually emit the VAR_DECL when needed, and things have the
-  // expected type.
-  foo (&__FUNCTION__);
-  baz (__FUNCTION__);
-  unover (&__FUNCTION__);
-  if (fail)
-    return 1;
-  
-  // __FUNCTION__ should be unique across functions with the same base name
-  // (it's a local static, _not_ a string).
-  baz ();
-  if (fail)
-    return 1;
-  if (baz (1))
-    return 1;
-  fn ();
-  
-  // Check the names of fn. They should all be distinct strings (though two
-  // will have the same value).
-  if (strings[0] == strings[1])
-    return 1;
-  if (strings[0] == strings[2])
-    return 1;
-  if (strings[1] == strings[2])
-    return 1;
-
-  // check the names of the template functions so invoked
-  if (tpls[0] != tpls[1])
-    return 1;
-  if (tpls[0] == tpls[2])
-    return 1;
-  
-  return 0;
-}
-- 
2.19.0


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2018-11-01  9:16 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-25 11:58 [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry Martin Liška
2017-04-25 12:08 ` Jakub Jelinek
2017-04-26 11:48   ` Martin Liška
2017-05-01 19:13     ` Jason Merrill
2017-07-14  8:35       ` Martin Liška
2017-07-14 20:00         ` Jim Wilson
2017-07-15  6:11           ` Jim Wilson
2017-07-27 12:56         ` Martin Liška
2017-08-10  8:57           ` Martin Liška
2017-08-10 20:23         ` Jason Merrill
2017-09-14  9:22           ` Martin Liška
2017-10-24 20:26             ` Jason Merrill
2018-05-21 13:49               ` Martin Liška
2018-05-21 17:59                 ` Jason Merrill
2018-06-04 12:11                   ` Martin Liška
2018-10-23  9:11                     ` [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts (PR c++/64266) Martin Liška
2018-10-23 12:20                       ` Richard Biener
2018-10-24 14:19                         ` Martin Liška
2018-10-24 14:26                           ` Richard Biener
2018-10-24 18:55                       ` Jason Merrill
2018-10-26  7:36                         ` Martin Liška
2018-10-29 14:22                           ` Jason Merrill
2018-10-31  6:27                             ` Jason Merrill
2018-11-01  9:16                               ` Martin Liška

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).