From patchwork Mon Mar 10 13:52:34 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathaniel Shead X-Patchwork-Id: 107580 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 8962C3858C54 for ; Mon, 10 Mar 2025 13:53:08 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pl1-x62c.google.com (mail-pl1-x62c.google.com [IPv6:2607:f8b0:4864:20::62c]) by sourceware.org (Postfix) with ESMTPS id 13A3C3858D20 for ; Mon, 10 Mar 2025 13:52:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 13A3C3858D20 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 13A3C3858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::62c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1741614762; cv=none; b=CkcTisNBTyejs8ik5+zB5Do3G6Fwb4f87lbxb9H/hgvjO7ujjLAczrKTAMYO87S/1BW40g4QjpvO98Wa/ALZ1fN07D5cHutOB7ES2GGDpHBIjVJoHRzLMsoEoINQut27bsDwer8MTr+wlUEMGdsm+dyatnT5po5FwBVBThdF1Hg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1741614762; c=relaxed/simple; bh=aGlXf9xagMztbLv0kJwJKqb/EODLmkyIvlXNvg11508=; h=DKIM-Signature:Message-ID:Date:From:To:Subject:MIME-Version; b=iE8MV+CIGjKG8vy6rtXBCZENz19ROyyek1lxMCFFkLYcfuIykqGwiNfDGGS3MYg4QNhrET1jAInN4ZJ8YhNfg50zKrfrh0lmcO5r/reo1HX/7vdUOAGcBTrw8ubB/w1b0WGeW6gF6jVs6fhKVUuio8pz9x0A/6yYJb+5A4vCA6A= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x62c.google.com with SMTP id d9443c01a7336-216513f8104so6926745ad.2 for ; Mon, 10 Mar 2025 06:52:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1741614761; x=1742219561; darn=gcc.gnu.org; h=content-disposition:mime-version:subject:cc:to:from:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=PTtStEXZ7aXUICBSMn4oIrfcrBvUk2ZCG7zif6YaXus=; b=C2yCK4GANdgLqs6pyfem/3eFjeTnSorbXtopvthGc9WTpuwCzkkH2B7oZEZOxvVCRk VmNcZ1f8EaG1OwFFiQHnVD1lsfbSvE+sP+VpPEvQrjKIWZa9D4JgOzwfsqE/OChkCs05 ee5Outrvtx2BXyZSaAlO5fApwSvxIHCfI6ne3xT3iVZOncMubtatTl/rMfRBAM6q8gSl jyFRLxH1T4p/RaZiU/e1lWhH8htTgDt+sCEFlD2SjKuTw3aGAzB9yBhRfNqzziki7V9x aeuWCAhX19HtrYFAbGGMNWKIUy6jKVo++ZgXjRVVN46Fc/r9lomIEZY72uKxbFCfRcba NthQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741614761; x=1742219561; h=content-disposition:mime-version:subject:cc:to:from:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=PTtStEXZ7aXUICBSMn4oIrfcrBvUk2ZCG7zif6YaXus=; b=ZMyeUK+m9flgYAlw6FAUzhNGOQwHRmT4jHEtP9eKMByu58BOl4PDM73jRklk5kdaEq DsnfSCRoLk5KT4FML2fV3cNXKr3S1UPrxWyfTcgw/xahNMBDzir5UByIB2hiIghO2Rq6 PAV3r7Eaz1QIJovfOl9JT1BwtVGkXBDoC2mo0XF1KsvuiL4rpOp2FH1cszd6o8FxoJrw 6i/wHY7ckbBtfLz2QtT6VO++HrHR23TERk62S7OG8ucVgjq+WBiGep4R8Far1a6holqf CPsge2kxyjgCr8uUljCc/WazN76MV2TPqCilkqs3SXrJH/GKKQB/FG828hIxdCVX4fEF R3Kw== X-Gm-Message-State: AOJu0YwzFyeXjVijuZoy0LPfpAIdU240rxa5R/PSvJPbfKPuhO8LFpz5 Xs1q3IKqZ3aaWcBUzc2E5WWMu27WP/sHvejYxqI6CyGAkVcDIUBqRIVWhA== X-Gm-Gg: ASbGncs2gSaCyRMLjfaq+/T5kQQgZwLsv6YQ3N2EiXW1j4H4gQObY6KE8CLH7YAEQOt EIFs8NGqVBupAYwULw2AggUi334PheOKaMs5RK+uR3CVorl+N3Ml2XIw826KaIpZT4a/fvifHuZ yRm2mtAM/HDS8vyjc1Ezw/CyUCY9V2APhw9MYTvaYl34Ml5kQE4GYbr2Q4oum2iIQ8odiUlXdta sc3m0ULXp4tAcHkmzMlsc6JDem6C4FCTro2O09Wy1vL9p5zbrStkUwuqNMc5xtuWQo+UJHdzFW1 366ivYiPE73Rr4CTRxbc2163EqnEpN182OgI2/bV58CDBtHxRwGjLs/XBLhggvlPNkKZQCuIvw7 6Heyr1vqVuTyY X-Google-Smtp-Source: AGHT+IFXy4IKQXpyhbtmqCZYvwRYkvvzVMJCLR95QCzsj+v8krWD235/M/vqB9g12is4ZGlhzgQanA== X-Received: by 2002:a17:902:dac4:b0:223:5c33:56ae with SMTP id d9443c01a7336-22541f92259mr53104385ad.11.1741614759460; Mon, 10 Mar 2025 06:52:39 -0700 (PDT) Received: from Thaum. (163-47-68-46.ipv4.originbroadband.com.au. [163.47.68.46]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-22410a9194csm77894525ad.172.2025.03.10.06.52.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 10 Mar 2025 06:52:39 -0700 (PDT) Message-ID: <67ceeea7.170a0220.1d2d45.b238@mx.google.com> X-Google-Original-Message-ID: Date: Tue, 11 Mar 2025 00:52:34 +1100 From: Nathaniel Shead To: gcc-patches@gcc.gnu.org Cc: Jason Merrill Subject: c++/modules: Stream section, tls_model, and comdat_group MIME-Version: 1.0 Content-Disposition: inline X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, POISEN_SPAM_PILL, POISEN_SPAM_PILL_1, POISEN_SPAM_PILL_3, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP 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: , Errors-To: gcc-patches-bounces~patchwork=sourceware.org@gcc.gnu.org Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? Or should this wait for GCC16? -- >8 -- While looking at PR c++/119154 I noticed some more properties that we don't stream or check properly. This patch adds the ones we were missing, and adds checks that the values don't clash with existing decls. There's probably a lot more duplicate_decls-like work that should be done in is_matching_decl (notably decl attributes?) but this is at least a slight improvement to the status-quo. gcc/cp/ChangeLog: * module.cc (trees_out::core_vals): Stream TLS model, section name, and comdat group. (trees_in::core_vals): Read them. (trees_out::decl_value): No longer need to stream TLS model. (trees_in::decl_value): Likewise; free symtab node of discarded decls when we're done with them. (trees_in::is_matching_decl): Add checks for corresponding TLS model and section name. gcc/testsuite/ChangeLog: * g++.dg/modules/attrib-3_a.C: New test. * g++.dg/modules/attrib-3_b.C: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/module.cc | 104 +++++++++++++++++++--- gcc/testsuite/g++.dg/modules/attrib-3_a.C | 9 ++ gcc/testsuite/g++.dg/modules/attrib-3_b.C | 14 +++ 3 files changed, 117 insertions(+), 10 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/attrib-3_a.C create mode 100644 gcc/testsuite/g++.dg/modules/attrib-3_b.C diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 8e0fa3c5bfc..9aa86c939f6 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -6399,6 +6399,9 @@ trees_out::core_vals (tree t) /* Decls. */ case VAR_DECL: + if (streaming_p ()) + if (CP_DECL_THREAD_LOCAL_P (t)) + u (decl_tls_model (t)); if (DECL_CONTEXT (t) && TREE_CODE (DECL_CONTEXT (t)) != FUNCTION_DECL) { @@ -6691,6 +6694,22 @@ trees_out::core_vals (tree t) break; } + if (VAR_OR_FUNCTION_DECL_P (t) + && (DECL_EXTERNAL (t) || TREE_PUBLIC (t) || TREE_STATIC (t))) + { + if (streaming_p ()) + { + const char *name = DECL_SECTION_NAME (t); + size_t len = name ? strlen (name) : 0; + str (name, len); + } + + tree comdat_group = NULL_TREE; + if (DECL_ONE_ONLY (t)) + comdat_group = symtab_node::get (t)->get_comdat_group (); + WT (comdat_group); + } + if (CODE_CONTAINS_STRUCT (code, TS_TYPED)) { /* We want to stream the type of a expression-like nodes /after/ @@ -6933,6 +6952,11 @@ trees_in::core_vals (tree t) /* Decls. */ case VAR_DECL: + if (CP_DECL_THREAD_LOCAL_P (t)) + { + enum tls_model model = tls_model (u()); + set_decl_tls_model (t, model); + } if (DECL_CONTEXT (t) && TREE_CODE (DECL_CONTEXT (t)) != FUNCTION_DECL) { @@ -7227,6 +7251,23 @@ trees_in::core_vals (tree t) break; } + if (VAR_OR_FUNCTION_DECL_P (t) + && (DECL_EXTERNAL (t) || TREE_PUBLIC (t) || TREE_STATIC (t))) + { + size_t section_len = 0; + const char *section_name = str (§ion_len); + if (section_len) + set_decl_section_name (t, section_name); + + if (tree comdat_group = tree_node ()) + { + if (TREE_CODE (t) == FUNCTION_DECL) + cgraph_node::get_create (t)->set_comdat_group (comdat_group); + else + varpool_node::get_create (t)->set_comdat_group (comdat_group); + } + } + if (CODE_CONTAINS_STRUCT (code, TS_TYPED)) { tree type = tree_node (); @@ -8316,9 +8357,6 @@ trees_out::decl_value (tree decl, depset *dep) decl, cloned_p ? "" : "not "); } - if (streaming_p () && VAR_P (decl) && CP_DECL_THREAD_LOCAL_P (decl)) - u (decl_tls_model (decl)); - if (streaming_p ()) dump (dumper::TREE) && dump ("Written decl:%d %C:%N", tag, TREE_CODE (decl), decl); @@ -8757,6 +8795,12 @@ trees_in::decl_value () DECL_CONTEXT (parm) = e_inner; } + /* We will not need to track the comdat group etc of the + to-be-discarded decl, so remove it from the symbol table. */ + if (VAR_OR_FUNCTION_DECL_P (inner)) + if (struct symtab_node *snode = symtab_node::get (inner)) + snode->remove (); + /* And our result is the existing node. */ decl = existing; } @@ -8802,13 +8846,6 @@ trees_in::decl_value () } } - if (VAR_P (decl) && CP_DECL_THREAD_LOCAL_P (decl)) - { - enum tls_model model = tls_model (u ()); - if (is_new) - set_decl_tls_model (decl, model); - } - if (!NAMESPACE_SCOPE_P (inner) && ((TREE_CODE (inner) == TYPE_DECL && !is_typedef @@ -12098,6 +12135,53 @@ trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef) if (!DECL_EXTERNAL (d_inner)) DECL_EXTERNAL (e_inner) = false; + if (VAR_P (d_inner)) + { + if (CP_DECL_THREAD_LOCAL_P (d_inner) != CP_DECL_THREAD_LOCAL_P (e_inner)) + { + auto_diagnostic_group d; + error_at (DECL_SOURCE_LOCATION (decl), + "conflicting % for %#qD", decl); + inform (DECL_SOURCE_LOCATION (existing), "existing declaration here"); + return false; + } + if (CP_DECL_THREAD_LOCAL_P (d_inner) + && decl_tls_model (d_inner) != decl_tls_model (e_inner)) + { + auto_diagnostic_group d; + error_at (DECL_SOURCE_LOCATION (decl), + "conflicting TLS model %qs for %q#D", + tls_model_names[decl_tls_model (d_inner)], decl); + inform (DECL_SOURCE_LOCATION (existing), + "existing declaration here as %qs", + tls_model_names[decl_tls_model (e_inner)]); + return false; + } + } + + if (VAR_OR_FUNCTION_DECL_P (d_inner)) + { + if (DECL_SECTION_NAME (d_inner) != DECL_SECTION_NAME (e_inner)) + { + auto_diagnostic_group d; + if (DECL_SECTION_NAME (d_inner)) + error_at (DECL_SOURCE_LOCATION (decl), + "conflicting section %qs for %q#D", + DECL_SECTION_NAME (d_inner), decl); + else + error_at (DECL_SOURCE_LOCATION (decl), + "conflicting section for %q#D", decl); + if (DECL_SECTION_NAME (e_inner)) + inform (DECL_SOURCE_LOCATION (existing), + "existing declaration here in section %qs", + DECL_SECTION_NAME (e_inner)); + else + inform (DECL_SOURCE_LOCATION (existing), + "existing declaration here with no section"); + return false; + } + } + if (TREE_CODE (decl) == TEMPLATE_DECL) { /* Merge default template arguments. */ diff --git a/gcc/testsuite/g++.dg/modules/attrib-3_a.C b/gcc/testsuite/g++.dg/modules/attrib-3_a.C new file mode 100644 index 00000000000..ea265762cbf --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/attrib-3_a.C @@ -0,0 +1,9 @@ +// { dg-additional-options "-fmodules" } + +export module M; +export extern "C++" { + inline void a() __attribute__((section("bar"))); + inline void a() {} + + inline void b() {} +} diff --git a/gcc/testsuite/g++.dg/modules/attrib-3_b.C b/gcc/testsuite/g++.dg/modules/attrib-3_b.C new file mode 100644 index 00000000000..67c78567883 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/attrib-3_b.C @@ -0,0 +1,14 @@ +// { dg-additional-options "-fmodules" } +// { dg-error "conflicting section" "" { target *-*-* } 0 } + +inline void a() {} // { dg-message "here" } + +inline void b() __attribute__((section("qux"))); +inline void b() {} // { dg-message "here" } + +import M; + +int main() { + a(); + b(); +}