Changeset 114309 in webkit for trunk/Source/JavaScriptCore


Ignore:
Timestamp:
Apr 16, 2012, 3:26:36 PM (13 years ago)
Author:
[email protected]
Message:

Exception stack traces aren't complete when the exception starts in native code
​https://bugs.webkit.org/show_bug.cgi?id=84073

Reviewed by Gavin Barraclough.

Source/JavaScriptCore:

Refactored building the stack trace to so that we can construct
it earlier, and don't rely on any prior work performed in the
exception handling machinery. Also updated LLInt and the DFG to
completely initialise the callframes of host function calls.

  • bytecode/CodeBlock.h:

(JSC::CodeBlock::codeOriginIndexForReturn):
(CodeBlock):

  • dfg/DFGOperations.cpp:
  • interpreter/Interpreter.cpp:

(JSC::Interpreter::getStackTrace):
(JSC::Interpreter::addStackTraceIfNecessary):
(JSC):
(JSC::Interpreter::throwException):

  • interpreter/Interpreter.h:

(Interpreter):

  • jit/JITStubs.cpp:

(JSC::DEFINE_STUB_FUNCTION):

  • jsc.cpp:

(functionJSCStack):

  • llint/LLIntSlowPaths.cpp:

(JSC::LLInt::handleHostCall):

  • parser/Parser.h:

(JSC::::parse):

  • runtime/Error.cpp:

(JSC::addErrorInfo):
(JSC::throwError):

  • runtime/Error.h:

(JSC):

LayoutTests:

Update tests to include new exception property ordering, and new functions

  • fast/js/exception-properties-expected.txt:
  • fast/js/script-tests/exception-properties.js:
  • fast/js/script-tests/stack-trace.js:

(selfRecursive1): Modified slightly so that we produce consistent traces

  • fast/js/stack-trace-expected.txt:
Location:
trunk/Source/JavaScriptCore
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r114274 r114309  
     12012-04-16  Oliver Hunt  <[email protected]>
     2
     3        Exception stack traces aren't complete when the exception starts in native code
     4        https://bugs.webkit.org/show_bug.cgi?id=84073
     5
     6        Reviewed by Gavin Barraclough.
     7
     8        Refactored building the stack trace to so that we can construct
     9        it earlier, and don't rely on any prior work performed in the
     10        exception handling machinery. Also updated LLInt and the DFG to
     11        completely initialise the callframes of host function calls.
     12
     13        * bytecode/CodeBlock.h:
     14        (JSC::CodeBlock::codeOriginIndexForReturn):
     15        (CodeBlock):
     16        * dfg/DFGOperations.cpp:
     17        * interpreter/Interpreter.cpp:
     18        (JSC::Interpreter::getStackTrace):
     19        (JSC::Interpreter::addStackTraceIfNecessary):
     20        (JSC):
     21        (JSC::Interpreter::throwException):
     22        * interpreter/Interpreter.h:
     23        (Interpreter):
     24        * jit/JITStubs.cpp:
     25        (JSC::DEFINE_STUB_FUNCTION):
     26        * jsc.cpp:
     27        (functionJSCStack):
     28        * llint/LLIntSlowPaths.cpp:
     29        (JSC::LLInt::handleHostCall):
     30        * parser/Parser.h:
     31        (JSC::::parse):
     32        * runtime/Error.cpp:
     33        (JSC::addErrorInfo):
     34        (JSC::throwError):
     35        * runtime/Error.h:
     36        (JSC):
     37
    1382012-04-16  Oliver Hunt  <[email protected]>
    239

  • trunk/Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.def

    r114255 r114309  
    213213    ?getPropertyNames@JSObject@JSC@@SAXPAV12@PAVExecState@2@AAVPropertyNameArray@2@W4EnumerationMode@2@@Z
    214214    ?getSlice@ArgList@JSC@@QBEXHAAV12@@Z
    215     ?getStackTrace@Interpreter@JSC@@SAXPAVJSGlobalData@2@HAAV?$Vector@UStackFrame@JSC@@$0A@@WTF@@@Z
     215    ?getStackTrace@Interpreter@JSC@@SAXPAVJSGlobalData@2@AAV?$Vector@UStackFrame@JSC@@$0A@@WTF@@@Z
    216216    ?getString@JSCell@JSC@@QBE?AVUString@2@PAVExecState@2@@Z
    217217    ?getString@JSCell@JSC@@QBE_NPAVExecState@2@AAVUString@2@@Z

  • trunk/Source/JavaScriptCore/bytecode/CodeBlock.h

    r113906 r114309  
    711711        }
    712712       
     713        int codeOriginIndexForReturn(ReturnAddressPtr returnAddress)
     714        {
     715            ASSERT(hasCodeOrigins());
     716            unsigned offset = getJITCode().offsetOf(returnAddress.value());
     717            CodeOriginAtCallReturnOffset* entry = binarySearch<CodeOriginAtCallReturnOffset, unsigned, getCallReturnOffsetForCodeOrigin>(codeOrigins().begin(), codeOrigins().size(), offset, WTF::KeyMustNotBePresentInArray);
     718            return entry - codeOrigins().begin();
     719        }
     720       
    713721        CodeOrigin codeOrigin(unsigned index)
    714722        {

  • trunk/Source/JavaScriptCore/dfg/DFGOperations.cpp

    r113930 r114309  
    793793   
    794794        if (callType == CallTypeHost) {
     795            execCallee->setCallee(asObject(callee));
    795796            globalData->hostCallReturnValue = JSValue::decode(callData.native.function(execCallee));
    796797            if (globalData->exception)
    … …  
    813814   
    814815    if (constructType == ConstructTypeHost) {
     816        execCallee->setCallee(asObject(callee));
    815817        globalData->hostCallReturnValue = JSValue::decode(constructData.native.function(execCallee));
    816818        if (globalData->exception)

  • trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

    r113363 r114309  
    6666#include <stdio.h>
    6767#include <wtf/Threading.h>
     68#include <wtf/text/StringBuilder.h>
    6869
    6970#if ENABLE(JIT)
    … …  
    953954}
    954955
    955 void Interpreter::getStackTrace(JSGlobalData* globalData, int line, Vector<StackFrame>& results)
     956void Interpreter::getStackTrace(JSGlobalData* globalData, Vector<StackFrame>& results)
    956957{
    957     CallFrame* callFrame = globalData->topCallFrame->removeHostCallFrameFlag()->trueCallFrameFromVMCode();
     958    CallFrame* callFrame = globalData->topCallFrame->removeHostCallFrameFlag();
    958959    if (!callFrame || callFrame == CallFrame::noCaller())
    959960        return;
    960 
    961     if (line == -1)
    962         line = getLineNumberForCallFrame(globalData, callFrame);
     961    int line = getLineNumberForCallFrame(globalData, callFrame);
     962
     963    callFrame = callFrame->trueCallFrameFromVMCode();
    963964
    964965    while (callFrame && callFrame != CallFrame::noCaller()) {
    … …  
    976977}
    977978
     979void Interpreter::addStackTraceIfNecessary(CallFrame* callFrame, JSObject* error)
     980{
     981    JSGlobalData* globalData = &callFrame->globalData();
     982    if (error->hasProperty(callFrame, globalData->propertyNames->stack))
     983        return;
     984
     985    Vector<StackFrame> stackTrace;
     986    getStackTrace(&callFrame->globalData(), stackTrace);
     987   
     988    if (stackTrace.isEmpty())
     989        return;
     990   
     991    JSGlobalObject* globalObject = 0;
     992    if (isTerminatedExecutionException(error) || isInterruptedExecutionException(error))
     993        globalObject = globalData->dynamicGlobalObject;
     994    else
     995        globalObject = error->globalObject();
     996    StringBuilder builder;
     997    for (unsigned i = 0; i < stackTrace.size(); i++) {
     998        builder.append(String(stackTrace[i].toString(globalObject->globalExec()).impl()));
     999        if (i != stackTrace.size() - 1)
     1000            builder.append('\n');
     1001    }
     1002   
     1003    error->putDirect(*globalData, globalData->propertyNames->stack, jsString(globalData, UString(builder.toString().impl())), ReadOnly | DontDelete);
     1004}
     1005
    9781006NEVER_INLINE HandlerInfo* Interpreter::throwException(CallFrame*& callFrame, JSValue& exceptionValue, unsigned bytecodeOffset)
    9791007{
    … …  
    9911019        if (codeBlock->hasExpressionInfo() && !hasErrorInfo(callFrame, exception)) {
    9921020            ASSERT(codeBlock->hasLineInfo());
    993 
    9941021            // FIXME: should only really be adding these properties to VM generated exceptions,
    9951022            // but the inspector currently requires these for all thrown objects.
    996             Vector<StackFrame> stackTrace;
    997             getStackTrace(&callFrame->globalData(), codeBlock->lineNumberForBytecodeOffset(bytecodeOffset), stackTrace);
    998             addErrorInfo(callFrame, exception, codeBlock->lineNumberForBytecodeOffset(bytecodeOffset), codeBlock->ownerExecutable()->source(), stackTrace);
     1023            addErrorInfo(callFrame, exception, codeBlock->lineNumberForBytecodeOffset(bytecodeOffset), codeBlock->ownerExecutable()->source());
    9991024        }
    10001025

  • trunk/Source/JavaScriptCore/interpreter/Interpreter.h

    r108444 r114309  
    223223        NEVER_INLINE void debug(CallFrame*, DebugHookID, int firstLine, int lastLine);
    224224        static const UString getTraceLine(CallFrame*, StackFrameCodeType, const UString&, int);
    225         JS_EXPORT_PRIVATE static void getStackTrace(JSGlobalData*, int line, Vector<StackFrame>& results);
     225        JS_EXPORT_PRIVATE static void getStackTrace(JSGlobalData*, Vector<StackFrame>& results);
     226        static void addStackTraceIfNecessary(CallFrame*, JSObject* error);
    226227
    227228        void dumpSampleData(ExecState* exec);

  • trunk/Source/JavaScriptCore/jit/JITStubs.cpp

    r114255 r114309  
    35193519    STUB_INIT_STACK_FRAME(stackFrame);
    35203520    JSGlobalData* globalData = stackFrame.globalData;
     3521    // It's possible for us to reach this point with incorrect origin metadata
     3522    // if a native function throws an exception after being planted in certain
     3523    // code paths as the native thunk doesn't can't unwind itself as if it were
     3524    // a JS function. So we redetermine the correct data here just to be safe.
     3525    if (CodeBlock* codeBlock = stackFrame.callFrame->codeBlock()) {
     3526#if ENABLE(DFG_JIT)
     3527        if (codeBlock->hasCodeOrigins())
     3528            stackFrame.callFrame->setBytecodeOffsetForNonDFGCode(codeBlock->codeOriginIndexForReturn(globalData->exceptionLocation));
     3529        else
     3530#endif
     3531            if (codeBlock->getJITType() == JITCode::BaselineJIT)
     3532            stackFrame.callFrame->setBytecodeOffsetForNonDFGCode(codeBlock->bytecodeOffset(stackFrame.callFrame, globalData->exceptionLocation));
     3533    }
    35213534    ExceptionHandler handler = jitThrow(globalData, stackFrame.callFrame, globalData->exception, globalData->exceptionLocation);
    35223535    STUB_SET_RETURN_ADDRESS(handler.catchRoutine);

  • trunk/Source/JavaScriptCore/jsc.cpp

    r113553 r114309  
    278278    String trace = "--> Stack trace:\n";
    279279    Vector<StackFrame> stackTrace;
    280     Interpreter::getStackTrace(&exec->globalData(), -1, stackTrace);
     280    Interpreter::getStackTrace(&exec->globalData(), stackTrace);
    281281    int i = 0;
    282282

  • trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp

    r113930 r114309  
    12421242   
    12431243        if (callType == CallTypeHost) {
     1244            execCallee->setCallee(asObject(callee));
    12441245            globalData.hostCallReturnValue = JSValue::decode(callData.native.function(execCallee));
    12451246           
    … …  
    12631264   
    12641265    if (constructType == ConstructTypeHost) {
     1266        execCallee->setCallee(asObject(callee));
    12651267        globalData.hostCallReturnValue = JSValue::decode(constructData.native.function(execCallee));
    12661268

  • trunk/Source/JavaScriptCore/parser/Parser.h

    r112555 r114309  
    994994            *exception = createSyntaxError(lexicalGlobalObject, errMsg);
    995995        else
    996             *exception = addErrorInfo(&lexicalGlobalObject->globalData(), createSyntaxError(lexicalGlobalObject, errMsg), errLine, *m_source, Vector<StackFrame>());
     996            *exception = addErrorInfo(lexicalGlobalObject->globalExec(), createSyntaxError(lexicalGlobalObject, errMsg), errLine, *m_source);
    997997    }
    998998

  • trunk/Source/JavaScriptCore/runtime/Error.cpp

    r109034 r114309  
    121121}
    122122
    123 JSObject* addErrorInfo(JSGlobalData* globalData, JSObject* error, int line, const SourceCode& source, const Vector<StackFrame>& stackTrace)
     123JSObject* addErrorInfo(CallFrame* callFrame, JSObject* error, int line, const SourceCode& source)
    124124{
     125    JSGlobalData* globalData = &callFrame->globalData();
    125126    const UString& sourceURL = source.provider()->url();
    126127
    … …  
    129130    if (!sourceURL.isNull())
    130131        error->putDirect(*globalData, Identifier(globalData, sourceURLPropertyName), jsString(globalData, sourceURL), ReadOnly | DontDelete);
    131     if (!stackTrace.isEmpty()) {
    132         JSGlobalObject* globalObject = 0;
    133         if (isTerminatedExecutionException(error) || isInterruptedExecutionException(error))
    134             globalObject = globalData->dynamicGlobalObject;
    135         else
    136             globalObject = error->globalObject();
    137         StringBuilder builder;
    138         for (unsigned i = 0; i < stackTrace.size(); i++) {
    139             builder.append(String(stackTrace[i].toString(globalObject->globalExec()).impl()));
    140             if (i != stackTrace.size() - 1)
    141                 builder.append('\n');
    142         }
    143132
    144         error->putDirect(*globalData, globalData->propertyNames->stack, jsString(globalData, UString(builder.toString().impl())), ReadOnly | DontDelete);
    145     }
     133    globalData->interpreter->addStackTraceIfNecessary(callFrame, error);
    146134
    147135    return error;
    148136}
    149137
    150 JSObject* addErrorInfo(ExecState* exec, JSObject* error, int line, const SourceCode& source, const Vector<StackFrame>& stackTrace)
    151 {
    152     return addErrorInfo(&exec->globalData(), error, line, source, stackTrace);
    153 }
    154138
    155139bool hasErrorInfo(ExecState* exec, JSObject* error)
    … …  
    161145JSValue throwError(ExecState* exec, JSValue error)
    162146{
     147    if (error.isObject())
     148        return throwError(exec, asObject(error));
    163149    exec->globalData().exception = error;
    164150    return error;
    … …  
    167153JSObject* throwError(ExecState* exec, JSObject* error)
    168154{
     155    Interpreter::addStackTraceIfNecessary(exec, error);
    169156    exec->globalData().exception = error;
    170157    return error;

  • trunk/Source/JavaScriptCore/runtime/Error.h

    r108112 r114309  
    5858    // Methods to add
    5959    bool hasErrorInfo(ExecState*, JSObject* error);
    60     JSObject* addErrorInfo(JSGlobalData*, JSObject* error, int line, const SourceCode&, const Vector<StackFrame>&);
    6160    // ExecState wrappers.
    62     JSObject* addErrorInfo(ExecState*, JSObject* error, int line, const SourceCode&, const Vector<StackFrame>&);
     61    JSObject* addErrorInfo(ExecState*, JSObject* error, int line, const SourceCode&);
    6362
    6463    // Methods to throw Errors.
Note: See TracChangeset for help on using the changeset viewer.