public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Martin Liška" <mliska@suse.cz>
To: Richard Biener <richard.guenther@gmail.com>
Cc: Jason Merrill <jason@redhat.com>,
	Jakub Jelinek <jakub@redhat.com>, Pedro Alves <palves@redhat.com>,
	Jan Kratochvil <jan.kratochvil@redhat.com>,
	GCC Patches <gcc-patches@gcc.gnu.org>,
	Segher Boessenkool <segher@kernel.crashing.org>,
	wilson@gcc.gnu.org, Nathan Sidwell <nathan@acm.org>
Subject: Re: [PATCH] Make __PRETTY_FUNCTION__-like functions mergeable string csts (PR c++/64266).
Date: Wed, 24 Oct 2018 14:19:00 -0000	[thread overview]
Message-ID: <1f34660a-0162-99bc-497d-745fcd369b91@suse.cz> (raw)
In-Reply-To: <CAFiYyc263NE3DnEk=sqJAn7TtXPuOp6nNMpimKbvSq=WsFgTZg@mail.gmail.com>

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

  reply	other threads:[~2018-10-24 13:34 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1f34660a-0162-99bc-497d-745fcd369b91@suse.cz \
    --to=mliska@suse.cz \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=jan.kratochvil@redhat.com \
    --cc=jason@redhat.com \
    --cc=nathan@acm.org \
    --cc=palves@redhat.com \
    --cc=richard.guenther@gmail.com \
    --cc=segher@kernel.crashing.org \
    --cc=wilson@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).