519489 - Possible version-compatibility issue: Verifier allows OP_getslot allowed in cases where it should not

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Stan Switzer]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3
Build Identifier: Tamarin Central 24 April 2009

The verifier correctly prevents the use of OP_getslot across ABC files, but it allows some cases that may cause compatibility problems down the road.

Getslot is currently allowed if the base classes from other ABCs have no slots. The problem is that they might have no slots _now_ but that the base classes might have slots in some future version.  This will cause the verifier to reject code it had previously accepted.

Instead, the verifier should only allow getSlot if the all base classes except Object (or Any... for non-Object traits) are defined in the same ABC file.

Of course, making the Verifier more strict will cause those same ABCs to be rejected immediately, which is also a compatibility issue.  For lack of a time-machine, we should probably track this for when we rev the ABC version.

The problem is in this code:

 	bool Traits::allowEarlyBinding() const
 	{
 		// the compiler can early bind to a type's slots when it's defined
 		// or when the base class came from another abc file and has zero slots
 		// this ensures you cant use the early opcodes to access an external type's
 		// private members.
		TraitsBindingsp tb = this->base ? this->base->getTraitsBindings() : NULL;
		while (tb != NULL && tb->slotCount > 0)
		{
			if (tb->owner->pool != this->pool && tb->slotCount > 0)
			{
				return false;
			}
			tb = tb->base;
		}
		return true;
	}

Again, just to be clear: the current code does what it says.  It prevents access to an external type's slots.  But it allows the current type to use getslot in cases where _future_ changes to the external base class will cause a verification failure.  A type should not be allowed to early-bind if it inherits from any external type (except Object), whether or not that base class _currently_ has slots, because in the future it might.


Reproducible: Didn't try

Comment 1

[Stan Switzer]

One possible fix is to generalize slot access.  Since slot access can only have worked if the foreign base had no slots, then slot numbers can be understood _as if_ foreign bases had no slots.  That is: slot indexes number slots introduced in the referencing ABC, whether foreign bases have slots or not.

Comment 2

[Edwin Smith]

I'd thought of that a while back and its still worth considering, but here's what i'd thought of also:

lets say you're interpreting.  then, computing the real slot to access from the OP_getslot parameter is potentially expensive.  you need to know which base class is the first external one to start counting from.  (and what if the inheritance tree alternates internal/external base classes?)

if you're jit-compiling, any place you could do a getslot, you could also do a getproperty, assuming the slot has a name.  the code will be just as good as if getslot was used.  assuming this, the savings from getslot are just from eliding the name, or from maybe using a smaller operand (small slot number vs large name index)

now, if the goal *is* to eliminate the names, cool.  otherwise, generalization doesn't help a whole lot and might make the semantics more complicated.  (an abc linker for example now has to renumber OP_getslot operands, which requires more type knowlege than its normal job of just renumbering pool indexes).  maybe that too is okay, but its a factor in the decision.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Tamarin isn't maintained anymore. WONTFIX remaining bugs.