Ticket #5844 (new bug)

Opened 4 months ago

Last modified 5 weeks ago

Panic on generating Core code

Reported by: JamesFisher Owned by:
Priority: normal Milestone: 7.6.1
Component: External Core Version: 7.4.1
Keywords: Cc: jameshfisher@…, eventh@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Compile-time crash Difficulty: Unknown
Test Case: Blocked By:
Blocking: Related Tickets:

Description

STEPS TO REPRODUCE

1. Create the file "Panic.hs" with the contents:

module Main where
main = print 1

2. Compile with "ghc -fext-core Panic.hs"

EXPECTED BEHAVIOR

Generation of the Core file "Panic.hcr".

ACTUAL BEHAVIOR

$ ghc -fext-core Panic.hs
[1 of 1] Compiling Main             ( Panic.hs, Panic.o )
ghc: panic! (the 'impossible' happened)
  (GHC version 7.4.1 for x86_64-unknown-linux):
	MkExternalCore died: make_lit

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

$

As it states, I am using GHC version 7.4.1 for x86_64 Linux, on an x86-64 Linux box running Ubuntu 11.10 an Linux kernel 3.0.0-14-generic.

Attachments

log Download (88.0 KB) - added by JamesFisher 4 months ago.
result of ghc -v10 -fext-core Panic.hs &> log
core.png Download (95.3 KB) - added by JamesFisher 4 months ago.
Schematic of Core organization, current and proposed
external_core.xml Download (74.0 KB) - added by JamesFisher 3 months ago.
To be included as docs/users_guide/external_core.xml
0001-New-User-s-Guide-document-docs-users_guide-external_.patch Download (76.7 KB) - added by JamesFisher 3 months ago.
Patch to include previously attached XML file

Change History

  Changed 4 months ago by JamesFisher

This appears to be caused by the presence of the literal 1 in certain typing contexts. It crashes when I place the following in the position of 1 in the above test:

0
2
3 [...]
(-1)
(fromIntegral 1)
[1,2]
(Just 1)
(toInteger 5)
(1 :: Integer)
(1 :: Rational)

It does NOT crash when replaced with:

1e3
0x45
(1 :: Int)
"foo"
'a'
("foo" :: Integer)

Changed 4 months ago by JamesFisher

  • attachment log Download added

result of ghc -v10 -fext-core Panic.hs &> log

  Changed 4 months ago by JamesFisher

As the attached log notes, the panic happens after a warning about attempting to delete a non-existent .s file:

*** Deleting temp files:
Deleting: /tmp/ghc10588_0/ghc10588_0.s
Warning: deleting non-existent /tmp/ghc10588_0/ghc10588_0.s
*** Deleting temp dirs:
Deleting: /tmp/ghc10588_0
ghc: panic! (the 'impossible' happened)
  (GHC version 7.4.1 for x86_64-unknown-linux):
        MkExternalCore died: make_lit

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

  Changed 4 months ago by JamesFisher

(I should note that after GHC exits, the directory /tmp/ghc10588_0 is indeed deleted.)

  Changed 4 months ago by simonpj

  • difficulty set to Unknown

Whenever we extend Core we have to invent new External Core syntax to match. And we have not done that with type-level literals.

External Core needs love. Does anyone feel like being the source of that love? We'll gladly help with advice etc.

Simon

follow-up: ↓ 6   Changed 4 months ago by JamesFisher

  • cc jameshfisher@… added
  • os changed from Linux to Unknown/Multiple
  • component changed from Compiler to External Core
  • architecture changed from x86_64 (amd64) to Unknown/Multiple

in reply to: ↑ 5   Changed 4 months ago by simonmar

  • milestone set to 7.6.1

Replying to simonpj:

Whenever we extend Core we have to invent new External Core syntax to match. And we have not done that with type-level literals.

I think in fact it's the new Integer literals, not type-level literals, causing the reported problem here.

Replying to JamesFisher:

Ah, I see. Is this where we should be looking?:  http://hackage.haskell.org/trac/ghc/browser/compiler/coreSyn/PprExternalCore.lhs

Yes.

follow-ups: ↓ 9 ↓ 17   Changed 4 months ago by simonpj

Short answer Yes... but there is a longer and more interesting answer. External Core is designed to round-trip, thus:

  • External Core is a data type defined in coreSyn/ExternalCore.
  • Core is converted to the External Core data type by coreSyn/MkExternalCore
  • The External Core data type is printed by PprExternalCore into concrete syntax.
  • The modified External Core can be parsed by parser/ParserCore. However, it is not parsed into the External Core data type, but rather into "Iface Core" defined in iface/IfaceSyn.
  • The Iface Core data type (thus parsed) can be converted to Core by iface/TcIface.

What is Iface Core? Iface Core is a data type that GHC uses every time you compile a module. During compilation, GHC converts Core into Iface Core, and then serialises Iface Core into the "M.hi" interface file, in a binary format. This is done by iface/BinIface. Then, when GHC wants to read an interface file, it de-serialises M.hi into Iface Core, and then converts Iface Core into Core with iface/TcIface. What this means is that Iface Core gets plenty of love: it is on our critical path.

Why does "External Core" do the round trip via the External Core data type on the way our, but via Iface Core data type on the way in? This mis-match just an aretefact of an earlier era: when we first implemented External Core, there was no Iface Core.

I think the Right Thing to do is to complete the change that we have already started:

  • Abandon the Exgternal Core data type entirely.
  • The conversion from Core to Iface Core is already done (in iface/MkIface), because GHC uses it every time it compiles a moudle.
  • Change the pretty-printer for External Core to pretty-print the Iface Core rarther than External Core. In fact there already is a pretty-printer for Iface Core, in iface/LoadIface.pprModIface, although it might require minor modification.

The bottom line is that we'd be abandoning the External Core data type (though not its concrete syntax), and switching entirely to Iface Core, which is much better maintained. Less code, more robust to change; what's not to like?

Now, while I say that there is a pretty printer for Iface Core already, it is designed mainly for humans; it is used when you say ghc --show-iface M.hi, and for no other purpose. The External Core concrete syntax, on the other hand, is designed to be parseable as well, so that the round-trip mechanism (print Exgternal Core, modify it, and read it back in) works right. But I think it would be quite acceptable to modify the Iface Core pretty-printer to use External Core syntax, because the Iface Core pretty printer has an output-only role in GHC. (If you do this, I suggest you put the new pretty-printer in a module of its own, perhaps iface/PprIface.)

If this was done, the maintenance burden of External Core, if/when we add new features to Core, would be reduced to

  • Designing new concrete (ASCII) syntax for the new features
  • Writing the pretty-printer and parser for this new syntax

I do think that the concrete syntax of External Core should be described by a human-readable BNF grammar, not only by a Happy parser. We have a very old Latex document describing External Core (including is syntax); I'd like to see it become a full part of the documentation, perhaps by convrting it to SGML and making it a full part of the GHC user manual.

This isn't a big task, and it's one that is well separated from the rest of GHC. Would anyone like to take it on? I'd be happy to advise if so.

I wonder who uses External Core?

  Changed 4 months ago by JamesFisher

Does my attached drawing summarize the situation accurately?

in reply to: ↑ 7   Changed 4 months ago by JamesFisher

Replying to simonpj:

We have a very old Latex document describing External Core (including is syntax); I'd like to see it become a full part of the documentation, perhaps by convrting it to SGML and making it a full part of the GHC user manual.

Is  this the document you mean?

I'm a bit confused as to what the "official" GHC documentation is. There's the  Developer wiki,  Haskell wiki, and the user guide. Where would the External Core document go if converted?

  Changed 4 months ago by JamesFisher

(I ask because the user manual seems slightly strange place to document compiler internals. Perhaps not though.) Where is the source repository for the user manual?

  Changed 4 months ago by JamesFisher

Ignore me;  found it. Would it help if was to convert  ext-core/core.tex into, say  users_guide/core.xml?

  Changed 4 months ago by simonpj

Yes, your diagram is right, except that the arrow from .hi to Iface Core should be labelled iface/BinIface as well; BinIface does deserialisation as well a serialisation.

And rather that "External Iface" I might say "the output of ghc --show-iface or something like that.

Documenting in the user manual is slightly odd; but only slightly. After all, it's simply documenting what ghc -ext-core does; in particular, defining the language it produces into the .hcr file. The implementation diagram could form a useful part of the Commentary:  http://hackage.haskell.org/trac/ghc/wiki/Commentary.

Yes converting core.tex to core.xml would be a great start.

Simon

  Changed 4 months ago by JamesFisher

  • owner set to JamesFisher

I'm working on that conversion as of now.

I'm not experienced in ticketing systems so I'm not sure what the usual way of stating that I'm doing this is. For the moment I'm assigning this ticket to me.

Changed 4 months ago by JamesFisher

Schematic of Core organization, current and proposed

Changed 3 months ago by JamesFisher

To be included as docs/users_guide/external_core.xml

  Changed 3 months ago by JamesFisher

I've attached my DocBook? conversion of the External Core LaTeX document. It validates as a DocBook? chapter, and builds OK with dblatex, but I can't work out how to build the User's Guide, so someone should check it builds.

Changed 3 months ago by JamesFisher

Patch to include previously attached XML file

  Changed 3 months ago by JamesFisher

Please let me know if the attached patch is wrong in any way; I have as much experience of VC as of ticketing systems. I attempted to follow < http://hackage.haskell.org/trac/ghc/wiki/WorkingConventions/FixingBugs>.

  Changed 3 months ago by JamesFisher

  • status changed from new to patch

in reply to: ↑ 7   Changed 3 months ago by JamesFisher

Replying to simonpj:

I wonder who uses External Core?

 The results are in! Mouse-over for complete answer text.

 Poll hosted on Haskellers.com. Advertised on  Reddit and on Haskell Café.

  Changed 3 months ago by igloo@…

commit 8c0196b48d043fe16eb5b2d343f5544b7fdd5004

Author: Ian Lynagh <igloo@earth.li>
Date:   Tue Feb 14 14:43:08 2012 +0000

    Add docbook-ised external-core doc; from #5844, by James H. Fisher

 docs/users_guide/external_core.xml | 1807 ++++++++++++++++++++++++++++++++++++
 docs/users_guide/ug-book.xml.in    |    1 +
 docs/users_guide/ug-ent.xml.in     |    2 +
 3 files changed, 1810 insertions(+), 0 deletions(-)

  Changed 3 months ago by igloo

  • owner JamesFisher deleted
  • status changed from patch to new

Thanks, I've added your external_core.xml to the user guide, and removed the old doc.

follow-up: ↓ 21   Changed 3 months ago by simonpj

Thanks for doing this James. We'll leave the ticket open for the larger question of refactoring the External Core handling as per my long comment above.

in reply to: ↑ 20   Changed 3 months ago by JamesFisher

Replying to simonpj:

Thanks for doing this James.

No problem.

We'll leave the ticket open for the larger question of refactoring the External Core handling as per my long comment above.

IMO the next task would be to update the External Core document to represent the current code. Or would you consider this wasted effort if we're then changing things again?

Whatever the next task is, is the convention that I open a new ticket for it, or just to continue this discussion?

  Changed 3 months ago by simonpj

The two seems pretty independent to me: the doc is about the concrete syntax and how to drive it; the implementation will change but that should not radically change the external syntax.

I don't have a strong opinion about new vs old tickets. I'd stick to this one for now.

  Changed 5 weeks ago by Eventh

  • cc eventh@… added
Note: See TracTickets for help on using tickets.