290774 - inner function causes scope conflict with Object.prototype

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Yuh-Ruey Chen]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050317 Firefox/1.0.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050317 Firefox/1.0.2

If:

1) In the same scope that A was defined in (the "parent" scope, not the scope of
A), a var or function (denote as B) is declared that has same name as any native
internal member/method (not including any custom members/methods) of Object
(denote as C)
2) A function in the scope of A (inner function) is defined that includes any
statement that requires acessing a non-local object even if it doesn't exist
(does not include statements that declare or reference local variables/functions)

Then:

Referencing var/function B in function A (i.e. in the scope of A)
accesses C (property of Object.prototype) instead of B (variable in scope A)

Workaround: don't name vars and functions toString, valueOf, watch, unwatch,
eval, or any other Object method/member.

Reproducible: Always

Steps to Reproduce:
Simple test case:

var toString = 'test';
function foo() {
	alert(toString);
	function inner() {	
	
any_statement_that_requires_accessing_a_non_local_object_even_if_it_does_not_exist;
	}
}
foo();

Comprehensive test case:

function scope() {	//bug can also happen in global scope (comment this out to test)
	//define var or function that has same name as any native internal
member/method of Object
	//	(not including any custom members/methods):
	function valueOf() {
		return 'test1';
	}
	//following also triggers bug:
	var toString = function() {
		return 'test2';
	}
	//this too:
	var eval = 'test3';	//eval is deprecated method of Object
	var watch;	//only need to declare - don't need to even define
	var some_var = 'test5';	//bug doesn't affect this
	var parseInt = 'test6';	//bug doesn't affect members/methods of global object
	var sin = 'test7';	//bug doesn't affect this (doesn't conflict with any other
native object in global scope)
	function foo() {
		alert(valueOf);
		alert(toString);
		alert(eval);
		alert(watch);
		alert(some_var);	//bug doesn't affect this
		alert(parseInt);	//bug doesn't affect this
		alert(sin);	//bug doesn't affect this
		alert(unwatch);
		function inner() {	
		//var inner = function() {	//this also triggers bug
		
any_statement_that_requires_accessing_a_non_local_object_even_if_it_does_not_exist;
			//var x = 10; x;	//this doesn't trigger bug
		}
	}
	foo();
	var unwatch = 'test8';	//declaring after foo still triggers bug
	//note: if in global scope, declared yet undefined variables reference the
	//	corresponding global object's property
}
scope();

Actual Results:  
Simple test case:

function toString() {
	[native code]
}

Comprehensive test case:

function valueOf() {
	[native code]
}
function toString() {
	[native code]
}
function eval() {
	[native code]
}
function watch() {
	[native code]
}
test5
test6
test7
function unwatch() {
	[native code]
}


Expected Results:  
Simple test case:

test

Comprehensive test case:

function toString() {
	return 'test1';
}
function () {
	return 'test2';
}
test3
undefined
	(OR if in global scope:)
	function watch() {
		[native code]
	}
test5
test6
test7
undefined
	(OR if in global scope:)
	function unwatch() {
		[native code]
	}

Comment 1

[Yuh-Ruey Chen]

Attachment: simple test case

Comment 2

[Yuh-Ruey Chen]

Attachment: comprehensive test case

Comment 3

[Brendan Eich [:brendan]]

I believe you, but can you give a working example -- one that does not include
pseudo-code of the form:

any_statement_that_requires_accessing_a_non_local_object_even_if_it_does_not_exist;

/be

Comment 4

[Yuh-Ruey Chen]

Attachment: simple test case (clearer)

any_statement_that_requires_accessing_a_non_local_object_even_if_it_does_not_exist
actually isn't pseudo-code.  The engine tries to find a property on the global
object with that name.

In any case, this is a clearer test case.

Comment 5

[Yuh-Ruey Chen]

Attachment: comprehensive test case (clearer)

Comment 6

[Brendan Eich [:brendan]]

Thanks -- that wasn't clear at first.

It's true, we use a real object (of class Call, delegating to Call.prototype,
then to Object.prototype) for activations of functions containing functions that
use non-local names.  ECMA-262 Edition 3 fails to specify whether the
"activation object" (10.1.6) delegates to Object.prototype, but it censors this
object from the language, using it only as a specification device.

That does imply that an activation object never delegates to Object.prototype,
and defines only argumentes and the properties corresponding to the declared
arguments and local variables.

So I will try to fix this.

Bryant: were you reporting this because other browsers' JS implementations
behave differently?

/be

Comment 7

[Brendan Eich [:brendan]]

Attachment: fix

Easy to match what ECMA implies (and should specify explicitly, but the spec's
implication is clear enough).

/be

Comment 8

[Yuh-Ruey Chen]

(In reply to comment #6)
> Bryant: were you reporting this because other browsers' JS implementations
> behave differently?

Yeah, when I run the test cases in IE and Opera, I get the expected output.

I encountered the bug while writing a complex script, and it took me a while to
narrow it down to this really odd case.  Who would've thought the contents of an
inner function that isn't even called can affect scope?

Comment 9

[Brendan Eich [:brendan]]

Comment on attachment 181016 [details] [diff] [review]
fix

Trivial fix, checking in for 1.8b2 with optimistic r=shaver.

/be

Comment 10

[Brendan Eich [:brendan]]

Fixed.

/be

Comment 11

[Bob Clary [:bc] (inactive)]

added test 
/cvsroot/mozilla/js/tests/ecma_3/ExecutionContexts/10.6.1-01.js,v  <--  10.6.1-01.js
initial revision: 1.1

verified fixed with this patch.

Comment 12

[Mike Shaver (:shaver emeritus)]

Comment on attachment 181016 [details] [diff] [review]
fix

r=shaver.

Comment 13

[Erik Fabert]

*** Bug 297471 has been marked as a duplicate of this bug. ***