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+

Mark Lam

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

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 attachment proposed patch, testcase, etc.

Mark Lam

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

Mark Lam

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

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

WebKit Commit Bot

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

Geoffrey Garen

Comment 4 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?

Mark Lam

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

WebKit Commit Bot

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

Mark Lam

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

WebKit Commit Bot

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

Geoffrey Garen

Comment 9 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

Mark Lam

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

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

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