public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Markus Böck" <markus.boeck02@gmail.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: [PATCH][PR94156] Split COMDAT groups on target that do not support them
Date: Tue, 23 Mar 2021 17:27:30 +0100	[thread overview]
Message-ID: <CAEZ8vhnLo3uAj+uGb_AKgUZigdYheb2kzAUjau8ua0n_=gOb=g@mail.gmail.com> (raw)

GCC at the moment uses COMDAT groups for things like virtual thunks,
even on targets that do not support COMDAT groups. This has not been a
problem as on platforms not supporting these (such as PE COFF on
Windows), the backend handled it through directives to GAS. GCC would
simply use a .linkonce directive telling the assembler that this
symbol may occur multiple times, and GAS would translate that into a
"select any" COMDAT, containing only the symbol itself (as Windows
does support COMDAT, just not groups).

When using LTO on Windows however, a few problems occur: The COMDAT
group is transmitted as part of the IR and the linker (ld) will try to
resolve symbols. On Windows the COMDAT information is put into the
symbol table, instead of in sections, leading to the linker to error
out with a multiple reference error before even calling the
lto-wrapper and LTRANS on the IR, which would otherwise resolve the
use of COMDAT groups.

This patch removes comdat groups for symbols in the ipa-visibility
pass and instead puts them into their own comdat. An exception to this
rule are aliases which (at least on Windows) are also allowed to be in
the same comdat group as the symbol they are referencing.

This fixes PR94156: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94156
A previous discussion on the problems this patch attempts to fix were
had here: https://gcc.gnu.org/pipermail/gcc-patches/2020-July/550148.html

I tested this patch with a x86_64-w64-mingw32 target on a Linux host.
No regressions between before and after of this patch were noted. The
Test cases provided with this patch have been confirmed to reproduce
before this patch, and link and work after the application of the
patch.

Feedback is very welcome, especially on implications I might not be aware of.

gcc/ChangeLog:

2020-03-23  Markus Böck  <markus.boeck02@gmail.com>

        * ipa-visibility.c (function_and_variable_visibility): Split
COMDAT groups on targets not supporting them

gcc/testsuite/ChangeLog:

2020-03-23  Markus Böck  <markus.boeck02@gmail.com>

        * g++.dg/lto/pr94156.h: New test.
        * g++.dg/lto/pr94156_0.C: New test.
        * g++.dg/lto/pr94156_1.C: New test.

--------------
diff --git a/gcc/ipa-visibility.c b/gcc/ipa-visibility.c
index eb0ebf770e3..76f1a8ff72a 100644
--- a/gcc/ipa-visibility.c
+++ b/gcc/ipa-visibility.c
@@ -709,6 +709,14 @@ function_and_variable_visibility (bool whole_program)
      }
    node->dissolve_same_comdat_group_list ();
  }
+
+      if (!HAVE_COMDAT_GROUP && node->same_comdat_group
+          && !node->alias && !node->has_aliases_p())
+        {
+          node->remove_from_same_comdat_group();
+          node->set_comdat_group(DECL_ASSEMBLER_NAME_RAW(node->decl));
+        }
+
       gcc_assert ((!DECL_WEAK (node->decl)
    && !DECL_COMDAT (node->decl))
                  || TREE_PUBLIC (node->decl)
@@ -742,8 +750,11 @@ function_and_variable_visibility (bool whole_program)
      {
        gcc_checking_assert (DECL_COMDAT (node->decl)
     == DECL_COMDAT (decl_node->decl));
-       gcc_checking_assert (node->in_same_comdat_group_p (decl_node));
-       gcc_checking_assert (node->same_comdat_group);
+       if (HAVE_COMDAT_GROUP)
+                {
+                  gcc_checking_assert (node->in_same_comdat_group_p
(decl_node));
+                  gcc_checking_assert (node->same_comdat_group);
+                }
      }
    node->forced_by_abi = decl_node->forced_by_abi;
    if (DECL_EXTERNAL (decl_node->decl))
diff --git a/gcc/testsuite/g++.dg/lto/pr94156.h
b/gcc/testsuite/g++.dg/lto/pr94156.h
new file mode 100644
index 00000000000..3990ac46fcb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr94156.h
@@ -0,0 +1,20 @@
+class Base0 {
+public:
+  virtual ~Base0() {}
+};
+
+class Base1 {
+public:
+  virtual void foo() = 0;
+};
+
+class Base2 {
+public:
+  virtual void foo() = 0;
+};
+
+class Derived : public Base0, public Base1, public Base2 {
+public:
+  virtual ~Derived();
+  virtual void foo() override {}
+};
diff --git a/gcc/testsuite/g++.dg/lto/pr94156_0.C
b/gcc/testsuite/g++.dg/lto/pr94156_0.C
new file mode 100644
index 00000000000..1a2e30badc7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr94156_0.C
@@ -0,0 +1,6 @@
+// { dg-lto-do link }
+#include "pr94156.h"
+
+Derived::~Derived() {}
+
+int main() {}
diff --git a/gcc/testsuite/g++.dg/lto/pr94156_1.C
b/gcc/testsuite/g++.dg/lto/pr94156_1.C
new file mode 100644
index 00000000000..d7a40efa96c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr94156_1.C
@@ -0,0 +1,6 @@
+#include "pr94156.h"
+
+void bar(Derived* p)
+{
+  p->foo();
+}

             reply	other threads:[~2021-03-23 16:27 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-23 16:27 Markus Böck [this message]
2021-05-05 13:04 ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAEZ8vhnLo3uAj+uGb_AKgUZigdYheb2kzAUjau8ua0n_=gOb=g@mail.gmail.com' \
    --to=markus.boeck02@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).