Bug 155164

Created attachment 273280 [details] Patch

Created attachment 273281 [details] Patch

Comment on attachment 273281 [details] Patch r=me

Comment on attachment 273281 [details] Patch Clearing flags on attachment: 273281 Committed r197994: <http://trac.webkit.org/changeset/197994>

All reviewed patches have been landed. Closing bug.

(In reply to comment #4) > Comment on attachment 273281 [details] > Patch > > Clearing flags on attachment: 273281 > > Committed r197994: <http://trac.webkit.org/changeset/197994> It made 2 JSC stress tests crash (at least) on ARM 32-bit and 64-bit Linux bots. See https://bugs.webkit.org/show_bug.cgi?id=155355 for details.

Let's roll out.

Re-opened since this is blocked by bug 155368

I'll look into that later, this is not a critical patch, just nice to have.

Reopening to attach new patch.

Created attachment 273786 [details] Patch

That was trivial: branchAdd with immediate was missing blinding support.

Conf#1 Conf#2 3d-cube 5.0156+-0.1633 4.8687+-0.1295 might be 1.0302x faster 3d-morph 5.0690+-0.0673 4.9932+-0.0474 might be 1.0152x faster 3d-raytrace 5.4584+-0.3212 5.2790+-0.1017 might be 1.0340x faster access-binary-trees 2.0642+-0.0457 ? 2.1047+-0.0746 ? might be 1.0196x slower access-fannkuch 5.9108+-0.1256 5.9075+-0.0320 access-nbody 2.5262+-0.0326 ? 2.5738+-0.0687 ? might be 1.0188x slower access-nsieve 3.0632+-0.1334 2.9922+-0.0496 might be 1.0237x faster bitops-3bit-bits-in-byte 1.1284+-0.0188 ? 1.1367+-0.0268 ? bitops-bits-in-byte 2.9306+-0.1908 2.8443+-0.0557 might be 1.0303x faster bitops-bitwise-and 2.0470+-0.0190 ? 2.0562+-0.0361 ? bitops-nsieve-bits 3.1448+-0.0657 3.1123+-0.0526 might be 1.0104x faster controlflow-recursive 2.4241+-0.1033 2.3214+-0.0180 might be 1.0442x faster crypto-aes 4.1147+-0.0601 4.0947+-0.0811 crypto-md5 2.4777+-0.0579 ? 2.4813+-0.0644 ? crypto-sha1 2.3146+-0.0534 2.2861+-0.0220 might be 1.0124x faster date-format-tofte 6.5533+-0.1548 ? 6.8088+-0.2330 ? might be 1.0390x slower date-format-xparb 4.6342+-0.1291 4.5318+-0.0513 might be 1.0226x faster math-cordic 2.8395+-0.0559 ? 2.8521+-0.0414 ? math-partial-sums 4.8627+-0.0715 ? 4.8931+-0.1341 ? math-spectral-norm 1.9981+-0.0322 ? 2.0022+-0.0301 ? regexp-dna 6.4825+-0.1434 ? 6.5287+-0.1690 ? string-base64 4.8413+-0.1875 4.8092+-0.1618 string-fasta 5.7983+-0.0716 5.7192+-0.0556 might be 1.0138x faster string-tagcloud 8.1424+-0.1675 8.1003+-0.1894 string-unpack-code 19.3036+-0.6727 18.8612+-0.2572 might be 1.0235x faster string-validate-input 4.4333+-0.1465 4.2969+-0.0581 might be 1.0317x faster <arithmetic> 4.5992+-0.0331 4.5560+-0.0154 might be 1.0095x faster

Comment on attachment 273786 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=273786&action=review > Source/JavaScriptCore/assembler/MacroAssembler.h:1710 > + // If we don't have a scratch register available for use, we'll just > + // place a random number of nops. > + uint32_t nopCount = random() & 3; > + while (nopCount--) > + nop(); Is this really adequate as a replacement for constant blinding? Seems like this gives an attacker a 1 in 3 chance of getting what he wants. > Source/JavaScriptCore/dfg/DFGOSRExit.h:68 > SpeculationRecovery(SpeculationRecoveryType type, GPRReg dest, GPRReg src) > - : m_type(type) > + : m_src(src) > , m_dest(dest) > - , m_src(src) > + , m_type(type) > + { > + } Add an "ASSERT(type != SpeculativeAddImmediate)" here? > Source/JavaScriptCore/dfg/DFGOSRExit.h:75 > + SpeculationRecovery(SpeculationRecoveryType type, GPRReg dest, int32_t immediate) > + : m_immediate(immediate) > + , m_dest(dest) > + , m_type(type) > { > } Either add an "ASSERT(type == SpeculativeAddImmediate)" here, or just remove the option to pass in a type, and hardwire "m_type(SpeculativeAddImmediate)".

Comment on attachment 273786 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=273786&action=review r=me. Please consider suggestions for SpeculationRecovery before landing. >> Source/JavaScriptCore/assembler/MacroAssembler.h:1710 >> + nop(); > > Is this really adequate as a replacement for constant blinding? Seems like this gives an attacker a 1 in 3 chance of getting what he wants. Nevermind. Seems like this is already accepted standard practice in the code base.

Created attachment 276851 [details] Patch for landing

Comment on attachment 276851 [details] Patch for landing Clearing flags on attachment: 276851 Committed r199792: <http://trac.webkit.org/changeset/199792>

All reviewed patches have been landed. Closing bug.