Closed Bug 293782 Opened 15 years ago Closed 15 years ago

Local variables can cause predefined function object properties to be undefined

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.8beta2

People

(Reporter: jwatt, Assigned: brendan)

References

Details

(Keywords: js1.5)

Attachments

(3 files)

If a function has a local variable with the same name as a predefined function
object property (e.g. apply, call and name) then those properties will be
undefined for that function object. Test using the following:

javascript:function f(){var name=1;};alert(f.name);
Assignee: general → brendan
Flags: blocking1.8b3+
Keywords: js1.5
Priority: -- → P3
Target Milestone: --- → mozilla1.8beta3
Status: NEW → ASSIGNED
Priority: P3 → P1
Target Milestone: mozilla1.8beta3 → mozilla1.8beta2
Blocks: 290656
Yay, this patch removes the XXX'd hacks in jsinterp.c (SetFunctionSlot) and
jsobj.c (with_GetProperty) that try with too much code to work around the
pre-ECMA pollution of function objects with properties, used by the compiler
and by Call object reflection, describing their args and local vars.

/be
Attachment #183623 - Flags: review?(shaver)
Blocks: 294191
Comment on attachment 183623 [details] [diff] [review]
Fix SpiderMonkey to hide arg/var properties of function objects

I like -- perhaps love -- everything here but "FUNNY".	I appreciate a good
function pun, but wonder if _ACTIVATION or some such might be clearer. 
(Especially given the now-dead, but long-lasting, CHECK_FOR_FUNNY_ID macro,
though that might only matter to you and me.)

Not enough to keep this from hitting the tree, though, as it is indeed truly
righteous.

r+a=shaver
Attachment #183623 - Flags: review?(shaver)
Attachment #183623 - Flags: review+
Attachment #183623 - Flags: approval1.8b2+
(In reply to comment #2)
> (From update of attachment 183623 [details] [diff] [review] [edit])
> I like -- perhaps love -- everything here but "FUNNY".	I appreciate a good
> function pun, but wonder if _ACTIVATION or some such might be clearer. 

huh, yeah, that would have been clearer to me at least.
Checking in 10.1.6.js;
/cvsroot/mozilla/js/tests/js1_5/Function/10.1.6.js,v  <--  10.1.6.js
initial revision: 1.1
Flags: testcase+
- Renamed _FUNNY => _HIDDEN.
- Fixed a leftover js_LookupProperty in Function that was looking for duplicate
formal parameters -- of course it must now be js_LookupPropertyWithFlags(...,
JSLOOKUP_HIDDEN, ...).
- Robusticated call_resolve to insist that any hidden property found in the
callee or a prototype of the callee came from the same function as the callee's
fp->fun.  Necessary since (mutable) __proto__ was added.

/be
Fixed.  Marking deps fixed too.  Hooray!

/be
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Attached patch followup fixSplinter Review
There's no way around it, given id uniquely identifying a property in an
object: to hide function formarl parameter and local variable properties in a
disjoint set of properities requires a disjoint set of ids.  Hence this patch,
which undoes the regression of bug 137000.  Checking in shortly.

/be
Attachment #183781 - Flags: review?(shaver)
Comment on attachment 183781 [details] [diff] [review]
followup fix

r+a=shaver.
Attachment #183781 - Flags: review?(shaver)
Attachment #183781 - Flags: review+
Attachment #183781 - Flags: approval1.8b2+
I checked in the followup fix, with one slight tweak to jsemit.c (to test !ok
and break if so after the call to js_LookupHiddenProperty for the native obj
case in js_LookupCompileTimeConstant).

/be
latest patch looks to be causing tinderbox orange!
Sorry about that -- over-reliance on the JS testsuite, and neither shaver nor I
remembered the JS_CompileUCFunctionForPrincipals API impl, which defines formal
parameters.

/be
The inner variables of functions still show up in for-in statements. Try

function f(){var a, b;}
for (prop in f) alert('f['+prop+'] == '+f[prop])

Should I file a separate bug, or does this need to be reopened?
(In reply to comment #12)
> The inner variables of functions still show up in for-in statements. Try
> 
> function f(){var a, b;}
> for (prop in f) alert('f['+prop+'] == '+f[prop])
> 
> Should I file a separate bug, or does this need to be reopened?

Fixed.  This was just a matter of removing JSPROP_ENUMERATE attributes passed to
js_AddHiddenProperty for args and vars, per ECMA-262 Ed. 3, 10.1.3 and 10.2.3.

/be
Brendan, the problem seems to still exist in a build I made in the last hour or so.
jwatt: thanks for catching this -- one JSPROP_ENUMERATE was left behind.  Fixed.

/be
Status: RESOLVED → VERIFIED
testcase for enumerating function local variables.
Checking in 10.1.6-01.js;
/cvsroot/mozilla/js/tests/js1_5/Function/10.1.6-01.js,v  <--  10.1.6-01.js
initial revision: 1.1
done
You need to log in before you can comment on or make changes to this bug.