[pushed] Darwin: Fix constant CFString code-gen [PR105522].

Message ID 20240118135928.24103-1-iain@sandoe.co.uk
State New
Headers
Series [pushed] Darwin: Fix constant CFString code-gen [PR105522]. |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 fail Patch failed to apply

Commit Message

Iain Sandoe Jan. 18, 2024, 1:59 p.m. UTC
  @Richi, @Andrew - FIO since you were involved in the IRC discussion.

Tested on i686, powerpc, x86_64 Darwin (and x86_64 Linux), pushed
to trunk, thanks,
Iain

--- 8< ---

Although this only fires for one of the Darwin sub-ports, it is latent
elsewhere, it is also a regression c.f. the Darwin system compiler.

In the code we imported from an earlier branch, CFString objects (which
are constant aggregates) are constructed as CONST_DECLs.  Although our
current documentation suggests that these are reserved for enumeration
values, in fact they are used elsewhere in the compiler for constants.
This includes Objective-C where they are used to form NSString constants.

In the particular case, we take the address of the constant and that
triggers varasm.cc:decode_addr_constant, which does not currently support
CONST_DECL.

If there is a general intent to allow/encourage wider use of CONST_DECL,
then we should fix decode_addr_constant to look through these and evaluate
the initializer (a two-line patch, but I'm not suggesting it for stage-4).

We also need to update the GCC internals documentation to allow for the
additional uses.

This patch is Darwin-local and fixes the problem by making the CFString
constants into regular variable but TREE_CONSTANT+TREE_READONLY. I plan
to back-port this to the open branches once it has baked a while on trunk.

Since, for Darwin, the Objective-C default is to construct constant
NSString objects as CFStrings; this will also cover the majority of cases
there (this patch does not make any changes to Objective-C NSStrings).

	PR target/105522

gcc/ChangeLog:

	* config/darwin.cc (machopic_select_section): Handle C and C++
	CFStrings.
	(darwin_rename_builtins): Move this out of the CFString code.
	(darwin_libc_has_function): Likewise.
	(darwin_build_constant_cfstring): Create an anonymous var to
	hold each CFString.
	* config/darwin.h (ASM_OUTPUT_LABELREF): Handle constant
	CFstrings.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
---
 gcc/config/darwin.cc            | 100 ++++++++++++++++++--------------
 gcc/config/darwin.h             |   2 +
 gcc/testsuite/gcc.dg/pr105522.c |  17 ++++++
 3 files changed, 76 insertions(+), 43 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr105522.c
  

Patch

diff --git a/gcc/config/darwin.cc b/gcc/config/darwin.cc
index cf203dc4b3e..b15f3b1a1d9 100644
--- a/gcc/config/darwin.cc
+++ b/gcc/config/darwin.cc
@@ -1731,7 +1731,16 @@  machopic_select_section (tree decl,
 	    base_section = darwin_sections[zobj_data_section];
 	}
       else if (ro)
-	base_section = darwin_sections[const_data_section];
+	{
+	  if (VAR_P (decl) && TREE_TYPE (decl)
+	      && TREE_CODE (TREE_TYPE (decl)) == RECORD_TYPE
+	      && DECL_NAME (decl)
+	      && strncmp (IDENTIFIER_POINTER (DECL_NAME (decl)),
+			  "__anon_cfstring", 15) == 0)
+	   base_section = darwin_sections[cfstring_constant_object_section];
+	  else
+	    base_section = darwin_sections[const_data_section];
+	}
       else
 	base_section = data_section;
       break;
@@ -1795,8 +1804,6 @@  machopic_select_section (tree decl,
 	  else
 	    return darwin_sections[objc_string_object_section];
 	}
-      else if (!strcmp (IDENTIFIER_POINTER (name), "__builtin_CFString"))
-	return darwin_sections[cfstring_constant_object_section];
       else
 	return base_section;
     }
@@ -3612,6 +3619,29 @@  darwin_patch_builtins (void)
 }
 #endif
 
+void
+darwin_rename_builtins (void)
+{
+}
+
+/* Implementation for the TARGET_LIBC_HAS_FUNCTION hook.  */
+
+bool
+darwin_libc_has_function (enum function_class fn_class,
+			  tree type ATTRIBUTE_UNUSED)
+{
+  if (fn_class == function_sincos && darwin_macosx_version_min)
+    return (strverscmp (darwin_macosx_version_min, "10.9") >= 0);
+#if DARWIN_PPC && SUPPORT_DARWIN_LEGACY
+  if (fn_class == function_c99_math_complex
+      || fn_class == function_c99_misc)
+    return (TARGET_64BIT
+	    || (darwin_macosx_version_min &&
+		strverscmp (darwin_macosx_version_min, "10.3") >= 0));
+#endif
+  return default_libc_has_function (fn_class, type);
+}
+
 /*  CFStrings implementation.  */
 static GTY(()) tree cfstring_class_reference = NULL_TREE;
 static GTY(()) tree cfstring_type_node = NULL_TREE;
@@ -3629,7 +3659,7 @@  typedef struct GTY ((for_user)) cfstring_descriptor {
   /* The string literal.  */
   tree literal;
   /* The resulting constant CFString.  */
-  tree constructor;
+  tree ccf_str;
 } cfstring_descriptor;
 
 struct cfstring_hasher : ggc_ptr_hash<cfstring_descriptor>
@@ -3704,7 +3734,7 @@  darwin_init_cfstring_builtins (unsigned builtin_cfstring)
   /* Make a lang-specific section - dup_lang_specific_decl makes a new node
      in place of the existing, which may be NULL.  */
   DECL_LANG_SPECIFIC (cfsfun) = NULL;
-  (*lang_hooks.dup_lang_specific_decl) (cfsfun);
+  lang_hooks.dup_lang_specific_decl (cfsfun);
   set_decl_built_in_function (cfsfun, BUILT_IN_MD, darwin_builtin_cfstring);
   lang_hooks.builtin_function (cfsfun);
 
@@ -3715,7 +3745,7 @@  darwin_init_cfstring_builtins (unsigned builtin_cfstring)
 
   TREE_PUBLIC (cfstring_class_reference) = 1;
   DECL_ARTIFICIAL (cfstring_class_reference) = 1;
-  (*lang_hooks.decls.pushdecl) (cfstring_class_reference);
+  lang_hooks.decls.pushdecl (cfstring_class_reference);
   DECL_EXTERNAL (cfstring_class_reference) = 1;
   rest_of_decl_compilation (cfstring_class_reference, 0, 0);
 
@@ -3752,29 +3782,6 @@  darwin_fold_builtin (tree fndecl, int n_args, tree *argp,
   return NULL_TREE;
 }
 
-void
-darwin_rename_builtins (void)
-{
-}
-
-/* Implementation for the TARGET_LIBC_HAS_FUNCTION hook.  */
-
-bool
-darwin_libc_has_function (enum function_class fn_class,
-			  tree type ATTRIBUTE_UNUSED)
-{
-  if (fn_class == function_sincos && darwin_macosx_version_min)
-    return (strverscmp (darwin_macosx_version_min, "10.9") >= 0);
-#if DARWIN_PPC && SUPPORT_DARWIN_LEGACY
-  if (fn_class == function_c99_math_complex
-      || fn_class == function_c99_misc)
-    return (TARGET_64BIT
-	    || (darwin_macosx_version_min &&
-		strverscmp (darwin_macosx_version_min, "10.3") >= 0));
-#endif
-  return default_libc_has_function (fn_class, type);
-}
-
 hashval_t
 cfstring_hasher::hash (cfstring_descriptor *ptr)
 {
@@ -3872,29 +3879,36 @@  darwin_build_constant_cfstring (tree str)
 			     build_int_cst (TREE_TYPE (field), length));
 
       constructor = build_constructor (ccfstring_type_node, v);
-      TREE_READONLY (constructor) = 1;
-      TREE_CONSTANT (constructor) = 1;
-      TREE_STATIC (constructor) = 1;
-
-      /* Fromage: The C++ flavor of 'build_unary_op' expects constructor nodes
-	 to have the TREE_HAS_CONSTRUCTOR (...) bit set.  However, this file is
-	 being built without any knowledge of C++ tree accessors; hence, we shall
-	 use the generic accessor that TREE_HAS_CONSTRUCTOR actually maps to!  */
+      TREE_READONLY (constructor) = true;
+      TREE_CONSTANT (constructor) = true;
+      TREE_STATIC (constructor) = true;
+
+      /* This file is being built without any knowledge of C++ tree accessors;
+	 hence, we shall use the generic accessor to set TREE_HAS_CONSTRUCTOR.
+	 ??? Is this actually used any more? */
       if (darwin_running_cxx)
 	TREE_LANG_FLAG_4 (constructor) = 1;  /* TREE_HAS_CONSTRUCTOR  */
 
       /* Create an anonymous global variable for this CFString.  */
-      var = build_decl (input_location, CONST_DECL,
+      var = build_decl (input_location, VAR_DECL,
 			NULL, TREE_TYPE (constructor));
-      DECL_ARTIFICIAL (var) = 1;
-      TREE_STATIC (var) = 1;
+      char *name = xasprintf ("__anon_cfstring.%u", DECL_UID (var));
+      DECL_NAME (var) = get_identifier (name);
+      free (name);
+      DECL_ARTIFICIAL (var) = true;
+      TREE_STATIC (var) = true;
+      TREE_READONLY (var) = true;
+      TREE_CONSTANT (var) = true;
       DECL_INITIAL (var) = constructor;
-      /* FIXME: This should use a translation_unit_decl to indicate file scope.  */
+      /* global namespace.  */
       DECL_CONTEXT (var) = NULL_TREE;
-      desc->constructor = var;
+      DECL_INITIAL (var) = constructor;
+      lang_hooks.decls.pushdecl (var);
+      rest_of_decl_compilation (var, 1, 0);
+      desc->ccf_str = var;
     }
 
-  addr = build1 (ADDR_EXPR, pccfstring_type_node, desc->constructor);
+  addr = build1 (ADDR_EXPR, pccfstring_type_node, desc->ccf_str);
   TREE_CONSTANT (addr) = 1;
 
   return addr;
diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index b3300b504c8..e94a29c639c 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -869,6 +869,8 @@  int darwin_label_is_anonymous_local_objc_name (const char *name);
          fprintf (FILE, "\"%s\"", xname);				     \
        else if (darwin_label_is_anonymous_local_objc_name (xname))	     \
 	fprintf (FILE, "%c%s", flag_next_runtime ? 'L' : 'l', xname);	     \
+       else if (strncmp (xname, "__anon_cfstring", 15) == 0)		     \
+	fprintf (FILE, "L%s", xname);					     \
        else if (xname[0] != '"' && name_needs_quotes (xname))		     \
 	 asm_fprintf (FILE, "\"%U%s\"", xname);				     \
        else								     \
diff --git a/gcc/testsuite/gcc.dg/pr105522.c b/gcc/testsuite/gcc.dg/pr105522.c
new file mode 100644
index 00000000000..186cd60cd32
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr105522.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile { target *-*-darwin* } } */
+typedef const struct __CFString * CFStringRef;
+
+/* This used to ICE on powerpc darwin, in decode_addr_const.  */
+const void
+* create_usage_match(const unsigned int page, const unsigned int usage, int
+*okay)
+{
+
+  const void *keys[2] = 
+    { (void *) ((CFStringRef) __builtin___CFStringMakeConstantString (""
+      "DeviceUsagePage" ""))
+      , (void *) ((CFStringRef) __builtin___CFStringMakeConstantString (""
+      "DeviceUsage" ""))
+    };
+  return keys[1];
+}