Bug 1633741 - Use CheckedInt for js::ImmutableScriptData. r=jandem
authorTed Campbell <tcampbell@mozilla.com>
Wed, 29 Apr 2020 12:55:38 +0000
changeset 526653 666f5f689c8a57d710a9bfdb0e73c2fc79ac5018
parent 526652 41aa6a9b67e10a505e5b0083e0de4e19447ca02b
child 526654 04b81c9f1986009ea1edced726036adc001c4b6b
push id37361
push usermalexandru@mozilla.com
push dateWed, 29 Apr 2020 21:55:39 +0000
treeherdermozilla-central@bc0932b38478 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1633741
milestone77.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 1633741 - Use CheckedInt for js::ImmutableScriptData. r=jandem Cleanup the allocation of trailing arrays to use CheckedInt and follow the example of JitScript. Remove some static_asserts that made arrays harder to change and instead rely on the existing MOZ_ASSERT in initElements. Differential Revision: https://phabricator.services.mozilla.com/D72942
js/src/util/TrailingArray.h
js/src/vm/JSScript.cpp
js/src/vm/SharedStencil.h
--- a/js/src/util/TrailingArray.h
+++ b/js/src/util/TrailingArray.h
@@ -22,16 +22,20 @@ class TrailingArray {
   // Offsets are measured in bytes relative to 'this'.
   using Offset = uint32_t;
 
   // Test if offset is correctly aligned for type.
   template <typename T>
   static constexpr bool isAlignedOffset(Offset offset) {
     return offset % alignof(T) == 0;
   }
+  template <size_t N>
+  static constexpr bool isAlignedOffset(Offset offset) {
+    return offset % N == 0;
+  }
 
   // Translate an offset into a concrete pointer.
   template <typename T>
   T* offsetToPointer(Offset offset) {
     uintptr_t base = reinterpret_cast<uintptr_t>(this);
     return reinterpret_cast<T*>(base + offset);
   }
   template <typename T>
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -5,16 +5,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /*
  * JS script operations.
  */
 
 #include "vm/JSScript-inl.h"
 
+#include "mozilla/CheckedInt.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/PodOperations.h"
 #include "mozilla/ScopeExit.h"
 #include "mozilla/Span.h"  // mozilla::{Span,MakeSpan}
 #include "mozilla/Sprintf.h"
 #include "mozilla/Unused.h"
@@ -718,158 +719,116 @@ XDRResult js::PrivateScriptData::XDR(XDR
 
   // Verify marker to detect data corruption after decoding GC things. A
   // mismatch here indicates we will almost certainly crash in release.
   MOZ_TRY(xdr->codeMarker(0xF83B989A));
 
   return Ok();
 }
 
-/* static */ size_t ImmutableScriptData::AllocationSize(
-    uint32_t codeLength, uint32_t noteLength, uint32_t numResumeOffsets,
-    uint32_t numScopeNotes, uint32_t numTryNotes) {
-  size_t size = sizeof(ImmutableScriptData);
-
-  size += sizeof(Flags);
-  size += codeLength * sizeof(jsbytecode);
-  size += noteLength * sizeof(SrcNote);
-
-#ifdef DEBUG
-  // The compact arrays need to maintain uint32_t alignment. This should have
-  // been done by padding out source notes.
-  MOZ_ASSERT(size % sizeof(uint32_t) == 0,
-             "Source notes should have been padded already");
-#endif
-
-  unsigned numOptionalArrays = unsigned(numResumeOffsets > 0) +
-                               unsigned(numScopeNotes > 0) +
-                               unsigned(numTryNotes > 0);
-  size += numOptionalArrays * sizeof(Offset);
-
-  size += numResumeOffsets * sizeof(uint32_t);
-  size += numScopeNotes * sizeof(ScopeNote);
-  size += numTryNotes * sizeof(TryNote);
-
-  return size;
-}
-
 // Initialize the optional arrays in the trailing allocation. This is a set of
 // offsets that delimit each optional array followed by the arrays themselves.
 // See comment before 'ImmutableScriptData' for more details.
-void ImmutableScriptData::initOptionalArrays(size_t* pcursor,
-                                             ImmutableScriptData::Flags* flags,
+void ImmutableScriptData::initOptionalArrays(Offset* pcursor,
                                              uint32_t numResumeOffsets,
                                              uint32_t numScopeNotes,
                                              uint32_t numTryNotes) {
-  size_t cursor = (*pcursor);
+  Offset cursor = (*pcursor);
 
   // The byte arrays must have already been padded.
-  MOZ_ASSERT(cursor % sizeof(uint32_t) == 0);
+  MOZ_ASSERT(isAlignedOffset<CodeNoteAlign>(cursor),
+             "Bytecode and source notes should be padded to keep alignment");
 
   // Each non-empty optional array needs will need an offset to its end.
   unsigned numOptionalArrays = unsigned(numResumeOffsets > 0) +
                                unsigned(numScopeNotes > 0) +
                                unsigned(numTryNotes > 0);
 
   // Default-initialize the optional-offsets.
-  static_assert(alignof(ImmutableScriptData) >= alignof(Offset),
-                "Incompatible alignment");
   initElements<Offset>(cursor, numOptionalArrays);
   cursor += numOptionalArrays * sizeof(Offset);
 
   // Offset between optional-offsets table and the optional arrays. This is
   // later used to access the optional-offsets table as well as first optional
   // array.
   optArrayOffset_ = cursor;
 
   // Each optional array that follows must store an end-offset in the offset
   // table. Assign table entries by using this 'offsetIndex'. The index 0 is
   // reserved for implicit value 'optArrayOffset'.
   int offsetIndex = 0;
 
   // Default-initialize optional 'resumeOffsets'.
   MOZ_ASSERT(resumeOffsetsOffset() == cursor);
   if (numResumeOffsets > 0) {
-    static_assert(sizeof(Offset) >= alignof(uint32_t),
-                  "Incompatible alignment");
     initElements<uint32_t>(cursor, numResumeOffsets);
     cursor += numResumeOffsets * sizeof(uint32_t);
     setOptionalOffset(++offsetIndex, cursor);
   }
-  flags->resumeOffsetsEndIndex = offsetIndex;
+  flagsRef().resumeOffsetsEndIndex = offsetIndex;
 
   // Default-initialize optional 'scopeNotes'.
   MOZ_ASSERT(scopeNotesOffset() == cursor);
   if (numScopeNotes > 0) {
-    static_assert(sizeof(uint32_t) >= alignof(ScopeNote),
-                  "Incompatible alignment");
     initElements<ScopeNote>(cursor, numScopeNotes);
     cursor += numScopeNotes * sizeof(ScopeNote);
     setOptionalOffset(++offsetIndex, cursor);
   }
-  flags->scopeNotesEndIndex = offsetIndex;
+  flagsRef().scopeNotesEndIndex = offsetIndex;
 
   // Default-initialize optional 'tryNotes'
   MOZ_ASSERT(tryNotesOffset() == cursor);
   if (numTryNotes > 0) {
-    static_assert(sizeof(ScopeNote) >= alignof(TryNote),
-                  "Incompatible alignment");
     initElements<TryNote>(cursor, numTryNotes);
     cursor += numTryNotes * sizeof(TryNote);
     setOptionalOffset(++offsetIndex, cursor);
   }
-  flags->tryNotesEndIndex = offsetIndex;
+  flagsRef().tryNotesEndIndex = offsetIndex;
 
   MOZ_ASSERT(endOffset() == cursor);
   (*pcursor) = cursor;
 }
 
 ImmutableScriptData::ImmutableScriptData(uint32_t codeLength,
                                          uint32_t noteLength,
                                          uint32_t numResumeOffsets,
                                          uint32_t numScopeNotes,
                                          uint32_t numTryNotes)
     : codeLength_(codeLength) {
   // Variable-length data begins immediately after ImmutableScriptData itself.
-  size_t cursor = sizeof(*this);
+  Offset cursor = sizeof(ImmutableScriptData);
 
   // The following arrays are byte-aligned with additional padding to ensure
   // that together they maintain uint32_t-alignment.
   {
-    MOZ_ASSERT(cursor % CodeNoteAlign == 0);
+    MOZ_ASSERT(isAlignedOffset<CodeNoteAlign>(cursor));
 
     // Zero-initialize 'flags'
-    static_assert(CodeNoteAlign >= alignof(Flags), "Incompatible alignment");
+    MOZ_ASSERT(isAlignedOffset<Flags>(cursor));
     new (offsetToPointer<void>(cursor)) Flags{};
     cursor += sizeof(Flags);
 
-    static_assert(alignof(Flags) >= alignof(jsbytecode),
-                  "Incompatible alignment");
     initElements<jsbytecode>(cursor, codeLength);
     cursor += codeLength * sizeof(jsbytecode);
 
-    static_assert(alignof(jsbytecode) >= alignof(SrcNote),
-                  "Incompatible alignment");
     initElements<SrcNote>(cursor, noteLength);
     cursor += noteLength * sizeof(SrcNote);
 
-    MOZ_ASSERT(cursor % CodeNoteAlign == 0);
+    MOZ_ASSERT(isAlignedOffset<CodeNoteAlign>(cursor));
   }
 
   // Initialization for remaining arrays.
-  initOptionalArrays(&cursor, &flagsRef(), numResumeOffsets, numScopeNotes,
-                     numTryNotes);
+  initOptionalArrays(&cursor, numResumeOffsets, numScopeNotes, numTryNotes);
 
   // Check that we correctly recompute the expected values.
   MOZ_ASSERT(this->codeLength() == codeLength);
   MOZ_ASSERT(this->noteLength() == noteLength);
 
   // Sanity check
-  MOZ_ASSERT(AllocationSize(codeLength, noteLength, numResumeOffsets,
-                            numScopeNotes, numTryNotes) == cursor);
+  MOZ_ASSERT(endOffset() == cursor);
 }
 
 template <XDRMode mode>
 static XDRResult XDRImmutableScriptData(XDRState<mode>* xdr,
                                         UniquePtr<ImmutableScriptData>& isd) {
   uint32_t codeLength = 0;
   uint32_t noteLength = 0;
   uint32_t numResumeOffsets = 0;
@@ -3955,31 +3914,53 @@ bool ScriptSource::setSourceMapURL(JSCon
  * uint32_t         resumeOffsets()
  * ScopeNote        scopeNotes()
  * TryNote          tryNotes()
  */
 
 js::UniquePtr<ImmutableScriptData> js::ImmutableScriptData::new_(
     JSContext* cx, uint32_t codeLength, uint32_t noteLength,
     uint32_t numResumeOffsets, uint32_t numScopeNotes, uint32_t numTryNotes) {
-  // Compute size including trailing arrays
-  size_t size = AllocationSize(codeLength, noteLength, numResumeOffsets,
-                               numScopeNotes, numTryNotes);
-
-  // Allocate contiguous raw buffer
-  void* raw = cx->pod_malloc<uint8_t>(size);
+  // Take a count of which optional arrays will be used and need offset info.
+  unsigned numOptionalArrays = unsigned(numResumeOffsets > 0) +
+                               unsigned(numScopeNotes > 0) +
+                               unsigned(numTryNotes > 0);
+
+  // Compute size including trailing arrays.
+  CheckedInt<Offset> size = sizeof(ImmutableScriptData);
+  size += sizeof(Flags);
+  size += CheckedInt<Offset>(codeLength) * sizeof(jsbytecode);
+  size += CheckedInt<Offset>(noteLength) * sizeof(SrcNote);
+  size += CheckedInt<Offset>(numOptionalArrays) * sizeof(Offset);
+  size += CheckedInt<Offset>(numResumeOffsets) * sizeof(uint32_t);
+  size += CheckedInt<Offset>(numScopeNotes) * sizeof(ScopeNote);
+  size += CheckedInt<Offset>(numTryNotes) * sizeof(TryNote);
+  if (!size.isValid()) {
+    ReportAllocationOverflow(cx);
+    return nullptr;
+  }
+
+  // Allocate contiguous raw buffer.
+  void* raw = cx->pod_malloc<uint8_t>(size.value());
   MOZ_ASSERT(uintptr_t(raw) % alignof(ImmutableScriptData) == 0);
   if (!raw) {
     return nullptr;
   }
 
   // Constuct the ImmutableScriptData. Trailing arrays are uninitialized but
   // GCPtrs are put into a safe state.
   UniquePtr<ImmutableScriptData> result(new (raw) ImmutableScriptData(
       codeLength, noteLength, numResumeOffsets, numScopeNotes, numTryNotes));
+  if (!result) {
+    return nullptr;
+  }
+
+  // Sanity check
+  MOZ_ASSERT(result->endOffset() == size.value());
+
   return result;
 }
 
 RuntimeScriptData* js::RuntimeScriptData::new_(JSContext* cx, uint32_t natoms) {
   // Compute size including trailing arrays
   size_t size = AllocationSize(natoms);
 
   // Allocate contiguous raw buffer
@@ -4113,18 +4094,16 @@ PrivateScriptData::PrivateScriptData(uin
     : ngcthings(ngcthings) {
   // Variable-length data begins immediately after PrivateScriptData itself.
   // NOTE: Alignment is computed using cursor/offset so the alignment of
   // PrivateScriptData must be stricter than any trailing array type.
   size_t cursor = sizeof(*this);
 
   // Layout and initialize the gcthings array.
   {
-    static_assert(alignof(PrivateScriptData) >= alignof(JS::GCCellPtr),
-                  "Incompatible alignment");
     initElements<JS::GCCellPtr>(cursor, ngcthings);
 
     cursor += ngcthings * sizeof(JS::GCCellPtr);
   }
 
   // Sanity check
   MOZ_ASSERT(AllocationSize(ngcthings) == cursor);
 }
--- a/js/src/vm/SharedStencil.h
+++ b/js/src/vm/SharedStencil.h
@@ -292,24 +292,18 @@ class alignas(uint32_t) ImmutableScriptD
   }
   Offset tryNotesOffset() const {
     return getOptionalOffset(flags().scopeNotesEndIndex);
   }
   Offset endOffset() const {
     return getOptionalOffset(flags().tryNotesEndIndex);
   }
 
-  // Size to allocate
-  static size_t AllocationSize(uint32_t codeLength, uint32_t noteLength,
-                               uint32_t numResumeOffsets,
-                               uint32_t numScopeNotes, uint32_t numTryNotes);
-
-  void initOptionalArrays(size_t* cursor, Flags* flags,
-                          uint32_t numResumeOffsets, uint32_t numScopeNotes,
-                          uint32_t numTryNotes);
+  void initOptionalArrays(Offset* cursor, uint32_t numResumeOffsets,
+                          uint32_t numScopeNotes, uint32_t numTryNotes);
 
   // Initialize to GC-safe state
   ImmutableScriptData(uint32_t codeLength, uint32_t noteLength,
                       uint32_t numResumeOffsets, uint32_t numScopeNotes,
                       uint32_t numTryNotes);
 
   void setOptionalOffset(int index, Offset offset) {
     MOZ_ASSERT(index > 0);