1009289 - nsGlobalWindow cycle collection instrumentation has a printf type error

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

On 2012-08-03, this was added to dom/base/nsGlobalWindow.cpp

+    PR_snprintf(name, sizeof(name), "nsGlobalWindow #%ld", tmp->mWindowID);

This is bad, because mWindowID is a uint64_t, and %ld means int32_t (in NSPR's nonstandard printf dialect, but long int is also not uint64_t).
 
On 2014-05-09, it got worse:

+    PR_snprintf(name, sizeof(name), "nsGlobalWindow #%ld %s %s",
+                tmp->mWindowID, tmp->IsInnerWindow() ? "inner" : "outer",
+                uri.get());

On Linux on i386, this breaks silently, as long as tmp->mWindowID < 0x100000000, because uint64_t is 4-byte-aligned: the less significant half of mWindowID is printed, the most significant is 0 and thus prints as "(null)", the "inner" or "outer" gets printed as the second string, and the URI never shows up.

On Linux on ARM, this is a crash if tmp->mWindowID != 0, because uint64_t is 8-byte-aligned and it's passed after three 4-byte arguments: the alignment padding gets printed as the window ID, and then both halves of the actual window ID are treated as pointers.

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: bug1009289-printf-oops-hg0.diff

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Trying: https://tbpl.mozilla.org/?tree=Try&rev=6a775aaf4f46

Comment 3

[Andrew McCreight [:mccr8]]

Comment on attachment 8421445 [details] [diff] [review]
bug1009289-printf-oops-hg0.diff

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

Oops indeed...

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/439e73a0a4ba

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/439e73a0a4ba