542055 - Copy propagation and Null check removal conspire to affect visible semantics

Status: VERIFIED FIXED

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

Description

[Edwin Smith]

The scenario goes like this:

1. JIT performs copy propagation and load removal to optimize code
2. the notNull bit of each variable is propagated when copied
3. When a pointer is null checked, the Verifier copies the notNull=false bit to each copy of the same pointer.
4. OP_add = String if either input is a non-null String; Number or Object otherwise.
5. OP_pushscope copies the verified type to the scope stack
6. OP_newfunction|newclass capture the types of lexical scope objects
7. OP_findproperty only inspects declared properties of pushscope lexical scope objects, as opposed to seeing all runtime properties on pushwith scope objects.

Therefore its possible to write a program that causes a lexical scope to contain [... String ...] when the JIT is attached, and [... Object ...] otherwise, which then can change the lexical visibility of unqualified "length" properties or other delarations on String that aren't on Object.

(or even, varying with smarter or dumber JIT's).

Breaking any link in the above causal chain will fix the problem.

Ideal: Modifying String's type checker (step 4) to not inspect the notNull bit would allow the verifier to completely remove the notNull flag from FrameState, and would remove null-ability from the abstract model of the verifier.  As long as the jit has its own type modelling that's at least as good as the verifier, this fix is safe and fast.

Less Ideal: Do not implicitly propagate notNull to copies of a null checked pointer.  FrameState.notNull must remain as part of the verifier's abstract model.  However, since only stack values ever get null checked (implicitly as part of dereferecning them, the original copies in local variables remain unchanged, and the presence of notNull in the verifier's model is essentially noise.

Comment 1

[Edwin Smith]

It is also worth noting that it's impossible to write AS3 code that does step #4 then #5.  So, it is possible that this problem is rare or even nonexistent in real programs and either of the above fixes would never be noticed.

Comment 2

[Edwin Smith]

see also: 
Bug 415080 -  Verifier's removal of NULL type causes later verification failure

Comment 3

[Edwin Smith]

This code in the (apparently used a lot) JW Player triggers the same bug

http://developer.longtailvideo.com/trac/browser/trunk/as3/com/jeroenwijering/parsers/SMILParser.as?rev=88#L54

Comment 4

[Dan Smith]

Pushed to TR and TR argo: e01eeeb5b392
Marking as Resolved/Fixed

Comment 5

[Chris Peyer]

Testcase to be created in Bug 415080