Support the GHCi debugger with -fexternal-interpreter
ClosedPublic

Authored by simonmar on Wed, Jul 11, 8:41 AM.

Details

Summary
  • All the tests in tests/ghci.debugger now pass with -fexternal-interpreter. These tests are now run with the ghci-ext way in addition to the normal way so we won't break it in the future.
  • I removed all the unsafeCoerce# calls from RtClosureInspect. Yay!

The main changes are:

  • New messages: GetClosure and Seq. GetClosure is a remote interface to GHC.Exts.Heap.getClosureData, which required Binary instances for various datatypes. Fortunately this wasn't too painful thanks to DeriveGeneric.
  • No cheating by unsafeCoercing values when printing them. Now we have to turn the Closure representation back into the native representation when printing Int, Float, Double, Integer and Char. Of these, Integer was the most painful - we now have a dependency on integer-gmp due to needing access to the representation.
  • Fixed a bug in rts/Heap.c - it was bogusly returning stack content as pointers for an AP_STACK closure.
Test Plan
  • cd testsuite/tests/ghci.debugger && make
  • validate

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
simonmar created this revision.Wed, Jul 11, 8:41 AM
simonmar updated this revision to Diff 17271.Wed, Jul 11, 9:19 AM

fix various minor wibbles

I'm glad to see the Box/HValue flipping resolved. I think that reduces a lot of the friction between GHCi and the heap code.

You have my approval, though that probably shouldn't count for much ;)

I'm very happy to see these unsafeCoerces gone.

compiler/ghci/RtClosureInspect.hs
337–344

Let's write this in terms of ifTerm'. It took me a bit of staring to work out how these differed.

376

Does this work on 32-bit? valRaw looks to be a [Word] and we are matching on a singleton list above yet Double will be two words on a 32-bit machine.

384

Is there a particular test you have in mind here?

405

Indeed this is pretty yucky but fair enough. Perhaps we should at least add a comment alongside the Integer definition in integer-gmp to make the connection clear?

Also, don't we need to do the same for Natural?

ghc.mk
619

@snowleopard & @alpmestan: this will require Hadrian modification.

libraries/ghci/GHCi/Message.hs
429

Wibble: perhaps fromIntegral . wordToPtr and ptrToWord . fromIntegral would be better here?

rts/Heap.c
171

We probably ought to open a ticket for this.

bgamari requested changes to this revision.Thu, Jul 12, 10:03 AM
This revision now requires changes to proceed.Thu, Jul 12, 10:03 AM
simonmar updated this revision to Diff 17309.Fri, Jul 13, 11:28 AM

address comments; rebase

simonmar marked 4 inline comments as done.Fri, Jul 13, 11:29 AM

@bgamari thanks for the review - all done I think.

bgamari accepted this revision.Mon, Jul 16, 6:58 PM

Great, looks good to me!

This revision is now accepted and ready to land.Mon, Jul 16, 6:58 PM
This revision was automatically updated to reflect the committed changes.