Uninitialised value use in AutoJSExceptionReporter::~AutoJSExceptionReporter

RESOLVED FIXED in Firefox 38

Status

()

Core
Plug-ins
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jseward, Assigned: jseward)

Tracking

Trunk
mozilla40
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox37 unaffected, firefox38 fixed, firefox38.0.5 fixed, firefox39 fixed, firefox40 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
It appears that ~AutoJSExceptionReporter can check its mIsDestroyPending
field with that ever having been initialised.  Looking at the constructor
AutoJSExceptionReporter::AutoJSExceptionReporter, mIsDestroyPending is
simply copied from nsJSObjWrapper::mDestroyPending.  It may be that the
root error is that nsJSObjWrapper::nsJSObjWrapper doesn't set mDestroyPending.

Possibly related to bug 1135491.
(Assignee)

Comment 1

2 years ago
STR: test path is dom/base/test/csp/test_CSP.html

It's pretty easy to demonstrate, though, happens on quite a lot
of runs.

I ran this:

DISPLAY=:1.0 G_SLICE=always-malloc MOZ_DISABLE_NONLOCAL_CONNECTIONS=0 \
  ./mach mochitest-plain --e10s \
  --debugger=/home/sewardj/VgTRUNK/mozhx/Inst/bin/valgrind \
  --debugger-args="--fair-sched=yes --smc-check=all-non-file \
    --suppressions=/home/sewardj/MOZ/SUPPS/mochitest-mc.supp --error-limit=no \
    --trace-children=yes --child-silent-after-fork=yes \
    --trace-children-skip=/usr/bin/hg,/bin/rm,*/bin/certutil,*/bin/pk12util,*/bin/ssltunnel,*/bin/uname,*/bin/which,*/bin/ps,*/bin/grep,*/bin/java \
    --num-transtab-sectors=24 --tool=memcheck --freelist-vol=500000000 \
    --redzone-size=256 --gen-suppressions=no \
    --px-default=allregs-at-mem-access \
    --px-file-backed=unwindregs-at-mem-access --stats=yes \
    --partial-loads-ok=yes --show-mismatched-frees=no --read-inline-info=yes \
    --fullpath-after=-CURR/ --num-callers=16 --track-origins=yes" \
  dom/base/test/csp/test_CSP.html 2>&1 | tee logfile-05-mc
(Assignee)

Comment 2

2 years ago
Created attachment 8590752 [details]
Valgrind complainage
(Assignee)

Comment 3

2 years ago
Created attachment 8590757 [details] [diff] [review]
bug1153173-1.diff

A simple fix.
(Assignee)

Updated

2 years ago
Attachment #8590757 - Flags: review?(aklotz)
Comment on attachment 8590757 [details] [diff] [review]
bug1153173-1.diff

Review of attachment 8590757 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for catching this!
Attachment #8590757 - Flags: review?(aklotz) → review+
Blocks: 1116806
(Assignee)

Comment 5

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce79d0eb8636
Comment on attachment 8590757 [details] [diff] [review]
bug1153173-1.diff

Approval Request Comment
[Feature/regressing bug #]: bug 1135491
[User impact if declined]: Possibility of incorrect behavior of scripted plugin objects.
[Describe test coverage new/current, TreeHerder]: Caught by dom/base/test/csp/test_CSP.html + Valgrind
[Risks and why]: None, trivial patch that fixes an uninitialized variable
[String/UUID change made/needed]: None
Attachment #8590757 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/ce79d0eb8636
Assignee: nobody → jseward
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
(In reply to Aaron Klotz [:aklotz] (please use needinfo) from comment #6)
> [Feature/regressing bug #]: bug 1135491

Bug 1135491 was fixed in 38. Do we need to take the fix in 38 as well?
status-firefox37: --- → unaffected
status-firefox38: --- → affected
status-firefox39: --- → affected
Flags: needinfo?(aklotz)
Comment on attachment 8590757 [details] [diff] [review]
bug1153173-1.diff

Aurora+
Attachment #8590757 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Good point. I'd recommend that we do.
Flags: needinfo?(aklotz)
Comment on attachment 8590757 [details] [diff] [review]
bug1153173-1.diff

Approval Request Comment
[Feature/regressing bug #]: bug 1135491
[User impact if declined]: Possibility of incorrect behavior of scripted plugin objects.
[Describe test coverage new/current, TreeHerder]: Caught by dom/base/test/csp/test_CSP.html + Valgrind
[Risks and why]: None, trivial patch that fixes an uninitialized variable
[String/UUID change made/needed]: None
Attachment #8590757 - Flags: approval-mozilla-beta?
Comment on attachment 8590757 [details] [diff] [review]
bug1153173-1.diff

Should be in 38 beta 7.
Attachment #8590757 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8590757 [details] [diff] [review]
bug1153173-1.diff

[Triage Comment]
uplift to m-r as we did the 38 merge already
Attachment #8590757 - Flags: approval-mozilla-beta+ → approval-mozilla-release+
https://hg.mozilla.org/releases/mozilla-aurora/rev/22868e71c861
status-firefox39: affected → fixed
https://hg.mozilla.org/releases/mozilla-release/rev/92fb098ace7a
status-firefox38: affected → fixed
https://hg.mozilla.org/releases/mozilla-beta/rev/92fb098ace7a

Updated

2 years ago
Duplicate of this bug: 1158398
status-firefox38.0.5: --- → fixed
You need to log in before you can comment on or make changes to this bug.