Bug 31089 – Uninitialized member m_createdByParser in WebCore::ProcessingInstruction

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Dan Kegel]

Caught by the chromium valgrind layout bot.  Valgrind complains

Warning: set address range perms: large range [0x3901d000, 0x5901d000) (defined)
Conditional jump or move depends on uninitialised value(s)
   at WebCore::Document::addStyleSheetCandidateNode(WebCore::Node*, bool) (Document.cpp:2411)
   by WebCore::ProcessingInstruction::insertedIntoDocument() (ProcessingInstruction.cpp:262)
   by WebCore::dispatchChildInsertionEvents(WebCore::Node*) (ContainerNode.cpp:876)
   ...
 Uninitialised value was created by a heap allocation
   at 0x4C2312C: malloc (vg_replace_malloc.c:195)
   by 0x13B3CDA: WTF::fastMalloc(unsigned long) (FastMalloc.cpp:233)
   by 0x8B6AEE: WTF::FastAllocBase::operator new(unsigned long) (FastAllocBase.h:96)
   by 0xC1528A: WebCore::ProcessingInstruction::create(WebCore::Document*, WebCore::String const&, WebCore::String const&) (ProcessingInstruction.cpp:53)

Looks like somebody forgot to initialize m_createdByParser.  The following patch seems to resolve the problem:

--- WebCore/dom/ProcessingInstruction.cpp    (revision 50454)
+++ WebCore/dom/ProcessingInstruction.cpp    (working copy)
@@ -42,6 +42,7 @@
     , m_cachedSheet(0)
     , m_loading(false)
     , m_alternate(false)
+    , m_createdByParser(false)
 #if ENABLE(XSLT)
     , m_isXSL(false)
 #endif

[Alexey Proskuryakov]

The fix looks right to me. Would you be willing to submit it for review, as described in <http://webkit.org/coding/contributing.html>?

[Dan Kegel]

Sure, I'll do that in the morning, thanks.

[Dan Kegel]

Created attachment 42484 [details]
Add mising initializer

Here y'go!

[Eric Seidel (no email)]

Comment on attachment 42484 [details]
Add mising initializer

You can leave the "Reviewed by NOBODY (OOPS!)." line alone and our scripts will fill it in automatically with Alexey's full name when landing.  It would be better for you to paste an un-altered NOBODY line, or to correct that to say "Alexey Proskuryakov." (with a period), before we land this.

LGTM, and someone could land this manually with that fix, but we should get the ChangeLog completely right (either left as NOBODY (OOPS!) or changed to Alexey's full name with a period) if we want the commit-queue to land this.

Thanks for the patch!

[Dan Kegel]

Created attachment 42511 [details]
2nd draft, with right 'reviewed by'

In case it's useful, here is the patch with the suggested change.

[Dmitry Titov]

Comment on attachment 42511 [details]
2nd draft, with right 'reviewed by'

r+, cq+, commit bot to substitute the right reviewer.

[WebKit Commit Bot]

Comment on attachment 42511 [details]
2nd draft, with right 'reviewed by'

Clearing flags on attachment: 42511

Committed r50571: <http://trac.webkit.org/changeset/50571>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.