Closed Bug 635657 Opened 14 years ago Closed 14 years ago

TM: Tracing JIT returns wrong result

Categories

(Core :: JavaScript Engine, defect)

x86
Windows Vista
defect
Not set
major

Tracking

()

RESOLVED DUPLICATE of bug 620757

People

(Reporter: m_kato, Unassigned)

References

()

Details

Attachments

(1 file, 1 obsolete file)

Attached file test case (obsolete) —
attached js is minimal sample to reproduce this.  This isn't regression.  This problem still occurs on 3.6.  If using method JIT or interpreter, this doesn't occur.


# js -m attached.js
success: Holiday in Lieu

But, on tracing,

# js -j attached.js
error
We really need a way to track correctness bugs like this.  :(
Severity: normal → major
(In reply to comment #1)
> We really need a way to track correctness bugs like this.  :(

Can you clarify?  Eg. do you want a tracking bug for tracer correctness bugs or something?
Tracking bug, keyword, status whiteboard annotation we all agree on, flag, whatever.  Something to keep them from just disappearing into the abyss of minor spec compliance bugs and minor performance/arch issues...

In particular, I happen to consider a correctness bug that we have in only one of our jits more of a problem that something that happens across all modes, all else being equal, since it's way less predictable for JS authors.  But maybe that's just me.
(In reply to comment #3)
> In particular, I happen to consider a correctness bug that we have in only one
> of our jits more of a problem that something that happens across all modes, all
> else being equal, since it's way less predictable for JS authors.  But maybe
> that's just me.

It's not.  Not that I have any novel suggestions for tracking these differently than we have (or haven't tracked well) in the past.  I almost wonder if it requires a cultural shift to accepting that some portion of time should be spent to fix minor spec compliance bugs *even if* you'd never block a release on any of them.
(In reply to comment #4)
>
> I almost wonder if it
> requires a cultural shift to accepting that some portion of time should be
> spent to fix minor spec compliance bugs *even if* you'd never block a release
> on any of them.

Does anyone *not* think some portion of time should be spent doing that?
Now's a great time, if you don't have blockers to work on.  Queue up some correctness fixes.  I don't think we've had trouble getting them in except for the small endgame lockdown, but I might not be watching the right bugs.  Certainly we've taken a ton of them for ES5, Acid3, other test suites.
I think the minor fixes do tend to be under-emphasized at times, yes, especially in vaguely endgame-ish times.  But no, it's not a universal thing, I miswrote the comment a little if that was your interpretation (a reasonable interpretation, too, in hindsight).
Be the change you want to see. :-)
I'll give myself a gold star in advance, because I was planning to take a look at this today even before comment 1 was posted! :)
(In reply to comment #6)
> 
> Certainly we've taken a ton of them for ES5, Acid3, other test suites.

Has our Acid3 score gone above 97?
No, but to get there required a handful of JS fixes (evaluation order mostly, IIRC)
(The Acid3 score being 97 and not 100 is a distinctly different issue that doesn't have to do with taking minor fixes.  See roc's blog for the SVG fonts issues there.)

(In reply to comment #8)
I try.

Yet at the same time it's easy to find one person saying minor fixes are a reasonable thing to work on at this point, and another person saying blockers blockers blockers blockers blockers.  The js-engine.internals group had some of the latter several days ago, for example, at least by my reading of it.

So then it comes down to the minor stuff being fixed only if the person doing the fixing is willing to be assertive enough to ignore conflicting directives, and is willing to do so for particular minor bugs.  I have done and will do that when I think it necessary, but the relative uncertainty poses at least a minor mental roadblock.  Maybe that's unavoidable.

And this is all getting afield of the bug, of course.  If people are bothered even slightly, I'm happy to continue this somewhere else if people think that's helpful.
Given the switch statement, this might be related to bug 620757.
The thread on internals was about landing things on tracemonkey, not about writing patches if you couldn't find blockers to help with.
I didn't interpret Brendan's, or Andreas's, mails in that thread that way.
They were pointing out that there were blockers that anyone could help with, yes.  Even if that is no longer true, or once it no longer becomes true, we don't want more churn on tm right now, so people should be holding their patch-fire until we can see the whites of FF4's eyes.
(In reply to comment #2)
> > We really need a way to track correctness bugs like this.  :(
> 
> Can you clarify?  Eg. do you want a tracking bug for tracer correctness bugs or
> something?

I'd like to see failing tests checked straight into HG, without having to wait for the bug to be fixed. That means if the bug falls off the radar, the test case will still be in our sights, mocking us.
(In reply to comment #17)
> (In reply to comment #2)
> > > We really need a way to track correctness bugs like this.  :(
> > 
> > Can you clarify?  Eg. do you want a tracking bug for tracer correctness bugs or
> > something?
> 
> I'd like to see failing tests checked straight into HG, without having to wait
> for the bug to be fixed. That means if the bug falls off the radar, the test
> case will still be in our sights, mocking us.

You could add it to jstests, which has known-failure annotations in the manifest file. But that set has a lot of known failures so adding it there might not have much effect. It would be nice if we could separate known failures we care much about from those we don't.
The way to track work to do is file a bug and take assignment. Checking in the tests with skip lines in the manifest is fine. Checking tests in that are expected fails is ok too but as comment 18 notes this won't really help call them out, since we have many.

Nothing stops work from happening in bugs and user repos now. OTOH I chased two JM crashes (possibly due to only one bug) over the weekend that reproduced for me easily, so I doubt we are free from having to look at blockers.

/be
The given test depends on the timezone.  Using California time it doesn't print anything;  using Melbourne time it acts as expected.  This version sets the hour as well and works in both timezones.
Attachment #513933 - Attachment is obsolete: true
(In reply to comment #13)
> Given the switch statement, this might be related to bug 620757.

Yeah, I'm 95% sure it's a dupe.
I am a website owner of "holiday_logic_English.htm" in the URL column.

This malfunction is caused by strange works that the switch statement 
in the loop jumps to a block of "case 3" under conditions of "Value=5".

I prepared a simple inspection script of Test1 - Test4 for the following URL.
http://www.h3.dion.ne.jp/~sakatsu/FireFox_TMBugTest_E.htm
Depends on: 620757
[Depends on: bug 620757 ]
[#620757 Whiteboard: fixed-in-tracemonkey ]
I expected it whether it is reflected by "4.0rc1", and it is revised, 
but the problem of switch-case does not change in "4.0rc1" either.

Are not #620757 corrections yet reflected in "4.0rc1"?
Or did not #635657 have an effect by #620757 corrections?
(In reply to comment #23)
> [Depends on: bug 620757 ]
> [#620757 Whiteboard: fixed-in-tracemonkey ]
> I expected it whether it is reflected by "4.0rc1", and it is revised, 
> but the problem of switch-case does not change in "4.0rc1" either.

fixed-in-tracemonkey means that the fix is landed in tracemonkey repository.

You can download nightly of tracemonkey build from http://nightly.mozilla.org/js-preview.html.

This won't be fixed on 4.0 timeframe since it isn't blocker-2.0.
Thanks.

I tested "4.0b13pre".

All the results of FireFox_TMBugTest_E.htm became "Success".

I stopped JaegerMonkey and confirmed that works of switch-case 
did not have any problem in a state only TraceMonkey.

I was able to make the holiday-list definitely, too.

I confirmed that this bug was revised in "4.0b13pre".
K.Tsunoda:  thanks for the confirmation.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: