My patch at https://bugs.webkit.org/show_bug.cgi?id=202832 was rolled out, because it was using or32 on a now-16bits field. I am currently fixing it, which includes implementing or16 in the 32 and 64-bits x86 and ARM MacroAssembler backends. I cannot fix the MIPS backend as I have no way to test it and don't even know which processors it is supposed to work on; so once I am done and lands the patch again it will probably break the MIPS port. The solution is simply to implement or16(TrustedImm32, AbsoluteAddress) in MacroAssemblerMIPS.h. There is already a 32-bit version, so it should not be too difficult.
Created attachment 383417 [details] WIP - Patch This seems to be the proper implementation. I'm running tests to validate it.
Created attachment 383476 [details] WIP - Patch Diff from current https://bugs.webkit.org/show_bug.cgi?id=202832
Comment on attachment 383476 [details] WIP - Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383476&action=review > Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:-491 > - m_assembler.lw(immTempRegister, addrTempRegister, adr & 0xffff); This patch appears to be altering the implementation of or32, instead of adding or16?
Comment on attachment 383476 [details] WIP - Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383476&action=review >> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:-491 >> - m_assembler.lw(immTempRegister, addrTempRegister, adr & 0xffff); > > This patch appears to be altering the implementation of or32, instead of adding or16? Actually, this is a diff from your current patch that already landed. I'm uploading a Patch to this soon.
Created attachment 383533 [details] Patch
Comment on attachment 383533 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383533&action=review > JSTests/stress/regress-57020338.js:6 > + for (let j = 0; j < 10000; j++) { I’m not sure this is a valid is an appropriate change to make. Robin, is 10000 sufficient to repro the problem in x86_64? If not,, then the timeout issue on slow ports should be handled in a different way.
Created attachment 383552 [details] Patch
(In reply to Mark Lam from comment #6) > Comment on attachment 383533 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=383533&action=review > > > JSTests/stress/regress-57020338.js:6 > > + for (let j = 0; j < 10000; j++) { > > I’m not sure this is a valid is an appropriate change to make. Robin, is > 10000 sufficient to repro the problem in x86_64? If not,, then the timeout > issue on slow ports should be handled in a different way. To reproduce the issue, I changed `ArithProfile<BitfieldType>::emitUnconditionalSet` to use `or32` instead of `or16`. I was able to see crashes in x86_64, but not every time. Now, I changed another place and I was able to reproduce segfault 100/100 tries in my x86_64 macOS build. I also tested this on jsc-only and reproduced the crash.
ping
Comment on attachment 383552 [details] Patch rs=me
For the record, both issues of stress/ensure-code-block-is-not-precise-allocation.js.dfg-eager (MIPS) and stress/toctou-having-a-bad-time-new-array.js (ARMv7) are not related with this Patch. The former is due the broken OSR to LLInt into MIPS and later is a OOM on ToT (see one report here https://build.webkit.org/builders/JSCOnly%20Linux%20ARMv7%20Thumb2%20Release/builds/9979/steps/jscore-test/logs/stdio)
Comment on attachment 383552 [details] Patch Thank you very much for the review!
Comment on attachment 383552 [details] Patch Clearing flags on attachment: 383552 Committed r252463: <https://trac.webkit.org/changeset/252463>
All reviewed patches have been landed. Closing bug.
<rdar://problem/57200550>