Changeset 201830 in webkit


Ignore:
Timestamp:
Jun 8, 2016 1:59:49 PM (8 years ago)
Author:
mark.lam@apple.com
Message:

Simplify Interpreter::StackFrame.
​https://bugs.webkit.org/show_bug.cgi?id=158498

Reviewed by Saam Barati.

Previously, Interpreter::StackFrame (which is used to capture info for
Error.stack) eagerly extracts info out of CodeBlock and duplicates the work that
CodeBlock does to compute line and column numbers (amongst other things).

This patch does away with the eager extraction and only stashes the CodeBlock
pointer in the Interpreter::StackFrame. Instead, Interpreter::StackFrame will
provide methods for computing the desired values on request later.

One difference in implementation: the old StackFrame offers a sourceURL and a
friendlySourceURL(). The only difference between the 2 is that for native
functions, sourceURL returns an empty string, and friendlySourceURL() returns
"[native code]". This is how it affects the clients of StackFrame:

  • In the old code, the Error object's addErrorInfoAndGetBytecodeOffset() and the inspector's createScriptCallStackFromException() would check if sourceURL is empty. If so, they will use this as an indicator to use alternate source info in the Error object e.g. url and line numbers from the parser that produced a SyntaxError.
  • In the new implementation, StackFrame only has a sourceURL() function that behaves like the old friendlySourceURL(). The client code which were relying on sourceURL being empty, will now explicitly check if the StackFrame is for native code using a new isNative() query in addition to the sourceURL being empty. This achieve functional parity with the old behavior.

Also fix Error.cpp's addErrorInfoAndGetBytecodeOffset() to take a bytecodeOffset
pointer instead of a reference. The bytecodeOffset arg is supposed to be
optional, but was implemented in a unclear way. This change clarifies it.

  • inspector/ScriptCallStackFactory.cpp:

(Inspector::createScriptCallStackFromException):

  • interpreter/Interpreter.cpp:

(JSC::StackFrame::sourceID):
(JSC::StackFrame::sourceURL):
(JSC::StackFrame::functionName):
(JSC::eval):
(JSC::Interpreter::isOpcode):
(JSC::StackFrame::computeLineAndColumn):
(JSC::StackFrame::toString):
(JSC::GetStackTraceFunctor::operator()):
(JSC::StackFrame::friendlySourceURL): Deleted.
(JSC::StackFrame::friendlyFunctionName): Deleted.
(JSC::getStackFrameCodeType): Deleted.
(JSC::StackFrame::expressionInfo): Deleted.

  • interpreter/Interpreter.h:

(JSC::StackFrame::isNative):

  • runtime/Error.cpp:

(JSC::addErrorInfoAndGetBytecodeOffset):
(JSC::addErrorInfo):

  • runtime/Error.h:
  • runtime/ErrorInstance.cpp:

(JSC::ErrorInstance::finishCreation):

Location:
trunk/Source/JavaScriptCore
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r201825 r201830  
     12016-06-08  Mark Lam  <mark.lam@apple.com>
     2
     3        Simplify Interpreter::StackFrame.
     4        https://bugs.webkit.org/show_bug.cgi?id=158498
     5
     6        Reviewed by Saam Barati.
     7
     8        Previously, Interpreter::StackFrame (which is used to capture info for
     9        Error.stack) eagerly extracts info out of CodeBlock and duplicates the work that
     10        CodeBlock does to compute line and column numbers (amongst other things).
     11
     12        This patch does away with the eager extraction and only stashes the CodeBlock
     13        pointer in the Interpreter::StackFrame.  Instead, Interpreter::StackFrame will
     14        provide methods for computing the desired values on request later.
     15
     16        One difference in implementation: the old StackFrame offers a sourceURL and a
     17        friendlySourceURL().  The only difference between the 2 is that for native
     18        functions, sourceURL returns an empty string, and friendlySourceURL() returns
     19        "[native code]".  This is how it affects the clients of StackFrame:
     20
     21            - In the old code, the Error object's addErrorInfoAndGetBytecodeOffset() and
     22              the inspector's createScriptCallStackFromException() would check if
     23              sourceURL is empty.  If so, they will use this as an indicator to use
     24              alternate source info in the Error object e.g. url and line numbers from
     25              the parser that produced a SyntaxError.
     26
     27            - In the new implementation, StackFrame only has a sourceURL() function that
     28              behaves like the old friendlySourceURL().  The client code which were
     29              relying on sourceURL being empty, will now explicitly check if the
     30              StackFrame is for native code using a new isNative() query in addition to
     31              the sourceURL being empty.  This achieve functional parity with the old
     32              behavior.
     33
     34        Also fix Error.cpp's addErrorInfoAndGetBytecodeOffset() to take a bytecodeOffset
     35        pointer instead of a reference.  The bytecodeOffset arg is supposed to be
     36        optional, but was implemented in a unclear way.  This change clarifies it.
     37
     38        * inspector/ScriptCallStackFactory.cpp:
     39        (Inspector::createScriptCallStackFromException):
     40        * interpreter/Interpreter.cpp:
     41        (JSC::StackFrame::sourceID):
     42        (JSC::StackFrame::sourceURL):
     43        (JSC::StackFrame::functionName):
     44        (JSC::eval):
     45        (JSC::Interpreter::isOpcode):
     46        (JSC::StackFrame::computeLineAndColumn):
     47        (JSC::StackFrame::toString):
     48        (JSC::GetStackTraceFunctor::operator()):
     49        (JSC::StackFrame::friendlySourceURL): Deleted.
     50        (JSC::StackFrame::friendlyFunctionName): Deleted.
     51        (JSC::getStackFrameCodeType): Deleted.
     52        (JSC::StackFrame::expressionInfo): Deleted.
     53        * interpreter/Interpreter.h:
     54        (JSC::StackFrame::isNative):
     55        * runtime/Error.cpp:
     56        (JSC::addErrorInfoAndGetBytecodeOffset):
     57        (JSC::addErrorInfo):
     58        * runtime/Error.h:
     59        * runtime/ErrorInstance.cpp:
     60        (JSC::ErrorInstance::finishCreation):
     61
    1622016-06-08  Keith Miller  <keith_miller@apple.com>
    263

  • trunk/Source/JavaScriptCore/inspector/ScriptCallStackFactory.cpp

    r201766 r201830  
    144144        unsigned column;
    145145        stackTrace[i].computeLineAndColumn(line, column);
    146         String functionName = stackTrace[i].friendlyFunctionName(vm);
    147         frames.append(ScriptCallFrame(functionName, stackTrace[i].friendlySourceURL(), static_cast<SourceID>(stackTrace[i].sourceID), line, column));
     146        String functionName = stackTrace[i].functionName(vm);
     147        frames.append(ScriptCallFrame(functionName, stackTrace[i].sourceURL(), static_cast<SourceID>(stackTrace[i].sourceID()), line, column));
    148148    }
    149149
    … …  
    159159            frames.append(ScriptCallFrame(String(), exceptionSourceURL, noSourceID, lineNumber, columnNumber));
    160160        } else {
    161             if (stackTrace[0].sourceURL.isEmpty()) {
     161            if (stackTrace[0].isNative() || stackTrace[0].sourceURL().isEmpty()) {
    162162                const ScriptCallFrame& firstCallFrame = frames.first();
    163163                extractSourceInformationFromException(exec, exceptionObject, &lineNumber, &columnNumber, &exceptionSourceURL);
    164                 frames[0] = ScriptCallFrame(firstCallFrame.functionName(), exceptionSourceURL, stackTrace[0].sourceID, lineNumber, columnNumber);
     164                frames[0] = ScriptCallFrame(firstCallFrame.functionName(), exceptionSourceURL, stackTrace[0].sourceID(), lineNumber, columnNumber);
    165165            }
    166166        }

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

    r201766 r201830  
    8888namespace JSC {
    8989
    90 String StackFrame::friendlySourceURL() const
    91 {
    92     String traceLine;
    93    
    94     switch (codeType) {
    95     case StackFrameEvalCode:
    96     case StackFrameModuleCode:
    97     case StackFrameFunctionCode:
    98     case StackFrameGlobalCode:
    99         if (!sourceURL.isEmpty())
    100             traceLine = sourceURL.impl();
    101         break;
    102     case StackFrameNativeCode:
    103         traceLine = "[native code]";
    104         break;
    105     }
    106     return traceLine.isNull() ? emptyString() : traceLine;
    107 }
    108 
    109 String StackFrame::friendlyFunctionName(VM& vm) const
    110 {
    111     String traceLine;
    112     JSObject* stackFrameCallee = callee.get();
    113 
    114     switch (codeType) {
    115     case StackFrameEvalCode:
    116         traceLine = "eval code";
    117         break;
    118     case StackFrameModuleCode:
    119         traceLine = "module code";
    120         break;
    121     case StackFrameNativeCode:
    122         if (callee)
    123             traceLine = getCalculatedDisplayName(vm, stackFrameCallee).impl();
    124         break;
    125     case StackFrameFunctionCode:
    126         traceLine = getCalculatedDisplayName(vm, stackFrameCallee).impl();
    127         break;
    128     case StackFrameGlobalCode:
    129         traceLine = "global code";
    130         break;
    131     }
    132     return traceLine.isNull() ? emptyString() : traceLine;
     90intptr_t StackFrame::sourceID() const
     91{
     92    if (!codeBlock)
     93        return noSourceID;
     94    return codeBlock->ownerScriptExecutable()->sourceID();
     95}
     96
     97String StackFrame::sourceURL() const
     98{
     99    if (!codeBlock)
     100        return ASCIILiteral("[native code]");
     101
     102    String sourceURL = codeBlock->ownerScriptExecutable()->sourceURL();
     103    if (!sourceURL.isNull())
     104        return sourceURL;
     105    return emptyString();
     106}
     107
     108String StackFrame::functionName(VM& vm) const
     109{
     110    if (codeBlock) {
     111        switch (codeBlock->codeType()) {
     112        case EvalCode:
     113            return ASCIILiteral("eval code");
     114        case ModuleCode:
     115            return ASCIILiteral("module code");
     116        case FunctionCode:
     117            break;
     118        case GlobalCode:
     119            return ASCIILiteral("global code");
     120        default:
     121            ASSERT_NOT_REACHED();
     122        }
     123    }
     124    String name;
     125    if (callee)
     126        name = getCalculatedDisplayName(vm, callee.get()).impl();
     127    return name.isNull() ? emptyString() : name;
    133128}
    134129
    … …  
    450445}
    451446
    452 static StackFrameCodeType getStackFrameCodeType(StackVisitor& visitor)
    453 {
    454     switch (visitor->codeType()) {
    455     case StackVisitor::Frame::Eval:
    456         return StackFrameEvalCode;
    457     case StackVisitor::Frame::Module:
    458         return StackFrameModuleCode;
    459     case StackVisitor::Frame::Function:
    460         return StackFrameFunctionCode;
    461     case StackVisitor::Frame::Global:
    462         return StackFrameGlobalCode;
    463     case StackVisitor::Frame::Native:
    464         ASSERT_NOT_REACHED();
    465         return StackFrameNativeCode;
    466     }
    467     RELEASE_ASSERT_NOT_REACHED();
    468     return StackFrameGlobalCode;
    469 }
    470 
    471447void StackFrame::computeLineAndColumn(unsigned& line, unsigned& column)
    472448{
    … …  
    480456    int unusedStartOffset = 0;
    481457    int unusedEndOffset = 0;
    482     unsigned divotLine = 0;
    483     unsigned divotColumn = 0;
    484     expressionInfo(divot, unusedStartOffset, unusedEndOffset, divotLine, divotColumn);
    485 
    486     line = divotLine + lineOffset;
    487     column = divotColumn + (divotLine ? 1 : firstLineColumnOffset);
    488 
     458    codeBlock->expressionRangeForBytecodeOffset(bytecodeOffset, divot, unusedStartOffset, unusedEndOffset, line, column);
     459
     460    ScriptExecutable* executable = codeBlock->ownerScriptExecutable();
    489461    if (executable->hasOverrideLineNumber())
    490462        line = executable->overrideLineNumber();
    491463}
    492464
    493 void StackFrame::expressionInfo(int& divot, int& startOffset, int& endOffset, unsigned& line, unsigned& column)
    494 {
    495     codeBlock->expressionRangeForBytecodeOffset(bytecodeOffset, divot, startOffset, endOffset, line, column);
    496     divot += characterOffset;
    497 }
    498 
    499465String StackFrame::toString(VM& vm)
    500466{
    501467    StringBuilder traceBuild;
    502     String functionName = friendlyFunctionName(vm);
    503     String sourceURL = friendlySourceURL();
     468    String functionName = this->functionName(vm);
     469    String sourceURL = this->sourceURL();
    504470    traceBuild.append(functionName);
    505471    if (!sourceURL.isEmpty()) {
    … …  
    507473            traceBuild.append('@');
    508474        traceBuild.append(sourceURL);
    509         if (codeType != StackFrameNativeCode) {
     475        if (codeBlock) {
    510476            unsigned line;
    511477            unsigned column;
    … …  
    547513                && !isWebAssemblyExecutable(visitor->codeBlock()->ownerExecutable())
    548514                && !visitor->codeBlock()->unlinkedCodeBlock()->isBuiltinFunction()) {
    549                 CodeBlock* codeBlock = visitor->codeBlock();
    550515                StackFrame s = {
    551516                    Strong<JSObject>(vm, visitor->callee()),
    552                     getStackFrameCodeType(visitor),
    553                     Strong<ScriptExecutable>(vm, codeBlock->ownerScriptExecutable()),
    554                     Strong<UnlinkedCodeBlock>(vm, codeBlock->unlinkedCodeBlock()),
    555                     codeBlock->source(),
    556                     codeBlock->ownerScriptExecutable()->firstLine(),
    557                     codeBlock->firstLineColumnOffset(),
    558                     codeBlock->sourceOffset(),
    559                     visitor->bytecodeOffset(),
    560                     visitor->sourceURL(),
    561                     visitor->sourceID(),
     517                    Strong<CodeBlock>(vm, visitor->codeBlock()),
     518                    visitor->bytecodeOffset()
    562519                };
    563520                m_results.append(s);
    564521            } else {
    565                 StackFrame s = { Strong<JSObject>(vm, visitor->callee()), StackFrameNativeCode, Strong<ScriptExecutable>(), Strong<UnlinkedCodeBlock>(), 0, 0, 0, 0, 0, String(), noSourceID};
     522                StackFrame s = {
     523                    Strong<JSObject>(vm, visitor->callee()),
     524                    Strong<CodeBlock>(),
     525                    0 // unused value because codeBlock is null.
     526                };
    566527                m_results.append(s);
    567528            }

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

    r201766 r201830  
    11/*
    2  * Copyright (C) 2008, 2013, 2015 Apple Inc. All rights reserved.
     2 * Copyright (C) 2008, 2013, 2015-2016 Apple Inc. All rights reserved.
    33 * Copyright (C) 2012 Research In Motion Limited. All rights reserved.
    44 *
    … …  
    8686    struct StackFrame {
    8787        Strong<JSObject> callee;
    88         StackFrameCodeType codeType;
    89         Strong<ScriptExecutable> executable;
    90         Strong<UnlinkedCodeBlock> codeBlock;
    91         RefPtr<SourceProvider> code;
    92         int lineOffset;
    93         unsigned firstLineColumnOffset;
    94         unsigned characterOffset;
     88        Strong<CodeBlock> codeBlock;
    9589        unsigned bytecodeOffset;
    96         String sourceURL;
    97         intptr_t sourceID;
     90
     91        bool isNative() const { return !codeBlock; }
     92
     93        void computeLineAndColumn(unsigned& line, unsigned& column);
     94        String functionName(VM&) const;
     95        intptr_t sourceID() const;
     96        String sourceURL() const;
    9897        String toString(VM&);
    99         String friendlySourceURL() const;
    100         String friendlyFunctionName(VM&) const;
    101         JS_EXPORT_PRIVATE void computeLineAndColumn(unsigned& line, unsigned& column);
    102 
    103     private:
    104         void expressionInfo(int& divot, int& startOffset, int& endOffset, unsigned& line, unsigned& column);
    10598    };
    10699

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

    r200338 r201830  
    22 *  Copyright (C) 1999-2001 Harri Porten (porten@kde.org)
    33 *  Copyright (C) 2001 Peter Kelly (pmk@post.com)
    4  *  Copyright (C) 2003, 2004, 2005, 2006, 2008, 2016 Apple Inc. All rights reserved.
     4 *  Copyright (C) 2003-2006, 2008, 2016 Apple Inc. All rights reserved.
    55 *  Copyright (C) 2007 Eric Seidel (eric@webkit.org)
    66 *
    … …  
    140140};
    141141
    142 bool addErrorInfoAndGetBytecodeOffset(ExecState* exec, VM& vm, JSObject* obj, bool useCurrentFrame, CallFrame*& callFrame, unsigned &bytecodeOffset)
     142bool addErrorInfoAndGetBytecodeOffset(ExecState* exec, VM& vm, JSObject* obj, bool useCurrentFrame, CallFrame*& callFrame, unsigned* bytecodeOffset)
    143143{
    144144    Vector<StackFrame> stackTrace = Vector<StackFrame>();
    145145
    146     if (exec && stackTrace.isEmpty())
    147         vm.interpreter->getStackTrace(stackTrace);
    148 
     146    vm.interpreter->getStackTrace(stackTrace);
    149147    if (!stackTrace.isEmpty()) {
    150148
    151149        ASSERT(exec == vm.topCallFrame || exec == exec->lexicalGlobalObject()->globalExec() || exec == exec->vmEntryGlobalObject()->globalExec());
    152150
    153         StackFrame* stackFrame = nullptr;
     151        StackFrame* firstNonNativeFrame;
    154152        for (unsigned i = 0 ; i < stackTrace.size(); ++i) {
    155             stackFrame = &stackTrace.at(i);
    156             if (stackFrame->bytecodeOffset)
     153            firstNonNativeFrame = &stackTrace.at(i);
     154            if (!firstNonNativeFrame->isNative())
    157155                break;
    158156        }
    … …  
    163161            callFrame = functor.foundCallFrame();
    164162            unsigned stackIndex = functor.index();
    165             bytecodeOffset = stackTrace.at(stackIndex).bytecodeOffset;
     163            *bytecodeOffset = stackTrace.at(stackIndex).bytecodeOffset;
    166164        }
    167165       
    168166        unsigned line;
    169167        unsigned column;
    170         stackFrame->computeLineAndColumn(line, column);
     168        firstNonNativeFrame->computeLineAndColumn(line, column);
    171169        obj->putDirect(vm, vm.propertyNames->line, jsNumber(line), ReadOnly | DontDelete);
    172170        obj->putDirect(vm, vm.propertyNames->column, jsNumber(column), ReadOnly | DontDelete);
    173171
    174         if (!stackFrame->sourceURL.isEmpty())
    175             obj->putDirect(vm, vm.propertyNames->sourceURL, jsString(&vm, stackFrame->sourceURL), ReadOnly | DontDelete);
    176    
     172        String frameSourceURL = firstNonNativeFrame->sourceURL();
     173        if (!frameSourceURL.isEmpty())
     174            obj->putDirect(vm, vm.propertyNames->sourceURL, jsString(&vm, frameSourceURL), ReadOnly | DontDelete);
     175
    177176        if (!useCurrentFrame)
    178177            stackTrace.remove(0);
    … …  
    187186{
    188187    CallFrame* callFrame = nullptr;
    189     unsigned bytecodeOffset = 0;
    190     addErrorInfoAndGetBytecodeOffset(exec, exec->vm(), obj, useCurrentFrame, callFrame, bytecodeOffset);
     188    addErrorInfoAndGetBytecodeOffset(exec, exec->vm(), obj, useCurrentFrame, callFrame);
    191189}
    192190

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

    r197614 r201830  
    6464
    6565
    66 bool addErrorInfoAndGetBytecodeOffset(ExecState*, VM&, JSObject*, bool, CallFrame*&, unsigned&);
     66bool addErrorInfoAndGetBytecodeOffset(ExecState*, VM&, JSObject*, bool, CallFrame*&, unsigned* = nullptr);
    6767
    6868bool hasErrorInfo(ExecState*, JSObject* error);

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

    r196302 r201830  
    143143        putDirect(vm, vm.propertyNames->message, jsString(&vm, message), DontEnum);
    144144
    145     unsigned bytecodeOffset = hasSourceAppender();
     145    unsigned bytecodeOffset = 0;
    146146    CallFrame* callFrame = nullptr;
    147     bool hasTrace = addErrorInfoAndGetBytecodeOffset(exec, vm, this, useCurrentFrame, callFrame, bytecodeOffset);
     147    bool hasTrace = addErrorInfoAndGetBytecodeOffset(exec, vm, this, useCurrentFrame, callFrame, hasSourceAppender() ? &bytecodeOffset : nullptr);
    148148
    149149    if (hasTrace && callFrame && hasSourceAppender()) {
Note: See TracChangeset for help on using the changeset viewer.