WebKit Bugzilla
Attachment 373369 Details for
Bug 199425
: REGRESSION(r247041): broke some iOS arm64e tests (Requested by keith_miller on #webkit).
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
ROLLOUT of r247041
bug-199425-20190702162433.patch (text/plain), 16.59 KB, created by
WebKit Commit Bot
on 2019-07-02 16:24:34 PDT
(
hide
)
Description:
ROLLOUT of r247041
Filename:
MIME Type:
Creator:
WebKit Commit Bot
Created:
2019-07-02 16:24:34 PDT
Size:
16.59 KB
patch
obsolete
>Subversion Revision: 247078 >diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog >index 3c57d4e39767c43676ceae9c2b298d5e07e50dde..11304a7d959e8b3cf43998b8df781499f036dd03 100644 >--- a/Source/JavaScriptCore/ChangeLog >+++ b/Source/JavaScriptCore/ChangeLog >@@ -1,3 +1,18 @@ >+2019-07-02 Commit Queue <commit-queue@webkit.org> >+ >+ Unreviewed, rolling out r247041. >+ https://bugs.webkit.org/show_bug.cgi?id=199425 >+ >+ broke some iOS arm64e tests (Requested by keith_miller on >+ #webkit). >+ >+ Reverted changeset: >+ >+ "PACCage should first cage leaving PAC bits intact then >+ authenticate" >+ https://bugs.webkit.org/show_bug.cgi?id=199372 >+ https://trac.webkit.org/changeset/247041 >+ > 2019-07-02 Keith Miller <keith_miller@apple.com> > > Frozen Arrays length assignment should throw in strict mode >diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog >index 984cbef606f8433f08a330bf3bca1352b36c6548..3a0b1fc91fbd3a979a3c6cb4f3904f394bfc9ba2 100644 >--- a/Source/WTF/ChangeLog >+++ b/Source/WTF/ChangeLog >@@ -1,3 +1,18 @@ >+2019-07-02 Commit Queue <commit-queue@webkit.org> >+ >+ Unreviewed, rolling out r247041. >+ https://bugs.webkit.org/show_bug.cgi?id=199425 >+ >+ broke some iOS arm64e tests (Requested by keith_miller on >+ #webkit). >+ >+ Reverted changeset: >+ >+ "PACCage should first cage leaving PAC bits intact then >+ authenticate" >+ https://bugs.webkit.org/show_bug.cgi?id=199372 >+ https://trac.webkit.org/changeset/247041 >+ > 2019-07-02 Chris Dumez <cdumez@apple.com> > > ThreadSafeRefCounted<DestructionThread::Main> is not safe to use in the UIProcess >diff --git a/Source/bmalloc/ChangeLog b/Source/bmalloc/ChangeLog >index 97b57cb31dcfec9fa5a6fa949061d8ab1d39cdb2..5731f60cca5353188d18a4fa96f27fe3d86dd322 100644 >--- a/Source/bmalloc/ChangeLog >+++ b/Source/bmalloc/ChangeLog >@@ -1,3 +1,18 @@ >+2019-07-02 Commit Queue <commit-queue@webkit.org> >+ >+ Unreviewed, rolling out r247041. >+ https://bugs.webkit.org/show_bug.cgi?id=199425 >+ >+ broke some iOS arm64e tests (Requested by keith_miller on >+ #webkit). >+ >+ Reverted changeset: >+ >+ "PACCage should first cage leaving PAC bits intact then >+ authenticate" >+ https://bugs.webkit.org/show_bug.cgi?id=199372 >+ https://trac.webkit.org/changeset/247041 >+ > 2019-07-02 Keith Miller <keith_miller@apple.com> > > PACCage should first cage leaving PAC bits intact then authenticate >diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h b/Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h >index 9c6e775e517445efcb5cb8c078b79b05a08918ae..fd7eec808551bdf7287afc1b1ec453f2f82082d2 100644 >--- a/Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h >+++ b/Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h >@@ -40,7 +40,6 @@ using Assembler = TARGET_ASSEMBLER; > class MacroAssemblerARM64E : public MacroAssemblerARM64 { > public: > static constexpr unsigned numberOfPACBits = 25; >- static constexpr uintptr_t nonPACBitsMask = (1ull << (64 - numberOfPACBits)) - 1; > > ALWAYS_INLINE void tagReturnAddress() > { >diff --git a/Source/JavaScriptCore/assembler/testmasm.cpp b/Source/JavaScriptCore/assembler/testmasm.cpp >index 3cbaed8a4751de591ac8d04a579c7afffb13747d..1050b39983e4e65052824c04f0bb1254a313e2c8 100644 >--- a/Source/JavaScriptCore/assembler/testmasm.cpp >+++ b/Source/JavaScriptCore/assembler/testmasm.cpp >@@ -1015,8 +1015,6 @@ void testMoveDoubleConditionally64() > > static void testCagePreservesPACFailureBit() > { >-#if GIGACAGE_ENABLED >- ASSERT(!Gigacage::isDisablingPrimitiveGigacageDisabled()); > auto cage = compile([] (CCallHelpers& jit) { > jit.emitFunctionPrologue(); > jit.cageConditionally(Gigacage::Primitive, GPRInfo::argumentGPR0, GPRInfo::argumentGPR1, GPRInfo::argumentGPR2); >@@ -1027,15 +1025,10 @@ static void testCagePreservesPACFailureBit() > > void* ptr = Gigacage::tryMalloc(Gigacage::Primitive, 1); > void* taggedPtr = tagArrayPtr(ptr, 1); >- ASSERT(hasOneBitSet(Gigacage::size(Gigacage::Primitive) << 2)); >- void* notCagedPtr = reinterpret_cast<void*>(reinterpret_cast<uintptr_t>(ptr) + (Gigacage::size(Gigacage::Primitive) << 2)); >- CHECK_NOT_EQ(Gigacage::caged(Gigacage::Primitive, notCagedPtr), notCagedPtr); >- void* taggedNotCagedPtr = tagArrayPtr(notCagedPtr, 1); >- >+ dataLogLn("starting test"); > if (isARM64E()) { > // FIXME: This won't work if authentication failures trap but I don't know how to test for that right now. > CHECK_NOT_EQ(invoke<void*>(cage, taggedPtr, 2), ptr); >- CHECK_EQ(invoke<void*>(cage, taggedNotCagedPtr, 1), untagArrayPtr(taggedPtr, 2)); > } else > CHECK_EQ(invoke<void*>(cage, taggedPtr, 2), ptr); > >@@ -1049,17 +1042,16 @@ static void testCagePreservesPACFailureBit() > jit.ret(); > }); > >- CHECK_EQ(invoke<void*>(cageWithoutAuthentication, taggedPtr), taggedPtr); > if (isARM64E()) { > // FIXME: This won't work if authentication failures trap but I don't know how to test for that right now. >- CHECK_NOT_EQ(invoke<void*>(cageWithoutAuthentication, taggedNotCagedPtr), taggedNotCagedPtr); >- CHECK_NOT_EQ(untagArrayPtr(invoke<void*>(cageWithoutAuthentication, taggedNotCagedPtr), 1), notCagedPtr); >- CHECK_NOT_EQ(invoke<void*>(cageWithoutAuthentication, taggedNotCagedPtr), taggedPtr); >- CHECK_NOT_EQ(untagArrayPtr(invoke<void*>(cageWithoutAuthentication, taggedNotCagedPtr), 1), ptr); >- } >+ CHECK_NOT_EQ(invoke<void*>(cageWithoutAuthentication, untagArrayPtr(taggedPtr, 2)), ptr); >+ } else >+ CHECK_EQ(invoke<void*>(cageWithoutAuthentication, untagArrayPtr(taggedPtr, 2)), ptr); >+ >+ CHECK_EQ(untagArrayPtr(taggedPtr, 1), ptr); >+ CHECK_EQ(invoke<void*>(cageWithoutAuthentication, untagArrayPtr(taggedPtr, 1)), ptr); > > Gigacage::free(Gigacage::Primitive, ptr); >-#endif > } > > #define RUN(test) do { \ >diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp >index 1af74148c95200fdadfd0d674c6261fbef4b8d93..55a2897e0052912ae64949b9f633727a4c384138 100644 >--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp >+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp >@@ -14177,7 +14177,18 @@ private: > > LValue caged(Gigacage::Kind kind, LValue ptr, LValue base) > { >+#if CPU(ARM64E) >+ if (kind == Gigacage::Primitive) { >+ LValue size = m_out.load32(base, m_heaps.JSArrayBufferView_length); >+ ptr = untagArrayPtr(ptr, size); >+ } >+#else >+ UNUSED_PARAM(kind); >+ UNUSED_PARAM(base); >+#endif >+ > #if GIGACAGE_ENABLED >+ UNUSED_PARAM(base); > if (!Gigacage::isEnabled(kind)) > return ptr; > >@@ -14195,7 +14206,7 @@ private: > LValue result = m_out.add(masked, basePtr); > > #if CPU(ARM64E) >- if (kind == Gigacage::Primitive) { >+ { > PatchpointValue* merge = m_out.patchpoint(pointerType()); > merge->append(result, B3::ValueRep(B3::ValueRep::SomeLateRegister)); > merge->appendSomeRegister(ptr); >@@ -14203,12 +14214,9 @@ private: > jit.move(params[2].gpr(), params[0].gpr()); > jit.bitFieldInsert64(params[1].gpr(), 0, 64 - MacroAssembler::numberOfPACBits, params[0].gpr()); > }); >- >- LValue size = m_out.load32(base, m_heaps.JSArrayBufferView_length); >- result = untagArrayPtr(merge, size); >+ result = merge; > } >-#endif // CPU(ARM64E) >- >+#endif > // Make sure that B3 doesn't try to do smart reassociation of these pointer bits. > // FIXME: In an ideal world, B3 would not do harmful reassociations, and if it did, it would be able > // to undo them during constant hoisting and regalloc. As it stands, if you remove this then Octane >@@ -14222,9 +14230,6 @@ private: > // https://bugs.webkit.org/show_bug.cgi?id=175493 > return m_out.opaque(result); > #endif >- >- UNUSED_PARAM(kind); >- UNUSED_PARAM(base); > return ptr; > } > >diff --git a/Source/JavaScriptCore/jit/AssemblyHelpers.h b/Source/JavaScriptCore/jit/AssemblyHelpers.h >index 3eead2d8e06ef5c689234857cb2a0c0d9e5eafd6..a77d43a501086730d7dc9985f42db5219a2f9259 100644 >--- a/Source/JavaScriptCore/jit/AssemblyHelpers.h >+++ b/Source/JavaScriptCore/jit/AssemblyHelpers.h >@@ -1586,41 +1586,42 @@ public: > // length may be the same register as scratch. > void cageConditionally(Gigacage::Kind kind, GPRReg storage, GPRReg length, GPRReg scratch) > { >-#if GIGACAGE_ENABLED >- if (Gigacage::isEnabled(kind)) { >- if (kind != Gigacage::Primitive || Gigacage::isDisablingPrimitiveGigacageDisabled()) >- cageWithoutUntagging(kind, storage); >- else { >-#if CPU(ARM64E) >- if (length == scratch) >- scratch = getCachedMemoryTempRegisterIDAndInvalidate(); >-#endif >- loadPtr(&Gigacage::basePtr(kind), scratch); >- Jump done = branchTest64(Zero, scratch); >-#if CPU(ARM64E) >- GPRReg tempReg = getCachedDataTempRegisterIDAndInvalidate(); >- move(storage, tempReg); >- ASSERT(LogicalImmediate::create64(Gigacage::mask(kind)).isValid()); >- andPtr(TrustedImmPtr(Gigacage::mask(kind)), tempReg); >- addPtr(scratch, tempReg); >- bitFieldInsert64(tempReg, 0, 64 - numberOfPACBits, storage); >-#else >- andPtr(TrustedImmPtr(Gigacage::mask(kind)), storage); >- addPtr(scratch, storage); >-#endif // CPU(ARM64E) >- done.link(this); >- } >- } >-#endif >- > #if CPU(ARM64E) > if (kind == Gigacage::Primitive) > untagArrayPtr(length, storage); >-#endif >+#else > UNUSED_PARAM(kind); > UNUSED_PARAM(storage); > UNUSED_PARAM(length); >+#endif >+ >+#if GIGACAGE_ENABLED >+ if (!Gigacage::isEnabled(kind)) >+ return; >+ >+ if (kind != Gigacage::Primitive || Gigacage::isDisablingPrimitiveGigacageDisabled()) >+ cageWithoutUntagging(kind, storage); >+ else { >+ loadPtr(&Gigacage::basePtr(kind), scratch); >+ Jump done = branchTestPtr(Zero, scratch); >+#if CPU(ARM64E) >+ auto tempReg = getCachedMemoryTempRegisterIDAndInvalidate(); >+ move(storage, tempReg); >+ andPtr(TrustedImmPtr(Gigacage::mask(kind)), tempReg); >+ addPtr(scratch, tempReg); >+ bitFieldInsert64(tempReg, 0, 64 - numberOfPACBits, storage); >+#else >+ andPtr(TrustedImmPtr(Gigacage::mask(kind)), storage); >+ addPtr(scratch, storage); >+#endif >+ done.link(this); >+ >+ >+ } >+#else > UNUSED_PARAM(scratch); >+#endif >+ > } > > void emitComputeButterflyIndexingMask(GPRReg vectorLengthGPR, GPRReg scratchGPR, GPRReg resultGPR) >diff --git a/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm b/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm >index 6ba972063e60b68d3dbcfa49f8d9c9d95fcc3f5b..903635af525a8b1f447060314307d1083d3635ad 100644 >--- a/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm >+++ b/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm >@@ -422,36 +422,37 @@ macro checkSwitchToJITForLoop() > end) > end > >-macro cage(basePtr, mask, ptr, scratch) >+macro uncage(basePtr, mask, ptr, scratchOrLength) > if GIGACAGE_ENABLED and not (C_LOOP or C_LOOP_WIN) >- loadp basePtr, scratch >- btpz scratch, .done >+ loadp basePtr, scratchOrLength >+ btpz scratchOrLength, .done > andp mask, ptr >- addp scratch, ptr >+ addp scratchOrLength, ptr > .done: > end > end > >-macro cagedPrimitive(ptr, length, scratch, scratch2) >+macro loadCagedPrimitive(source, dest, scratchOrLength) >+ loadp source, dest > if ARM64E >- const source = scratch2 >- move ptr, scratch2 >+ const result = t7 >+ untagArrayPtr scratchOrLength, dest >+ move dest, result > else >- const source = ptr >+ const result = dest > end > if GIGACAGE_ENABLED >- cage(_g_gigacageBasePtrs + Gigacage::BasePtrs::primitive, constexpr Gigacage::primitiveGigacageMask, source, scratch) >+ uncage(_g_gigacageBasePtrs + Gigacage::BasePtrs::primitive, constexpr Gigacage::primitiveGigacageMask, result, scratchOrLength) > if ARM64E > const numberOfPACBits = constexpr MacroAssembler::numberOfPACBits >- bfiq scratch2, 0, 64 - numberOfPACBits, ptr >- untagArrayPtr length, ptr >+ bfiq result, 0, 64 - numberOfPACBits, dest > end > end > end > > macro loadCagedJSValue(source, dest, scratchOrLength) > loadp source, dest >- cage(_g_gigacageBasePtrs + Gigacage::BasePtrs::jsValue, constexpr Gigacage::jsValueGigacageMask, dest, scratchOrLength) >+ uncage(_g_gigacageBasePtrs + Gigacage::BasePtrs::jsValue, constexpr Gigacage::jsValueGigacageMask, dest, scratchOrLength) > end > > macro loadVariable(get, fieldName, valueReg) >@@ -1521,17 +1522,15 @@ llintOpWithMetadata(op_get_by_val, OpGetByVal, macro (size, get, dispatch, metad > # Sweet, now we know that we have a typed array. Do some basic things now. > > if ARM64E >- const length = t6 >- const scratch = t7 >- loadi JSArrayBufferView::m_length[t0], length >- biaeq t1, length, .opGetByValSlow >+ const scratchOrLength = t6 >+ loadi JSArrayBufferView::m_length[t0], scratchOrLength >+ biaeq t1, scratchOrLength, .opGetByValSlow > else >- # length and scratch are intentionally undefined on this branch because they are not used on other platforms. >+ const scratchOrLength = t0 > biaeq t1, JSArrayBufferView::m_length[t0], .opGetByValSlow > end > >- loadp JSArrayBufferView::m_vector[t0], t3 >- cagedPrimitive(t3, length, t0, scratch) >+ loadCagedPrimitive(JSArrayBufferView::m_vector[t0], t3, scratchOrLength) > > # Now bisect through the various types: > # Int8ArrayType, >diff --git a/Source/WTF/wtf/CagedPtr.h b/Source/WTF/wtf/CagedPtr.h >index b4383018b68a8f106ad5fc7f57db9690a563e259..ca473c4baaa9955bde8556ccd5eb1b9e103caadc 100644 >--- a/Source/WTF/wtf/CagedPtr.h >+++ b/Source/WTF/wtf/CagedPtr.h >@@ -49,21 +49,20 @@ public: > : m_ptr(shouldTag ? tagArrayPtr(ptr, size) : ptr) > { } > >+ > T* get(unsigned size) const > { > ASSERT(m_ptr); > T* ptr = PtrTraits::unwrap(m_ptr); >- T* cagedPtr = Gigacage::caged(kind, ptr); >- T* untaggedPtr = shouldTag ? untagArrayPtr(mergePointers(ptr, cagedPtr), size) : cagedPtr; >- return untaggedPtr; >+ T* untaggedPtr = shouldTag ? untagArrayPtr(ptr, size) : ptr; >+ return mergePointers(untaggedPtr, Gigacage::caged(kind, ptr)); > } > > T* getMayBeNull(unsigned size) const > { > T* ptr = PtrTraits::unwrap(m_ptr); >- T* cagedPtr = Gigacage::cagedMayBeNull(kind, ptr); >- T* untaggedPtr = shouldTag ? untagArrayPtr(mergePointers(ptr, cagedPtr), size) : cagedPtr; >- return untaggedPtr; >+ T* untaggedPtr = shouldTag ? untagArrayPtr(ptr, size) : ptr; >+ return mergePointers(untaggedPtr, Gigacage::cagedMayBeNull(kind, ptr)); > } > > T* getUnsafe() const >@@ -125,16 +124,11 @@ public: > } > > protected: >- static inline T* mergePointers(T* sourcePtr, T* cagedPtr) >+ static inline T* mergePointers(const T* untaggedPtr, const T* uncagedPtr) > { >-#if CPU(ARM64E) > constexpr unsigned numberOfPACBits = 25; > constexpr uintptr_t mask = (1ull << ((sizeof(T*) * CHAR_BIT) - numberOfPACBits)) - 1; >- return reinterpret_cast<T*>((reinterpret_cast<uintptr_t>(sourcePtr) & ~mask) | (reinterpret_cast<uintptr_t>(cagedPtr) & mask)); >-#else >- UNUSED_PARAM(sourcePtr); >- return cagedPtr; >-#endif >+ return reinterpret_cast<T*>((reinterpret_cast<uintptr_t>(untaggedPtr) & ~mask) | (reinterpret_cast<uintptr_t>(uncagedPtr) & mask)); > } > > typename PtrTraits::StorageType m_ptr; >diff --git a/Source/bmalloc/bmalloc/ProcessCheck.mm b/Source/bmalloc/bmalloc/ProcessCheck.mm >index b872c59f4fe44cc0b376d40aa3f9a77e131e6ca1..3bb63f3525429898b0994cc851793b5195f52b02 100644 >--- a/Source/bmalloc/bmalloc/ProcessCheck.mm >+++ b/Source/bmalloc/bmalloc/ProcessCheck.mm >@@ -66,9 +66,7 @@ bool shouldProcessUnconditionallyUseBmalloc() > result = contains(@"com.apple.WebKit") || contains(@"safari"); > } else { > NSString *processName = [[NSProcessInfo processInfo] processName]; >- result = [processName isEqualToString:@"jsc"] >- || [processName isEqualToString:@"wasm"] >- || [processName hasPrefix:@"test"]; >+ result = [processName isEqualToString:@"jsc"] || [processName isEqualToString:@"wasm"]; > } > }); >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=373369">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 199425
: 373369