Bug 1364615 - Baldr: use UniquePtr to own code (r=lth)
authorLuke Wagner <luke@mozilla.com>
Mon, 15 May 2017 14:27:10 -0500
changeset 406647 94bc2a2c7274340118d59e845ecc90bcd20ec268
parent 406646 b2a7fe15c578d666346a25b9879ab7fd6fcef9f7
child 406648 b8bbd7e9579a199a02caf025e7becebb67188724
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslth
bugs1364615
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1364615 - Baldr: use UniquePtr to own code (r=lth) MozReview-Commit-ID: 9WJ36B3gR6k
js/src/wasm/WasmCode.cpp
js/src/wasm/WasmCode.h
--- a/js/src/wasm/WasmCode.cpp
+++ b/js/src/wasm/WasmCode.cpp
@@ -58,18 +58,18 @@ static uint32_t
 RoundupCodeLength(uint32_t codeLength)
 {
     // codeLength is a multiple of the system's page size, but not necessarily
     // a multiple of ExecutableCodePageSize.
     MOZ_ASSERT(codeLength % gc::SystemPageSize() == 0);
     return JS_ROUNDUP(codeLength, ExecutableCodePageSize);
 }
 
-static uint8_t*
-AllocateCodeBytes(uint32_t codeLength)
+/* static */ CodeSegment::UniqueCodeBytes
+CodeSegment::AllocateCodeBytes(uint32_t codeLength)
 {
     codeLength = RoundupCodeLength(codeLength);
 
     if (wasmCodeAllocations >= MaxWasmCodeAllocations)
         return nullptr;
 
     void* p = AllocateExecutableMemory(codeLength, ProtectionSetting::Writable);
 
@@ -85,26 +85,28 @@ AllocateCodeBytes(uint32_t codeLength)
 
     if (!p)
         return nullptr;
 
     // We account for the bytes allocated in WasmModuleObject::create, where we
     // have the necessary JSContext.
 
     wasmCodeAllocations++;
-    return (uint8_t*)p;
+    return UniqueCodeBytes((uint8_t*)p, FreeCode(codeLength));
 }
 
-static void
-FreeCodeBytes(uint8_t* bytes, uint32_t codeLength)
+void
+CodeSegment::FreeCode::operator()(uint8_t* bytes)
 {
+    MOZ_ASSERT(codeLength);
+    MOZ_ASSERT(codeLength == RoundupCodeLength(codeLength));
+
     MOZ_ASSERT(wasmCodeAllocations > 0);
     wasmCodeAllocations--;
 
-    codeLength = RoundupCodeLength(codeLength);
 #ifdef MOZ_VTUNE
     vtune::UnmarkBytes(bytes, codeLength);
 #endif
     DeallocateExecutableMemory(bytes, codeLength);
 }
 
 static bool
 StaticallyLink(const CodeSegment& cs, const LinkData& linkData)
@@ -209,120 +211,111 @@ SendCodeRangesToProfiler(const CodeSegme
             vtune::MarkWasm(vtune::GenerateUniqueMethodID(), name.begin(), (void*)start, size);
 #endif
     }
 
     return;
 }
 
 /* static */ UniqueConstCodeSegment
-CodeSegment::create(jit::MacroAssembler& masm,
+CodeSegment::create(MacroAssembler& masm,
                     const ShareableBytes& bytecode,
                     const LinkData& linkData,
                     const Metadata& metadata)
 {
     // Round up the code size to page size since this is eventually required by
     // the executable-code allocator and for setting memory protection.
     uint32_t bytesNeeded = masm.bytesNeeded();
     uint32_t padding = ComputeByteAlignment(bytesNeeded, gc::SystemPageSize());
     uint32_t codeLength = bytesNeeded + padding;
 
     MOZ_ASSERT(linkData.functionCodeLength < codeLength);
 
-    uint8_t* codeBase = AllocateCodeBytes(codeLength);
-    if (!codeBase)
+    UniqueCodeBytes codeBytes = AllocateCodeBytes(codeLength);
+    if (!codeBytes)
         return nullptr;
 
     // We'll flush the icache after static linking, in initialize().
-    masm.executableCopy(codeBase, /* flushICache = */ false);
+    masm.executableCopy(codeBytes.get(), /* flushICache = */ false);
 
     // Zero the padding.
-    memset(codeBase + bytesNeeded, 0, padding);
+    memset(codeBytes.get() + bytesNeeded, 0, padding);
 
-    return create(codeBase, codeLength, bytecode, linkData, metadata);
+    return create(Move(codeBytes), codeLength, bytecode, linkData, metadata);
 }
 
 /* static */ UniqueConstCodeSegment
-CodeSegment::create(const Bytes& unlinkedBytes, const ShareableBytes& bytecode,
-                    const LinkData& linkData, const Metadata& metadata)
+CodeSegment::create(const Bytes& unlinkedBytes,
+                    const ShareableBytes& bytecode,
+                    const LinkData& linkData,
+                    const Metadata& metadata)
 {
     uint32_t codeLength = unlinkedBytes.length();
     MOZ_ASSERT(codeLength % gc::SystemPageSize() == 0);
 
-    uint8_t* codeBytes = AllocateCodeBytes(codeLength);
+    UniqueCodeBytes codeBytes = AllocateCodeBytes(codeLength);
     if (!codeBytes)
         return nullptr;
-    memcpy(codeBytes, unlinkedBytes.begin(), codeLength);
+    memcpy(codeBytes.get(), unlinkedBytes.begin(), codeLength);
 
-    return create(codeBytes, codeLength, bytecode, linkData, metadata);
+    return create(Move(codeBytes), codeLength, bytecode, linkData, metadata);
 }
 
 /* static */ UniqueConstCodeSegment
-CodeSegment::create(uint8_t* codeBase, uint32_t codeLength,
+CodeSegment::create(UniqueCodeBytes codeBytes,
+                    uint32_t codeLength,
                     const ShareableBytes& bytecode,
                     const LinkData& linkData,
                     const Metadata& metadata)
 {
     // These should always exist and should never be first in the code segment.
     MOZ_ASSERT(linkData.interruptOffset != 0);
     MOZ_ASSERT(linkData.outOfBoundsOffset != 0);
     MOZ_ASSERT(linkData.unalignedAccessOffset != 0);
 
     auto cs = js::MakeUnique<CodeSegment>();
-    if (!cs) {
-        FreeCodeBytes(codeBase, codeLength);
+    if (!cs)
         return nullptr;
-    }
 
-    if (!cs->initialize(codeBase, codeLength, bytecode, linkData, metadata))
+    if (!cs->initialize(Move(codeBytes), codeLength, bytecode, linkData, metadata))
         return nullptr;
 
     return UniqueConstCodeSegment(cs.release());
 }
 
 bool
-CodeSegment::initialize(uint8_t* codeBase, uint32_t codeLength,
+CodeSegment::initialize(UniqueCodeBytes codeBytes,
+                        uint32_t codeLength,
                         const ShareableBytes& bytecode,
                         const LinkData& linkData,
                         const Metadata& metadata)
 {
     MOZ_ASSERT(bytes_ == nullptr);
 
-    bytes_ = codeBase;
-    // This CodeSegment instance now owns the code bytes, and the CodeSegment's
-    // destructor will take care of freeing those bytes in the case of error.
+    bytes_ = Move(codeBytes);
     functionLength_ = linkData.functionCodeLength;
     length_ = codeLength;
-    interruptCode_ = codeBase + linkData.interruptOffset;
-    outOfBoundsCode_ = codeBase + linkData.outOfBoundsOffset;
-    unalignedAccessCode_ = codeBase + linkData.unalignedAccessOffset;
+    interruptCode_ = bytes_.get() + linkData.interruptOffset;
+    outOfBoundsCode_ = bytes_.get() + linkData.outOfBoundsOffset;
+    unalignedAccessCode_ = bytes_.get() + linkData.unalignedAccessOffset;
 
     if (!StaticallyLink(*this, linkData))
         return false;
 
-    ExecutableAllocator::cacheFlush(codeBase, RoundupCodeLength(codeLength));
+    ExecutableAllocator::cacheFlush(bytes_.get(), RoundupCodeLength(codeLength));
 
     // Reprotect the whole region to avoid having separate RW and RX mappings.
-    if (!ExecutableAllocator::makeExecutable(codeBase, RoundupCodeLength(codeLength)))
+    if (!ExecutableAllocator::makeExecutable(bytes_.get(), RoundupCodeLength(codeLength)))
         return false;
 
     SendCodeRangesToProfiler(*this, bytecode.bytes, metadata);
 
     return true;
 }
 
-CodeSegment::~CodeSegment()
-{
-    if (!bytes_)
-        return;
-
-    MOZ_ASSERT(length() > 0);
-    FreeCodeBytes(bytes_, length());
-}
-
 UniqueConstBytes
 CodeSegment::unlinkedBytesForDebugging(const LinkData& linkData) const
 {
     UniqueBytes unlinkedBytes = js::MakeUnique<Bytes>();
     if (!unlinkedBytes)
         return nullptr;
     if (!unlinkedBytes->append(base(), length()))
         return nullptr;
@@ -343,42 +336,40 @@ CodeSegment::addSizeOfMisc(mozilla::Mall
     *code += RoundupCodeLength(length_);
 }
 
 uint8_t*
 CodeSegment::serialize(uint8_t* cursor, const LinkData& linkData) const
 {
     cursor = WriteScalar<uint32_t>(cursor, length_);
     uint8_t* base = cursor;
-    cursor = WriteBytes(cursor, bytes_, length_);
+    cursor = WriteBytes(cursor, bytes_.get(), length_);
     StaticallyUnlink(base, linkData);
     return cursor;
 }
 
 const uint8_t*
 CodeSegment::deserialize(const uint8_t* cursor, const ShareableBytes& bytecode,
                          const LinkData& linkData, const Metadata& metadata)
 {
     uint32_t length;
     cursor = ReadScalar<uint32_t>(cursor, &length);
     if (!cursor)
         return nullptr;
 
     MOZ_ASSERT(length_ % gc::SystemPageSize() == 0);
-    uint8_t* bytes = AllocateCodeBytes(length);
+    UniqueCodeBytes bytes = AllocateCodeBytes(length);
     if (!bytes)
         return nullptr;
 
-    cursor = ReadBytes(cursor, bytes, length);
-    if (!cursor) {
-        FreeCodeBytes(bytes, length);
+    cursor = ReadBytes(cursor, bytes.get(), length);
+    if (!cursor)
         return nullptr;
-    }
 
-    if (!initialize(bytes, length, bytecode, linkData, metadata))
+    if (!initialize(Move(bytes), length, bytecode, linkData, metadata))
         return nullptr;
 
     return cursor;
 }
 
 size_t
 FuncExport::serializedSize() const
 {
--- a/js/src/wasm/WasmCode.h
+++ b/js/src/wasm/WasmCode.h
@@ -53,66 +53,73 @@ typedef RefPtr<const ShareableBytes> Sha
 // A wasm CodeSegment owns the allocated executable code for a wasm module.
 
 class CodeSegment;
 typedef UniquePtr<CodeSegment> UniqueCodeSegment;
 typedef UniquePtr<const CodeSegment> UniqueConstCodeSegment;
 
 class CodeSegment
 {
+    // Executable code must be deallocated specially.
+    struct FreeCode {
+        uint32_t codeLength;
+        FreeCode() : codeLength(0) {}
+        explicit FreeCode(uint32_t codeLength) : codeLength(codeLength) {}
+        void operator()(uint8_t* codeBytes);
+    };
+    typedef UniquePtr<uint8_t, FreeCode> UniqueCodeBytes;
+    static UniqueCodeBytes AllocateCodeBytes(uint32_t codeLength);
+
     // bytes_ points to a single allocation of executable machine code in
     // the range [0, length_).  The range [0, functionLength_) is
     // the subrange of [0, length_) which contains function code.
-    uint8_t* bytes_;
-    uint32_t functionLength_;
-    uint32_t length_;
+    UniqueCodeBytes bytes_;
+    uint32_t        functionLength_;
+    uint32_t        length_;
 
     // These are pointers into code for stubs used for asynchronous
     // signal-handler control-flow transfer.
     uint8_t* interruptCode_;
     uint8_t* outOfBoundsCode_;
     uint8_t* unalignedAccessCode_;
 
-    // This assumes ownership of the codeBytes, and deletes them in the event of error.
-    bool initialize(uint8_t* codeBase, uint32_t codeLength, const ShareableBytes& bytecode,
-                    const LinkData& linkData, const Metadata& metadata);
+    bool initialize(UniqueCodeBytes bytes,
+                    uint32_t codeLength,
+                    const ShareableBytes& bytecode,
+                    const LinkData& linkData,
+                    const Metadata& metadata);
 
-    // codeBytes must be executable memory.
-    // This assumes ownership of the codeBytes, and deletes them in the event of error.
-    static UniqueConstCodeSegment create(uint8_t* codeBytes,
+    static UniqueConstCodeSegment create(UniqueCodeBytes bytes,
                                          uint32_t codeLength,
                                          const ShareableBytes& bytecode,
                                          const LinkData& linkData,
                                          const Metadata& metadata);
   public:
     CodeSegment(const CodeSegment&) = delete;
     void operator=(const CodeSegment&) = delete;
 
     CodeSegment()
-      : bytes_(nullptr),
-        functionLength_(0),
+      : functionLength_(0),
         length_(0),
         interruptCode_(nullptr),
         outOfBoundsCode_(nullptr),
         unalignedAccessCode_(nullptr)
     {}
 
     static UniqueConstCodeSegment create(jit::MacroAssembler& masm,
                                          const ShareableBytes& bytecode,
                                          const LinkData& linkData,
                                          const Metadata& metadata);
 
-    static UniqueConstCodeSegment create(const Bytes& codeBytes,
+    static UniqueConstCodeSegment create(const Bytes& unlinkedBytes,
                                          const ShareableBytes& bytecode,
                                          const LinkData& linkData,
                                          const Metadata& metadata);
 
-    ~CodeSegment();
-
-    uint8_t* base() const { return bytes_; }
+    uint8_t* base() const { return bytes_.get(); }
     uint32_t length() const { return length_; }
 
     uint8_t* interruptCode() const { return interruptCode_; }
     uint8_t* outOfBoundsCode() const { return outOfBoundsCode_; }
     uint8_t* unalignedAccessCode() const { return unalignedAccessCode_; }
 
     // The range [0, functionBytes) is a subrange of [0, codeBytes) that
     // contains only function body code, not the stub code. This distinction is
@@ -124,16 +131,18 @@ class CodeSegment
         return pc >= base() && pc < (base() + functionLength_);
     }
     bool containsCodePC(const void* pc) const {
         return pc >= base() && pc < (base() + length_);
     }
 
     UniqueConstBytes unlinkedBytesForDebugging(const LinkData& linkData) const;
 
+    // Structured clone support:
+
     size_t serializedSize() const;
     uint8_t* serialize(uint8_t* cursor, const LinkData& linkData) const;
     const uint8_t* deserialize(const uint8_t* cursor, const ShareableBytes& bytecode,
                                const LinkData& linkData, const Metadata& metadata);
 
     void addSizeOfMisc(mozilla::MallocSizeOf mallocSizeOf, size_t* code, size_t* data) const;
 };