Closed Bug 1259843 Opened 8 years ago Closed 8 years ago

Destructor bodies can be treated as empty

Categories

(Core :: JavaScript: GC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: sfink, Assigned: sfink)

Details

(Keywords: sec-audit, Whiteboard: [post-critsmash-triage][adv-main48-])

Attachments

(2 files, 1 obsolete file)

Sadly, when I "implemented" MUST_NOT_THROW_EXPR, I thought it was just an ignorable annotation. It turns out that it's a *wrapper* around the body of a method, and at least in gcc 4.9 it's used for destructors (presumably destructors that cannot throw?)

Which means that all such destructors are treated as having empty bodies, and therefore will never GC.

This is obviously not correct.

I don't think there should be too many hazards exposed by this, because I think this should only have been in effect since upgrading gcc, but I'm going to mark this s-s for now.
Update to version of sixgill that pays attention to the body.
Assignee: nobody → sphink
Status: NEW → ASSIGNED
Attachment #8734929 - Attachment is obsolete: true
Comment on attachment 8734930 [details] [diff] [review]
Handle MUST_NOT_THROW_EXPR correctly (it is just a wrapper around the real expr)

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

Argh. Typed |hg bzexport| when I intended |hg export|. And yet, it was better than what I was trying to do. Oddly, bugzilla barfed on a direct bzexport, which was why I was doing the 2-step:

error uploading attachment: REST error on POST to https://bugzilla.mozilla.org/rest/bug/1259843/attachment: You asked Terrence Cole [:terrence] <terrence@mozilla.com> for review on bug 1259843, attachment 8734928, but that bug has been restricted to users in certain groups, and the user you asked isn't in all the groups to which the bug has been restricted. Please choose someone else to ask, or make the bug accessible to users on its CC: list and add that user to the list.
Attachment #8734930 - Flags: review?(terrence)
Huh. Same error with the web interface. Adding :terrence to CC list "fixed" it.

I tried switching it from general s-s to JS s-s, but it won't let me remove general s-s.
Group: core-security → javascript-core-security
Attachment #8734930 - Flags: review?(terrence) → review+
https://hg.mozilla.org/mozilla-central/rev/719ded1e521d
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Group: javascript-core-security → core-security-release
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main48-]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: