Closed Bug 87271 Opened 23 years ago Closed 23 years ago

Useless Expression Elimination eliminating useful expression

Categories

(Core :: JavaScript Engine, defect, P3)

x86
All
defect

Tracking

()

VERIFIED FIXED
mozilla0.9.4

People

(Reporter: gh221, Assigned: brendan)

Details

(Keywords: js1.5)

Attachments

(1 file)

* TITLE/SUMMARY Useless Expression Elimination eliminating useful expression. * STEPS TO REPRODUCE Compile and run the program at the bottom of this report. * WORKAROUND I haven't found any scripts to evaluate which have the issue other than plain numbers. To convert a string containing a number into a number jsval, use JS_ValueToNumber on a JSVAL_IS_STRING jsval containing your number. Thanks to Brendan Eich for pointing that out. * ACTUAL RESULT Outputs "16", then "undefined", and finally "16". * EXPECTED RESULT Output of "16" three times. * CONFIGURATIONS TESTED - Windows 2000 (compiled with Visual C++ 6 SP5) - Debian Linux (compiled with G++ 2.95.2, GNU Make 3.79.1) * CODE AFFECTED BY BUG JSRuntime* rt; JSContext* cx; JSObject* global; JSClass global_class = { "global",JSCLASS_HAS_PRIVATE, JS_PropertyStub,JS_PropertyStub,JS_PropertyStub,JS_PropertyStub, JS_EnumerateStub,JS_ResolveStub,JS_ConvertStub,JS_FinalizeStub }; static void testEval() { jsval rval; if (JS_EvaluateScript(cx,global,"16",2,"",0,&rval)) printf("%s\n",JS_GetStringBytes(JS_ValueToString(cx,rval))); } static JSBool xjsx(JSContext* cx, JSObject* obj, uintN argc, jsval* argv, jsval* rval) { testEval(); // Outputs "undefined" -- Bug return JS_TRUE; } int main(int argc, char **argv) { jsval rval; rt = JS_NewRuntime(0xFFFFFFFF); cx = JS_NewContext(rt, 0x20000); global = JS_NewObject(cx, &global_class, NULL, NULL); JS_InitStandardClasses(cx, global); JS_DefineFunction(cx, global, "x", ::xjsx, 0, 0); testEval(); // Outputs 16 JS_EvaluateScript(cx,global,"x()",3,"",0,&rval); testEval(); // Outputs 16 JS_DestroyContext(cx); JS_DestroyRuntime(rt); return 0; }
Formally confirming bug for consideration -
Status: UNCONFIRMED → NEW
Ever confirmed: true
The problem is that code-generation code looks at the top frame on the given cx to decide whether an otherwise useless result might be wanted (by the caller of JS_EvaluateScript, who passed &rval, e.g.). If, as in Grant's case, the top frame is from a native function that calls JS_EvaluateScript, then the current test for usefulness (http://lxr.mozilla.org/mozilla/source/js/src/jsemit.c#2148) will result in false. The compiler (parser and code generator) reuse topmost frames that seem suitable rather than pushing their own, mostly-zeroes, compilation frames. If we pushed such a frame, the bug would be fixed (because !cx->fp->fun would then be true). Alternative ideas involving setting a bit in cx->fp->special seem like hacky and risky to me. More in a bit; comments welcome. /be
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.3
Targeting. /be
Keywords: js1.5, mozilla0.9.4
Target Milestone: mozilla0.9.3 → mozilla0.9.4
One-line fix coming up. /be
Attached patch proposed fix, see comment (deleted) — Splinter Review
Review craved. The fix was easier than I had thought. The alternative of pushing a dummy frame, besides being less efficient, requires care in seeding the dummy frame's non-null members from the top-most frame on cx, so that code such as that in LookupArgOrVar (jsemit.c) still works properly. Grant, please test this patch and report back, if you can. Thanks. khanson, rogerl: r= welcome. shaver, jband: feel free to sr=! /be
Keywords: patch
With this patch applied, the code given with the bug report now works. Also, I note no negative effect with any of my code that relies on jseng. Looks like a good fix. Thanks Brendan.
r=rogerl
sr=jband
Fix is in. /be
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Marking Verified -
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: