Bug 168673 - Repatch assembly dump: allow having no CodeBlock
Summary: Repatch assembly dump: allow having no CodeBlock
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: JF Bastien
URL:
Keywords:
Depends on:
Blocks: 159775
  Show dependency treegraph
 
Reported: 2017-02-21 14:02 PST by JF Bastien
Modified: 2017-02-21 17:18 PST (History)
7 users (show)

See Also:


Attachments
patch (3.86 KB, patch)
2017-02-21 14:03 PST, JF Bastien
fpizlo: review-
Details | Formatted Diff | Diff
patch (17.06 KB, patch)
2017-02-21 16:41 PST, JF Bastien
fpizlo: review+
jfbastien: commit-queue-
Details | Formatted Diff | Diff
patch (17.05 KB, patch)
2017-02-21 16:48 PST, JF Bastien
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description JF Bastien 2017-02-21 14:02:17 PST
Repatch's assembly dump assumes there's a CodeBlock present. WebAssembly doesn't have one. It crashes.
Comment 1 JF Bastien 2017-02-21 14:03:34 PST
Created attachment 302304 [details]
patch
Comment 2 Filip Pizlo 2017-02-21 14:12:06 PST
Comment on attachment 302304 [details]
patch

I think you should make it easier to say this, since we say it in multiple places.  In fact, the usual dumping style for CodeBlock+CodeOrigin is:

dataLog(pointerDump(codeBlock), " ", codeOrigin)

I think that's what we want here as well.  But maybe we can take it a step further and create a helper for it.  The right way to go is something like:

dataLog(FullCodeOrigin(codeBock, codeOrigin))

Where FullCodeOrigin is a simple value class that holds a CodeBlock* and a CodeOrigin, and has a `void dump(PrintStream&) const` that prints them out.
Comment 3 JF Bastien 2017-02-21 16:41:47 PST
Created attachment 302337 [details]
patch

Add a helper class as Fil suggested, and use it in a bunch of places. This is nice.
Comment 4 WebKit Commit Bot 2017-02-21 16:42:59 PST
Attachment 302337 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/FullCodeOrigin.h:28:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 JF Bastien 2017-02-21 16:47:37 PST
Comment on attachment 302337 [details]
patch

Style fix.
Comment 6 JF Bastien 2017-02-21 16:48:55 PST
Created attachment 302339 [details]
patch

Fix style.
Comment 7 WebKit Commit Bot 2017-02-21 17:18:28 PST
Comment on attachment 302339 [details]
patch

Clearing flags on attachment: 302339

Committed r212782: <http://trac.webkit.org/changeset/212782>
Comment 8 WebKit Commit Bot 2017-02-21 17:18:32 PST
All reviewed patches have been landed.  Closing bug.