415080 - Verifier's removal of NULL type causes later verification failure

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P2)

Description

[Brent Baker]

! this was found in Beta 3. I did not check beta 2.

A function which contains the following structure will cause : 
VerifyError: Error #1068: String and String cannot be reconciled.

I removed variables from the code to get the simplest possible function to cause this error.

    private function _badFunction(a : String = 'b') : Boolean {
        switch (true) {
            case true :
                a.indexOf('e');
                if(true == true) {
                    return true;
                } 
        }
        return false;
    }
 
 Actual Results:
 VerifyError: Error #1068: String and String cannot be reconciled.
 
 Expected Results:
 no error
 
 Workaround (if any):
Dont put a return inside an if, after an indexOf inside a switch in a function which takes a string as an argument.
 
 
Transferred Comments:

Richard Butler - Tue Jan 29 07:06:55 CST 2008
I have a similar error myself: "VerifyError: Error #1068: Boolean and ArrayCollection cannot be reconciled."

public function filter(item:Article):Boolean
{
	var found:Boolean = true;
			
	switch (false)
	{
		//case (keywords && filterByKeywords(item)):
		case (tags && filterByTags(item)):
		{
			found = false;
			break;
		}
	}
			
	return found;
}

"tags" is an ArrayCollection and the "filterByTags" function returns a Boolean. It seems that Flash Player is trying to compare the "false" in the switch with the ArrayCollection itself, rather than the expression: (tags && filterByTags(item)), which will resolve to a Boolean.

Brent Baker - Thu Jan 31 09:42:38 CST 2008
Confirmed the issue, and it is with the vm not the compiler. 

tamarin-central throws a verifier error running the generated abc but tamarin-tracing does not, and runs the code properly.

Need to transfer to bugzilla

Comment 1

[Edwin Smith]

another example: uncomment the call to ForIn_7() in test/acceptance/as3/Statements/for-each-in/eforin_001.as, and you'll get a Number/* type merge error, presumably because of the continue in the for-each-in loop.  (code after the continue is dead, but probably is verified).

Comment 2

[Lars T Hansen]

ASC bug?  Back to Jeff for triage.

Comment 3

[Jeff Dyer]

This bug occurs because the verifier changes the type of local from String? to String along a path in which it proves it can't be null. ASC could insert a case to String?, but that is just arm wrestling with the verifier. An alternative fix is for the verifier to not make such an eager downcast unless it can prove that it can do so along all parallel paths.

A fix which removes the transition from String? to String is safe because it results in more programs (including the test case) being accepted. But, it will also gives back some speed that probably should not have been taken in the first place. There is little ASC can do to help with that optimization since there is not ABC name for String!.

Comment 4

[Lars T Hansen]

VerifyErrors are observable.

Comment 5

[Edwin Smith]

The JIT is what changes String? to String, and when verifying without the jit attached, the variable remains String? and there is no verify error.  

The insidious thing here is that any change to the way the jit does value numbering will potentially subtly change this behavior.

Comment 6

[Edwin Smith]

fixed by 413522

Comment 7

[Chris Peyer]

Verified Fixed tamarin-redux r4008

Comment 8

[Chris Peyer]

Attachment: Regression testcase.

Comment 9

[Brent Baker]

testcase patch pushed:
tr-argo -> 3921:ecbf85d7764b
tr -> 4288:ecbf85d7764b