PR target/35514: Gcc shoud generate symbol type for undefined symbol

On Sun, Jul 5, 2015 at 2:54 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sun, Jul 5, 2015 at 11:14 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> Update default_elf_asm_output_external to also output symbol type to
>> help ELF linker to properly issue diagnostic message.  We don't output
>> symbol type for reference to external TLS symbol since assembler will
>> generate TLS symbol type based on TLS relocation and Solaris assembler
>> only supports the @tls_obj type directive, not the @tls_object type
>> directive used by GNU assmbler, which doesn't understand the @tls_obj
>> type directive.
>>
>> Tested on Linux/x86-64.  OK for trunk?
>>
>> Thanks.
>>
>>
>> H.J.
>> ---
>> gcc/
>>
>>         PR target/35514
>>         * varasm.c (default_elf_asm_output_external): Also output symbol
>>         type.
>>
>> gcc/testsuite/
>>
>>         PR target/35514
>>         * lib/target-supports.exp (check_effective_target_elf): New.
>>         * gcc.dg/pr35514-1.c: New file.
>>         * gcc.dg/pr35514-2.c: Likewise.
>> ---
>>  gcc/testsuite/gcc.dg/pr35514-1.c      | 16 ++++++++++++++++
>>  gcc/testsuite/gcc.dg/pr35514-2.c      | 14 ++++++++++++++
>>  gcc/testsuite/lib/target-supports.exp | 20 ++++++++++++++++++++
>>  gcc/varasm.c                          | 22 +++++++++++++++++-----
>>  4 files changed, 67 insertions(+), 5 deletions(-)
>>  create mode 100644 gcc/testsuite/gcc.dg/pr35514-1.c
>>  create mode 100644 gcc/testsuite/gcc.dg/pr35514-2.c
>>
>> diff --git a/gcc/testsuite/gcc.dg/pr35514-1.c b/gcc/testsuite/gcc.dg/pr35514-1.c
>> new file mode 100644
>> index 0000000..8a1d99d
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/pr35514-1.c
>> @@ -0,0 +1,16 @@
>> +/* { dg-do compile { target { elf } } } */
>> +/* { dg-options "-O2" } */
>> +/* { dg-final { scan-assembler ".type\[^\\n\]*object, (%|@)object" } } */
>> +/* { dg-final { scan-assembler ".type\[^\\n\]*function, (%|@)function" } } */
>> +
>> +/* Verify that we generate proper symbol types for external reference.  */
>> +
>> +extern int object;
>> +void function (void);
>> +
>> +int
>> +foo (void)
>> +{
>> +  function ();
>> +  return object;
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/pr35514-2.c b/gcc/testsuite/gcc.dg/pr35514-2.c
>> new file mode 100644
>> index 0000000..f67af5a
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/pr35514-2.c
>> @@ -0,0 +1,14 @@
>> +/* { dg-do compile { target { elf } } } */
>> +/* { dg-require-effective-target tls } */
>> +/* { dg-options "-O2" } */
>> +/* { dg-final { scan-assembler-not ".type\[^\\n\]*object, (%|@)object" } } */
>> +
>> +/* Verify that we generate proper symbol types for external reference.  */
>> +
>> +extern __thread int object;
>> +
>> +int
>> +foo (void)
>> +{
>> +  return object;
>> +}
>> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
>> index bf512e9..c3ec209 100644
>> --- a/gcc/testsuite/lib/target-supports.exp
>> +++ b/gcc/testsuite/lib/target-supports.exp
>> @@ -6349,3 +6349,23 @@ proc check_effective_target_comdat_group {} {
>>         int (*fn) () = foo;
>>      }]
>>  }
>> +
>> +# Returns 1 if the target is ELF.
>> +
>> +proc check_effective_target_elf { } {
>> +    global check_effective_target_elf_saved
>> +    global tool
>> +
>> +    if [info exists check_effective_target_elf_saved] {
>> +        verbose "check_effective_target_elf returning saved $check_effective_target_elf_saved" 2
>> +    } else {
>> +       set objformat [gcc_target_object_format]
>> +       if { $objformat == "elf" } {
>> +           set check_effective_target_elf_saved 1
>> +       } else {
>> +           set check_effective_target_elf_saved 0
>> +       }
>> +        verbose "check_effective_target_elf returning $check_effective_target_elf_saved" 2
>> +    }
>> +    return $check_effective_target_elf_saved
>> +}
>> diff --git a/gcc/varasm.c b/gcc/varasm.c
>> index 3e76032..12f36f3 100644
>> --- a/gcc/varasm.c
>> +++ b/gcc/varasm.c
>> @@ -7509,7 +7509,7 @@ elf_record_gcc_switches (print_switch_type type, const char * name)
>>  }
>>
>>  /* Emit text to declare externally defined symbols. It is needed to
>> -   properly support non-default visibility.  */
>> +   properly support non-default visibility and specify symbol type.  */
>>  void
>>  default_elf_asm_output_external (FILE *file ATTRIBUTE_UNUSED,
>>                                  tree decl,
>> @@ -7518,10 +7518,22 @@ default_elf_asm_output_external (FILE *file ATTRIBUTE_UNUSED,
>>    /* We output the name if and only if TREE_SYMBOL_REFERENCED is
>>       set in order to avoid putting out names that are never really
>>       used.  Always output visibility specified in the source.  */
>> -  if (TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl))
>> -      && (DECL_VISIBILITY_SPECIFIED (decl)
>> -         || targetm.binds_local_p (decl)))
>> -    maybe_assemble_visibility (decl);
>> +  if (TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl)))
>> +    {
>> +      if (DECL_VISIBILITY_SPECIFIED (decl)
>> +         || targetm.binds_local_p (decl))
>> +       maybe_assemble_visibility (decl);
>> +
>> +#ifdef ASM_OUTPUT_TYPE_DIRECTIVE
>> +      if (TREE_CODE (decl) == FUNCTION_DECL)
>> +       ASM_OUTPUT_TYPE_DIRECTIVE (file, name, "function");
>> +      else if (TREE_CODE (decl) == VAR_DECL)
>> +       {
>> +         if (!DECL_THREAD_LOCAL_P (decl))
>> +           ASM_OUTPUT_TYPE_DIRECTIVE (file, name, "object");
>> +       }
>> +#endif
>> +    }
>>  }
>>
>>  /* The default hook for TARGET_ASM_OUTPUT_SOURCE_FILENAME.  */
>> --
>> 2.4.3
>
> This patch causes:
>
> FAIL: gcc.dg/fstack-protector-strong.c scan-assembler-times stack_chk_fail 12
> FAIL: gcc.dg/tree-ssa/loop-1.c scan-assembler-times foo 5
> FAIL: gcc.target/i386/memcpy-strategy-3.c scan-assembler-times memcpy 2
> FAIL: gcc.target/i386/memset-strategy-1.c scan-assembler-times memset 2
> FAIL: g++.dg/fstack-protector-strong.C  -std=gnu++11
> scan-assembler-times stack_chk_fail 7
> FAIL: g++.dg/fstack-protector-strong.C  -std=gnu++14
> scan-assembler-times stack_chk_fail 7
> FAIL: g++.dg/fstack-protector-strong.C  -std=gnu++98
> scan-assembler-times stack_chk_fail 7
> FAIL: g++.dg/opt/devirt2.C  -std=gnu++11  scan-assembler-times xyzzy 2
> FAIL: g++.dg/opt/devirt2.C  -std=gnu++14  scan-assembler-times xyzzy 2
> FAIL: g++.dg/opt/devirt2.C  -std=gnu++98  scan-assembler-times xyzzy 2
> FAIL: g++.dg/pr49718.C  -std=gnu++11  scan-assembler-times
> __cyg_profile_func_enter 1
> FAIL: g++.dg/pr49718.C  -std=gnu++14  scan-assembler-times
> __cyg_profile_func_enter 1
> FAIL: g++.dg/pr49718.C  -std=gnu++98  scan-assembler-times
> __cyg_profile_func_enter 1
> FAIL: g++.dg/stackprotectexplicit2.C  -std=gnu++11
> scan-assembler-times stack_chk_fail 2
> FAIL: g++.dg/stackprotectexplicit2.C  -std=gnu++14
> scan-assembler-times stack_chk_fail 2
> FAIL: g++.dg/stackprotectexplicit2.C  -std=gnu++98
> scan-assembler-times stack_chk_fail 2
> FAIL: gfortran.dg/bind_c_array_params_2.f90   -O
> scan-assembler-times [ \t][$,_0-9]*myBindC 1
>
> since there is an extra line of .type directive.
>

Here is the updated patch to adjust those tests by counting in
the .type directive for ELF targets.  There is no regression on
Linux/x86-64.  OK for trunk?

PR target/35514: Gcc shoud generate symbol type for undefined symbol

Commit Message

Patch