352455 - JS engine stops execution trying to uneval an object w/ some non-function getters/setters

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

z = ({});
z.x getter= /g/i;
print("This line should not be the last output you see."); 
try { print(uneval(z)); } catch(e) { print("Threw!"); print(e); } print('PASS');

The second line should probably throw "invalid getter usage".  If that's not possible, uneval() should throw or return something that isn't quite right instead of halting the entire script.

Comment 1

[Jesse Ruderman]

If you test this by pasting into the shell, you'll want "print('PASS');" on the same line as the try..catch.

Comment 2

[Brendan Eich [:brendan]]

Attachment: fix

That get/set js_obj_toSource code was not careful.

/be

Comment 3

[Brendan Eich [:brendan]]

Of course, this patch just avoids the silent out of memory.  What we really want is what Jesse said: runtime check for non-function object being bound as a getter or setter.

I'm thinking this bug is a post-js1.7/mozilla1.8.1 work item.  I'm putting it back in the [e-mail] pool.

/be

Comment 4

[Jesse Ruderman]

js> function() { return { x setter: 3 }; }
Assertion failure: strncmp(rval, js_function_str, 8) == 0 && rval[8] == ' ', at jsopcode.c:3191

Comment 5

[Brendan Eich [:brendan]]

(In reply to comment #4)
> js> function() { return { x setter: 3 }; }
> Assertion failure: strncmp(rval, js_function_str, 8) == 0 && rval[8] == ' ', at
> jsopcode.c:3191

Yes, that's a consequence of not insisting on a function object at runtime, if not at compile time, as you allude to in this part of comment 0:

> The second line should probably throw "invalid getter usage".

/be

Comment 6

[Brendan Eich [:brendan]]

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

Comment 7

[Brendan Eich [:brendan]]

Brian, could I interest you in this one?

/be

Comment 8

[Brian Crowder]

My patch for bug 61911 fixes this.  Alternatively, I can special-case the getter/setter code to explicitly also check for regexp or something else.  Thoughts?

Comment 9

[Brian Crowder]

Scratch that; my 61911 patch fixed the regexp problem.  More investigation needed.

Comment 10

[Brian Crowder]

Okay.  There are two different bugs here, one of which is genuinely fixed by my patch to bug 61911.  RegExp objects should not be considered as JSTYPE_FUNCTION (or if they are, they should be special cased to be invalid as getter/setters via an exception at runtime).  I'll write that patch if that's what Brendan wants to do.

The other bug (revealed by comment #4) is that the decompiler is making an assertion about something the compiler can't enforce.  That assertion should instead be a runtime error.  Separate bug?

Comment 11

[Jesse Ruderman]

Ok, that is now bug 355992.

Comment 12

[Brendan Eich [:brendan]]

Yeah, we should fix bug 61911 asap so that typeof /hi/ == "object".  If we do that and fix this bug (toSource, object graph to source) and the decompiler (Decompile, code to source), we are done.

/be

Comment 13

[Brian Crowder]

Still need code from comment 2 to land, then mark fixed?

Comment 14

[:Gavin Sharp [email: [e-mail]]]

mozilla/js/src/jsobj.c 	3.295

Comment 15

[Bob Clary [:bc] (inactive)]

Checking in regress-352455.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-352455.js,v  <--  regress-352455.js
initial revision: 1.1
done

Comment 16

[Bob Clary [:bc] (inactive)]

verified fixed 1.9 20061012 windows/linux

Comment 17

[Reed Loden [:reed]]

As this is blocking1.8.1.1+, please either request approval1.8.1.1 on the current patch or, if needed, attach a branch version of the patch and request approval1.8.1.1 on it.

Comment 18

[Brian Crowder]

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

Sorry, I missed this one in the mayhem (maybe didn't get the blocking approval mail somehow?)

Comment 19

[Daniel Veditz [:dveditz]]

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

approved for 1.8 branch, a=dveditz for drivers

Comment 20

[:Gavin Sharp [email: [e-mail]]]

mozilla/js/src/jsobj.c 	3.208.2.44 

Comment 21

[Bob Clary [:bc] (inactive)]

20061128 1.8.1.1 windows/linux/mac* browser|shell no longer stops execution but fails to throw the expected Syntax Error. Is this bug considered fixed and the failure to throw Syntax Error a different bug?

Comment 22

[Brian Crowder]

Probably need 61911 too?

Comment 23

[Andreas Gal :gal]

I changed the test to set '5' as getter, which is invalid. A regexp is a callable object and thus actually a valid getter/setter value.

http://hg.mozilla.org/tracemonkey/rev/dca026286095