Bug 138660 – Add printing functionality in JITted code for debugging purposes

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

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.

[Mark Lam]

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.

[Mark Lam]

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

[WebKit Commit Bot]

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.

[Geoffrey Garen]

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?

[Mark Lam]

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.

[WebKit Commit Bot]

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.

[Mark Lam]

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

[WebKit Commit Bot]

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.

[Geoffrey Garen]

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

r=me

[Mark Lam]

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