Re: [PATCH] PR target/35514: Gcc shoud generate symbol type for undefined symbol

["H.J. Lu" <hjl dot tools at gmail dot com>]

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.


-- 
H.J.