Layout test js/regexp-unicode.html is currently crashing for WinCairo, but only in Release mode. As a more pointed repro case, one can simply execute the line `/\u{1044f}/u.test("\ud801\udc4f")` in JSC. This seems to be another MSVC __forceinline issue.
Created attachment 338594 [details] Patch
Comment on attachment 338594 [details] Patch This feels like it's just covering up another bug. I've seen bugs like this before that are actually from using uninitialized memory.
Comment on attachment 338594 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=338594&action=review > Source/JavaScriptCore/ChangeLog:10 > + MSVC evidently can't handle __forceinline here. Can you please explain why __forceinline would result in a crash in the first place? It doesn't make sense that a change in inlining behavior would resolve a crash short of working around a compiler bug. You'll need to provide proof if that is the case.
(In reply to Alex Christensen from comment #2) > Comment on attachment 338594 [details] > Patch > > This feels like it's just covering up another bug. I've seen bugs like this > before that are actually from using uninitialized memory. That is indeed the type of crash here, but it claims to be occurring at: https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/runtime/RegExpInlines.h#L279 ...which seems like it's probably not accurate due to optimization. Unfortunately, there's no repro with RelWithDebInfo, so I'm a bit confused how to proceed...
(In reply to Mark Lam from comment #3) > Comment on attachment 338594 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=338594&action=review > > > Source/JavaScriptCore/ChangeLog:10 > > + MSVC evidently can't handle __forceinline here. > > Can you please explain why __forceinline would result in a crash in the > first place? It doesn't make sense that a change in inlining behavior would > resolve a crash short of working around a compiler bug. You'll need to > provide proof if that is the case. I agree. In particular, it's perplexing that AppleWin doesn't seem to have such an issue (though I imagine this is almost certainly a 32-bit/64-bit difference).
It appears that during the call to YarrJIT::execute here, %rdi gets overwritten (to 0xDC00): https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/runtime/RegExpInlines.h#L269 > call rax > movups xmm1,xmmword ptr [rax] Now, when we reach the return here, which destination register gets chosen evidently depends on __forceinline: https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/runtime/RegExpInlines.h#L279 Specifically, with `inline` we have... > movups xmmword ptr [rbx],xmm1 ...but with `__forceinline` we have... > movups xmmword ptr [rdi],xmm1 As established, %rdi has already been changed to 0xDC00, so the latter crashes with a write violation.
Created attachment 338762 [details] Patch
Comment on attachment 338762 [details] Patch r=me
Comment on attachment 338762 [details] Patch Clearing flags on attachment: 338762 Committed r231009: <https://trac.webkit.org/changeset/231009>
All reviewed patches have been landed. Closing bug.
<rdar://problem/39730640>