Bug 138038 - [iOS8][ARMv7(s)] Optimized Object.create in 'use strict' context sometimes breaks.
Summary: [iOS8][ARMv7(s)] Optimized Object.create in 'use strict' context sometimes br...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: iPhone / iPad iOS 8.0
: P2 Critical
Assignee: Benjamin Poulain
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-10-23 21:13 PDT by Stefan Penner
Modified: 2015-09-14 06:07 PDT (History)
15 users (show)

See Also:


Attachments
Patch (6.81 KB, patch)
2015-05-27 18:22 PDT, Benjamin Poulain
msaboff: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Penner 2014-10-23 21:13:06 PDT
(iOS8 seems to be missing from the OS dropdown, so I labeled it as other).


It some that in sometimes (appears only when optimized, but not always) the new instance produced by `Object.create` somehow fails to be a proper instance, and setting properties results in a thrown exception.

Example:

```js
var a = Object.create(other);
a.foo = 1; // => throws: TypeError Attempted to assign to readonly property
```

Interestingly, if the exception is caught and `a` inspected: `a.__proto__.foo` was set to `5`, not `a.foo`

My suspicion is that it appears to be JIT related issue, as it manifests only in hot-sots and not specifically tied to any one occurrence of `Object.create`.


affected:
---------

iOS 8.1 (12B411) on iPhone 5.
iPhone 4S iOS 8 
iPhone 4
iPhone 5 Simulator
iPhone 5c iOS 8

not-affected:
-------------

iOS 8.0 (12A365) Simulator
iPhone 6 iOS8
iPhone 5s iOS 8
iPad Retina Mini iOS 8

Demo: 
-----

(unfortunately I was not able to isolated further)

http://emberjs.jsbin.com/vecuz/1/ all iOS 8 non-64bit devices
http://jsbin.com/kegiya/6 (one scenario patched, but another creeps up when the page is added to the home screen)

Workaround:
-----------

* remove 'use strict' from the context using the `Object.create`
* another option, although poor is to set build of the new object as a pojo, and then set __proto__ after the fact (https://github.com/emberjs/ember.js/issues/5629)

related issues:
---------------

http://stackoverflow.com/questions/25174594/typeerror-attempted-to-assign-to-readonly-property-on-ios8-safari
https://github.com/emberjs/ember.js/issues/5606
https://github.com/emberjs/ember.js/issues/5629 (same as above, one early workaround)
https://github.com/mozilla/pdf.js/issues/5341


Similar symptoms but I believe to not be the same:
-----------------------------------------
as our issue is specific to non-64bit architectures and doesn't appear to have anything to do with WebIDL
https://bugs.webkit.org/show_bug.cgi?id=49739
https://github.com/Polymer/platform/issues/66
https://github.com/uhop/dcl/issues/15
https://github.com/angular/angular.js/issues/9128
https://github.com/ibm-js/delite/issues/259
Comment 1 Stefan Penner 2014-10-23 21:19:06 PDT
So basically this doesn't affect all, but it affects a good amount of ember 1.7+ (since we added 'use strict') apps in the wild.

Scouring stack overflow it appears several angular users have the same problem. I suspect any app that has a hot Object.create  in a 'use strict' context could suffer from this.

Typically the result is a hard failure, and a broken app experience.
Comment 2 Stefan Penner 2014-10-23 21:26:53 PDT
Also, for any ember users who stumble upon this.

1.7 -> 1.8beta5 (and those related canary builds) are affected as they include 'use strict' contexts

There is a very strong probability that 1.8 final, will ship with 'use strict' stripped.

But as a stop-gap, I am maintaining 1.7 and canary versions of ember that have 'use strict stripped' and do not appear to have this issue.

1.7:
----

bower users: -> yapplabs/ember-ios8-fix#1.7.0
https://github.com/yapplabs/ember-ios8-fix/tree/1.7.0


canary:
-------

bower users: -> yapplabs/ember-ios8-fix
https://github.com/yapplabs/ember-ios8-fix/tree/master
Comment 3 Geoffrey Garen 2014-10-24 11:21:01 PDT
Do we have a way to reproduce this failure (even if not a reduced way)?
Comment 4 Stefan Penner 2014-10-24 11:41:07 PDT
The JSBIn's included in DEMO should consistently fail on the affected devices.

I can also make myself available for further discussion/questions if needed.
Comment 5 Radar WebKit Bug Importer 2014-10-27 22:38:36 PDT
<rdar://problem/18792571>
Comment 6 mjstaffa 2015-02-16 10:47:15 PST
This is also affecting angularjs applications. Is this being worked on or is there something we can do to speed up the process?
Comment 7 Jack Preston 2015-04-14 04:43:56 PDT
+1 this is affecting our app. Response has been to remove use strict declarations but that's pretty undesirable. Is any work going on for the radar yet?
Comment 8 Jack Preston 2015-04-14 04:47:13 PDT
Also for more information towards the original report, in our codebase instead of "setting properties results in a thrown exception" we were often seeing what I'd describe as "setting properties invisibly does not happen". At one point, code like:

```js
this.data = {};
console.log(this.data);
```

Would output `undefined`. At other points it would crash Safari, and we would get the app reload with the grey notification bar. In almost 100% of cases it would cease any useful communication with Safari desktop dev tools.
Comment 9 Jack Preston 2015-04-14 04:51:04 PDT
Lastly, great job pinning down a reproducible case for this Stefan, this is a seriously slippery bug in my recent experience.
Comment 10 Benjamin Poulain 2015-04-15 14:58:15 PDT
Still broken on ToT :(
Comment 11 Jack Preston 2015-04-16 06:27:41 PDT
Update: we are still seeing the effects of this bug even after totally removing `'use strict';` declarations from our code, so as of the moment there's not a known workaround that is good for everyone.
Comment 12 Stefan Penner 2015-04-16 07:09:34 PDT
@jack remember that JSC will inline code. As far as I can tell, if a non-use strict module that contains Object.create is inlined into a use strict module the problem returns. With ember, we also still saw the issues until all potential Object.create inline sites in the framework where also stripped of `use strict`. This is extremely brittle, as we do not control user-land code. And if the code is slightly refactored we may once again regress.

We cant just strip use strict from our entire program, as we rely on subtle differences. This really needs to get fixed, hopefully with the recent burst of attention it will happen sooner.
Comment 13 Jack Preston 2015-04-16 07:38:34 PDT
Agreed, there's no even semi-good way round this. Good to see an @webkit edit to the ticket in the last few days, even if they didn't comment :(
Comment 14 Stefan Penner 2015-04-16 08:06:55 PDT
I likely haven't done a good job of bringing attention to this issue. So I appreciate the extra eyes and support its getting now. My appologies for not doing more earlier
Comment 15 Aaron Harnly 2015-05-14 10:53:33 PDT
We (Amplify Education) are also seeing this bug on 32-bit iPads, manifesting as an angularjs error when creating a new object.
Comment 16 Benjamin Poulain 2015-05-27 18:22:10 PDT
Created attachment 253823 [details]
Patch
Comment 17 Michael Saboff 2015-05-27 21:33:57 PDT
Comment on attachment 253823 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=253823&action=review

r=me

> Source/JavaScriptCore/ChangeLog:24
> +         and we fail the put_by_id because we try to set a property a on number.

Wording - "... a on number", should this be "on a number"?
Comment 18 Benjamin Poulain 2015-05-28 13:58:03 PDT
Committed r184960: <http://trac.webkit.org/changeset/184960>
Comment 19 Jack Preston 2015-05-29 03:13:39 PDT
Great to see a resolution. Next question is when this will make its way into mobile Safari?
Comment 20 Spencer Williams 2015-07-06 05:17:23 PDT
Great to see a patch/resolution for this. I do echo Jack's question though: is there any word yet on a timeline for fixing iOS Safari? This causes sporadic/nondeterministic issues with Angular.js webapps, and there's no real workaround except to recommend installing Chrome for its JS engine.
Comment 21 David Kilzer (:ddkilzer) 2015-07-06 07:14:35 PDT
This was fixed in iOS 9 Dev Seed 2.
Comment 22 Spencer Williams 2015-07-06 07:18:23 PDT
(In reply to comment #21)
> This was fixed in iOS 9 Dev Seed 2.

Great! Thanks for the quick response. Are there any plans for a backport for iOS8 users? (pardon me if it's a question with an obvious answer; I'm not very familiar with the iOS ecosystem in general)
Comment 23 David Kilzer (:ddkilzer) 2015-07-06 07:36:07 PDT
(In reply to comment #22)
> (In reply to comment #21)
> > This was fixed in iOS 9 Dev Seed 2.
> 
> Great! Thanks for the quick response. Are there any plans for a backport for
> iOS8 users? (pardon me if it's a question with an obvious answer; I'm not
> very familiar with the iOS ecosystem in general)

It is unlikely that this will be backported to iOS 8.x.
Comment 24 Mark 2015-07-07 06:17:17 PDT
This jsfiddle example still seems to fail for me on iOS 9 Beta 2.

http://jsfiddle.net/adrian_vasiliu/ftyafr0d/
Comment 25 Stefan Penner 2015-07-07 10:41:30 PDT
> It is unlikely that this will be backported to iOS 8.x.

This would be unfortunate, a large amount of devices and sites are affected by this. The solution is basically, hand tailor scenario by scenario and wait for armv7 devices on iOS8 to age out.

Luckily iOS age out faster then android, Unfortunately this will still be years of time. Time where app authors have no reasonable solution.
Comment 26 Stefan Penner 2015-07-07 10:42:57 PDT
(In reply to comment #24)
> This jsfiddle example still seems to fail for me on iOS 9 Beta 2.
> 
> http://jsfiddle.net/adrian_vasiliu/ftyafr0d/

This fiddle illustrates a different issue. Confusingly it manifests in a similar way as the one this bug is tracking.
Comment 27 David Kilzer (:ddkilzer) 2015-07-07 13:55:43 PDT
(In reply to comment #26)
> (In reply to comment #24)
> > This jsfiddle example still seems to fail for me on iOS 9 Beta 2.
> > 
> > http://jsfiddle.net/adrian_vasiliu/ftyafr0d/
> 
> This fiddle illustrates a different issue. Confusingly it manifests in a
> similar way as the one this bug is tracking.

Do we have a new bug tracking this issue?
Comment 28 Sivakumar Kailasam 2015-09-08 01:28:14 PDT
(In reply to comment #25)
> > It is unlikely that this will be backported to iOS 8.x.
> 
> This would be unfortunate, a large amount of devices and sites are affected
> by this. The solution is basically, hand tailor scenario by scenario and
> wait for armv7 devices on iOS8 to age out.
> 
> Luckily iOS age out faster then android, Unfortunately this will still be
> years of time. Time where app authors have no reasonable solution.

Although I'm happy that this is fixed, I feel concerned that this isn't getting backported to iOS8. For those of us serving enterprise web apps this is a blocking issue since most of our users just can't use our apps because of this issue. The 'use strict' removal didn't work for us. 

Not entirely sure about the webkit process but there have been instances where such issues have been fixed at least in one prior version.  Given that iOS9 isn't still out and would take a while for everyone to adapt please consider backporting this.
Comment 29 David Kilzer (:ddkilzer) 2015-09-08 03:46:53 PDT
Thanks, Sivakumar.  We will take this into consideration, but we rarely release updates to the previous major version of iOS after shipping a new major version.

I would encourage you to continue looking for a workaround if possible.
Comment 30 Jack Preston 2015-09-14 06:07:03 PDT
If I might, I'll register again that this is impacting a large swathe of our users too, with no attempts at workarounds working. If you're not prepared to backport, I feel a certain amount of support towards the effort of finding viable workarounds would be a responsible way to treat this. It's a breaking issue in a core part of code.