[Chromium]Android x86 content shell debug build warning for uninitialized value used as error with gcc 4.6

• Component: Web Template Framework
• Status: RESOLVED FIXED

[Wei James (wistoch)]

[Chromium]Debug build warning for uninitialized value used

[Wei James (wistoch)]

Created attachment 167505 [details]
Patch

[Wei James (wistoch)]

In file included from ../../third_party/WebKit/Source/WTF/wtf/HashSet.h:25:0,
                 from ../../third_party/WebKit/Source/WTF/wtf/ListHashSet.h:25,
                 from ../../third_party/WebKit/Source/WebCore/dom/DocumentStyleSheetCollection.h:32,
                 from ../../third_party/WebKit/Source/WebCore/rendering/RenderObject.h:30,
                 from ../../third_party/WebKit/Source/WebCore/rendering/RenderLayerModelObject.h:26,
                 from ../../third_party/WebKit/Source/WebCore/rendering/RenderBoxModelObject.h:28,
                 from ../../third_party/WebKit/Source/WebCore/rendering/RenderBox.h:26,
                 from ../../third_party/WebKit/Source/WebCore/rendering/RenderBlock.h:29,
                 from ../../third_party/WebKit/Source/WebCore/rendering/RenderTable.h:30,
                 from ../../third_party/WebKit/Source/WebCore/rendering/RenderTableSection.h:28,
                 from ../../third_party/WebKit/Source/WebCore/rendering/RenderTableSection.cpp:27:
../../third_party/WebKit/Source/WTF/wtf/HashFunctions.h: In member function 'void WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ch
pTranslator<WTF::HashMapValueTraits<WTF::HashTraits<std::pair<const WebCore::RenderTableCell*, int> >, WTF::HashTraits<WebCore::CollapsedBorderValue> >, WTF:
T = std::pair<const WebCore::RenderTableCell*, int>, Key = std::pair<const WebCore::RenderTableCell*, int>, Value = WTF::KeyValuePair<std::pair<const WebCore
alue>, Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<std::pair<const WebCore::RenderTableCell*, int>, WebCore::CollapsedBorderValue> >, HashFun
ll*, int>, Traits = WTF::HashMapValueTraits<WTF::HashTraits<std::pair<const WebCore::RenderTableCell*, int> >, WTF::HashTraits<WebCore::CollapsedBorderValue>
Core::RenderTableCell*, int> >]':
../../third_party/WebKit/Source/WTF/wtf/HashFunctions.h:158:115: error: 'deletedValueBuffer.WTF::KeyValuePair<std::pair<const WebCore::RenderTableCell*, int>
onst WebCore::RenderTableCell*, int>::second' may be used uninitialized in this function [-Werror=uninitialized]
../../third_party/WebKit/Source/WTF/wtf/HashTable.h:589:67: note: 'deletedValueBuffer.WTF::KeyValuePair<std::pair<const WebCore::RenderTableCell*, int>, WebC
ebCore::RenderTableCell*, int>::second' was declared here
../../third_party/WebKit/Source/WTF/wtf/HashFunctions.h: In member function 'void WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ch
tyHashTranslator<WTF::PairHash<const WebCore::RenderTableCell*, int> >, T = std::pair<const WebCore::RenderTableCell*, int>, Key = std::pair<const WebCore::R
d::pair<const WebCore::RenderTableCell*, int>, WebCore::CollapsedBorderValue>, Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<std::pair<const We
rderValue> >, HashFunctions = WTF::PairHash<const WebCore::RenderTableCell*, int>, Traits = WTF::HashMapValueTraits<WTF::HashTraits<std::pair<const WebCore::
CollapsedBorderValue> >, KeyTraits = WTF::HashTraits<std::pair<const WebCore::RenderTableCell*, int> >]':
../../third_party/WebKit/Source/WTF/wtf/HashFunctions.h:158:115: error: 'deletedValueBuffer.WTF::KeyValuePair<std::pair<const WebCore::RenderTableCell*, int>
onst WebCore::RenderTableCell*, int>::second' may be used uninitialized in this function [-Werror=uninitialized]
../../third_party/WebKit/Source/WTF/wtf/HashTable.h:589:67: note: 'deletedValueBuffer.WTF::KeyValuePair<std::pair<const WebCore::RenderTableCell*, int>, WebC
ebCore::RenderTableCell*, int>::second' was declared here
cc1plus: all warnings being treated as errors

[Wei James (wistoch)]

Created attachment 167509 [details]
Patch

[Benjamin Poulain]

Comment on attachment 167509 [details]
Patch

memset is a bad idea performance wise for small data structures.

I also don't see why this needs to be initialized outside it's trait's constructDeletedValue. Zeroing memory is not free.

[Wei James (wistoch)]

(In reply to comment #4)
> (From update of attachment 167509 [details])
> memset is a bad idea performance wise for small data structures.
> 
> I also don't see why this needs to be initialized outside it's trait's constructDeletedValue. Zeroing memory is not free.

benjamin, thanks for the review. 

further investigation shows that this building warning only appears in android x86 gcc 4.6 debug build. 

I can build it successfully with x86 gcc 4.4.3 and gcc 4.6 works well for arm platform. 

So i suspect it is an issue of x86 gcc instead of chromium code. 

So i added -Wno-uninitialized into cflags for webcore_rendering for andorid x86 with gcc 4.6. 

is it ok? thanks

[Wei James (wistoch)]

Created attachment 167903 [details]
Patch

[Benjamin Poulain]

> So i suspect it is an issue of x86 gcc instead of chromium code. 
> 
> So i added -Wno-uninitialized into cflags for webcore_rendering for andorid x86 with gcc 4.6. 
> 
> is it ok? thanks

Yes, it is ok.
It is not the first time we have to workaround compiler bugs. You probably need to mention it is a compiler bug in the comment, otherwise it looks like you disable a warning instead of fixing the bug.

This becomes a Chromium-Android fix, I am not the right person to accept this in the tree.

[Wei James (wistoch)]

Created attachment 167924 [details]
Patch

[Wei James (wistoch)]

(In reply to comment #7)
> > So i suspect it is an issue of x86 gcc instead of chromium code. 
> > 
> > So i added -Wno-uninitialized into cflags for webcore_rendering for andorid x86 with gcc 4.6. 
> > 
> > is it ok? thanks
> 
> Yes, it is ok.
> It is not the first time we have to workaround compiler bugs. You probably need to mention it is a compiler bug in the comment, otherwise it looks like you disable a warning instead of fixing the bug.
> 
> This becomes a Chromium-Android fix, I am not the right person to accept this in the tree.

thanks!

abarth, peter, 

could you help to review it? it blocks the building of chromium android x86. thanks

[Adam Barth]

Comment on attachment 167924 [details]
Patch

This seems fine to me.

[WebKit Review Bot]

Comment on attachment 167924 [details]
Patch

Clearing flags on attachment: 167924

Committed r130925: <http://trac.webkit.org/changeset/130925>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.