Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.

["Martin Liška" <mliska@suse.cz>]

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