The following testcase crashes on mozilla-central revision 8af276ab8636 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2 --unboxed-objects --ion-eager --ion-offthread-compile=off): (function() { function f (v, i) { var c = v[i]; } var v = [ 0, 0.0, 0.1, 1, 1.0, 1.1, ]; for (var i = 0; i < 100; i++) f(({ __defineSetter__: 0, length: 1 }), i % v.length); })(); Backtrace: Program received signal SIGSEGV, Segmentation fault. 0xf7fd63f0 in ?? () #0 0xf7fd63f0 in ?? () eax 0x2 2 ebx 0xf7fd6160 -134389408 ecx 0x1 1 edx 0xdeadbeef -559038737 esi 0x2 2 edi 0xf6357120 -164269792 ebp 0x14 20 esp 0xffffba30 4294949424 eip 0xf7fd63f0 4160578544 => 0xf7fd63f0: mov -0xc(%ecx),%edx 0xf7fd63f3: cmp %eax,%edx

JSBugMon: Bisection requested, result: === Treeherder Build Bisection Results by autoBisect === The "good" changeset has the timestamp "20150310043920" and the hash "01bd19b3f56b". The "bad" changeset has the timestamp "20150310051620" and the hash "f785209f088d". Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=01bd19b3f56b&tochange=f785209f088d

Needinfo from jandem based on comment 1.

Revision f785209f088d probably exposed this but I think this is an unboxed objects bug. Here's a slightly simpler test: $ js --unboxed-objects --ion-eager --no-threads test.js Segmentation fault: 11 (function() { function f(v, i) { var c = v[i]; } for (var i = 0; i < 40; i++) { f({x: 0, length: 1}, 10); } })();

We compile a JSOP_GETELEM on the preliminary objects for some group as if the object is native, but when those objects are later converted to unboxed objects we get messed up treating the object as if it is still native. The fix here is to do more extensive checking for preliminary objects during compilation. This patch is modified from one of the patches in bug 1146597, which also addresses this problem.

(In reply to Brian Hackett (:bhackett) from comment #4) > We compile a JSOP_GETELEM on the preliminary objects for some group as if > the object is native, but when those objects are later converted to unboxed > objects we get messed up treating the object as if it is still native. Hm so we don't invalidate code when we go from preliminary to unboxed? > The fix here is to do more extensive checking for preliminary objects during > compilation. Could this cause performance issues, when we don't inline certain operations anymore? For instance if we construct a few global arrays and use them everywhere?

(In reply to Jan de Mooij [:jandem] from comment #5) > Hm so we don't invalidate code when we go from preliminary to unboxed? We don't want to have to invalidate Ion code when this happens. If we Ion compile a script that is using objects in a group which is still preliminary, we abort the builder, and then force analysis of the preliminary objects in whatever state they are currently in. Now, we can still generate baseline caches that operate on preliminary objects without forcing analysis of the group. These caches are stripped once an access site sees non-preliminary objects, to avoid polluting information used during Ion compilation (the preliminary objects might have a different shape than the later objects). > Could this cause performance issues, when we don't inline certain operations > anymore? For instance if we construct a few global arrays and use them > everywhere? We will still inline the operations. I think maybe the patch is confusing because hasPreliminaryGroups() is effectful, and if it returns true and we are compiling rather than analyzing the script, we will abort the builder, analyze the groups and then try compiling the script again shortly. The patch in the other bug named this function checkPreliminaryGroups(), which is maybe better but didn't make it obvious what the return value is. Maybe shouldAbortOnPreliminaryGroups()?

Comment on attachment 8590892 [details] [diff] [review] patch Review of attachment 8590892 [details] [diff] [review]: ----------------------------------------------------------------- Ah right, hasPreliminaryGroups sounds idempotent, I like shouldAbortOnPreliminaryGroups.

This patch fixed the regression I reported on https://bugzilla.mozilla.org/show_bug.cgi?id=1135423#c6

AWFY detected a improvement on: - slave: Mac OS X 10.10 32-bit (Mac Pro, shell) - mode: Ion Regression(s)/Improvement(s): - kraken: crypto-pbkdf2: -5.72% (improvement) More details: http://arewefastyet.com/regressions/#/regression/734353