Jump to page: 1 2
Thread overview
[Issue 11777] New: [ICE] Compiler segfault in `callfunc` in `e2ir.c`
Dec 19, 2013
Denis Shelomovskij
Dec 19, 2013
Denis Shelomovskij
Dec 24, 2013
yebblies
Dec 24, 2013
Vladimir Panteleev
Dec 24, 2013
Denis Shelomovskij
Dec 24, 2013
Denis Shelomovskij
Dec 24, 2013
Denis Shelomovskij
[Issue 11777] [ICE] dmd memory corruption as `Scope::pop` `free`s `fieldinit` used also in `enclosing`
Dec 24, 2013
Kenji Hara
Dec 25, 2013
Denis Shelomovskij
Dec 31, 2013
Walter Bright
Mar 19, 2014
Denis Shelomovskij
Mar 19, 2014
Denis Shelomovskij
Apr 04, 2014
Kenji Hara
Apr 06, 2014
Kenji Hara
December 19, 2013
https://d.puremagic.com/issues/show_bug.cgi?id=11777

           Summary: [ICE] Compiler segfault in `callfunc` in `e2ir.c`
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Keywords: ice
          Severity: regression
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: verylonglogin.reg@gmail.com


--- Comment #0 from Denis Shelomovskij <verylonglogin.reg@gmail.com> 2013-12-19 22:08:47 MSK ---
The issue is caused by refactoring commit 1e99eed73c06ae450c1c13352021e4b629d2bba8 [1] from dmd pull 2771 [2].


Remove `mem.free(fieldinit)` added at line 192 of `scope.c` by the causing commit [1] to detrigger the issue.


At first sight looks like `free`d memory is used. But the compiler segfaults at line 203 of `callfunc` in `e2ir.c` because `arg` has invalid vtable address (0x0037382d for me):
---
ea = arg->toElem(irs);
---

Also `arg` is derived from `Expression` so is at least `sizeof(Expression)` =
22 bytes long, but if we look at its memory:
  4 bytes: invalid vtable
 10 bytes: allocated but uninitialized (0xCD byte in MS CRT)
  4 bytes: allocation end guard (0xFD byte in MS CRT)

So it is only 14 bytes long and the only initialized part is vtable pointer. Clear memory corruption.


Sorry, the testcase is big and proprietary and can be only obtained directly from the issue author by e-mailing him.


[1] https://github.com/D-Programming-Language/dmd/commit/1e99eed73c06ae450c1c13352021e4b629d2bba8 [2] https://github.com/D-Programming-Language/dmd/pull/2771

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
December 19, 2013
https://d.puremagic.com/issues/show_bug.cgi?id=11777



--- Comment #1 from Denis Shelomovskij <verylonglogin.reg@gmail.com> 2013-12-19 22:13:15 MSK ---
A workarounding pull: https://github.com/D-Programming-Language/dmd/pull/2990

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
December 24, 2013
https://d.puremagic.com/issues/show_bug.cgi?id=11777


yebblies <yebblies@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |yebblies@gmail.com


--- Comment #2 from yebblies <yebblies@gmail.com> 2013-12-24 12:36:03 EST ---
https://github.com/D-Programming-Language/dmd/commit/89e778a9eee645d2975cbb134e5cfd578bc1ab01

This will be much more likely to stay fixed if you can find a reduced test case...

A possible way to find it would be to NULL out the array instead of freeing.

Please confirm it's fixed in your program.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
December 24, 2013
https://d.puremagic.com/issues/show_bug.cgi?id=11777


Vladimir Panteleev <thecybershadow@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |thecybershadow@gmail.com


--- Comment #3 from Vladimir Panteleev <thecybershadow@gmail.com> 2013-12-24 03:39:05 EET ---
DustMite can be used to reduce AND obfuscate test cases. I'd be glad to assist in using DustMite if you're having any problems applying it - just contact me.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
December 24, 2013
https://d.puremagic.com/issues/show_bug.cgi?id=11777



--- Comment #4 from Denis Shelomovskij <verylonglogin.reg@gmail.com> 2013-12-24 16:10:31 MSK ---
(In reply to comment #2)
> https://github.com/D-Programming-Language/dmd/commit/89e778a9eee645d2975cbb134e5cfd578bc1ab01
> 
> This will be much more likely to stay fixed if you can find a reduced test case...

Please, no. Memory corruption cause must be detected. I will investigate today.

> A possible way to find it would be to NULL out the array instead of freeing.

Nop. MS CRT already marks uninitialized and freed memory appropriately, it also checks previously freed memory isn't changed when allocation returns previously freed pointer.

> Please confirm it's fixed in your program.

Of course the bug is detriggered as I'm the pull author. )

(In reply to comment #3)
> DustMite can be used to reduce AND obfuscate test cases. I'd be glad to assist in using DustMite if you're having any problems applying it - just contact me.

Thanks, but I don't think it will help a lot with random memory corruption.



So, the investigation:

`callfunc` is unrelated, just a random victim who was unlucky to first access corrupted memory. There are random failures in other functions too. Here is the guilty code, it is in from `Scope::pop`:
---
for (size_t i = 0; i < dim; i++)
    enclosing->fieldinit[i] |= fieldinit[i]; // line 2
mem.free(fieldinit); // line 3
---

The issue trace:
1. There is a `Scope` with `fieldinit` and there is another reference to
`fieldinit`.
2. `Scope::pop` `free`-s `fieldinit` at line 3.
3. This memory is reused for different purpose.
4. Now there is another scope with same `fieldinit`.
5. `Scope::pop` executed with such scope as `enclosing` and corrupts reused
memory at line 2.
6. Random crashed/wrong code/whatever.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
December 24, 2013
https://d.puremagic.com/issues/show_bug.cgi?id=11777



--- Comment #5 from Denis Shelomovskij <verylonglogin.reg@gmail.com> 2013-12-24 16:35:06 MSK ---
The more precise issue trace:
1. There is a `Scope` with `fieldinit == enclosing->fieldinit`
2. This scope's `pop` `free`-s `fieldinit`.
3. `free`-d memory is reused for different purpose.
5. `pop` on another scope with same `enclosing` corrupts reused memory.
6. Same bad end.

Also in my case "another scope" is initially nested in the first scope (3-level child, if we call 1-level a direct child).

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
December 24, 2013
https://d.puremagic.com/issues/show_bug.cgi?id=11777



--- Comment #6 from Denis Shelomovskij <verylonglogin.reg@gmail.com> 2013-12-24 16:42:30 MSK ---
So now everyone can add `assert(enclosing->fieldinit != fieldinit);` check in `Scope::pop` and test if the issue is triggered in his codebase, as in the worst case it may silently occur without visible effect but with code corruption.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
December 24, 2013
https://d.puremagic.com/issues/show_bug.cgi?id=11777



--- Comment #7 from Kenji Hara <k.hara.pg@gmail.com> 2013-12-24 15:21:31 PST ---
(In reply to comment #6)
> So now everyone can add `assert(enclosing->fieldinit != fieldinit);` check in `Scope::pop` and test if the issue is triggered in his codebase, as in the worst case it may silently occur without visible effect but with code corruption.

Denis, can you test this patch?

 src/scope.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/scope.c b/src/scope.c
index f51a30c..c22b585 100644
--- a/src/scope.c
+++ b/src/scope.c
@@ -188,14 +188,17 @@ Scope *Scope::pop()
         enclosing->callSuper |= callSuper;
         if (enclosing->fieldinit && fieldinit)
         {
+            assert(enclosing->fieldinit != fieldinit);
+
             size_t dim = fieldinit_dim;
             for (size_t i = 0; i < dim; i++)
                 enclosing->fieldinit[i] |= fieldinit[i];
-            /* Workaround regression @@@BUG11777@@@.
-            Probably this memory is used in future.
-            mem.free(fieldinit);
-            */
-            fieldinit = NULL;
+
+            if (!nofree)
+            {
+                mem.free(fieldinit);
+                fieldinit = NULL;
+            }
         }
     }

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
December 25, 2013
https://d.puremagic.com/issues/show_bug.cgi?id=11777



--- Comment #8 from Denis Shelomovskij <verylonglogin.reg@gmail.com> 2013-12-25 14:09:38 MSK ---
(In reply to comment #7)
> (In reply to comment #6)
> > So now everyone can add `assert(enclosing->fieldinit != fieldinit);` check in `Scope::pop` and test if the issue is triggered in his codebase, as in the worst case it may silently occur without visible effect but with code corruption.
> 
> Denis, can you test this patch?

I don't understand. The `assert` will definitely fail as I wrote. Did you mean this:
---
assert(nofree || enclosing->fieldinit != fieldinit);
---
?
Anyway this `assert` fails too.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
December 31, 2013
https://d.puremagic.com/issues/show_bug.cgi?id=11777


Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |bugzilla@digitalmars.com
         Resolution|                            |FIXED


--- Comment #9 from Walter Bright <bugzilla@digitalmars.com> 2013-12-31 14:07:41 PST ---
Since https://github.com/D-Programming-Language/dmd/pull/2990 was pulled, I'm going to mark this as resolved.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
« First   ‹ Prev
1 2