Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow moving the metadata part of JSC::BuiltinExecutables::createExecutable to a generator script #26

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Source/JavaScriptCore/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1404,6 +1404,8 @@ set(JavaScriptCore_PRIVATE_FRAMEWORK_HEADERS

runtime/JSModuleEnvironment.h
runtime/JSModuleEnvironmentInlines.h

runtime/BuiltinExecutableMetadata.h
)

if(USE_INSPECTOR_SOCKET_SERVER)
Expand Down
7 changes: 6 additions & 1 deletion Source/JavaScriptCore/builtins/BuiltinExecutableCreator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,10 @@ UnlinkedFunctionExecutable* createBuiltinExecutable(VM& vm, const SourceCode& so
{
return BuiltinExecutables::createExecutable(vm, source, ident, implementationVisibility, kind, ability, NeedsClassFieldInitializer::No);
}


UnlinkedFunctionExecutable* createBuiltinExecutable(VM& vm, const SourceCode& source, const Identifier& ident, ImplementationVisibility implementationVisibility, ConstructorKind kind, ConstructAbility ability, const BuiltinExecutableMetadata& metadata)
{
return BuiltinExecutables::createExecutable(vm, source, ident, implementationVisibility, kind, ability, NeedsClassFieldInitializer::No, PrivateBrandRequirement::None, metadata);
}

} // namespace JSC
3 changes: 3 additions & 0 deletions Source/JavaScriptCore/builtins/BuiltinExecutableCreator.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@
#include "ImplementationVisibility.h"
#include "ParserModes.h"
#include "SourceCode.h"
#include "BuiltinExecutableMetadata.h"

namespace JSC {

JS_EXPORT_PRIVATE UnlinkedFunctionExecutable* createBuiltinExecutable(VM&, const SourceCode&, const Identifier&, ImplementationVisibility, ConstructorKind, ConstructAbility);
JS_EXPORT_PRIVATE UnlinkedFunctionExecutable* createBuiltinExecutable(VM&, const SourceCode&, const Identifier&, ImplementationVisibility, ConstructorKind, ConstructAbility, const BuiltinExecutableMetadata&);


} // namespace JSC
78 changes: 52 additions & 26 deletions Source/JavaScriptCore/builtins/BuiltinExecutables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "JSCJSValueInlines.h"
#include "Parser.h"
#include <wtf/NeverDestroyed.h>
#include "BuiltinExecutableMetadata.h"

namespace JSC {

Expand Down Expand Up @@ -75,42 +76,47 @@ UnlinkedFunctionExecutable* BuiltinExecutables::createDefaultConstructor(Constru

UnlinkedFunctionExecutable* BuiltinExecutables::createBuiltinExecutable(const SourceCode& code, const Identifier& name, ImplementationVisibility implementationVisibility, ConstructorKind constructorKind, ConstructAbility constructAbility)
{
return createExecutable(m_vm, code, name, implementationVisibility, constructorKind, constructAbility, NeedsClassFieldInitializer::No);
StringView view = code.view();
RELEASE_ASSERT(!view.isNull());
RELEASE_ASSERT(view.is8Bit());
BuiltinExecutableMetadata meta(reinterpret_cast<const unsigned char*>(view.characters8()), view.length());
return createExecutable(m_vm, code, name, implementationVisibility, constructorKind, constructAbility, NeedsClassFieldInitializer::No, PrivateBrandRequirement::None, meta);
}

UnlinkedFunctionExecutable* BuiltinExecutables::createExecutable(VM& vm, const SourceCode& source, const Identifier& name, ImplementationVisibility implementationVisibility, ConstructorKind constructorKind, ConstructAbility constructAbility, NeedsClassFieldInitializer needsClassFieldInitializer, PrivateBrandRequirement privateBrandRequirement)
{
// FIXME: Can we just make MetaData computation be constexpr and have the compiler do this for us?
// https://bugs.webkit.org/show_bug.cgi?id=193272
// Someone should get mad at me for writing this code. But, it prevents us from recursing into
// the parser, and hence, from throwing stack overflow when parsing a builtin.
StringView view = source.view();
RELEASE_ASSERT(!view.isNull());
RELEASE_ASSERT(view.is8Bit());
auto* characters = view.characters8();
BuiltinExecutableMetadata meta(reinterpret_cast<const unsigned char*>(view.characters8()), view.length());
return createExecutable(vm, source, name, implementationVisibility, constructorKind, constructAbility, needsClassFieldInitializer, privateBrandRequirement, meta);
}

BuiltinExecutableMetadata::BuiltinExecutableMetadata(const unsigned char* characters, unsigned length)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be moved in BuiltinExecutableMetadata.cpp, but it would make rebasing harder, but also alot of stuff in this function is already going to merge conflict.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it needs to be kept in sync with whatever variables change below it

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is going to make merging harder and there is an open issue by an apple employee is this a candidate to push to webkit (see if they are open to it)?

{
const char* regularFunctionBegin = "(function (";
const char* asyncFunctionBegin = "(async function (";
RELEASE_ASSERT(view.length() >= strlen("(function (){})"));
bool isAsyncFunction = view.length() >= strlen("(async function (){})") && !memcmp(characters, asyncFunctionBegin, strlen(asyncFunctionBegin));
RELEASE_ASSERT(length >= strlen("(function (){})"));
isAsyncFunction = length >= strlen("(async function (){})") && !memcmp(characters, asyncFunctionBegin, strlen(asyncFunctionBegin));
RELEASE_ASSERT(isAsyncFunction || !memcmp(characters, regularFunctionBegin, strlen(regularFunctionBegin)));

unsigned asyncOffset = isAsyncFunction ? strlen("async ") : 0;
unsigned parametersStart = strlen("function (") + asyncOffset;
unsigned startColumn = parametersStart;
int functionKeywordStart = strlen("(") + asyncOffset;
int functionNameStart = parametersStart;
bool isInStrictContext = false;
bool isArrowFunctionBodyExpression = false;
asyncOffset = isAsyncFunction ? strlen("async ") : 0;
parametersStart = strlen("function (") + asyncOffset;
startColumn = parametersStart;
functionKeywordStart = strlen("(") + asyncOffset;
functionNameStart = parametersStart;
isInStrictContext = false;
isArrowFunctionBodyExpression = false;

unsigned parameterCount;
parameterCount = 0;
{
unsigned i = parametersStart + 1;
unsigned commas = 0;
bool insideCurlyBrackets = false;
bool sawOneParam = false;
bool hasRestParam = false;
while (true) {
ASSERT(i < view.length());
ASSERT(i < length);
if (characters[i] == ')')
break;

Expand All @@ -125,7 +131,7 @@ UnlinkedFunctionExecutable* BuiltinExecutables::createExecutable(VM& vm, const S
else if (!Lexer<LChar>::isWhiteSpace(characters[i]))
sawOneParam = true;

if (i + 2 < view.length() && characters[i] == '.' && characters[i + 1] == '.' && characters[i + 2] == '.') {
if (i + 2 < length && characters[i] == '.' && characters[i + 1] == '.' && characters[i + 2] == '.') {
hasRestParam = true;
i += 2;
}
Expand All @@ -146,11 +152,11 @@ UnlinkedFunctionExecutable* BuiltinExecutables::createExecutable(VM& vm, const S
}
}

unsigned lineCount = 0;
unsigned endColumn = 0;
unsigned offsetOfLastNewline = 0;
lineCount = 0;
endColumn = 0;
offsetOfLastNewline = 0;
std::optional<unsigned> offsetOfSecondToLastNewline;
for (unsigned i = 0; i < view.length(); ++i) {
for (unsigned i = 0; i < length; ++i) {
if (characters[i] == '\n') {
if (lineCount)
offsetOfSecondToLastNewline = offsetOfLastNewline;
Expand All @@ -162,7 +168,7 @@ UnlinkedFunctionExecutable* BuiltinExecutables::createExecutable(VM& vm, const S

if (!isInStrictContext && (characters[i] == '"' || characters[i] == '\'')) {
const unsigned useStrictLength = strlen("use strict");
if (i + 1 + useStrictLength < view.length()) {
if (i + 1 + useStrictLength < length) {
if (!memcmp(characters + i + 1, "use strict", useStrictLength)) {
isInStrictContext = true;
i += 1 + useStrictLength;
Expand All @@ -171,14 +177,34 @@ UnlinkedFunctionExecutable* BuiltinExecutables::createExecutable(VM& vm, const S
}
}

unsigned positionBeforeLastNewlineLineStartOffset = offsetOfSecondToLastNewline ? *offsetOfSecondToLastNewline + 1 : 0;
positionBeforeLastNewlineLineStartOffset = offsetOfSecondToLastNewline ? *offsetOfSecondToLastNewline + 1 : 0;

int closeBraceOffsetFromEnd = 1;
closeBraceOffsetFromEnd = 1;
while (true) {
if (characters[view.length() - closeBraceOffsetFromEnd] == '}')
if (characters[length - closeBraceOffsetFromEnd] == '}')
break;
++closeBraceOffsetFromEnd;
}
}

UnlinkedFunctionExecutable* BuiltinExecutables::createExecutable(VM& vm, const SourceCode& source, const Identifier& name, ImplementationVisibility implementationVisibility, ConstructorKind constructorKind, ConstructAbility constructAbility, NeedsClassFieldInitializer needsClassFieldInitializer, PrivateBrandRequirement privateBrandRequirement, const BuiltinExecutableMetadata& meta)
{
StringView view = source.view();
unsigned startColumn = meta.startColumn;
unsigned endColumn = meta.endColumn;
int functionKeywordStart = meta.functionKeywordStart;
int functionNameStart = meta.functionNameStart;
unsigned parametersStart = meta.parametersStart;
bool isInStrictContext = meta.isInStrictContext;
bool isArrowFunctionBodyExpression = meta.isArrowFunctionBodyExpression;
bool isAsyncFunction = meta.isAsyncFunction;

unsigned lineCount = meta.lineCount;
unsigned offsetOfLastNewline = meta.offsetOfLastNewline;
unsigned positionBeforeLastNewlineLineStartOffset = meta.positionBeforeLastNewlineLineStartOffset;
unsigned closeBraceOffsetFromEnd = meta.closeBraceOffsetFromEnd;
unsigned asyncOffset = meta.asyncOffset;
unsigned parameterCount = meta.parameterCount;

JSTextPosition positionBeforeLastNewline;
positionBeforeLastNewline.line = lineCount;
Expand Down
3 changes: 2 additions & 1 deletion Source/JavaScriptCore/builtins/BuiltinExecutables.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ namespace JSC {
class UnlinkedFunctionExecutable;
class Identifier;
class VM;
class BuiltinExecutableMetadata;

#define BUILTIN_NAME_ONLY(name, functionName, overriddenName, length) name,
enum class BuiltinCodeIndex {
Expand All @@ -61,7 +62,7 @@ SourceCode name##Source();
UnlinkedFunctionExecutable* createDefaultConstructor(ConstructorKind, const Identifier& name, NeedsClassFieldInitializer, PrivateBrandRequirement);

static UnlinkedFunctionExecutable* createExecutable(VM&, const SourceCode&, const Identifier&, ImplementationVisibility, ConstructorKind, ConstructAbility, NeedsClassFieldInitializer, PrivateBrandRequirement = PrivateBrandRequirement::None);

static UnlinkedFunctionExecutable* createExecutable(VM&, const SourceCode&, const Identifier&, ImplementationVisibility, ConstructorKind, ConstructAbility, NeedsClassFieldInitializer, PrivateBrandRequirement, const BuiltinExecutableMetadata&);
void finalizeUnconditionally(CollectionScope);

private:
Expand Down
3 changes: 3 additions & 0 deletions Source/JavaScriptCore/builtins/BuiltinUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ class Identifier;
class SourceCode;
class UnlinkedFunctionExecutable;
class VM;
class BuiltinExecutableMetadata;

JS_EXPORT_PRIVATE UnlinkedFunctionExecutable* createBuiltinExecutable(VM&, const SourceCode&, const Identifier&, ImplementationVisibility, ConstructorKind, ConstructAbility);
JS_EXPORT_PRIVATE UnlinkedFunctionExecutable* createBuiltinExecutable(VM&, const SourceCode&, const Identifier&, ImplementationVisibility, ConstructorKind, ConstructAbility, const BuiltinExecutableMetadata&);


} // namespace JSC
46 changes: 46 additions & 0 deletions Source/JavaScriptCore/runtime/BuiltinExecutableMetadata.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#pragma once

namespace JSC {

class BuiltinExecutableMetadata {
public:
BuiltinExecutableMetadata(const unsigned char* data, unsigned length);
BuiltinExecutableMetadata() = default;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this constructor BuiltinExecutableMetadata() should not exist

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it most definitely should exist because that is how we run it in a generator script

BuiltinExecutableMetadata(unsigned startColumn, unsigned endColumn, int functionKeywordStart, int functionNameStart, unsigned parametersStart, bool isInStrictContext, bool isArrowFunctionBodyExpression, bool isAsyncFunction, unsigned asyncOffset, unsigned parameterCount, unsigned lineCount, unsigned offsetOfLastNewline, unsigned positionBeforeLastNewlineLineStartOffset, unsigned closeBraceOffsetFromEnd)
: startColumn(startColumn)
, endColumn(endColumn)
, functionKeywordStart(functionKeywordStart)
, functionNameStart(functionNameStart)
, parametersStart(parametersStart)
, isInStrictContext(isInStrictContext)
, isArrowFunctionBodyExpression(isArrowFunctionBodyExpression)
, isAsyncFunction(isAsyncFunction)
, asyncOffset(asyncOffset)
, parameterCount(parameterCount)
, lineCount(lineCount)
, offsetOfLastNewline(offsetOfLastNewline)
, positionBeforeLastNewlineLineStartOffset(positionBeforeLastNewlineLineStartOffset)
, closeBraceOffsetFromEnd(closeBraceOffsetFromEnd)
{
}

unsigned startColumn;
unsigned endColumn;

int functionKeywordStart;
int functionNameStart;
unsigned parametersStart;
bool isInStrictContext;
bool isArrowFunctionBodyExpression;
bool isAsyncFunction;

unsigned asyncOffset;
unsigned parameterCount;

unsigned lineCount;
unsigned offsetOfLastNewline;
unsigned positionBeforeLastNewlineLineStartOffset;
unsigned closeBraceOffsetFromEnd;
};

} // namespace JSC