[RFA] cgraph: A COMDAT decl always has non-zero address.

Message ID 20200205205936.25389-1-jason@redhat.com
State New
Headers show
Series
  • [RFA] cgraph: A COMDAT decl always has non-zero address.
Related show

Commit Message

Jason Merrill Feb. 5, 2020, 8:59 p.m.
We should be able to assume that a template instantiation or other COMDAT
has non-zero address even if MAKE_DECL_ONE_ONLY for the target sets
DECL_WEAK and we haven't yet decided to emit a definition in this
translation unit.

Tested x86_64-pc-linux-gnu, OK for trunk?

	PR c++/92003
	* symtab.c (symtab_node::nonzero_address): A DECL_COMDAT decl has
	non-zero address even if weak and not yet defined.
---
 gcc/symtab.c                                    | 10 +++++-----
 gcc/testsuite/g++.dg/cpp0x/constexpr-static13.C | 17 +++++++++++++++++
 2 files changed, 22 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-static13.C


base-commit: fa0c6e297b22d5883857d0db4a6a8be0967cb16f
-- 
2.18.1

Comments

Jan Hubicka Feb. 5, 2020, 11:19 p.m. | #1
> We should be able to assume that a template instantiation or other COMDAT

> has non-zero address even if MAKE_DECL_ONE_ONLY for the target sets

> DECL_WEAK and we haven't yet decided to emit a definition in this

> translation unit.

> 

> Tested x86_64-pc-linux-gnu, OK for trunk?

> 

> 	PR c++/92003

> 	* symtab.c (symtab_node::nonzero_address): A DECL_COMDAT decl has

> 	non-zero address even if weak and not yet defined.

OK, thanks!

Honza
> ---

>  gcc/symtab.c                                    | 10 +++++-----

>  gcc/testsuite/g++.dg/cpp0x/constexpr-static13.C | 17 +++++++++++++++++

>  2 files changed, 22 insertions(+), 5 deletions(-)

>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-static13.C

> 

> diff --git a/gcc/symtab.c b/gcc/symtab.c

> index eae891ab211..a879c095a1a 100644

> --- a/gcc/symtab.c

> +++ b/gcc/symtab.c

> @@ -2058,22 +2058,22 @@ symtab_node::nonzero_address ()

>       bind to NULL. This is on by default on embedded targets only.

>  

>       Otherwise all non-WEAK symbols must be defined and thus non-NULL or

> -     linking fails.  Important case of WEAK we want to do well are comdats.

> -     Those are handled by later check for definition.

> +     linking fails.  Important case of WEAK we want to do well are comdats,

> +     which also must be defined somewhere.

>  

>       When parsing, beware the cases when WEAK attribute is added later.  */

> -  if (!DECL_WEAK (decl)

> +  if ((!DECL_WEAK (decl) || DECL_COMDAT (decl))

>        && flag_delete_null_pointer_checks)

>      {

>        refuse_visibility_changes = true;

>        return true;

>      }

>  

> -  /* If target is defined and either comdat or not extern, we know it will be

> +  /* If target is defined and not extern, we know it will be

>       output and thus it will bind to non-NULL.

>       Play safe for flag_delete_null_pointer_checks where weak definition may

>       be re-defined by NULL.  */

> -  if (definition && (!DECL_EXTERNAL (decl) || DECL_COMDAT (decl))

> +  if (definition && !DECL_EXTERNAL (decl)

>        && (flag_delete_null_pointer_checks || !DECL_WEAK (decl)))

>      {

>        if (!DECL_WEAK (decl))

> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-static13.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-static13.C

> new file mode 100644

> index 00000000000..644f9f7f893

> --- /dev/null

> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-static13.C

> @@ -0,0 +1,17 @@

> +// PR c++/92003

> +// { dg-do compile { target c++11 } }

> +// { dg-prune-output "narrowing conversion" }

> +

> +constexpr char const* get_c_str() { return "abc"; }

> +constexpr bool use_get_c_str_in_constexpr_context{get_c_str()}; // works

> +

> +template <char... Cs>

> +struct string {

> +  static constexpr char const* c_str() { return c; }

> +

> + private:

> +  static constexpr char c[]{Cs..., '\0'};

> +};

> +

> +constexpr char const* cstr{string<'a', 'b', 'c'>::c_str()};

> +constexpr bool use_cstr_in_constexpr_context{cstr}; // doesn't work

> 

> base-commit: fa0c6e297b22d5883857d0db4a6a8be0967cb16f

> -- 

> 2.18.1

>

Patch

diff --git a/gcc/symtab.c b/gcc/symtab.c
index eae891ab211..a879c095a1a 100644
--- a/gcc/symtab.c
+++ b/gcc/symtab.c
@@ -2058,22 +2058,22 @@  symtab_node::nonzero_address ()
      bind to NULL. This is on by default on embedded targets only.
 
      Otherwise all non-WEAK symbols must be defined and thus non-NULL or
-     linking fails.  Important case of WEAK we want to do well are comdats.
-     Those are handled by later check for definition.
+     linking fails.  Important case of WEAK we want to do well are comdats,
+     which also must be defined somewhere.
 
      When parsing, beware the cases when WEAK attribute is added later.  */
-  if (!DECL_WEAK (decl)
+  if ((!DECL_WEAK (decl) || DECL_COMDAT (decl))
       && flag_delete_null_pointer_checks)
     {
       refuse_visibility_changes = true;
       return true;
     }
 
-  /* If target is defined and either comdat or not extern, we know it will be
+  /* If target is defined and not extern, we know it will be
      output and thus it will bind to non-NULL.
      Play safe for flag_delete_null_pointer_checks where weak definition may
      be re-defined by NULL.  */
-  if (definition && (!DECL_EXTERNAL (decl) || DECL_COMDAT (decl))
+  if (definition && !DECL_EXTERNAL (decl)
       && (flag_delete_null_pointer_checks || !DECL_WEAK (decl)))
     {
       if (!DECL_WEAK (decl))
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-static13.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-static13.C
new file mode 100644
index 00000000000..644f9f7f893
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-static13.C
@@ -0,0 +1,17 @@ 
+// PR c++/92003
+// { dg-do compile { target c++11 } }
+// { dg-prune-output "narrowing conversion" }
+
+constexpr char const* get_c_str() { return "abc"; }
+constexpr bool use_get_c_str_in_constexpr_context{get_c_str()}; // works
+
+template <char... Cs>
+struct string {
+  static constexpr char const* c_str() { return c; }
+
+ private:
+  static constexpr char c[]{Cs..., '\0'};
+};
+
+constexpr char const* cstr{string<'a', 'b', 'c'>::c_str()};
+constexpr bool use_cstr_in_constexpr_context{cstr}; // doesn't work