Patch forthcoming.
Created attachment 332233 [details] maybe the patch
Created attachment 332319 [details] the patch
Comment on attachment 332319 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=332319&action=review r=me w/ some naming nits and other tidbits > Source/JavaScriptCore/runtime/DirectArguments.h:122 > + return *preciseIndexMaskPtr(offset.offset(), std::max(m_length, m_minCapacity), dynamicPoison(type(), DirectArgumentsType, ptr)); could this max() be spectre trained in a way that hurts us? > Source/WTF/wtf/MathExtras.h:492 > +inline unsigned preciseIndexMaskShiftForSize(unsigned size) > +{ > + return size * 8 - 1; > +} > + > +template<typename T> > +unsigned preciseIndexMaskShift() > +{ > + return preciseIndexMaskShiftForSize(sizeof(T)); > +} constexpr for both of these? > Source/WTF/wtf/MathExtras.h:497 > + asm("" : "+r"(pointer)); asm volatile? > Source/WTF/wtf/MathExtras.h:516 > + (static_cast<uintptr_t>(opaque(actual) ^ expected) << 40)); Is it worth giving this 40 a name since it's hard coded inside JSC in a few places too?
(In reply to Saam Barati from comment #3) > Comment on attachment 332319 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=332319&action=review > > r=me w/ some naming nits and other tidbits > > > Source/JavaScriptCore/runtime/DirectArguments.h:122 > > + return *preciseIndexMaskPtr(offset.offset(), std::max(m_length, m_minCapacity), dynamicPoison(type(), DirectArgumentsType, ptr)); > > could this max() be spectre trained in a way that hurts us? No. At worst, you'll get a smaller limit. > > > Source/WTF/wtf/MathExtras.h:492 > > +inline unsigned preciseIndexMaskShiftForSize(unsigned size) > > +{ > > + return size * 8 - 1; > > +} > > + > > +template<typename T> > > +unsigned preciseIndexMaskShift() > > +{ > > + return preciseIndexMaskShiftForSize(sizeof(T)); > > +} > > constexpr for both of these? I'll try that. > > > Source/WTF/wtf/MathExtras.h:497 > > + asm("" : "+r"(pointer)); > > asm volatile? I don't need it to be volatile. > > > Source/WTF/wtf/MathExtras.h:516 > > + (static_cast<uintptr_t>(opaque(actual) ^ expected) << 40)); > > Is it worth giving this 40 a name since it's hard coded inside JSC in a few > places too? I'll do that.
Created attachment 332327 [details] patch for landing
<rdar://problem/36887421>
Landed in https://trac.webkit.org/changeset/227643/webkit
Comment on attachment 332327 [details] patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=332327&action=review > Source/JavaScriptCore/jit/AssemblyHelpers.cpp:1049 > + UNUSED_PARAM(actualValue); This is a typo, and as a result, you're breaking the 32-bit builds.
Comment on attachment 332327 [details] patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=332327&action=review >> Source/JavaScriptCore/jit/AssemblyHelpers.cpp:1049 >> + UNUSED_PARAM(actualValue); > > This is a typo, and as a result, you're breaking the 32-bit builds. Fixed 32-bit build in r227644: <https://trac.webkit.org/r227644>.
This also broke the windows build. Looks like its a syntax error and another error: MathExtras.h(497): error C2059: syntax error: ':' https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/7382
(In reply to Matt Lewis from comment #10) > This also broke the windows build. > > Looks like its a syntax error and another error: > > MathExtras.h(497): error C2059: syntax error: ':' > > https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/ > 7382 Fixed in https://trac.webkit.org/changeset/227648/webkit