mozilla :: #ionmonkey

10 Oct 2017
08:28bbouvierdecoder, sounds sensible
08:30decoderI already found the problem
08:30decoderjs/src/Unified_cpp_js_src38.gcda:Merge mismatch for function 583
08:30decoderthe gcda file seems to be damaged somehow
12:50bbouvierjonco, hi! thanks for the review in bug 1406883! what do you think of poisoning the value after it's been rooted, in case we return false and try to reuse it? (it's not used, but better be safe than sorry_)
13:08joncobbouvier: that would work, or just poison it when we return false
13:09bbouvieris there a mozilla::Poison or something
13:09joncobbouvier: I would just set it to undefined
13:09bbouvieri see mozWritePoison
13:10bbouvierjonco, it is not suposed to be used when the function returns false, so i'd rather have it blow up if it's used
13:15joncobbouvier: normally poisoning only happens on debug builds (or on nightly)
13:16joncobbouvier: there is js::PoisonedObjectValue() which puts an arbitrary integer into an object value
13:18bbouvierjonco, great, i'll use that then, thanks!
13:29joncobbouvier: huh, did you accidentally file bug 1406881 too? looks like the same thing
13:31bbouvierjonco, indeed, thanks for the heads up
13:32bbouvieroh right, i remember what happened: i submitted the patch, accidentally Ctrl+W (old terminal habit), reopened the page, click filed, bugzilla complained it didn't find the patch, i re-upload it, submit the bug, can't see the patch, attach the patch
13:32bbouvierprobably a bug in bugzilla
13:55evilpiejandem: I can set allowDoubleResult to true for getprop_super right? http://searchfox.org/mozilla-central/source/js/src/jit/IonCacheIRCompiler.cpp#402
13:56jandemevilpie: yep because of the type barrier there
13:56evilpiegreat, I implemented your suggestions. OOL calls are easy
15:30evilpieintermittent failure :/
19:13evilpiejandem: can you take a look? I am terrible at this
19:22tcampbellevilpie: what is intermittent?
19:22evilpiethe test failure in bug 1378186, but it does reproduce most of the time, so it's not terrible
19:23tcampbellcan you get it in rr?
19:24evilpieI don't have rr
19:24tcampbellI can take a look. I'm just between tasks anyways
19:25evilpiethanks. I should really check if rr works now. Last time it didn't
19:38tcampbellI'd fixed the static analysis complaints
19:39tcampbellevilpie: https://treeherder.mozilla.org/logviewer.html#?job_id=135949479&repo=try&lineNumber=9522
20:13evilpietcampbell: I already fixed that locally
20:13tcampbellevilpie: ah. I have it in rr, but only the opt build
20:13evilpieoh I guess I always use opt-debug
20:14tcampbelloh look. this time the debug captured
20:14tcampbellnow this should be easy..
20:15tcampbell*easier
20:28tcampbellevilpie: I think there is some output register clobbering
20:29tcampbellThe failing line has |this === 2|, and thus this.__proto__ === Number(2).__proto__
20:30evilpiedo you know which instruction? superbase or getprop_super
20:34tcampbellfollowing the trail back now.. got distracted
20:43tcampbellevilpie: I'm 90% sure it is GETELEM
20:43tcampbellwe are invoking Y.b() with this === 2
20:43tcampbellso I think your Math.random() is to blame for the intermittent
21:03tcampbellevilpie: I've got to get back to some other things, but here is the minimized test: https://pastebin.mozilla.org/9069735
21:04evilpieinteresting, this means in CacheIR terms we should use GetPropSuper and not GetElemSuper, because the keys are constant
21:04tcampbellthe keys are runtime strings
21:04tcampbellit is still a GETELEM
21:05tcampbellif you push the strings into the brackets, the test passes
21:06tcampbellwhat is odd is the need for |if (i < 4) return|
21:07evilpieok, I was talking about basically this http://searchfox.org/mozilla-central/source/js/src/jit/CodeGenerator.cpp#10174
21:07evilpiemaybe something with invalidation
21:07tcampbelloh, interesting
21:14tcampbellevilpie: you are right that it is turned into a GetPropSuper
11 Oct 2017
No messages
   
Last message: 6 days and 16 hours ago