Opened 5 years ago

Closed 5 years ago

#9208 closed bug (fixed)

panic - attempt to prod-split strictness call demand C(S(C(C(S(LS)))))

Reported by: luite Owned by:
Priority: normal Milestone: 7.8.3
Component: Compiler Version: 7.8.2
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case: stranal/should_compile/T9208
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

tested with GHC 7.8 branch commit 32b4bf33989bdda4dffed1866f7a61a7da4ca275 and GHC 7.8.2

compile with -O or -O2

Attachments (1)

Eval.hs (2.8 KB) - added by luite 5 years ago.

Download all attachments as: .zip

Change History (16)

Changed 5 years ago by luite

Attachment: Eval.hs added

comment:1 Changed 5 years ago by simonpj

Test Case: stranal/should_compile/T9208

Good point, thank you.

In the test program you use unsafeCoerce, and in fact you end up doing something like

(coerce ()) arg

that is, you take () (produced by loadTHData) and apply it to something (in runTH). So you are going to get some bizarre runtime crash. Maybe your real example was not as perverse as this one.

But regardless, it shouldn't crash the compiler, I agree. I've fixed that; patch coming.

comment:2 Changed 5 years ago by luite

Yes, in my real code I'm not quite doing this, but since it's part of the ghcjs-prim package I had to remove some code (foreign import javascript) that's not supported by normal GHC. The actual loadTHData would do some JavaScript runtime calls to load the code in the ByteString, initialise the info tables and then return a closure of the expected type.

It's part of my work to decouple Template Haskell from the GHC process and thus the host platform. GHC sets up a connection to a TH service and incrementally sends compiled code (target architecture) to it, the TH service can query GHC for reification. For GHCJS this is a node.js server, but when when my proof of concept works, I'll ask on ghc-devs if there's interest in extending this for general cross compilation in GHC.

comment:3 Changed 5 years ago by Simon Peyton Jones <simonpj@…>

In 2e362ddebf2286409b7423d3dd49152117c1ae56/ghc:

Make splitStrProdDmd (and similarly Use) more robust

The issue here is avoiding a GHC crash when a program uses
unsafeCoerce is a dangerous (or even outright-wrong) way.

See Trac #9208

comment:4 Changed 5 years ago by simonpj

Status: newmerge

Fix is above. Merge to 7.8 branch when it's next convenient. Luite: yell if you really, really need this in 7.8.3.

Simon

Last edited 5 years ago by simonpj (previous) (diff)

comment:5 Changed 5 years ago by luite

Thanks for the fix. I have a workaround, so for me it's not essential for it to be in 7.8.3

comment:6 Changed 5 years ago by nomeata

There is something wrong with the test case. With -DDEBUG, I get

Compile failed (status 256) errors were:
ghc-stage2: panic! (the 'impossible' happened)
  (GHC version 7.9.20140622 for x86_64-unknown-linux):
	ASSERT failed! file compiler/codeGen/StgCmmExpr.hs, line 643

Please report this as a GHC bug:  http://www.haskell.org/ghc/reportabug


*** unexpected failure for T9208(optasm)

(full log at https://s3.amazonaws.com/archive.travis-ci.org/jobs/28166586/log.txt) while it worked without -DDEBUG.

(Didn’t look into it deeper. Greetings from OPLSS! :-))

comment:7 Changed 5 years ago by simonpj

No, there's nothing wrong with the test case, but I should have commented on it.

There really is a terrible bug in the program: it takes the value (), unsafely casts it to a function, and applies it. As it happens the ASSERT in StgCmmExpr can see this happening, and complains. (But a slightly more complicated example might conceal the problem.) So something bizarre is going to happen, but that's what the programmer asked for.

In general GHC should never crash, no matter how bizarre the program. You could argue for a civilised error messaage. Or you could argue to remove the ASSERT. But I don't want to spend precious cycles thinking about programs that are wrong anyway.

The strongest case for doing something is that building stage-2 with -DDEBUG is really a good plan for flushing out latent bugs, but this test will make it look as if there is a failure. I'm open to suggestions about how to fix that. Perhaps the best way would be to switch on CmmLint as well as -DDEBUG, and make sure CmmLint detects this particular problem.

Simon

comment:8 Changed 5 years ago by thoughtpolice

Milestone: 7.8.3

comment:9 Changed 5 years ago by Joachim Breitner <mail@…>

In 518ada5cda08d3256826ed0383888111f8096de5/ghc:

Mark T9208 as broken when debugging is on

this seems to be expected, as explained by SPJ in comment 7 of #9208.

comment:10 Changed 5 years ago by Simon Peyton Jones <simonpj@…>

In 8a0aa198f78cac1ca8d0695bd711778e8ad086aa/ghc:

Comment the expect_broken for Trac #9208

comment:11 Changed 5 years ago by thoughtpolice

Resolution: fixed
Status: mergeclosed

Merged into 7.8.3.

comment:12 Changed 5 years ago by thoughtpolice

Status: closedmerge

Aw crap, I forgot to merge the last two changes to the testsuite as well to mark T9208 as broken. Those will be incoming soon.

comment:13 Changed 5 years ago by dfeuer

Wouldn't it be more correct to remove the assertion and try to come up with a better one in a future release than to ship with an invalid assertion?

comment:14 Changed 5 years ago by simonpj

Probably. Feel free to have a go. It's so much a corner case that I can't bear to spend the time to think about it.

Simon

comment:15 Changed 5 years ago by thoughtpolice

Status: mergeclosed

OK, merged.

Note: See TracTickets for help on using tickets.