Closed Bug 352455 Opened 13 years ago Closed 13 years ago

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

Categories

(Core :: JavaScript Engine, defect, major)

defect
Not set
major

Tracking

()

VERIFIED FIXED

People

(Reporter: jruderman, Assigned: crowderbt)

References

(Blocks 1 open bug)

Details

(Keywords: fixed1.8.1.1, testcase)

Attachments

(1 file)

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.
If you test this by pasting into the shell, you'll want "print('PASS');" on the same line as the try..catch.
Attached patch fixSplinter Review
That get/set js_obj_toSource code was not careful.

/be
Assignee: general → brendan
Status: NEW → ASSIGNED
Attachment #238150 - Flags: review?(igor.bukanov)
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 general@js.bugs pool.

/be
Assignee: brendan → general
Status: ASSIGNED → NEW
Attachment #238150 - Flags: review?(igor.bukanov) → review+
js> function() { return { x setter: 3 }; }
Assertion failure: strncmp(rval, js_function_str, 8) == 0 && rval[8] == ' ', at jsopcode.c:3191
(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
*** Bug 352986 has been marked as a duplicate of this bug. ***
Brian, could I interest you in this one?

/be
Assignee: general → crowder
Status: NEW → ASSIGNED
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?
Scratch that; my 61911 patch fixed the regexp problem.  More investigation needed.
Summary: JS engine stops execution trying to uneval an object with a regexp getter → JS engine stops execution trying to uneval an object w/ some non-function getters/setters
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?
Ok, that is now bug 355992.
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
Still need code from comment 2 to land, then mark fixed?
Whiteboard: [checkin needed]
Blocks: js1.7src
Flags: blocking1.8.1.1?
mozilla/js/src/jsobj.c 	3.295
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
OS: Mac OS X 10.4 → All
Hardware: Macintosh → All
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Checking in regress-352455.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-352455.js,v  <--  regress-352455.js
initial revision: 1.1
done
Flags: in-testsuite+
verified fixed 1.9 20061012 windows/linux
Status: RESOLVED → VERIFIED
Flags: blocking1.8.1.1? → blocking1.8.1.1+
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 on attachment 238150 [details] [diff] [review]
fix

Sorry, I missed this one in the mayhem (maybe didn't get the blocking approval mail somehow?)
Attachment #238150 - Flags: approval1.8.1.1?
Comment on attachment 238150 [details] [diff] [review]
fix

approved for 1.8 branch, a=dveditz for drivers
Attachment #238150 - Flags: approval1.8.1.1? → approval1.8.1.1+
mozilla/js/src/jsobj.c 	3.208.2.44 
Keywords: fixed1.8.1.1
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?
Probably need 61911 too?
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
You need to log in before you can comment on or make changes to this bug.