On Wed, Oct 14, 2015 at 6:21 PM, H.J. Lu <hongjiu...@intel.com> wrote:
> By default, there is no visibility on builtin functions. When there is
> explicitly declared visibility on the C library function which a builtin
> function fall back on, we should honor the explicit visibility on the
> the C library function.
>
> There are 2 issues:
>
> 1. We never update visibility of the fall back C library function.
> 2. init_block_move_fn and init_block_clear_fn used to implement builtin
> memcpy and memset generate the library call to memcpy and memset
> directly without checking if there is explicitly declared visibility on
> them.
>
> This patch updates builtin function with explicit visibility and checks
> visibility on builtin memcpy/memset when generating library call.
>
> Tested on Linux/x86-64 without regressions. OK for trunk?
Doesn't the C++ FE have the same issue?
>
> H.J.
> ---
> gcc/c/
>
> PR middle-end/67220
> * c-decl.c (diagnose_mismatched_decls): Copy explicit visibility
> to builtin function.
>
> gcc/
>
> PR middle-end/67220
> * expr.c (init_block_move_fn): Copy visibility from the builtin
> memcpy.
> (init_block_clear_fn): Copy visibility from the builtin memset.
>
> gcc/testsuite/
>
> PR middle-end/67220
> * gcc.target/i386/pr67220-1.c: New test.
> * gcc.target/i386/pr67220-2.c: Likewise.
> * gcc.target/i386/pr67220-3.c: Likewise.
> * gcc.target/i386/pr67220-4.c: Likewise.
> * gcc.target/i386/pr67220-5.c: Likewise.
> * gcc.target/i386/pr67220-6.c: Likewise.
> ---
> gcc/c/c-decl.c | 21 +++++++++++++++++----
> gcc/expr.c | 12 ++++++++++--
> gcc/testsuite/gcc.target/i386/pr67220-1.c | 15 +++++++++++++++
> gcc/testsuite/gcc.target/i386/pr67220-2.c | 15 +++++++++++++++
> gcc/testsuite/gcc.target/i386/pr67220-3.c | 15 +++++++++++++++
> gcc/testsuite/gcc.target/i386/pr67220-4.c | 15 +++++++++++++++
> gcc/testsuite/gcc.target/i386/pr67220-5.c | 14 ++++++++++++++
> gcc/testsuite/gcc.target/i386/pr67220-6.c | 14 ++++++++++++++
> 8 files changed, 115 insertions(+), 6 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/i386/pr67220-1.c
> create mode 100644 gcc/testsuite/gcc.target/i386/pr67220-2.c
> create mode 100644 gcc/testsuite/gcc.target/i386/pr67220-3.c
> create mode 100644 gcc/testsuite/gcc.target/i386/pr67220-4.c
> create mode 100644 gcc/testsuite/gcc.target/i386/pr67220-5.c
> create mode 100644 gcc/testsuite/gcc.target/i386/pr67220-6.c
>
> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> index ce8406a..26460eb 100644
> --- a/gcc/c/c-decl.c
> +++ b/gcc/c/c-decl.c
> @@ -2232,11 +2232,24 @@ diagnose_mismatched_decls (tree newdecl, tree olddecl,
> /* warnings */
> /* All decls must agree on a visibility. */
> if (CODE_CONTAINS_STRUCT (TREE_CODE (newdecl), TS_DECL_WITH_VIS)
> - && DECL_VISIBILITY_SPECIFIED (newdecl) && DECL_VISIBILITY_SPECIFIED
> (olddecl)
> - && DECL_VISIBILITY (newdecl) != DECL_VISIBILITY (olddecl))
> + && DECL_VISIBILITY_SPECIFIED (newdecl))
> {
> - warned |= warning (0, "redeclaration of %q+D with different visibility
> "
> - "(old visibility preserved)", newdecl);
> + if (DECL_VISIBILITY_SPECIFIED (olddecl))
> + {
> + if (DECL_VISIBILITY (newdecl) != DECL_VISIBILITY (olddecl))
> + warned |= warning (0, "redeclaration of %q+D with different "
> + "visibility (old visibility preserved)",
> + newdecl);
> + }
> + else if (TREE_CODE (olddecl) == FUNCTION_DECL
> + && DECL_BUILT_IN (olddecl))
> + {
> + enum built_in_function fncode = DECL_FUNCTION_CODE (olddecl);
> + tree fndecl = builtin_decl_explicit (fncode);
> + gcc_assert (fndecl && !DECL_VISIBILITY_SPECIFIED (fndecl));
> + DECL_VISIBILITY (fndecl) = DECL_VISIBILITY (newdecl);
> + DECL_VISIBILITY_SPECIFIED (fndecl) = 1;
> + }
> }
>
> if (TREE_CODE (newdecl) == FUNCTION_DECL)
> diff --git a/gcc/expr.c b/gcc/expr.c
> index 595324d..a12db96 100644
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -1390,7 +1390,11 @@ init_block_move_fn (const char *asmspec)
> TREE_PUBLIC (fn) = 1;
> DECL_ARTIFICIAL (fn) = 1;
> TREE_NOTHROW (fn) = 1;
> - DECL_VISIBILITY (fn) = VISIBILITY_DEFAULT;
> + tree fndecl = builtin_decl_explicit (BUILT_IN_MEMCPY);
> + if (fndecl)
> + DECL_VISIBILITY (fn) = DECL_VISIBILITY (fndecl);
> + else
> + DECL_VISIBILITY (fn) = VISIBILITY_DEFAULT;
> DECL_VISIBILITY_SPECIFIED (fn) = 1;
>
> attr_args = build_tree_list (NULL_TREE, build_string (1, "1"));
> @@ -2846,7 +2850,11 @@ init_block_clear_fn (const char *asmspec)
> TREE_PUBLIC (fn) = 1;
> DECL_ARTIFICIAL (fn) = 1;
> TREE_NOTHROW (fn) = 1;
> - DECL_VISIBILITY (fn) = VISIBILITY_DEFAULT;
> + tree fndecl = builtin_decl_explicit (BUILT_IN_MEMSET);
> + if (fndecl)
> + DECL_VISIBILITY (fn) = DECL_VISIBILITY (fndecl);
> + else
> + DECL_VISIBILITY (fn) = VISIBILITY_DEFAULT;
> DECL_VISIBILITY_SPECIFIED (fn) = 1;
>
> block_clear_fn = fn;
> diff --git a/gcc/testsuite/gcc.target/i386/pr67220-1.c
> b/gcc/testsuite/gcc.target/i386/pr67220-1.c
> new file mode 100644
> index 0000000..06af0ed
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr67220-1.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile { target fpic } } */
> +/* { dg-options "-O2 -fPIC" } */
> +
> +typedef __SIZE_TYPE__ size_t;
> +extern void *memcpy (void *, const void *, size_t);
> +extern void *memcpy (void *, const void *, size_t)
> + __attribute__ ((visibility ("hidden")));
> +
> +void
> +bar (void *d, void *s, size_t n)
> +{
> + memcpy (d, s, n);
> +}
> +
> +/* { dg-final { scan-assembler-not "memcpy@PLT" } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr67220-2.c
> b/gcc/testsuite/gcc.target/i386/pr67220-2.c
> new file mode 100644
> index 0000000..71e1d1e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr67220-2.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile { target fpic } } */
> +/* { dg-options "-O2 -fPIC" } */
> +
> +typedef __SIZE_TYPE__ size_t;
> +extern void *memset (void *, int, size_t);
> +extern void *memset (void *, int, size_t)
> + __attribute__ ((visibility ("hidden")));
> +
> +void
> +bar (void *s, size_t n)
> +{
> + memset (s, '\0', n);
> +}
> +
> +/* { dg-final { scan-assembler-not "memset@PLT" } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr67220-3.c
> b/gcc/testsuite/gcc.target/i386/pr67220-3.c
> new file mode 100644
> index 0000000..fa54530
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr67220-3.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile { target fpic } } */
> +/* { dg-options "-O2 -fPIC" } */
> +
> +typedef __SIZE_TYPE__ size_t;
> +extern void *memcpy (void *, const void *, size_t)
> + __attribute__ ((visibility ("hidden")));
> +extern void *memcpy (void *, const void *, size_t);
> +
> +void
> +bar (void *d, void *s, size_t n)
> +{
> + __builtin_memcpy (d, s, n);
> +}
> +
> +/* { dg-final { scan-assembler-not "memcpy@PLT" } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr67220-4.c
> b/gcc/testsuite/gcc.target/i386/pr67220-4.c
> new file mode 100644
> index 0000000..9ce02f4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr67220-4.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile { target fpic } } */
> +/* { dg-options "-O2 -fPIC" } */
> +
> +typedef __SIZE_TYPE__ size_t;
> +extern void *memset (void *, int, size_t)
> + __attribute__ ((visibility ("hidden")));
> +extern void *memset (void *, int, size_t);
> +
> +void
> +bar (void *s, size_t n)
> +{
> + __builtin_memset (s, '\0', n);
> +}
> +
> +/* { dg-final { scan-assembler-not "memset@PLT" } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr67220-5.c
> b/gcc/testsuite/gcc.target/i386/pr67220-5.c
> new file mode 100644
> index 0000000..e2c8ba3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr67220-5.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile { target fpic } } */
> +/* { dg-options "-O2 -fPIC" } */
> +
> +extern int strcmp (const char *, const char *);
> +extern int strcmp (const char *, const char *)
> + __attribute__ ((visibility ("hidden")));
> +
> +int
> +bar (const char *d, const char *s)
> +{
> + return __builtin_strcmp (d, s);
> +}
> +
> +/* { dg-final { scan-assembler-not "strcmp@PLT" } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr67220-6.c
> b/gcc/testsuite/gcc.target/i386/pr67220-6.c
> new file mode 100644
> index 0000000..c83d4c2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr67220-6.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile { target fpic } } */
> +/* { dg-options "-O2 -fPIC" } */
> +
> +extern int strcmp (const char *, const char *)
> + __attribute__ ((visibility ("hidden")));
> +extern int strcmp (const char *, const char *);
> +
> +int
> +bar (const char *d, const char *s)
> +{
> + return __builtin_strcmp (d, s);
> +}
> +
> +/* { dg-final { scan-assembler-not "strcmp@PLT" } } */
> --
> 2.4.3
>
