diff mbox series

[committed,GCC9] d: Fix wrong vtable offset in virtual function call

Message ID 20200516222342.29585-1-ibuclaw@gdcproject.org
State New
Headers show
Series [committed,GCC9] d: Fix wrong vtable offset in virtual function call | expand

Commit Message

Iain Buclaw May 16, 2020, 10:23 p.m. UTC
Hi,

This patch fixes PR95155, which prevented the D front-end in gcc-9 from
being able to bootstrap a self-hosted D compiler.

The Semantic (pass 1) analysis for classes is handled by
ClassDeclaration::semantic.  For a given class, this method may be ran
multiple times in order to resolve forward references.  The method
incrementally tries to resolve the types referred to by the members of
the class.

The subsequent calls to this method are short-circuited if the class
members have been fully analyzed.  For this the code tests that it is
not the first/main call to the method (semanticRun == PASS.init else
branch), scx is not set, and that the this->symtab is already set.  If
all these conditions are met, the method returns.  But before returning,
the method was setting this->semanticRun to PASSsemanticdone.  It should
not set semanticRun since the class has not been fully analyzed yet.
The base class analysis for this class could be pending and as a result
vtable may not have been fully created.

This fake setting of semanticRun results in the semantic analyzer to
believe that the class has been fully analyzed.  As exposed by the
issues in upstream, it may result in compile time errors when a derived
type class is getting analyzed and because of this fake semanticdone on
the base class, the semantic analysis construes that an overriden method
is not defined in the base class.  PR95155 exposes anoter scenario where
a buggy vtable may be created and a call to a class method may result in
execution of some adhoc code.

Backported from r10-1131.

Regression tested on x86_64-linux-gnu, committed to gcc-9 branch.

Regards
Iain

---
gcc/d/ChangeLog:

	PR d/95155
	* dmd/dclass.c (ClassDeclaration::semantic): Don't prematurely
	set done on semantic analysis.

gcc/testsuite/ChangeLog:

	PR d/95155
	* gdc.test/compilable/imports/pr9471a.d: New test.
	* gdc.test/compilable/imports/pr9471b.d: New test.
	* gdc.test/compilable/imports/pr9471c.d: New test.
	* gdc.test/compilable/imports/pr9471d.d: New test.
	* gdc.test/compilable/pr9471.d: New test.
---
 gcc/d/dmd/dclass.c                             |  1 -
 .../gdc.test/compilable/imports/pr9471a.d      |  2 ++
 .../gdc.test/compilable/imports/pr9471b.d      |  5 +++++
 .../gdc.test/compilable/imports/pr9471c.d      | 18 ++++++++++++++++++
 .../gdc.test/compilable/imports/pr9471d.d      |  1 +
 gcc/testsuite/gdc.test/compilable/pr9471.d     |  6 ++++++
 6 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gdc.test/compilable/imports/pr9471a.d
 create mode 100644 gcc/testsuite/gdc.test/compilable/imports/pr9471b.d
 create mode 100644 gcc/testsuite/gdc.test/compilable/imports/pr9471c.d
 create mode 100644 gcc/testsuite/gdc.test/compilable/imports/pr9471d.d
 create mode 100644 gcc/testsuite/gdc.test/compilable/pr9471.d
diff mbox series

Patch

diff --git a/gcc/d/dmd/dclass.c b/gcc/d/dmd/dclass.c
index 572b3e24387..66869361dcb 100644
--- a/gcc/d/dmd/dclass.c
+++ b/gcc/d/dmd/dclass.c
@@ -395,7 +395,6 @@  void ClassDeclaration::semantic(Scope *sc)
     }
     else if (symtab && !scx)
     {
-        semanticRun = PASSsemanticdone;
         return;
     }
     semanticRun = PASSsemantic;
diff --git a/gcc/testsuite/gdc.test/compilable/imports/pr9471a.d b/gcc/testsuite/gdc.test/compilable/imports/pr9471a.d
new file mode 100644
index 00000000000..79b78e1e52a
--- /dev/null
+++ b/gcc/testsuite/gdc.test/compilable/imports/pr9471a.d
@@ -0,0 +1,2 @@ 
+import imports.pr9471c;
+class AggregateDeclaration : ScopeDsymbol { }
diff --git a/gcc/testsuite/gdc.test/compilable/imports/pr9471b.d b/gcc/testsuite/gdc.test/compilable/imports/pr9471b.d
new file mode 100644
index 00000000000..a46a12c496f
--- /dev/null
+++ b/gcc/testsuite/gdc.test/compilable/imports/pr9471b.d
@@ -0,0 +1,5 @@ 
+import imports.pr9471a;
+class ClassDeclaration : AggregateDeclaration
+{
+    void isBaseOf();
+}
diff --git a/gcc/testsuite/gdc.test/compilable/imports/pr9471c.d b/gcc/testsuite/gdc.test/compilable/imports/pr9471c.d
new file mode 100644
index 00000000000..d80a61480ce
--- /dev/null
+++ b/gcc/testsuite/gdc.test/compilable/imports/pr9471c.d
@@ -0,0 +1,18 @@ 
+import imports.pr9471b;
+
+struct Array(T)
+{
+    static if (is(typeof(T.opCmp))) { }
+}
+alias ClassDeclarations = Array!ClassDeclaration;
+
+class Dsymbol
+{
+    void addObjcSymbols(ClassDeclarations);
+}
+
+class ScopeDsymbol : Dsymbol
+{
+    import imports.pr9471d;
+    void importScope();
+}
diff --git a/gcc/testsuite/gdc.test/compilable/imports/pr9471d.d b/gcc/testsuite/gdc.test/compilable/imports/pr9471d.d
new file mode 100644
index 00000000000..187b9083294
--- /dev/null
+++ b/gcc/testsuite/gdc.test/compilable/imports/pr9471d.d
@@ -0,0 +1 @@ 
+// Module needs to be imported to trigger bug.
diff --git a/gcc/testsuite/gdc.test/compilable/pr9471.d b/gcc/testsuite/gdc.test/compilable/pr9471.d
new file mode 100644
index 00000000000..37ff32e4957
--- /dev/null
+++ b/gcc/testsuite/gdc.test/compilable/pr9471.d
@@ -0,0 +1,6 @@ 
+// PERMUTE_ARGS:
+// EXTRA_FILES: imports/pr9471a.d imports/pr9471b.d imports/pr9471c.d imports/pr9471d.d
+import imports.pr9471a;
+import imports.pr9471b;
+
+static assert (__traits(getVirtualIndex, ClassDeclaration.isBaseOf) == 7);