Skip to content
Permalink
Browse files
WebAssembly: Wasm::IndexOrName has a raw pointer to Name
https://bugs.webkit.org/show_bug.cgi?id=176644

Reviewed by Michael Saboff.

IndexOrName now keeps a RefPtr to its original NameSection, which
holds the Name (or references nullptr if Index). Holding onto the
entire section seems like the better thing to do, since backtraces
probably contain multiple names from the same Module.

* JavaScriptCore.xcodeproj/project.pbxproj:
* interpreter/Interpreter.cpp:
(JSC::GetStackTraceFunctor::operator() const):
* interpreter/StackVisitor.h: Frame is no longer POD because of the
RefPtr.
* runtime/StackFrame.cpp:
(JSC::StackFrame::StackFrame):
* runtime/StackFrame.h: Drop the union, size is now 40 bytes.
(JSC::StackFrame::StackFrame): Deleted. Initialized in class instead.
(JSC::StackFrame::wasm): Deleted. Make it a ctor instead.
* wasm/WasmBBQPlanInlines.h:
(JSC::Wasm::BBQPlan::initializeCallees):
* wasm/WasmCallee.cpp:
(JSC::Wasm::Callee::Callee):
* wasm/WasmCallee.h:
(JSC::Wasm::Callee::create):
* wasm/WasmFormat.h: Move NameSection to its own header.
(JSC::Wasm::isValidNameType):
(JSC::Wasm::NameSection::get): Deleted.
* wasm/WasmIndexOrName.cpp:
(JSC::Wasm::IndexOrName::IndexOrName):
(JSC::Wasm::makeString):
* wasm/WasmIndexOrName.h:
(JSC::Wasm::IndexOrName::IndexOrName):
(JSC::Wasm::IndexOrName::isEmpty const):
(JSC::Wasm::IndexOrName::isIndex const):
* wasm/WasmModuleInformation.cpp:
(JSC::Wasm::ModuleInformation::ModuleInformation):
* wasm/WasmModuleInformation.h:
(JSC::Wasm::ModuleInformation::ModuleInformation): Deleted.
* wasm/WasmNameSection.h:
(JSC::Wasm::NameSection::get):
(JSC::Wasm::NameSection::create): Deleted.
* wasm/WasmNameSectionParser.cpp:
(JSC::Wasm::NameSectionParser::parse):
* wasm/WasmNameSectionParser.h:
* wasm/WasmOMGPlan.cpp:
(JSC::Wasm::OMGPlan::work):


Canonical link: https://commits.webkit.org/195223@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@224272 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
jfbastien committed Nov 1, 2017
1 parent 9991515 commit e58bf0fd78f8f290b66fa52005ef9f608f8fae5d
Showing 18 changed files with 128 additions and 77 deletions.
@@ -1,3 +1,54 @@
2017-10-31 JF Bastien <jfbastien@apple.com>

WebAssembly: Wasm::IndexOrName has a raw pointer to Name
https://bugs.webkit.org/show_bug.cgi?id=176644

Reviewed by Michael Saboff.

IndexOrName now keeps a RefPtr to its original NameSection, which
holds the Name (or references nullptr if Index). Holding onto the
entire section seems like the better thing to do, since backtraces
probably contain multiple names from the same Module.

* JavaScriptCore.xcodeproj/project.pbxproj:
* interpreter/Interpreter.cpp:
(JSC::GetStackTraceFunctor::operator() const):
* interpreter/StackVisitor.h: Frame is no longer POD because of the
RefPtr.
* runtime/StackFrame.cpp:
(JSC::StackFrame::StackFrame):
* runtime/StackFrame.h: Drop the union, size is now 40 bytes.
(JSC::StackFrame::StackFrame): Deleted. Initialized in class instead.
(JSC::StackFrame::wasm): Deleted. Make it a ctor instead.
* wasm/WasmBBQPlanInlines.h:
(JSC::Wasm::BBQPlan::initializeCallees):
* wasm/WasmCallee.cpp:
(JSC::Wasm::Callee::Callee):
* wasm/WasmCallee.h:
(JSC::Wasm::Callee::create):
* wasm/WasmFormat.h: Move NameSection to its own header.
(JSC::Wasm::isValidNameType):
(JSC::Wasm::NameSection::get): Deleted.
* wasm/WasmIndexOrName.cpp:
(JSC::Wasm::IndexOrName::IndexOrName):
(JSC::Wasm::makeString):
* wasm/WasmIndexOrName.h:
(JSC::Wasm::IndexOrName::IndexOrName):
(JSC::Wasm::IndexOrName::isEmpty const):
(JSC::Wasm::IndexOrName::isIndex const):
* wasm/WasmModuleInformation.cpp:
(JSC::Wasm::ModuleInformation::ModuleInformation):
* wasm/WasmModuleInformation.h:
(JSC::Wasm::ModuleInformation::ModuleInformation): Deleted.
* wasm/WasmNameSection.h:
(JSC::Wasm::NameSection::get):
(JSC::Wasm::NameSection::create): Deleted.
* wasm/WasmNameSectionParser.cpp:
(JSC::Wasm::NameSectionParser::parse):
* wasm/WasmNameSectionParser.h:
* wasm/WasmOMGPlan.cpp:
(JSC::Wasm::OMGPlan::work):

2017-10-31 Tim Horton <timothy_horton@apple.com>

Clean up some drag and drop feature flags
@@ -1488,6 +1488,7 @@
AD5C36EC1F75AD7C000BCAAF /* WasmToJS.h in Headers */ = {isa = PBXBuildFile; fileRef = ADD09AEE1F5F623F001313C2 /* WasmToJS.h */; settings = {ATTRIBUTES = (Private, ); }; };
AD5C36EF1F7A263A000BCAAF /* WasmMemoryMode.h in Headers */ = {isa = PBXBuildFile; fileRef = AD5C36EE1F7A2629000BCAAF /* WasmMemoryMode.h */; settings = {ATTRIBUTES = (Private, ); }; };
AD7438C01E0457A400FD0C2A /* WasmSignature.h in Headers */ = {isa = PBXBuildFile; fileRef = AD7438BF1E04579200FD0C2A /* WasmSignature.h */; settings = {ATTRIBUTES = (Private, ); }; };
AD7B4B2E1FA3E29800C9DF79 /* WasmNameSection.h in Headers */ = {isa = PBXBuildFile; fileRef = AD7B4B2D1FA3E28600C9DF79 /* WasmNameSection.h */; settings = {ATTRIBUTES = (Private, ); }; };
AD86A93E1AA4D88D002FE77F /* WeakGCMapInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = AD86A93D1AA4D87C002FE77F /* WeakGCMapInlines.h */; settings = {ATTRIBUTES = (Private, ); }; };
AD8FF3981EB5BDB20087FF82 /* WasmIndexOrName.h in Headers */ = {isa = PBXBuildFile; fileRef = AD8FF3951EB5BD850087FF82 /* WasmIndexOrName.h */; settings = {ATTRIBUTES = (Private, ); }; };
AD9E852F1E8A0C7C008DE39E /* JSWebAssemblyCodeBlock.h in Headers */ = {isa = PBXBuildFile; fileRef = AD9E852E1E8A0C6E008DE39E /* JSWebAssemblyCodeBlock.h */; settings = {ATTRIBUTES = (Private, ); }; };
@@ -4238,6 +4239,7 @@
AD5C36F01F7A26BF000BCAAF /* WasmMemoryMode.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = WasmMemoryMode.cpp; sourceTree = "<group>"; };
AD7438BE1E04579200FD0C2A /* WasmSignature.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WasmSignature.cpp; sourceTree = "<group>"; };
AD7438BF1E04579200FD0C2A /* WasmSignature.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WasmSignature.h; sourceTree = "<group>"; };
AD7B4B2D1FA3E28600C9DF79 /* WasmNameSection.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WasmNameSection.h; sourceTree = "<group>"; };
AD86A93D1AA4D87C002FE77F /* WeakGCMapInlines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WeakGCMapInlines.h; sourceTree = "<group>"; };
AD8DD6CF1F67089F0004EB52 /* JSToWasm.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = JSToWasm.h; path = js/JSToWasm.h; sourceTree = "<group>"; };
AD8DD6D01F6708A30004EB52 /* JSToWasm.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; name = JSToWasm.cpp; path = js/JSToWasm.cpp; sourceTree = "<group>"; };
@@ -6153,6 +6155,7 @@
53F40E961D5A7BEC0099A1B6 /* WasmModuleParser.cpp */,
53F40E941D5A7AEF0099A1B6 /* WasmModuleParser.h */,
AD5B416E1EBAFB65008EFA43 /* WasmName.h */,
AD7B4B2D1FA3E28600C9DF79 /* WasmNameSection.h */,
ADD8FA441EB3077100DF542F /* WasmNameSectionParser.cpp */,
ADD8FA431EB3077100DF542F /* WasmNameSectionParser.h */,
5311BD481EA581E500525281 /* WasmOMGPlan.cpp */,
@@ -8262,6 +8265,7 @@
0F96303C1D4192CD005609D9 /* DestructionMode.h in Headers */,
A77A423E17A0BBFD00A8DB81 /* DFGAbstractHeap.h in Headers */,
A704D90317A0BAA8006BA554 /* DFGAbstractInterpreter.h in Headers */,
AD7B4B2E1FA3E29800C9DF79 /* WasmNameSection.h in Headers */,
A704D90417A0BAA8006BA554 /* DFGAbstractInterpreterInlines.h in Headers */,
0F620177143FCD3F0068B77C /* DFGAbstractValue.h in Headers */,
0FD3E4021B618AAF00C80E1E /* DFGAdaptiveInferredPropertyValueWatchpoint.h in Headers */,
@@ -507,7 +507,7 @@ class GetStackTraceFunctor {

if (m_remainingCapacityForFrameCapture) {
if (visitor->isWasmFrame()) {
m_results.append(StackFrame::wasm(visitor->wasmFunctionIndexOrName()));
m_results.append(StackFrame(visitor->wasmFunctionIndexOrName()));
} else if (!!visitor->codeBlock() && !visitor->codeBlock()->unlinkedCodeBlock()->isBuiltinFunction()) {
m_results.append(
StackFrame(m_vm, m_owner, visitor->callee().asCell(), visitor->codeBlock(), visitor->bytecodeOffset()));
@@ -121,9 +121,9 @@ class StackVisitor {
size_t m_index;
size_t m_argumentCountIncludingThis;
unsigned m_bytecodeOffset;
Wasm::IndexOrName m_wasmFunctionIndexOrName;
bool m_callerIsEntryFrame : 1;
bool m_isWasmFrame : 1;
Wasm::IndexOrName m_wasmFunctionIndexOrName;

friend class StackVisitor;
};
@@ -35,7 +35,6 @@ namespace JSC {

StackFrame::StackFrame(VM& vm, JSCell* owner, JSCell* callee)
: m_callee(vm, owner, callee)
, m_bytecodeOffset(UINT_MAX)
{
}

@@ -46,6 +45,12 @@ StackFrame::StackFrame(VM& vm, JSCell* owner, JSCell* callee, CodeBlock* codeBlo
{
}

StackFrame::StackFrame(Wasm::IndexOrName indexOrName)
: m_wasmFunctionIndexOrName(indexOrName)
, m_isWasmFrame(true)
{
}

intptr_t StackFrame::sourceID() const
{
if (!m_codeBlock)
@@ -37,21 +37,9 @@ class SlotVisitor;

class StackFrame {
public:
StackFrame()
: m_bytecodeOffset(UINT_MAX)
{ }

StackFrame(VM&, JSCell* owner, JSCell* callee);

StackFrame(VM&, JSCell* owner, JSCell* callee, CodeBlock*, unsigned bytecodeOffset);

static StackFrame wasm(Wasm::IndexOrName indexOrName)
{
StackFrame result;
result.m_isWasmFrame = true;
result.m_wasmFunctionIndexOrName = indexOrName;
return result;
}
StackFrame(Wasm::IndexOrName);

bool hasLineAndColumnInfo() const { return !!m_codeBlock; }

@@ -73,10 +61,8 @@ class StackFrame {
private:
WriteBarrier<JSCell> m_callee { };
WriteBarrier<CodeBlock> m_codeBlock { };
union {
unsigned m_bytecodeOffset;
Wasm::IndexOrName m_wasmFunctionIndexOrName;
};
Wasm::IndexOrName m_wasmFunctionIndexOrName;
unsigned m_bytecodeOffset { UINT_MAX };
bool m_isWasmFrame { false };
};

@@ -30,6 +30,7 @@
#include "CalleeBits.h"
#include "WasmBBQPlan.h"
#include "WasmCallee.h"
#include "WasmNameSection.h"

namespace JSC { namespace Wasm {

@@ -47,7 +48,7 @@ void BBQPlan::initializeCallees(const Functor& callback)

InternalFunction* function = m_wasmInternalFunctions[internalFunctionIndex].get();
size_t functionIndexSpace = internalFunctionIndex + m_moduleInformation->importFunctionCount();
Ref<Wasm::Callee> wasmEntrypointCallee = Wasm::Callee::create(WTFMove(function->entrypoint), functionIndexSpace, m_moduleInformation->nameSection.get(functionIndexSpace));
Ref<Wasm::Callee> wasmEntrypointCallee = Wasm::Callee::create(WTFMove(function->entrypoint), functionIndexSpace, m_moduleInformation->nameSection->get(functionIndexSpace));
MacroAssembler::repatchPointer(function->calleeMoveLocation, CalleeBits::boxWasm(wasmEntrypointCallee.ptr()));

callback(internalFunctionIndex, WTFMove(embedderEntrypointCallee), WTFMove(wasmEntrypointCallee));
@@ -38,9 +38,9 @@ Callee::Callee(Entrypoint&& entrypoint)
registerCode(m_entrypoint.compilation->codeRef().executableMemory()->start(), m_entrypoint.compilation->codeRef().executableMemory()->end());
}

Callee::Callee(Entrypoint&& entrypoint, size_t index, const Name* name)
Callee::Callee(Entrypoint&& entrypoint, size_t index, std::pair<const Name*, RefPtr<NameSection>>&& name)
: m_entrypoint(WTFMove(entrypoint))
, m_indexOrName(index, name)
, m_indexOrName(index, WTFMove(name))
{
registerCode(m_entrypoint.compilation->codeRef().executableMemory()->start(), m_entrypoint.compilation->codeRef().executableMemory()->end());
}
@@ -44,9 +44,9 @@ class Callee : public ThreadSafeRefCounted<Callee> {
return adoptRef(*callee);
}

static Ref<Callee> create(Wasm::Entrypoint&& entrypoint, size_t index, const Name* name)
static Ref<Callee> create(Wasm::Entrypoint&& entrypoint, size_t index, std::pair<const Name*, RefPtr<NameSection>>&& name)
{
Callee* callee = new Callee(WTFMove(entrypoint), index, name);
Callee* callee = new Callee(WTFMove(entrypoint), index, WTFMove(name));
return adoptRef(*callee);
}

@@ -57,7 +57,7 @@ class Callee : public ThreadSafeRefCounted<Callee> {

private:
JS_EXPORT_PRIVATE Callee(Wasm::Entrypoint&&);
JS_EXPORT_PRIVATE Callee(Wasm::Entrypoint&&, size_t, const Name*);
JS_EXPORT_PRIVATE Callee(Wasm::Entrypoint&&, size_t, std::pair<const Name*, RefPtr<NameSection>>&&);

Wasm::Entrypoint m_entrypoint;
IndexOrName m_indexOrName;
@@ -34,6 +34,7 @@
#include "RegisterAtOffsetList.h"
#include "WasmMemoryInformation.h"
#include "WasmName.h"
#include "WasmNameSection.h"
#include "WasmOps.h"
#include "WasmPageCount.h"
#include "WasmSignature.h"
@@ -253,15 +254,6 @@ inline bool isValidNameType(Int val)
}
return false;
}

struct NameSection {
Name moduleName;
Vector<Name> functionNames;
const Name* get(size_t functionIndexSpace)
{
return functionIndexSpace < functionNames.size() ? &functionNames[functionIndexSpace] : nullptr;
}
};

struct UnlinkedWasmToWasmCall {
CodeLocationNearCall callLocation;
@@ -28,26 +28,28 @@

namespace JSC { namespace Wasm {

IndexOrName::IndexOrName(Index index, const Name* name)
IndexOrName::IndexOrName(Index index, std::pair<const Name*, RefPtr<NameSection>>&& name)
{
static_assert(sizeof(m_index) == sizeof(m_name), "bit-tagging depends on sizes being equal");
static_assert(sizeof(m_index) == sizeof(*this), "bit-tagging depends on object being the size of the union's types");
static_assert(sizeof(m_indexName.index) == sizeof(m_indexName.name), "bit-tagging depends on sizes being equal");

if ((index & allTags) || (bitwise_cast<Index>(name) & allTags))
if ((index & allTags) || (bitwise_cast<Index>(name.first) & allTags))
*this = IndexOrName();
else if (name)
m_name = name;
else
m_index = indexTag | index;
else {
if (name.first)
m_indexName.name = name.first;
else
m_indexName.index = indexTag | index;
m_nameSection = WTFMove(name.second);
}
}

String makeString(const IndexOrName& ion)
{
if (ion.isEmpty())
return String();
if (ion.isIndex())
return String::number(ion.m_index & ~IndexOrName::indexTag);
return String(ion.m_name->data(), ion.m_name->size());
return String::number(ion.m_indexName.index & ~IndexOrName::indexTag);
return String(ion.m_indexName.name->data(), ion.m_indexName.name->size());
};

} } // namespace JSC::Wasm
@@ -26,29 +26,32 @@
#pragma once

#include "WasmName.h"
#include "WasmNameSection.h"
#include <wtf/RefPtr.h>
#include <wtf/StdLibExtras.h>
#include <wtf/text/WTFString.h>

namespace JSC { namespace Wasm {

struct NameSection;

struct IndexOrName {
typedef size_t Index;

IndexOrName()
: m_index(emptyTag)
{ }
IndexOrName(Index, const Name*);
bool isEmpty() const { return bitwise_cast<Index>(*this) & emptyTag; }
bool isIndex() const { return bitwise_cast<Index>(*this) & indexTag; }
IndexOrName() { m_indexName.index = emptyTag; }
IndexOrName(Index, std::pair<const Name*, RefPtr<NameSection>>&&);
bool isEmpty() const { return bitwise_cast<Index>(m_indexName) & emptyTag; }
bool isIndex() const { return bitwise_cast<Index>(m_indexName) & indexTag; }
bool isName() const { return !(isEmpty() || isName()); }

friend String makeString(const IndexOrName&);

private:
union {
Index m_index;
const Name* m_name;
};
Index index;
const Name* name;
} m_indexName;
RefPtr<NameSection> m_nameSection;

// Use the top bits as tags. Neither pointers nor the function index space should use them.
static constexpr Index indexTag = 1ull << (CHAR_BIT * sizeof(Index) - 1);
@@ -28,8 +28,15 @@

#if ENABLE(WEBASSEMBLY)

#include "WasmNameSection.h"

namespace JSC { namespace Wasm {

ModuleInformation::ModuleInformation(Vector<uint8_t>&& sourceBytes)
: source(WTFMove(sourceBytes))
, nameSection(new NameSection())
{
}
ModuleInformation::~ModuleInformation() { }

} } // namespace JSC::Wasm
@@ -32,6 +32,14 @@
namespace JSC { namespace Wasm {

struct ModuleInformation : public ThreadSafeRefCounted<ModuleInformation> {
ModuleInformation() = delete;
ModuleInformation(const ModuleInformation&) = delete;
ModuleInformation(ModuleInformation&&) = delete;

ModuleInformation(Vector<uint8_t>&& sourceBytes);

JS_EXPORT_PRIVATE ~ModuleInformation();

size_t functionIndexSpaceSize() const { return importFunctionSignatureIndices.size() + internalFunctionSignatureIndices.size(); }
bool isImportedFunctionFromFunctionIndexSpace(size_t functionIndex) const
{
@@ -48,13 +56,6 @@ struct ModuleInformation : public ThreadSafeRefCounted<ModuleInformation> {
uint32_t importFunctionCount() const { return importFunctionSignatureIndices.size(); }
uint32_t internalFunctionCount() const { return internalFunctionSignatureIndices.size(); }

ModuleInformation(Vector<uint8_t>&& sourceBytes)
: source(WTFMove(sourceBytes))
{
}

JS_EXPORT_PRIVATE ~ModuleInformation();

const Vector<uint8_t> source;

Vector<Import> imports;
@@ -74,7 +75,7 @@ struct ModuleInformation : public ThreadSafeRefCounted<ModuleInformation> {
Vector<Global> globals;
unsigned firstInternalGlobal { 0 };
Vector<CustomSection> customSections;
NameSection nameSection;
RefPtr<NameSection> nameSection;
};


@@ -28,21 +28,17 @@
#include "WasmName.h"
#include <wtf/ThreadSafeRefCounted.h>
#include <wtf/Vector.h>
#include <utility>

namespace JSC { namespace Wasm {

struct NameSection : ThreadSafeRefCounted<NameSection> {
static Ref<NameSection> create()
struct NameSection : public ThreadSafeRefCounted<NameSection> {
std::pair<const Name*, RefPtr<NameSection>> get(size_t functionIndexSpace)
{
return adoptRef(*new NameSection());
return functionIndexSpace < functionNames.size() ? std::make_pair(&functionNames[functionIndexSpace], RefPtr<NameSection>(this)) : std::pair<const Name*, RefPtr<NameSection>>(nullptr, nullptr);
}

Name moduleName;
Vector<Name> functionNames;
const Name* get(size_t functionIndexSpace)
{
return functionIndexSpace < functionNames.size() ? &functionNames[functionIndexSpace] : nullptr;
}
};

} } // namespace JSC::Wasm

0 comments on commit e58bf0f

Please sign in to comment.