Fix Issue 18871 & 18819 - DMD illegal hardware instruction crash

[JinShil]

I'm still trying to create a reduced test case. When I print out ea.toChars(), I get this:

CallExp::interpret() _ArrayDtor([Vector(), Vector()])
1 ea = Vector[] [Vector(), Vector()]
2 ea = Vector[], arrayliteral [Vector(), Vector()]

If someone knows how to get a reduced test case from that. Please let me know.

[dlang-bot]

Thanks for your pull request, @JinShil!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	18819	normal	DMD compilation crash
✓	18871	regression	DMD "illegal hardware instruction" crash

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "stable + dmd#8260"

[RazvanN7]

If someone knows how to get a reduced test case from that. Please let me know.

You can use dustmite [1].

I added the "Needs work" label due to the missing test.

[1] https://github.com/CyberShadow/DustMite/wiki

[JinShil]

Figured it out. This is ready for review.

[dnadlinger]

Is _ArrayDtor supposed to be a public API?

[JinShil]

Is _ArrayDtor supposed to be a public API?

I don't think so, but so far, that's the only way I've been able to reproduce the bug without adding dependencies on std.experimental.allocator. I created that test based on what I saw in the compiler's lowering.

[JinShil]

Actually, I should probably wait until dlang/druntime#2147 and #8067 are pulled and test this again.

[dnadlinger]

Yep, I just tried your test case with _ArrayDtor replaced by a call to a copy (void foo(T)(T[] a) { foreach_reverse (ref T e; a) e.__xdtor(); }) in the same module, as well as from an imported module. I couldn't reproduce the crash.

Edit: Oh, of course, that code is in a special case for _Array{Postblit,Dtor}. Perhaps we can remove the special case entirely after #8067 is in?

[JinShil]

Sorry, I don't understand. I'm not seeing the special case.

[dnadlinger]

The whole if statement special-cases calls to the two functions I mentioned: https://github.com/dlang/dmd/pull/8260/files#diff-264b29261e215213d3026cd44003c2ebR4826

[dnadlinger]

Either way, it would be great if we had a test case that didn't directly depend on _ArrayDtor – both to avoid it being unnecessarily brittle, and because the bug might actually lie elsewhere: Calling destructors directly on an ArrayLiteralExp seems somewhat weird.

[JinShil]

Calling destructors directly on an ArrayLiteralExp seems somewhat weird

Yeah, that doesn't seem right, does it. I'll use digger to see if I can identify what caused the regression.

[JinShil]

It appears the regression was caused by dlang/phobos#6411

I don't think this is the right fix. Calling a destructor on an array literal is nonsense.

[dnadlinger]

Well, it's still a DMD bug, as it certainly shouldn't crash like that…

[JinShil]

Ok, reopened in case someone wants to merge it.

[JinShil]

Fixed test case thanks to reduction in given in https://issues.dlang.org/show_bug.cgi?id=18819#c4