Bug 118758 - fourthTier: Change JSStack to grow from high to low addresses
Summary: fourthTier: Change JSStack to grow from high to low addresses
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords:
Depends on: 121055 121056 121057
Blocks: 116888
  Show dependency treegraph
 
Reported: 2013-07-16 18:19 PDT by Michael Saboff
Modified: 2013-09-13 11:03 PDT (History)
13 users (show)

See Also:


Attachments
Work in progress (44.89 KB, patch)
2013-07-16 18:24 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
LLInt runs testapi cleanly (48.24 KB, patch)
2013-07-17 16:34 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
LLInt now passes all JS tests (66.57 KB, patch)
2013-07-23 13:56 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
Passing JS tests with LLint + Baseline JIT (87.73 KB, patch)
2013-07-24 11:30 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
LLInt passes layout tests, Baseline JIT passed JS test and fails on 12 Layout/fast/js tests (87.95 KB, patch)
2013-07-25 16:07 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
Baseline JIT passes JS tests and Layout/fast/js tests (93.95 KB, patch)
2013-08-13 14:16 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
LLInt + Baseline JIT + DFG Passes testapi - working on JS tests (109.08 KB, patch)
2013-08-20 17:41 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
LLInt + Baseline JIT + DFG Passes all but one JS test (143.46 KB, patch)
2013-08-26 16:01 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
LLInt + Baseline JIT + DFG Passes all JS tests (144.87 KB, patch)
2013-08-26 16:18 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
LLInt/Baseline JIT/DFG 64 bit with all but 26 fast/js tests passing (149.36 KB, patch)
2013-08-27 15:39 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
LLInt/Baseline JIT/DFG 64 bit with all but 14 fast/js tests passing (150.55 KB, patch)
2013-08-27 20:31 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
64 Bit LLInt, Baseline JIT, DFG Passes all but one test (152.30 KB, patch)
2013-09-02 21:44 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
LLInt/Baseline JIT/DFG 64 bit pass tests - LLInt/Baseline JIT 32 bit pass tests - 10 tests fail in 32 bit DFG (267.71 KB, patch)
2013-09-06 17:21 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
LLint/Baseline JIT/DFG 32&64 bit pass all JS tests (267.76 KB, patch)
2013-09-06 17:34 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
Patch (96.91 KB, patch)
2013-09-11 12:01 PDT, Michael Saboff
oliver: review+
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 (894.43 KB, application/zip)
2013-09-11 13:09 PDT, Build Bot
no flags Details
Patch addressing review and ewe bot issues (97.55 KB, patch)
2013-09-11 15:19 PDT, Michael Saboff
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 (858.10 KB, application/zip)
2013-09-11 17:24 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion (880.53 KB, application/zip)
2013-09-11 23:56 PDT, Build Bot
no flags Details
Another Patch with speculative build / test fixes (98.55 KB, patch)
2013-09-12 10:59 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
Patch with final fix (106.53 KB, patch)
2013-09-12 16:23 PDT, Michael Saboff
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2013-07-16 18:19:29 PDT
Currently the JS stack grows from low address towards high addresses.  The native C/C++ stack grows from high to low addresses.  Before using the C stack for JS processing, the direction of the stack should change.
Comment 1 Michael Saboff 2013-07-16 18:24:29 PDT
Created attachment 206832 [details]
Work in progress

Compiles and runs testapi with LLint with one exception error and with value profiling turned off.

I'll fix those and then try running all of JS tests.
Comment 2 Michael Saboff 2013-07-17 16:34:36 PDT
Created attachment 206929 [details]
LLInt runs testapi cleanly

Now on to JS tests
Comment 3 Michael Saboff 2013-07-23 13:56:02 PDT
Created attachment 207346 [details]
LLInt now passes all JS tests

Bring on the baseline JIT
Comment 4 Michael Saboff 2013-07-24 11:30:00 PDT
Created attachment 207406 [details]
Passing JS tests with LLint  + Baseline JIT

Now will work through the fast/js Layout failures for the LLInt and baseline JIT
Comment 5 Michael Saboff 2013-07-25 16:07:02 PDT
Created attachment 207491 [details]
LLInt passes layout tests, Baseline JIT passed JS test and fails on 12 Layout/fast/js tests
Comment 6 Michael Saboff 2013-08-13 14:16:53 PDT
Created attachment 208674 [details]
Baseline JIT passes JS tests and Layout/fast/js tests

Running with LLInt and DFG disabled.

There are 4 failures in the fast/js layout tests:
Regressions: Unexpected text-only failures (3)
  fast/js/exception-sequencing.html [ Failure ]
  fast/js/exception-thrown-from-new.html [ Failure ]
  fast/js/kde/Array.html [ Failure ]

Regressions: Unexpected crashes (1)
  fast/js/global-recursion-on-full-stack.html [ Crash ]

The three unexpected failures match without the changes.  The crash is due to stack overflow checking.  I'll address that later.

Going on to DFG.
Comment 7 Michael Saboff 2013-08-20 17:41:44 PDT
Created attachment 209241 [details]
LLInt + Baseline JIT + DFG Passes testapi - working on JS tests
Comment 8 Michael Saboff 2013-08-26 16:01:43 PDT
Created attachment 209686 [details]
LLInt + Baseline JIT + DFG Passes all but one JS test
Comment 9 Michael Saboff 2013-08-26 16:18:56 PDT
Created attachment 209690 [details]
LLInt + Baseline JIT + DFG Passes all JS tests
Comment 10 Oliver Hunt 2013-08-26 17:00:16 PDT
(In reply to comment #9)
> Created an attachment (id=209690) [details]
> LLInt + Baseline JIT + DFG Passes all JS tests

32 and 64bit?
Comment 11 Michael Saboff 2013-08-26 17:26:14 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > Created an attachment (id=209690) [details] [details]
> > LLInt + Baseline JIT + DFG Passes all JS tests
> 
> 32 and 64bit?

64 bit only at this point.
Comment 12 Michael Saboff 2013-08-27 15:39:13 PDT
Created attachment 209813 [details]
LLInt/Baseline JIT/DFG 64 bit with all but 26 fast/js tests passing

Can you say "Arguments"….

Regressions: Unexpected text-only failures (2)
  fast/js/regress/external-arguments-putbyval.html [ Failure ]
  fast/js/regress/inline-arguments-local-escape.html [ Failure ]

Regressions: Unexpected crashes (24)
  fast/js/dfg-arguments-alias-activation.html [ Crash ]
  fast/js/dfg-arguments-alias-one-block-osr-exit.html [ Crash ]
  fast/js/dfg-arguments-alias-one-block-overwrite-arguments.html [ Crash ]
  fast/js/dfg-arguments-alias-one-block.html [ Crash ]
  fast/js/dfg-arguments-out-of-bounds.html [ Crash ]
  fast/js/dfg-arguments-unexpected-escape.html [ Crash ]
  fast/js/dfg-create-inlined-arguments-in-closure-inline.html [ Crash ]
  fast/js/dfg-inline-arguments-become-double.html [ Crash ]
  fast/js/dfg-inline-arguments-become-int32.html [ Crash ]
  fast/js/dfg-inline-arguments-int32.html [ Crash ]
  fast/js/dfg-inline-arguments-osr-exit-and-capture.html [ Crash ]
  fast/js/dfg-inline-arguments-reset-changetype.html [ Crash ]
  fast/js/dfg-inline-arguments-reset.html [ Crash ]
  fast/js/dfg-inline-arguments-simple.html [ Crash ]
  fast/js/dfg-inline-arguments-use-directly-from-inlined-code.html [ Crash ]
  fast/js/dfg-inline-arguments-use-from-uninlined-code.html [ Crash ]
  fast/js/global-recursion-on-full-stack.html [ Crash ]
  fast/js/regress/aliased-arguments-getbyval.html [ Crash ]
  fast/js/regress/delay-tear-off-arguments-strictmode.html [ Crash ]
  fast/js/regress/direct-arguments-getbyval.html [ Crash ]
  fast/js/regress/external-arguments-getbyval.html [ Crash ]
  fast/js/regress/tear-off-arguments-simple.html [ Crash ]
  fast/js/regress/tear-off-arguments.html [ Crash ]
  fast/js/stack-trace.html [ Crash ]
Comment 13 Michael Saboff 2013-08-27 20:31:30 PDT
Created attachment 209841 [details]
LLInt/Baseline JIT/DFG 64 bit with all but 14 fast/js tests passing

Still more argument / tear off work:

  fast/js/dfg-arguments-alias-activation.html [ Crash ]
  fast/js/dfg-arguments-alias-one-block-osr-exit.html [ Crash ]
  fast/js/dfg-arguments-alias-one-block.html [ Crash ]
  fast/js/dfg-arguments-out-of-bounds.html [ Crash ]
  fast/js/dfg-create-inlined-arguments-in-closure-inline.html [ Crash ]
  fast/js/global-recursion-on-full-stack.html [ Crash ]
  fast/js/regress/aliased-arguments-getbyval.html [ Crash ]
  fast/js/regress/delay-tear-off-arguments-strictmode.html [ Crash ]
  fast/js/regress/direct-arguments-getbyval.html [ Crash ]
  fast/js/regress/external-arguments-getbyval.html [ Crash ]
  fast/js/regress/external-arguments-putbyval.html [ Crash ]
  fast/js/regress/inline-arguments-local-escape.html [ Crash ]
  fast/js/regress/tear-off-arguments-simple.html [ Crash ]
  fast/js/regress/tear-off-arguments.html [ Crash ]
Comment 14 Michael Saboff 2013-09-02 21:44:23 PDT
Created attachment 210319 [details]
64 Bit LLInt, Baseline JIT, DFG Passes all but one test

DFG passes all DFG tests.  Baseline JIT fails stack overflow check.  LLint passes this test.  Will investigate.

fast/js/global-recursion-on-full-stack.html
Comment 15 Filip Pizlo 2013-09-02 21:53:12 PDT
(In reply to comment #14)
> Created an attachment (id=210319) [details]
> 64 Bit LLInt, Baseline JIT, DFG Passes all but one test
> 
> DFG passes all DFG tests.  Baseline JIT fails stack overflow check.  LLint passes this test.  Will investigate.

That's awesome!

> 
> fast/js/global-recursion-on-full-stack.html
Comment 16 Michael Saboff 2013-09-06 17:21:31 PDT
Created attachment 210819 [details]
LLInt/Baseline JIT/DFG 64 bit pass tests - LLInt/Baseline JIT 32 bit pass tests - 10 tests fail in 32 bit DFG
Comment 17 Michael Saboff 2013-09-06 17:34:52 PDT
Created attachment 210822 [details]
LLint/Baseline JIT/DFG 32&64 bit pass all JS tests

32bit WebKit tests next
Comment 18 Michael Saboff 2013-09-06 17:54:46 PDT
(In reply to comment #17)
> Created an attachment (id=210822) [details]
> LLint/Baseline JIT/DFG 32&64 bit pass all JS tests
> 
> 32bit WebKit tests next

There are 3 WebKit tests failures running 32 bit:
  fast/js/invalid-syntax-for-function.html [ Crash ]
  fast/js/regress/function-dot-apply.html [ Crash ]
  fast/js/typed-array-access.html [ Crash ]
Comment 19 Michael Saboff 2013-09-06 18:55:12 PDT
(In reply to comment #18)
> (In reply to comment #17)
> > Created an attachment (id=210822) [details] [details]
> > LLint/Baseline JIT/DFG 32&64 bit pass all JS tests
> > 
> > 32bit WebKit tests next
> 
> There are 3 WebKit tests failures running 32 bit:
>   fast/js/invalid-syntax-for-function.html [ Crash ]
>   fast/js/regress/function-dot-apply.html [ Crash ]
>   fast/js/typed-array-access.html [ Crash ]

Down to one failing test:
  fast/js/invalid-syntax-for-function.html [ Crash ]

It only fails when run with a group of other tests and doesn't fail by itself.  The crash is due to an ASSERT failure in
void JSStack::disableErrorStackReserve()
{
    char* useableEnd = reinterpret_cast<char*>(reservationEnd()) + commitSize;
    m_useableEnd = reinterpret_cast_ptr<Register*>(useableEnd);

    // By the time we get here, we are guaranteed to be destructing the last
    // Interpreter::ErrorHandlingMode that enabled this reserve in the first
    // place. That means the stack space beyond m_useableEnd before we
    // enabled the reserve was not previously in use. Hence, it is safe to
    // shrink back to that m_useableEnd.
    if (m_end < m_useableEnd) {
        ASSERT(m_topCallFrame->frameExtent() >= m_useableEnd);  <==
        shrink(m_useableEnd);
    }
}

m_topCallFrame is null, therefore the crash.
Comment 20 Michael Saboff 2013-09-11 12:01:46 PDT
Created attachment 211338 [details]
Patch

I reverted the change made in XX to return a VirtualRegister from localToOperand().  I also changed stackOffset bit field in CodeOrigin to be "int" instead of "signed int".  check-webkit-style doesn't like this, but I'll fix that separately.
Comment 21 WebKit Commit Bot 2013-09-11 12:04:53 PDT
Attachment 211338 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/assembler/MacroAssembler.h', u'Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h', u'Source/JavaScriptCore/assembler/X86Assembler.h', u'Source/JavaScriptCore/bytecode/CodeBlock.cpp', u'Source/JavaScriptCore/bytecode/CodeBlock.h', u'Source/JavaScriptCore/bytecode/CodeOrigin.h', u'Source/JavaScriptCore/bytecode/Operands.h', u'Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp', u'Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h', u'Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp', u'Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp', u'Source/JavaScriptCore/dfg/DFGJITCompiler.cpp', u'Source/JavaScriptCore/dfg/DFGOSREntry.cpp', u'Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp', u'Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp', u'Source/JavaScriptCore/dfg/DFGOperations.cpp', u'Source/JavaScriptCore/dfg/DFGScoreBoard.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp', u'Source/JavaScriptCore/dfg/DFGValidate.cpp', u'Source/JavaScriptCore/ftl/FTLLink.cpp', u'Source/JavaScriptCore/heap/ConservativeRoots.cpp', u'Source/JavaScriptCore/interpreter/CallFrame.cpp', u'Source/JavaScriptCore/interpreter/CallFrame.h', u'Source/JavaScriptCore/interpreter/Interpreter.cpp', u'Source/JavaScriptCore/interpreter/JSStack.cpp', u'Source/JavaScriptCore/interpreter/JSStack.h', u'Source/JavaScriptCore/interpreter/JSStackInlines.h', u'Source/JavaScriptCore/jit/JIT.cpp', u'Source/JavaScriptCore/jit/JITCall.cpp', u'Source/JavaScriptCore/jit/JITCall32_64.cpp', u'Source/JavaScriptCore/jit/JITOpcodes.cpp', u'Source/JavaScriptCore/jit/JITOpcodes32_64.cpp', u'Source/JavaScriptCore/jit/JITStubs.cpp', u'Source/JavaScriptCore/jit/ThunkGenerators.cpp', u'Source/JavaScriptCore/llint/LLIntData.cpp', u'Source/JavaScriptCore/llint/LLIntSlowPaths.cpp', u'Source/JavaScriptCore/llint/LowLevelInterpreter.asm', u'Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm', u'Source/JavaScriptCore/llint/LowLevelInterpreter64.asm', u'Source/JavaScriptCore/runtime/ArgList.cpp', u'Source/JavaScriptCore/runtime/ArgList.h', u'Source/JavaScriptCore/runtime/Arguments.cpp', u'Source/JavaScriptCore/runtime/ArrayConstructor.cpp', u'Source/JavaScriptCore/runtime/CommonSlowPaths.cpp', u'Source/JavaScriptCore/runtime/JSActivation.h', u'Source/JavaScriptCore/runtime/JSArray.h', u'Source/JavaScriptCore/runtime/JSGlobalObject.cpp', u'Source/JavaScriptCore/runtime/JSGlobalObject.h', u'Source/JavaScriptCore/runtime/JSString.h', u'Source/JavaScriptCore/runtime/Operations.h', u'Source/JavaScriptCore/runtime/SymbolTable.h', u'Source/WTF/wtf/DataLog.cpp']" exit_code: 1
Source/JavaScriptCore/bytecode/CodeOrigin.h:100:  Please declare integral type bitfields with either signed or unsigned.  [runtime/bitfields] [5]
Total errors found: 1 in 53 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 22 Oliver Hunt 2013-09-11 12:09:12 PDT
Comment on attachment 211338 [details]
Patch

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

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:3434
> +    if (symbolTable()) {
> +        ConcurrentJITLocker locker(symbolTable()->m_lock);
> +        SymbolTable::Map::iterator end = symbolTable()->end(locker);
> +        for (SymbolTable::Map::iterator ptr = symbolTable()->begin(locker); ptr != end; ++ptr) {
> +            if (ptr->value.getIndex() == registerNumber) {
> +                // FIXME: This won't work from the compilation thread.
> +                // https://bugs.webkit.org/show_bug.cgi?id=115300
> +                return String(ptr->key);
> +            }

Why is this change necessary in this patch?

> Source/JavaScriptCore/runtime/JSArray.h:312
> -
> +    

grrrr

> Source/JavaScriptCore/runtime/JSArray.h:317
> -
> +    

grrrrr

> Source/JavaScriptCore/runtime/Operations.h:90
> +    for (int i = 0; i < static_cast<int>(count); ++i) {
> +        JSValue v = strings[-i].jsValue();

I would rather i remain an unsigned, and the strings[-i] be strings[-static_cat<int>(i)]

> Source/WTF/wtf/DataLog.cpp:43
> -#define DATA_LOG_TO_FILE 0
> +#define DATA_LOG_TO_FILE 1

Whoops :D
Comment 23 Early Warning System Bot 2013-09-11 12:16:31 PDT
Comment on attachment 211338 [details]
Patch

Attachment 211338 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1743623
Comment 24 Geoffrey Garen 2013-09-11 12:17:47 PDT
Comment on attachment 211338 [details]
Patch

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

>> Source/JavaScriptCore/bytecode/CodeBlock.cpp:3434
>> +            }
> 
> Why is this change necessary in this patch?

When is symbolTable() null? This seems like a bug.

> Source/JavaScriptCore/jit/JITOpcodes.cpp:-848
> -    emitPutVirtualRegister(dst);

Why is it OK not to store the activation to the stack here? This seems like a bug.

> Source/JavaScriptCore/jit/JITOpcodes.cpp:-1119
> -    signExtend32ToPtr(regT1, regT1);

Does add32 automatically zero the high bits in regT1? If not, we still need to sign-extend.
Comment 25 Early Warning System Bot 2013-09-11 12:18:17 PDT
Comment on attachment 211338 [details]
Patch

Attachment 211338 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1743622
Comment 26 Geoffrey Garen 2013-09-11 12:50:25 PDT
> > Source/JavaScriptCore/jit/JITOpcodes.cpp:-848
> > -    emitPutVirtualRegister(dst);
> 
> Why is it OK not to store the activation to the stack here? This seems like a bug.

I see: the above instruction stores to dst, too. Can you edit the instruction to use "dst"? Then the change will be clearer.
Comment 27 Build Bot 2013-09-11 13:09:54 PDT
Comment on attachment 211338 [details]
Patch

Attachment 211338 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1790062

New failing tests:
js/regress/inline-arguments-local-escape.html
js/dfg-inline-arguments-reset.html
js/dfg-inline-arguments-int32.html
js/dfg-inline-arguments-simple.html
js/dfg-inline-arguments-use-directly-from-inlined-code.html
js/dfg-inline-arguments-use-from-uninlined-code.html
js/dfg-inline-arguments-become-double.html
js/dfg-inline-arguments-become-int32.html
js/js-correct-exception-handler.html
js/dfg-inline-arguments-use-from-all-the-places-broken.html
js/dfg-inline-arguments-use-from-all-the-places.html
js/dfg-create-inlined-arguments-in-closure-inline.html
js/regress/external-arguments-getbyval.html
js/dfg-inline-arguments-use-from-getter.html
js/dfg-inline-arguments-osr-exit-and-capture.html
js/dfg-inline-arguments-reset-changetype.html
Comment 28 Build Bot 2013-09-11 13:09:56 PDT
Created attachment 211343 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.4
Comment 29 Michael Saboff 2013-09-11 15:19:41 PDT
Created attachment 211359 [details]
Patch addressing review and ewe bot issues

Removed the "if (symbolTable())" check in CodeBlock.cpp.

Have speculative fixes for Qt compile failure (moved definition of trustedImm32ForShift() up in the file).

Have speculative fix for failure found in mac-wk2-ews - Made stackOffset int in CodeOrigin::stackOffset() and inlinedFrameOffset in StackVisitor.cpp.
Comment 30 WebKit Commit Bot 2013-09-11 15:21:43 PDT
Attachment 211359 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/assembler/MacroAssembler.h', u'Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h', u'Source/JavaScriptCore/assembler/X86Assembler.h', u'Source/JavaScriptCore/bytecode/CodeBlock.cpp', u'Source/JavaScriptCore/bytecode/CodeBlock.h', u'Source/JavaScriptCore/bytecode/CodeOrigin.h', u'Source/JavaScriptCore/bytecode/Operands.h', u'Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp', u'Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h', u'Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp', u'Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp', u'Source/JavaScriptCore/dfg/DFGJITCompiler.cpp', u'Source/JavaScriptCore/dfg/DFGOSREntry.cpp', u'Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp', u'Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp', u'Source/JavaScriptCore/dfg/DFGOperations.cpp', u'Source/JavaScriptCore/dfg/DFGScoreBoard.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp', u'Source/JavaScriptCore/dfg/DFGValidate.cpp', u'Source/JavaScriptCore/ftl/FTLLink.cpp', u'Source/JavaScriptCore/heap/ConservativeRoots.cpp', u'Source/JavaScriptCore/interpreter/CallFrame.cpp', u'Source/JavaScriptCore/interpreter/CallFrame.h', u'Source/JavaScriptCore/interpreter/Interpreter.cpp', u'Source/JavaScriptCore/interpreter/JSStack.cpp', u'Source/JavaScriptCore/interpreter/JSStack.h', u'Source/JavaScriptCore/interpreter/JSStackInlines.h', u'Source/JavaScriptCore/interpreter/StackVisitor.cpp', u'Source/JavaScriptCore/jit/JIT.cpp', u'Source/JavaScriptCore/jit/JITCall.cpp', u'Source/JavaScriptCore/jit/JITCall32_64.cpp', u'Source/JavaScriptCore/jit/JITOpcodes.cpp', u'Source/JavaScriptCore/jit/JITOpcodes32_64.cpp', u'Source/JavaScriptCore/jit/JITStubs.cpp', u'Source/JavaScriptCore/jit/ThunkGenerators.cpp', u'Source/JavaScriptCore/llint/LLIntData.cpp', u'Source/JavaScriptCore/llint/LLIntSlowPaths.cpp', u'Source/JavaScriptCore/llint/LowLevelInterpreter.asm', u'Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm', u'Source/JavaScriptCore/llint/LowLevelInterpreter64.asm', u'Source/JavaScriptCore/runtime/ArgList.cpp', u'Source/JavaScriptCore/runtime/ArgList.h', u'Source/JavaScriptCore/runtime/Arguments.cpp', u'Source/JavaScriptCore/runtime/ArrayConstructor.cpp', u'Source/JavaScriptCore/runtime/CommonSlowPaths.cpp', u'Source/JavaScriptCore/runtime/JSActivation.h', u'Source/JavaScriptCore/runtime/JSArray.h', u'Source/JavaScriptCore/runtime/JSGlobalObject.cpp', u'Source/JavaScriptCore/runtime/JSGlobalObject.h', u'Source/JavaScriptCore/runtime/JSString.h', u'Source/JavaScriptCore/runtime/Operations.h', u'Source/JavaScriptCore/runtime/SymbolTable.h']" exit_code: 1
Source/JavaScriptCore/bytecode/CodeOrigin.h:100:  Please declare integral type bitfields with either signed or unsigned.  [runtime/bitfields] [5]
Total errors found: 1 in 54 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 31 Build Bot 2013-09-11 17:24:15 PDT
Comment on attachment 211359 [details]
Patch addressing review and ewe bot issues

Attachment 211359 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1815007

New failing tests:
js/regress/inline-arguments-local-escape.html
js/dfg-inline-arguments-reset.html
js/dfg-inline-arguments-int32.html
js/dfg-inline-arguments-simple.html
js/dfg-inline-arguments-use-directly-from-inlined-code.html
js/dfg-inline-arguments-use-from-uninlined-code.html
js/dfg-inline-arguments-become-double.html
js/dfg-inline-arguments-become-int32.html
js/dfg-inline-arguments-osr-exit-and-capture.html
js/dfg-inline-arguments-use-from-all-the-places-broken.html
js/dfg-inline-arguments-use-from-all-the-places.html
js/dfg-create-inlined-arguments-in-closure-inline.html
js/regress/external-arguments-getbyval.html
js/dfg-inline-arguments-use-from-getter.html
js/dfg-inline-arguments-reset-changetype.html
Comment 32 Build Bot 2013-09-11 17:24:18 PDT
Created attachment 211368 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.4
Comment 33 EFL EWS Bot 2013-09-11 20:38:54 PDT
Comment on attachment 211359 [details]
Patch addressing review and ewe bot issues

Attachment 211359 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1806045
Comment 34 Build Bot 2013-09-11 23:56:13 PDT
Comment on attachment 211359 [details]
Patch addressing review and ewe bot issues

Attachment 211359 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1828029

New failing tests:
js/regress/inline-arguments-local-escape.html
js/dfg-inline-arguments-reset.html
js/dfg-inline-arguments-int32.html
js/dfg-inline-arguments-simple.html
js/dfg-inline-arguments-use-directly-from-inlined-code.html
js/dfg-inline-arguments-use-from-uninlined-code.html
js/dfg-inline-arguments-become-double.html
js/dfg-inline-arguments-become-int32.html
js/dfg-inline-arguments-osr-exit-and-capture.html
js/dfg-inline-arguments-use-from-all-the-places-broken.html
js/dfg-inline-arguments-use-from-all-the-places.html
js/dfg-create-inlined-arguments-in-closure-inline.html
js/regress/external-arguments-getbyval.html
js/dfg-inline-arguments-use-from-getter.html
js/dfg-inline-arguments-reset-changetype.html
Comment 35 Build Bot 2013-09-11 23:56:16 PDT
Created attachment 211404 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.4
Comment 36 Michael Saboff 2013-09-12 10:59:33 PDT
Created attachment 211446 [details]
Another Patch with speculative build / test fixes
Comment 37 Michael Saboff 2013-09-12 16:23:01 PDT
Created attachment 211490 [details]
Patch with final fix

Fixed a bug in Arguments::tearOff() for the inlined call frame case where the Register* for the torn off arguments wasn't being calculated correctly.  The calculation for the non-inline case was correct.
Comment 38 Geoffrey Garen 2013-09-12 16:59:13 PDT
> Fixed a bug in Arguments::tearOff() for the inlined call frame case where the Register* for the torn off arguments wasn't being calculated correctly.

Can we improve the disassembly diff-er to catch the cases you've been fixing?
Comment 39 Michael Saboff 2013-09-12 17:04:29 PDT
(In reply to comment #38)
> > Fixed a bug in Arguments::tearOff() for the inlined call frame case where the Register* for the torn off arguments wasn't being calculated correctly.
> 
> Can we improve the disassembly diff-er to catch the cases you've been fixing?

The fixes I made yesterday and today aren't in code that generates instructions.  I believe that this is the final patch.
Comment 40 Michael Saboff 2013-09-13 11:03:14 PDT
Committed r155711: <http://trac.webkit.org/changeset/155711>