Closed Bug 503427 Opened 15 years ago Closed 15 years ago

jstracer.cpp creates ill-typed LIR: i2f (i2f (arg)) and variants thereof


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
status1.9.2 --- beta1-fixed


(Reporter: jseward, Assigned: dvander)



(Whiteboard: fixed-in-tracemonkey)


(2 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5
Build Identifier: 

TM branch, x86-linux build.  Basically jstracer.cpp creates 
LIR_i2f applied to non-int32 args; 64-bit float and quad args
have been observed.

The reason this doesn't cause havoc is because, in all such cases,
the outer (or, in general, ill-typed) application of i2f is dead,
and so DeadCodeFilter later in the pipeline removes it.

I have no idea if this is merely a harmless bug or whether it
signifies something more serious.  However, it would be good to
remove it, since it produces what amount to false-positives in 
in-progress work on LIR typechecking.

How to repro: take a debug build of jsshell, do:

TMFLAGS=minimal,readlir ./BuildD/js -j ./trace-test.js

and search for "i2f 0:0" in the output.  This is i2f being
applied to a 64-bit constant, when the arg should be 32-bit int.

Finding an easily reproduced case for i2f (i2f (arg)) is not
so easy; still in progress.

Reproducible: Always
Gross. Nice catch.
Attached file test case
This happens when a loop wants a double at the edge, but there's a promoteInt instead. We insert i2f, despite the fact that a promoteInt is always a quad in LIR. This works because writeBack() will ::demote and strip away the redundant i2f, and the erroneous i2f will die.

Attached is a trace-tests case which emits such weird LIR.

This is a lot easier to fix against the patch in bug 497948 but I think I can hack it into the old code as well.
(In reply to comment #2)
> test case


xxx     0xf7f982d0 i2f3 = i2f add1
xxx     0xf7f98bb0 i2f4 = i2f i2f3
LirChecker: for argument 1 of the node
   i2f4 = i2f i2f3
   type error: can't reconcile Flt64 with Int32
Attached patch hacky fix (obsolete) — Splinter Review
Here is a hacky fix.  I added an assert which did not hit in trace-tests.
Any better idea how to fix this?
Ever confirmed: true
Flags: blocking1.9.2?
Attached patch fixes bugSplinter Review
Attachment #388362 - Attachment is obsolete: true
Attachment #398800 - Flags: review?(gal)
Attachment #398800 - Flags: review?(gal) → review+
Closed: 15 years ago
Resolution: --- → FIXED
Not blocking per danderson.
Flags: blocking1.9.2? → blocking1.9.2-
Assignee: general → dvander
You need to log in before you can comment on or make changes to this bug.