From 82a2a67f07d9a63a1d16bd779b001eb55a5bc694 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Thu, 16 Mar 2023 13:38:26 +0100 Subject: [PATCH 1/3] No Issue - Ensure more OsiSpace ensureOsiSpace(); was very much under-used. This adds more instances where it makes sense to ensure Osi space before placing masm calls. --- js/src/jit/CodeGenerator.cpp | 6 ++++++ js/src/jit/shared/CodeGenerator-shared.cpp | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index 3f1b7251a3..78d39bfc52 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -3757,6 +3757,7 @@ CodeGenerator::visitCallNative(LCallNative* call) if (jitInfo && jitInfo->type() == JSJitInfo::IgnoresReturnValueNative) native = jitInfo->ignoresReturnValueMethod; } + ensureOsiSpace(); masm.callWithABI(JS_FUNC_TO_DATA_PTR(void*, native)); emitTracelogStopEvent(TraceLogger_Call); @@ -3881,6 +3882,7 @@ CodeGenerator::visitCallDOMNative(LCallDOMNative* call) masm.passABIArg(argObj); masm.passABIArg(argPrivate); masm.passABIArg(argArgs); + ensureOsiSpace(); masm.callWithABI(JS_FUNC_TO_DATA_PTR(void*, target->jitInfo()->method)); if (target->jitInfo()->isInfallible) { @@ -4006,6 +4008,7 @@ CodeGenerator::visitCallGeneric(LCallGeneric* call) // Finally call the function in objreg. masm.bind(&makeCall); + ensureOsiSpace(); uint32_t callOffset = masm.callJit(objreg); markSafepointAt(callOffset, call); @@ -4104,6 +4107,7 @@ CodeGenerator::visitCallKnown(LCallKnown* call) masm.Push(Imm32(descriptor)); // Finally call the function in objreg. + ensureOsiSpace(); uint32_t callOffset = masm.callJit(objreg); markSafepointAt(callOffset, call); @@ -4436,6 +4440,7 @@ CodeGenerator::emitApplyGeneric(T* apply) masm.bind(&rejoin); // Finally call the function in objreg, as assigned by one of the paths above. + ensureOsiSpace(); uint32_t callOffset = masm.callJit(objreg); markSafepointAt(callOffset, apply); @@ -11244,6 +11249,7 @@ CodeGenerator::visitGetDOMProperty(LGetDOMProperty* ins) masm.passABIArg(ObjectReg); masm.passABIArg(PrivateReg); masm.passABIArg(ValueReg); + ensureOsiSpace(); masm.callWithABI(JS_FUNC_TO_DATA_PTR(void*, ins->mir()->fun())); if (ins->mir()->isInfallible()) { diff --git a/js/src/jit/shared/CodeGenerator-shared.cpp b/js/src/jit/shared/CodeGenerator-shared.cpp index 08e9d311cf..16e082745c 100644 --- a/js/src/jit/shared/CodeGenerator-shared.cpp +++ b/js/src/jit/shared/CodeGenerator-shared.cpp @@ -1112,7 +1112,6 @@ CodeGeneratorShared::ensureOsiSpace() } MOZ_ASSERT_IF(!masm.oom(), masm.currentOffset() - lastOsiPointOffset_ >= Assembler::PatchWrite_NearCallSize()); - lastOsiPointOffset_ = masm.currentOffset(); } uint32_t @@ -1124,6 +1123,7 @@ CodeGeneratorShared::markOsiPoint(LOsiPoint* ins) uint32_t offset = masm.currentOffset(); SnapshotOffset so = ins->snapshot()->snapshotOffset(); masm.propagateOOM(osiIndices_.append(OsiIndex(offset, so))); + lastOsiPointOffset_ = offset; return offset; } From fd5015faeb5334e15c10d5466b3453eb8e87f1eb Mon Sep 17 00:00:00 2001 From: Moonchild Date: Thu, 16 Mar 2023 14:05:45 +0100 Subject: [PATCH 2/3] [GFX] Add some sanity checks and clamps to SurfaceData calculations. --- gfx/2d/DataSurfaceHelpers.cpp | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/gfx/2d/DataSurfaceHelpers.cpp b/gfx/2d/DataSurfaceHelpers.cpp index 99dfe063a3..c9a4945a25 100644 --- a/gfx/2d/DataSurfaceHelpers.cpp +++ b/gfx/2d/DataSurfaceHelpers.cpp @@ -84,8 +84,9 @@ DataAtOffset(DataSourceSurface* aSurface, MOZ_ASSERT(Factory::CheckSurfaceSize(aSurface->GetSize()), "surface size overflows - this should have been prevented when the surface was created"); - uint8_t* data = aMap->mData + aPoint.y * aMap->mStride + - aPoint.x * BytesPerPixel(aSurface->GetFormat()); + uint8_t* data = aMap->mData + + size_t(aPoint.y) * size_t(aMap->mStride) + + size_t(aPoint.x) * size_t(BytesPerPixel(aSurface->GetFormat())); if (data < aMap->mData) { MOZ_CRASH("GFX: out-of-range data access"); @@ -124,22 +125,29 @@ void CopySurfaceDataToPackedArray(uint8_t* aSrc, uint8_t* aDst, IntSize aSrcSize, int32_t aSrcStride, int32_t aBytesPerPixel) { - MOZ_ASSERT(aBytesPerPixel > 0, - "Negative stride for aDst not currently supported"); - MOZ_ASSERT(BufferSizeFromStrideAndHeight(aSrcStride, aSrcSize.height) > 0, - "How did we end up with a surface with such a big buffer?"); + CheckedInt packedStride(aBytesPerPixel); + packedStride *= aSrcSize.width; + if (!packedStride.isValid()) { + MOZ_ASSERT(false, "Invalid stride"); + return; + } - int packedStride = aSrcSize.width * aBytesPerPixel; + CheckedInt totalSize(aSrcStride); + totalSize *= aSrcSize.height; + if (!totalSize.isValid()) { + MOZ_ASSERT(false, "Invalid surface size"); + return; + } - if (aSrcStride == packedStride) { + if (size_t(aSrcStride) == packedStride.value()) { // aSrc is already packed, so we can copy with a single memcpy. - memcpy(aDst, aSrc, packedStride * aSrcSize.height); + memcpy(aDst, aSrc, totalSize.value()); } else { // memcpy one row at a time. for (int row = 0; row < aSrcSize.height; ++row) { - memcpy(aDst, aSrc, packedStride); + memcpy(aDst, aSrc, packedStride.value()); aSrc += aSrcStride; - aDst += packedStride; + aDst += packedStride.value(); } } } From 05d29a53dbbad230e854bc4b1f571f5683cc1d34 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Fri, 17 Mar 2023 18:26:07 +0100 Subject: [PATCH 3/3] Issue #2117 - Follow-up: Run precomplete cmd during package staging We rely on this for all OSes except Mac where precomplete should not be in the root (it uses Mozilla's fallback packaging that places it in $RESPATH) --- toolkit/mozapps/installer/packager-uxp.mk | 1 - toolkit/mozapps/installer/packager.mk | 4 ++++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/toolkit/mozapps/installer/packager-uxp.mk b/toolkit/mozapps/installer/packager-uxp.mk index 856ca434c3..8d7cb4e62d 100644 --- a/toolkit/mozapps/installer/packager-uxp.mk +++ b/toolkit/mozapps/installer/packager-uxp.mk @@ -15,7 +15,6 @@ ifeq (,$(filter SunOS Linux WINNT,$(OS_ARCH))) else $(MAKE) stage-package make-buildinfo-file @echo 'Compressing...' - cd $(DIST)/$(MOZ_PKG_DIR); $(CREATE_PRECOMPLETE_CMD) ifeq (WINNT,$(OS_ARCH)) cd $(DIST); $(CYGWIN_WRAPPER) 7z a -t7z -m0=lzma2 -mx=9 -aoa -bb3 $(PKG_BASENAME).7z $(MOZ_PKG_DIR) else diff --git a/toolkit/mozapps/installer/packager.mk b/toolkit/mozapps/installer/packager.mk index 41a07cbfd5..394c480ffe 100644 --- a/toolkit/mozapps/installer/packager.mk +++ b/toolkit/mozapps/installer/packager.mk @@ -55,6 +55,10 @@ stage-package: $(MOZ_PKG_MANIFEST) $(MOZ_PKG_MANIFEST_DEPS) $(MOZ_PKG_MANIFEST) '$(DIST)' '$(DIST)'/$(STAGEPATH)$(MOZ_PKG_DIR)$(if $(MOZ_PKG_MANIFEST),,$(_BINPATH)) \ $(if $(filter omni,$(MOZ_PACKAGER_FORMAT)),$(if $(NON_OMNIJAR_FILES),--non-resource $(NON_OMNIJAR_FILES))) $(PYTHON) $(MOZILLA_DIR)/toolkit/mozapps/installer/find-dupes.py $(DIST)/$(STAGEPATH)$(MOZ_PKG_DIR) +ifneq (Darwin, $(OS_ARCH)) + # We don't want to do this on Mac as it will end up in the package root which is not how Mac packages things + cd $(DIST)/$(MOZ_PKG_DIR) && $(CREATE_PRECOMPLETE_CMD) +endif ifdef MOZ_PACKAGE_JSSHELL # Package JavaScript Shell @echo 'Packaging JavaScript Shell...'