From patchwork Thu Jan 18 13:59:28 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Iain Sandoe X-Patchwork-Id: 84345 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4F59B385783C for ; Thu, 18 Jan 2024 14:00:05 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-wm1-x329.google.com (mail-wm1-x329.google.com [IPv6:2a00:1450:4864:20::329]) by sourceware.org (Postfix) with ESMTPS id B30423858299 for ; Thu, 18 Jan 2024 13:59:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B30423858299 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org B30423858299 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::329 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705586375; cv=none; b=nVpH1iMTaaHLPFTubCRI17SxLRNHQdsGO+qKg7Rup+sM3gE7Jm7xYvZxso0Wwz3//e1Hgowji2y5LcBWt+KPaqvR8+m5uRVIsyHwwcijfQklxLtwGv0vnCgRBM8mmaHMnz/gb4nqROiGSOUu9xUYWUZgYSBfe8lu8Pd2lkSezRo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705586375; c=relaxed/simple; bh=Vyu7DOr1ZMOlw+eB/Kpb6uTHabriDEISa5HsgoRF7ww=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=iaQAlgSf3ZhN1qU46TSsYfeR57pgXYfELRl7yWZC+0hw5we3oG6wM67KiXCmmdVIrAWExLBo8fI2jFnOx99o8EubEkE0t6+rvij7klCxNB/jjWMtcTw9wB5RPE4cLt8mG+9Wp8u/lkiX5V5Jff47P+lM7RCqk4ycQxpPHHw9FOc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wm1-x329.google.com with SMTP id 5b1f17b1804b1-40e8fec0968so8806035e9.1 for ; Thu, 18 Jan 2024 05:59:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1705586370; x=1706191170; darn=gcc.gnu.org; h=content-transfer-encoding:mime-version:reply-to:message-id:date :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=NDrTcbgfwVSOZHe77+LLS1ug52jg1viZZPNTsvtwknY=; b=kBe1LOGlA6dkA/8rqenANyLQPaE8OfXJ1e+Xg1s8Yq8KTFzi/6Uk1HuSL4YU087nil WW+8dhJFGgCyQlDyM+Ht8VecapRAvdhsMnhTymgYhn2I+6oT1v+cRwohLbFuXjXxYYK5 cPkVDt6HnGwHWnTRDu9dvK59s1NPkDeYgpr4JtNlVccQZt7G8Mg+KuNZBCVWwut5tTGn fEhO7ctma+RVd/LOSH21SUVbOEcTdLidIsTCaKeaWhdrighoPNAtVXJJAkR2UKNdBLDP Z3aQtk1JhOo5SNzcaw3Y85GK1JUa0xLFUSprg1bw9yor2IRKp569AsJIhQi0Gj7PhD0C CSuQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705586370; x=1706191170; h=content-transfer-encoding:mime-version:reply-to:message-id:date :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=NDrTcbgfwVSOZHe77+LLS1ug52jg1viZZPNTsvtwknY=; b=sc87BuH3+lRBDdK1lwZYMxqVN0fr0ZNyJLwafO0HYemVlWD4y+61961rlr07+T7k82 ug1JlcDj4NlN+tEeELRiGL9Qb91q+ROylIYPFrPBSkpgNIpWBYrB84BSmrYSmZow6cnE y62wFpcjkby/j4lqy/XafcyWm2w5r6NKAFi7vUtt35aI4/6Q29RNtZKwAbppUcMn8TRG jfe3bdAbYSjQRn4oIcuVjawTa+rYHxLdTs3czIaOcsmPizi8oBwMbIMettkJxdIaYwis oOL/hsHgeU3S2A9IY7SCG6RRDtKdxo4TW3r9NfCy3eXN10328zmRouJJIJMX5oXKBW3/ spnQ== X-Gm-Message-State: AOJu0YzHH7XaEpchEH+DICxE6E6kjWWoeSboPkPPyAv9LqVGf0Kqh+dp +scMHt53oaCzODca3ktm6qRXDj5vYDMXIa+PZMAnxs51WcI0Br1mvgt2IqS2 X-Google-Smtp-Source: AGHT+IFrNSXMx2AAGvNPZ18asJg4ulCkcY81ADtu9J7CzzLgsvxbzK7n1C0yTBNsBu7ngekV2CNy8w== X-Received: by 2002:a7b:cb19:0:b0:40e:6ba4:e061 with SMTP id u25-20020a7bcb19000000b0040e6ba4e061mr473359wmj.204.1705586369950; Thu, 18 Jan 2024 05:59:29 -0800 (PST) Received: from localhost.localdomain (host81-138-1-83.in-addr.btopenworld.com. [81.138.1.83]) by smtp.gmail.com with ESMTPSA id jb13-20020a05600c54ed00b0040e418494absm25318990wmb.46.2024.01.18.05.59.29 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Thu, 18 Jan 2024 05:59:29 -0800 (PST) From: Iain Sandoe X-Google-Original-From: Iain Sandoe To: gcc-patches@gcc.gnu.org Cc: rguenther@suse.de, pinskia@gmail.com Subject: [pushed] Darwin: Fix constant CFString code-gen [PR105522]. Date: Thu, 18 Jan 2024 13:59:28 +0000 Message-Id: <20240118135928.24103-1-iain@sandoe.co.uk> X-Mailer: git-send-email 2.39.2 (Apple Git-143) MIME-Version: 1.0 X-Spam-Status: No, score=-8.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: iain@sandoe.co.uk Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org @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 --- 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 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 @@ -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]; +}