138660 – Add printing functionality in JITted code for debugging purposes

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

Summary: Add printing functionality in JITted code for debugging purposes

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	JavaScriptCore (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Mark Lam

URL:
Keywords:

Depends on:	138661 138671
Blocks:
	Show dependency tree / graph

Reported:	2014-11-12 09:22 PST by Mark Lam
Modified:	2014-11-17 14:03 PST (History)
CC List:	7 users (show)

See Also:

Attachments
the payola for all the recent probe changes (30.47 KB, patch) 2014-11-14 19:03 PST, Mark Lam	ggaren: review+	Details \| Formatted Diff \| Diff
patch 2 (19.66 KB, patch) 2014-11-17 12:51 PST, Mark Lam	no flags	Details \| Formatted Diff \| Diff
patch 3: removed some unused code due to a failed attempt at supporting indentation when printing AllRegisters. (19.56 KB, patch) 2014-11-17 12:55 PST, Mark Lam	ggaren: review+	Details \| Formatted Diff \| Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

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>.