Bug 138660

Summary:

Add printing functionality in JITted code for debugging purposes

Product:

WebKit

Reporter:

Mark Lam <mark.lam>

Component:

JavaScriptCore

Assignee:

Mark Lam <mark.lam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, fpizlo, ggaren, mhahnenb, mmirman, msaboff, oliver

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

138661, 138671

Bug Blocks:

Attachments:

Description	Flags
the payola for all the recent probe changes	ggaren: review+
patch 2	none
patch 3: removed some unused code due to a failed attempt at supporting indentation when printing AllRegisters.	ggaren: review+

Description Mark Lam 2014-11-12 09:22:58 PST

Sometimes, it's be nice to be able to just print the values of constants or registers that JITted code is using, or even just a string to log that certain pieces of JITted code has been executed.  We can implement this some enhancements to the JIT probe mechanism.

Comment 1 Mark Lam 2014-11-12 09:25:14 PST

This is functionality that I've developed in recent debugging work.  Just cleaning it up to land so that I don't have to re-invent it the next time it is needed.

Comment 2 Mark Lam 2014-11-14 19:03:28 PST

Created attachment 241654 [details]
the payola for all the recent probe changes

Comment 3 WebKit Commit Bot 2014-11-14 19:05:11 PST

Attachment 241654 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:852:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:851:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:855:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:864:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:863:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:867:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:876:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:875:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:879:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:888:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:887:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:891:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARM.cpp:104:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARM.cpp:110:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARM.cpp:112:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARM.cpp:118:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARMv7.cpp:39:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARMv7.cpp:45:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARMv7.cpp:47:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARMv7.cpp:53:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
Total errors found: 20 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 4 Geoffrey Garen 2014-11-17 11:45:21 PST

Comment on attachment 241654 [details]
the payola for all the recent probe changes

View in context: https://bugs.webkit.org/attachment.cgi?id=241654&action=review

> Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:945
> +    struct PrintCPU {

I think this would make more sense if you called it "AllRegisters". Then you wouldn't need a comment to explain that it meant all registers.

Why does this struct also accept an indentation level? That's pretty random. It seems like either you should remove the concept of indentation level, or you should add indentation as a separate argument to the PrintArg constructor.

> Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:1288
> +        extern void macroAssemblerPrint(ProbeContext*);

Please remove.

> Source/JavaScriptCore/assembler/MacroAssemblerARM.cpp:127
> +        void* voidPtr;

Should this be GPRStorageType?

> Source/JavaScriptCore/assembler/MacroAssemblerARM.cpp:138
> +        double doubleValue;

FPRStorageType?

Comment 5 Mark Lam 2014-11-17 12:51:47 PST

Created attachment 241736 [details]
patch 2

Per offline discussion with Geoff, I've removed all uses of the GPRStorageType, FPRStorageType, and SPRStortageType abstractions, and replaced them with the concrete types that they map to.

Comment 6 WebKit Commit Bot 2014-11-17 12:54:24 PST

Attachment 241736 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:852:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:851:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:855:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:864:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:863:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:867:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:876:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:875:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:879:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:888:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:887:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:891:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:958:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARM.cpp:104:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARM.cpp:110:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARM.cpp:112:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARM.cpp:118:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARMv7.cpp:39:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARMv7.cpp:45:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARMv7.cpp:47:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARMv7.cpp:53:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
Total errors found: 21 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 7 Mark Lam 2014-11-17 12:55:16 PST

Created attachment 241737 [details]
patch 3: removed some unused code due to a failed attempt at supporting indentation when printing AllRegisters.

Comment 8 WebKit Commit Bot 2014-11-17 12:57:21 PST

Attachment 241737 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:852:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:851:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:855:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:864:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:863:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:867:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:876:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:875:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:879:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:888:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:887:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:891:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:958:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARM.cpp:104:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARM.cpp:110:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARM.cpp:112:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARM.cpp:118:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARMv7.cpp:39:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARMv7.cpp:45:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARMv7.cpp:47:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerARMv7.cpp:53:  preprocessor directives (e.g., #ifdef, #define, #import) should never be indented.  [whitespace/indent] [4]
Total errors found: 21 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 9 Geoffrey Garen 2014-11-17 13:59:17 PST

Comment on attachment 241737 [details]
patch 3: removed some unused code due to a failed attempt at supporting indentation when printing AllRegisters.

r=me

Comment 10 Mark Lam 2014-11-17 14:03:43 PST

Thanks.  Landed in r176233: <http://trac.webkit.org/r176233>.