Closed Bug 920294 Opened 6 years ago Closed 6 years ago

Fix DumpJSStack

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: evilpie, Assigned: evilpie)

Details

Attachments

(1 file, 2 obsolete files)

Attached patch dumpstack (obsolete) — Splinter Review
I fixed paramters and kind of fixed locals. For some reason all the values are undefined, is that because of an optimization?
Attachment #809517 - Flags: feedback?(jdemooij)
So I don't really remember what Jan told me, but I think it boiled down to that supporting this for the JITs needs some work. Also I think he recommended using something different from debug scopes. Jan would you mind clarifying this again? :)
Flags: needinfo?(jdemooij)
Comment on attachment 809517 [details] [diff] [review]
dumpstack

Review of attachment 809517 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Tom Schuster [:evilpie] from comment #1)
> So I don't really remember what Jan told me, but I think it boiled down to
> that supporting this for the JITs needs some work. Also I think he
> recommended using something different from debug scopes. Jan would you mind
> clarifying this again? :)

Yeah I wouldn't use GetDebugScopeForFrame here. I think you can just add a ScriptFrameIter::unaliasedLocal method like the unaliasedActual method you're already using.

Note that these methods work fine for Interpreter and Baseline frames but will assert for Ion frames I think, so please make sure you're handling that correctly (the simplest thing to do is to not print args/locals if iter.isIon()).
Attachment #809517 - Flags: feedback?(jdemooij)
Flags: needinfo?(jdemooij)
Attached patch wip v2 (obsolete) — Splinter Review
Made some progress here, I think right now I am missing something like AliasedFormalIter but for formals.
Assignee: nobody → evilpies
Attachment #809517 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Oh looks like the locals are wrong even for simple functions, need to investigate this more.
Maybe we can just punt on locals and fix the arguments at least?
Attached patch v1Splinter Review
Attachment #812250 - Attachment is obsolete: true
Attachment #822244 - Flags: review?(jdemooij)
Attachment #822244 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/1268bd66a094
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.