diff mbox series

[GCC13,committed] d: Fix crash in d/dmd/root/aav.d:127 dmd_aaGetRvalue from DsymbolTable::lookup (PR110113)

Message ID 20230626005142.333366-1-ibuclaw@gdcproject.org
State New
Headers show
Series [GCC13,committed] d: Fix crash in d/dmd/root/aav.d:127 dmd_aaGetRvalue from DsymbolTable::lookup (PR110113) | expand

Commit Message

Iain Buclaw June 26, 2023, 12:51 a.m. UTC 
Hi,

This backports patch from upstream dmd mainline for fixing PR110113.

The data being Mem.xrealloc'd contains many Array(T) fields, some of
which have self references in their data.ptr field thanks to the
smallarray optimization used by Array.

Naturally then, the memcpy from old GC data to new retains those self
referenced addresses, and the GC marks the old data as "free". Some time
later GC.malloc will return a pointer to said "free" data. So now we
have two GC references to the same memory. One that is treating the data
as an Array(VarDeclaration) in dmd.escape.escapeByStorage, and the other
as an AA in the symtab of a dmd.dsymbol.ScopeDsymbol.

Fix this memory corruption by not storing the data in a global variable
for reuse.  If there are no more live references, the GC will free it.

Bootstrapped and regression tested on x86_64-linux-gnu/-m32, committed
to releases/gcc-13, and backported to releases/gcc-12.

Regards,
Iain.

---
	PR d/110113

gcc/d/ChangeLog:

	* dmd/escape.d (checkMutableArguments): Always allocate new buffer for
	computing escapeBy.

gcc/testsuite/ChangeLog:

	* gdc.test/compilable/test23978.d: New test.

Reviewed-on: https://github.com/dlang/dmd/pull/15302
---
 gcc/d/dmd/escape.d                            | 24 +--------------
 gcc/testsuite/gdc.test/compilable/test23978.d | 30 +++++++++++++++++++
 2 files changed, 31 insertions(+), 23 deletions(-)
 create mode 100644 gcc/testsuite/gdc.test/compilable/test23978.d
diff mbox series

Patch

diff --git a/gcc/d/dmd/escape.d b/gcc/d/dmd/escape.d
index 420fa7f80bb..7586e5c7184 100644
--- a/gcc/d/dmd/escape.d
+++ b/gcc/d/dmd/escape.d
@@ -93,22 +93,7 @@  bool checkMutableArguments(Scope* sc, FuncDeclaration fd, TypeFunction tf,
         bool isMutable;         // true if reference to mutable
     }
 
-    /* Store escapeBy as static data escapeByStorage so we can keep reusing the same
-     * arrays rather than reallocating them.
-     */
-    __gshared EscapeBy[] escapeByStorage;
-    auto escapeBy = escapeByStorage;
-    if (escapeBy.length < len)
-    {
-        auto newPtr = cast(EscapeBy*)mem.xrealloc(escapeBy.ptr, len * EscapeBy.sizeof);
-        // Clear the new section
-        memset(newPtr + escapeBy.length, 0, (len - escapeBy.length) * EscapeBy.sizeof);
-        escapeBy = newPtr[0 .. len];
-        escapeByStorage = escapeBy;
-    }
-    else
-        escapeBy = escapeBy[0 .. len];
-
+    auto escapeBy = new EscapeBy[len];
     const paramLength = tf.parameterList.length;
 
     // Fill in escapeBy[] with arguments[], ethis, and outerVars[]
@@ -228,13 +213,6 @@  bool checkMutableArguments(Scope* sc, FuncDeclaration fd, TypeFunction tf,
         escape(i, eb, false);
     }
 
-    /* Reset the arrays in escapeBy[] so we can reuse them next time through
-     */
-    foreach (ref eb; escapeBy)
-    {
-        eb.er.reset();
-    }
-
     return errors;
 }
 
diff --git a/gcc/testsuite/gdc.test/compilable/test23978.d b/gcc/testsuite/gdc.test/compilable/test23978.d
new file mode 100644
index 00000000000..cc30f728dee
--- /dev/null
+++ b/gcc/testsuite/gdc.test/compilable/test23978.d
@@ -0,0 +1,30 @@ 
+// REQUIRED_ARGS: -preview=dip1021 -lowmem
+// https://issues.dlang.org/show_bug.cgi?id=23978
+
+// Note: this is a memory corruption bug.
+// Memory returned by `GC.realloc` retains references to old memory in it,
+// mostly because of the smallarray optimization for `Array(T)`.
+// If this fails again, it might not be consistent, so try running it multiple times.
+
+class LUBench { }
+void lup(ulong , ulong , int , int = 1)
+{
+    new LUBench;
+}
+void lup_3200(ulong iters, ulong flops)
+{
+    lup(iters, flops, 3200);
+}
+void raytrace()
+{
+    struct V
+    {
+        float x, y, z;
+        auto normalize() { }
+        struct Tid { }
+        auto spawnLinked() { }
+        string[] namesByTid;
+        class MessageBox { }
+        auto cross() { }
+    }
+}