Sitelet https://github.com/emscripten-core/emscripten-fastcomp/pull/231
Skip to content
This repository was archived by the owner on Nov 1, 2021. It is now read-only.

Add a workaround for Safari 9 ARM iOS right shift by non-immediate zero JIT bug - #231

Merged
juj merged 1 commit into
emscripten-core:incomingfrom
juj:workaround_ios_9_right_shift_zero_bug
Oct 3, 2018
Merged

juj merged 1 commit into
emscripten-core:incomingfrom
juj:workaround_ios_9_right_shift_zero_bug

Conversation

@juj

@juj juj commented Sep 25, 2018 •

Copy link
Copy Markdown
Collaborator

I do not unfortunately have a small test case, since it somehow interacts with JIT, and simple emscripten_request_animation_frame() based attempts to build a repro that'd get fed to a JIT after some warm-up runs ended up empty.

The original issue occurs on stb_image codebase in function

https://github.com/nothings/stb/blob/e6afb9cbae4064da8c3e69af3ff5c4629579c1d2/deprecated/stb_image.c#L1999-L2005

on the right shift on the last return line, when loading up a png image; where it is observed that v==16222142, a non-zero non-negative i32 number, and bits==16, yielding a shift right by 0, but the expression would compute 16222142 >> 0 == 0 and return 0.

Verified this patch to fix the above stb_image loading code. The odd thing is that the patch was supposed to have landed on Feb 15th 2016, but iOS 9.3.5 was released only much later on August 25th, but the observed symptoms fit the description of the issue in https://bugs.webkit.org/show_bug.cgi?id=151514 perfectly, so very probable that was the cause.

@kripken

kripken commented Sep 28, 2018

Copy link
Copy Markdown
Member

Odd bug...

I'm not sure this is completely enough, though, as we may emit a >> b in other places. It may be better to add a JS optimizer pass for this - should be about the same amount of work. (I'm probably more familiar with that code so I could do it if you want.) May be slightly simpler that way as then it's all in a single repo.

@kripken

kripken commented Sep 29, 2018

Copy link
Copy Markdown
Member

On the other hand, that would be slower to run, and this does seem fairly simple. lgtm.

@juj

juj commented Oct 3, 2018

Copy link
Copy Markdown
Collaborator Author

I did go through all shifts I could find in JSBackend, and looked for non-immediate shifts that could be zero, this was the only place I found. If you know of places off the top of your head, let me know. Apart from that, iPhone 4s with iOS 9.3.5 is so old target that may not be worth spending time to look too deep into this, unless someone runs into a real problem that needs debugging further.

@juj
juj merged commit 069fd96 into emscripten-core:incoming Oct 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants