]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
d: Fix wrong vtable offset in virtual function call
authorIain Buclaw <ibuclaw@gdcproject.org>
Sat, 16 May 2020 21:33:15 +0000 (23:33 +0200)
committerIain Buclaw <ibuclaw@gdcproject.org>
Sat, 16 May 2020 22:20:09 +0000 (00:20 +0200)
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.

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/ChangeLog
gcc/d/dmd/dclass.c
gcc/testsuite/ChangeLog
gcc/testsuite/gdc.test/compilable/imports/pr9471a.d [new file with mode: 0644]
gcc/testsuite/gdc.test/compilable/imports/pr9471b.d [new file with mode: 0644]
gcc/testsuite/gdc.test/compilable/imports/pr9471c.d [new file with mode: 0644]
gcc/testsuite/gdc.test/compilable/imports/pr9471d.d [new file with mode: 0644]
gcc/testsuite/gdc.test/compilable/pr9471.d [new file with mode: 0644]

index 9cc59c8ad1d5282f00ac77177f005a5342738117..c3f565902985cec9769f684b223edc5cd8e92f8e 100644 (file)
@@ -1,3 +1,9 @@
+2020-05-16  Iain Buclaw  <ibuclaw@gdcproject.org>
+
+       PR d/95155
+       * dmd/dclass.c (ClassDeclaration::semantic): Don't prematurely
+       set done on semantic analysis.
+
 2020-04-07  Iain Buclaw  <ibuclaw@gdcproject.org>
 
        PR d/94240
index 572b3e24387a62305e36f2522724ab0ab94bbbbc..66869361dcb0e41c57bac79e36036ebc2853b2d8 100644 (file)
@@ -395,7 +395,6 @@ void ClassDeclaration::semantic(Scope *sc)
     }
     else if (symtab && !scx)
     {
-        semanticRun = PASSsemanticdone;
         return;
     }
     semanticRun = PASSsemantic;
index b69958e23d09375fa8110e20f4e4cb5257d2c6da..0b70f660fb85ab532fa9430fc39f5347fe26e046 100644 (file)
@@ -1,3 +1,12 @@
+2020-05-16  Iain Buclaw  <ibuclaw@gdcproject.org>
+
+       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.
+
 2020-05-14  Szabolcs Nagy  <szabolcs.nagy@arm.com>
 
        Backport from mainline.
diff --git a/gcc/testsuite/gdc.test/compilable/imports/pr9471a.d b/gcc/testsuite/gdc.test/compilable/imports/pr9471a.d
new file mode 100644 (file)
index 0000000..79b78e1
--- /dev/null
@@ -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 (file)
index 0000000..a46a12c
--- /dev/null
@@ -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 (file)
index 0000000..d80a614
--- /dev/null
@@ -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 (file)
index 0000000..187b908
--- /dev/null
@@ -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 (file)
index 0000000..37ff32e
--- /dev/null
@@ -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);