To see the bigger picture of this changeset. Check out this wiki page: https://ghc.haskell.org/trac/ghc/wiki/DWARF/80Status

Abandoning. As far as I understand, D1196 together with D1198 strictly supersedes this. Also see https://ghc.haskell.org/trac/ghc/wiki/DWARF/80Status for clearer status page.

Awesome that you fixed the code style to @bgamari! :)

Cool! Looked through a bit. Did not read every line in detail though. Do you plan to migrate the test cases from my patch too? Would for sure boost my confidence in the correctness of the good.

Sweet! :D

Wohoo! Great job @bgamari!!!

Finally had time to go through this in detail. It looks really good and exciting!

Nice!

Looks good!

Looks good! :)

@scpmw and @bgamari, did the experimentation done in D1156 bring any conclusions related to this patch? If not I would like us to continue to see if this patch can be merged. What needs to be done?

LGTM. But somebody who knows dwarf have to review this! :)

Nice that you build upon D963 so that patch doesn't get lost. ^^

Cool! Awesome progress @bgamari, such productivity! :)

@bgamari, actually Codemap is just a shrink-down of the old Dwarf.c, which was Codemap.c + dwarf-parts. I had a prototype in D662 even. So I guess current Codemap.c can be expanded to incorporate Dwarf in the future. It used libdwarf and not libdw, not sure if that matters.

@bgamari, nice. I updated some documentation. Now I'll try to ping our other colleagues. :)

Update docs and comments

Rebase so patch can be applied (and validate)

Add (c) notices to rts files

Alright, the 'dyn' tests pass now as well. Let me know it there's anything more to fix for this patch. Otherwise what needs to be done in order to get this to land?

Make tests run with 'dyn' as well

Cool. Seems like this patch is looking ready then. I'll just have to look at the segfault. Hopefully a minor fix. :)

I'll iterate on this again because of the core dumped issue in executionStack002 test. Anyway, I think this patch can be reviewed still. :)

Remove extra whitespace in testlib.py

Small fixes

For some very weird reason I have started to get this error locally now. I'm pushing my changes to see if the build machine also suddenly have started to fail or not.

• Fix @bgmari's comments. Remove unused Codemap-code

Thanks for the review @bgamari! I'll try to iterate on it early this week. :)

I would really appreciate another round of review guys. :)

Make it ./validate with elf as well.

• Only run tests when GHC has libelf

Hmm, I have a couple of questions. I would appreciate any help:

• Whoups, ELF not DWARF :)
• Only run tests when GHC has libelf

• Mark *ip as UNUSED for no USE_ELF
• Remove redundant import

• Mark *ip as UNUSED for no USE_ELF

• Mark *ip as UNUSED for no USE_ELF

Ok, so the validation errors is basically part of this build step: make binary-dist-prep && make test_bindist TEST_PREP=YES.

Hmm, there's some new ./validate errors locally I'll have to fix. But please review the code in itself. :)

Okay. I'll be happy to receive another round of review. :)

Updated the arc patch description

Moved testing stuff to Internal. Minimized public interface.

Hmm, @scpmw, what do you think about exporting getCurrentStackFrames :: IO [StackFrame] and printStackFrames :: [StackFrames] -> IO ()? Then I hide the rest of the exports under -- * Internal. I still need to test the code somehow so I do need to export stuff under -- * Internal. Besides, GHC.Stack exports stuff like that at the moment. So it can't be that bad if @simonmar does it! ;)

@scpmw, I like your comments. it gives very good perspective. Allow me to say how I reason around this.

@scpmw, do you have any thoughts on what to do with the Haskell modules? I would really appreciate your comments on this. If you're not sure, I can start just playing around and move stuff around until I think it looks sensible and upload a new patch.

• Update comments in Codemap.h

So I fixed the "easy" parts of what you guys commented on. However, I do need some advice on how to proceed with the whole GHC.Stack vs GHC.ExecutionStack story.

• Remove file ".env" commited by mistake

Some code fixes:

Thanks for the comments. I'll try to iterate on a new patch early this coming week. :)

Good point @austin, can that be a follow-up commit (If we just say people to not use GHC.ExecutionStack)? Or should it be in this commit? I thought of doing some reconciliation a long time ago, but @scpmw discouraged me in order to keep it simple to begin with (get something working first, basically). But maybe now is the time?

Adding @simonmar as a reviewer. :)

Seems like the build environment does not have libelf. And I would like to have some help on how to proceed.

I'll add the reviewers and subscribers from D662 once I see that validation passes (so people don't get spammed by my tiny code amendments).

Abandoning to clear reviewers watch list. I just opened this diff to show a "direction" for Haskell land stack traces and I think this diff did that. Look forward to a new patch as I described in D661. :)

Not really this particular patch. I'm working on a new patch, which tries to provide some actual value like D662 does, but tries to stay "minimal" like this Diff.

@scpmw, So I'm not sure exactly what I will be needing. I noticed that in Dwarf.c, you do a lot of the libelf stuff as well. I figured I should trim down that Dwarf.c to just do the libelf stuff. More precisely, I should use the Elf information in the binary to look up symbol names, using the instruction pointers that reside on the Haskell stack. Does that "plan" make sense @scpmw? It wouldn't use any of the dwarf information, but we can still retrieve names from the symbol table. Is that correct?

@scpmw, Did you have time to look anything at a minified version of Dwarf.c?

Anyway, starting tomorrow. I'll be on vacation for 3 weeks. I'll continue to iterate on this patch after that. :)

@simonmar, should I send the ENTRY_CODE implementations in their own diff so the diffs I send are as small as possible (while still being valuable on their own)?

Fix comments from @simonmar

Thanks for your comments Simon! I just stumbled upon a small road-block on GET_ENTRY. I'm about to submit my patch once it validates. :)

I'm still reading through the patch

I was about to answer you @tibbe, but I see that @scpmw already gave a very good and complete answer (as always :)). I would just like to say that I think that it would be very hard to implement freezing efficiently. I'm not even sure it's possible. If it's a GC safe data structure, doesn't it need to traversed during GC anyway? I wrote a lot about efficient stack reification in my master thesis. http://www.arashrouhani.com/papers/master-thesis.pdf, I think the tl;dr is to stick with the simplest solution (O(n) copy).

I agree that this patch can be hard to argue about since it doesn't provide any value on it's own.

Actually, once D661 is out of the way. I think I could submit this as another partial yet compilable patch by splitting out the stack-reification part of this into it's own patch set. The thing is that stack-reification (even when in Haskell land) does not require the dwarf parts. Would that make sense?

Fixed test cases and amended stuff

• Validation fixes

fix validation error in countStackSize

Set default limit 100 on max stack reification size

Fix unsigned/signed bug. Thanks Harbormaster and static analysis!

Hmm, Peter, how did you get Phabricator to understand that D396 depends on D169? I can't get this one to understand that this diff depends on D661. Hmm :-/

Just append to summary

Ok, uploaded D662 now which should show how this patch can be used.

Hi guys, I started with a small patch towards stack traces. I plan to very soon upload a bigger diff which shows the direction as a whole I'm thinking about.

Awesome job on landing this. Hehe, will do a lot of rebasing during the christmas holidays then! :)

Cool! :D

Oh my, I can't believe I missed this patch set! Fantastic that this patch set is also reviewed and underway, good job!

Thanks for the update. Keep them coming. :)

Cool! What is remaining for this patch set to be merged now?

Nice to see all this progress! Have you considered to also just fix the lint warnings that were introduced in your patches?

Yea, I also heard about the GHC freeze. Also note that this patch set does not include the DWARF stuff yet, am I correct?

@scpmw, are you still on this? It would be nice to continue to see the nice progress on this! :)

Perfect, thanks Austin! :)

Hmm, I tried to land my change but couldn't (I told arc to ignore the build failure). I get this error:

Cool! It's awesome that it builds it for me, but the failure I got is not due to me right? It seems like all the 90 builds prior to mine failed as well.