Closed Bug 563243 Opened 15 years ago Closed 15 years ago

(64-bit) Invalid read / write when testcase is run in valgrind

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
blocking1.9.2 --- .14+
status1.9.2 --- .14-fixed
blocking1.9.1 --- .17+
status1.9.1 --- .17-fixed

People

(Reporter: gkw, Assigned: mrbkap)

References

Details

(4 keywords, Whiteboard: [sg:critical?][critsmash:resolved] fixed-in-tracemonkey)

Attachments

(4 files, 1 obsolete file)

valgrind log
(deleted), text/plain
Details
gdb info for an opt 1.9.2 shell build
(deleted), text/plain
Details
Proposed fix
(deleted), patch
jorendorff
: review+
Details | Diff | Splinter Review
fix for 192 анд 191
(deleted), patch
christian
: approval1.9.2.14+
christian
: approval1.9.1.17+
Details | Diff | Splinter Review
Attached file valgrind log (deleted) —
(function () { for (a in (function () { yield Array.reduce() })()) function () {} })() causes invalid reads and writes when run in a 1.9.2 64-bit Linux shell without -j on 1.9.2 tip. (Pass into 1.9.2 shell as a CLI argument, e.g. ./js a.js) A valgrind log is attached. This does not seem to occur with TM tip. Setting s-s because I'm not sure how bad this is.
This shows up as a crash when jsfunfuzz is being run.
Keywords: crash
Group: core-security
Tested on Ubuntu Linux 10.04 AMD64 on this 1.9.2 changeset: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/d1aab61eb130
Whiteboard: [sg:critical?]
Attached patch Proposed fix (deleted) — — Splinter Review
We haven't been able to assume anything about vp[2 + n] for n >= argc since 2006 or so... We don't actually fill missing args for fast natives.
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #443173 - Flags: review?(jorendorff)
Note: the changes to the slow version weren't necessary, but the two versions should probably match each other in behavior.
Attachment #443173 - Flags: review?(jorendorff) → review+
Keywords: valgrind
Whiteboard: [sg:critical?] → [sg:critical?][critsmash:patch]
When did this regress? Does it affect any of our shipping branches?
This is a regression from bug 412296. It affects all active branches (note: it does *not* affect 1.9.0).
Blocks: 412296
(In reply to comment #7) > This is a regression from bug 412296. It affects all active branches (note: it > does *not* affect 1.9.0). Strange - might just be me, but I couldn't seem to reproduce on TM tip, as per comment 0.
Keywords: regressionwindow-wanted
http://hg.mozilla.org/tracemonkey/rev/0f5867192284 Yeah, I saw that, but I've definitely reproduced it on all branches. Not sure what you're seeing.
Whiteboard: [sg:critical?][critsmash:patch] → [sg:critical?][critsmash:patch] fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/0f5867192284
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical?][critsmash:patch] fixed-in-tracemonkey → [sg:critical?][critsmash:resolved] fixed-in-tracemonkey
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
status1.9.1: ? → wanted
status1.9.2: ? → wanted
Blocks: 563133
Fixes bug 563133 for branches
blocking1.9.1: ? → .17+
blocking1.9.2: ? → .14+
Attached patch fix for 192 (obsolete) (deleted) — — Splinter Review
The patch is a trivial backport of the tm patch. Here is a plain diff between tm and this patch to confirm this: 24c24 < if (!js_ComputeThis(cx, vp + 2)) --- > if (!js_ComputeThis(cx, JS_FALSE, vp + 2)) 59,60c59,60 < @@ -4333,15 +4325,8 @@ js_generic_native_method_dispatcher(JSCo < js_GetTopStackFrame(cx)->thisv = argv[-1]; --- > @@ -4463,15 +4455,8 @@ js_generic_native_method_dispatcher(JSCo > js_GetTopStackFrame(cx)->thisp = JSVAL_TO_OBJECT(argv[-1]);
Attachment #505425 - Flags: approval1.9.2.14?
Attached patch fix for 192 анд 191 (deleted) — — Splinter Review
The previous attachment had an unrelated diff.
Attachment #505425 - Attachment is obsolete: true
Attachment #505425 - Flags: approval1.9.2.14?
Attachment #505426 - Flags: approval1.9.2.14?
Comment on attachment 505426 [details] [diff] [review] fix for 192 анд 191 The patch applies to 191 as-is
Attachment #505426 - Attachment description: fix for 192 for real → fix for 192 анд 191
Attachment #505426 - Flags: approval1.9.1.17?
Comment on attachment 505426 [details] [diff] [review] fix for 192 анд 191 Approved for 1.9.2.14 and 1.9.1.17. Please land this as soon as possible!
Attachment #505426 - Flags: approval1.9.2.14?
Attachment #505426 - Flags: approval1.9.2.14+
Attachment #505426 - Flags: approval1.9.1.17?
Attachment #505426 - Flags: approval1.9.1.17+
Group: core-security
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: