Bug 31089

Summary: Uninitialized member m_createdByParser in WebCore::ProcessingInstruction
Product: WebKit Reporter: Dan Kegel <dank>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Add mising initializer
eric: review+, eric: commit-queue-
2nd draft, with right 'reviewed by' none

Description Dan Kegel 2009-11-03 15:19:18 PST
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
Comment 1 Alexey Proskuryakov 2009-11-03 20:26:53 PST
The fix looks right to me. Would you be willing to submit it for review, as described in <http://webkit.org/coding/contributing.html>?
Comment 2 Dan Kegel 2009-11-03 22:07:38 PST
Sure, I'll do that in the morning, thanks.
Comment 3 Dan Kegel 2009-11-04 08:38:09 PST
Created attachment 42484 [details]
Add mising initializer

Here y'go!
Comment 4 Eric Seidel (no email) 2009-11-04 10:09:25 PST
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!
Comment 5 Dan Kegel 2009-11-04 12:26:59 PST
Created attachment 42511 [details]
2nd draft, with right 'reviewed by'

In case it's useful, here is the patch with the suggested change.
Comment 6 Dmitry Titov 2009-11-05 11:44:22 PST
Comment on attachment 42511 [details]
2nd draft, with right 'reviewed by'

r+, cq+, commit bot to substitute the right reviewer.
Comment 7 WebKit Commit Bot 2009-11-05 11:57:17 PST
Comment on attachment 42511 [details]
2nd draft, with right 'reviewed by'

Clearing flags on attachment: 42511

Committed r50571: <http://trac.webkit.org/changeset/50571>
Comment 8 WebKit Commit Bot 2009-11-05 11:57:23 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Lucas Forschler 2019-02-06 09:02:32 PST
Mass moving XML DOM bugs to the "DOM" Component.