Bug 169598 – Fix uninitialized public members in NetworkProcess

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Tomas Popela]

Found by Coverity scan:

webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.h:51: member_decl: Class member declaration for "privateBrowsingEnabled".
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.cpp:39: uninit_member: Non-static class member "privateBrowsingEnabled" is not initialized in this constructor nor in any functions that it calls.
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.h:52: member_decl: Class member declaration for "cacheModel".
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.cpp:39: uninit_member: Non-static class member "cacheModel" is not initialized in this constructor nor in any functions that it calls.
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.h:54: member_decl: Class member declaration for "canHandleHTTPSServerTrustEvaluation".
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.cpp:39: uninit_member: Non-static class member "canHandleHTTPSServerTrustEvaluation" is not initialized in this constructor nor in any functions that it calls.
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.h:59: member_decl: Class member declaration for "shouldEnableNetworkCache".
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.cpp:39: uninit_member: Non-static class member "shouldEnableNetworkCache" is not initialized in this constructor nor in any functions that it calls.
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.h:60: member_decl: Class member declaration for "shouldEnableNetworkCacheEfficacyLogging".
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.cpp:39: uninit_member: Non-static class member "shouldEnableNetworkCacheEfficacyLogging" is not initialized in this constructor nor in any functions that it calls.
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.h:62: member_decl: Class member declaration for "shouldEnableNetworkCacheSpeculativeRevalidation".
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.cpp:39: uninit_member: Non-static class member "shouldEnableNetworkCacheSpeculativeRevalidation" is not initialized in this constructor nor in any functions that it calls.
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.h:74: member_decl: Class member declaration for "shouldUseTestingNetworkSession".
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.cpp:39: uninit_member: Non-static class member "shouldUseTestingNetworkSession" is not initialized in this constructor nor in any functions that it calls.
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.h:94: member_decl: Class member declaration for "cookiePersistentStorageType".
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.cpp:39: uninit_member: Non-static class member "cookiePersistentStorageType" is not initialized in this constructor nor in any functions that it calls.
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.h:95: member_decl: Class member declaration for "cookieAcceptPolicy".
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.cpp:39: uninit_member: Non-static class member "cookieAcceptPolicy" is not initialized in this constructor nor in any functions that it calls.
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.h:96: member_decl: Class member declaration for "ignoreTLSErrors".
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.cpp:39: uninit_member: Non-static class member "ignoreTLSErrors" is not initialized in this constructor nor in any functions that it calls.
#   37|   NetworkProcessCreationParameters::NetworkProcessCreationParameters()
#   38|   {
#   39|-> }
#   40|   
#   41|   void NetworkProcessCreationParameters::encode(IPC::Encoder& encoder) const


/opt/rh/devtoolset-6/root/usr/include/c++/6.2.1/chrono:373: member_decl: Class member declaration for "__r".
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp:354: uninit_member: Non-static class member field "epochRelativeTimeStamp.__r" is not initialized in this constructor nor in any functions that it calls.
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp:361: member_decl: Class member declaration for "headerSize".
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp:354: uninit_member: Non-static class member "headerSize" is not initialized in this constructor nor in any functions that it calls.
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp:363: member_decl: Class member declaration for "bodySize".
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp:354: uninit_member: Non-static class member "bodySize" is not initialized in this constructor nor in any functions that it calls.
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp:364: member_decl: Class member declaration for "isBodyInline".
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp:354: uninit_member: Non-static class member "isBodyInline" is not initialized in this constructor nor in any functions that it calls.
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp:367: member_decl: Class member declaration for "headerOffset".
webkitgtk-2.14.5/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp:354: uninit_member: Non-static class member "headerOffset" is not initialized in this constructor nor in any functions that it calls.
#  352|           : cacheStorageVersion(Storage::version)
#  353|           , key(key)
#  354|->     { }
#  355|   
#  356|       unsigned cacheStorageVersion;

[Tomas Popela]

Created attachment 304374 [details]
Patch

[Build Bot]

Comment on attachment 304374 [details]
Patch

Attachment 304374 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3320362

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 304378 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 304374 [details]
Patch

Attachment 304374 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3320418

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 304381 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Michael Catanzaro]

LGTM, needs an owner.

[WebKit Commit Bot]

Comment on attachment 304374 [details]
Patch

Clearing flags on attachment: 304374

Committed r213915: <http://trac.webkit.org/changeset/213915>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

(In reply to comment #7)
> Comment on attachment 304374 [details]
> Patch
> 
> Clearing flags on attachment: 304374
> 
> Committed r213915: <http://trac.webkit.org/changeset/213915>

This change appears to have caused LayoutTests to exit early with timeouts on WK2:

https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK2%20%28Tests%29/builds/4362
https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK2%20%28Tests%29/builds/13970
https://build.webkit.org/builders/Apple%20iOS%2010%20Simulator%20Release%20WK2%20%28Tests%29/builds/4575

[Ryan Haddad]

Reverted r213915 for reason:

Caused WK2 LayoutTests to exit early with timeouts.

Committed r213927: <http://trac.webkit.org/changeset/213927>

[Michael Catanzaro]

Alexey asked in bug #169603#c4:

(In reply to comment #4)
> Comment on attachment 304377 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=304377&action=review
> 
> > Source/WebKit2/NetworkProcess/NetworkProcessCreationParameters.h:52
> > +    bool privateBrowsingEnabled { false };
> 
> Can any of the creation parameters actually be uninitialized? Using default
> values seems as much of a problem as using uninitialised one - perhaps even
> worse, as UBSan won't catch that.

Yes, in fact there already exist NetworkProcessCreationParameters members that are never initialized. Valgrind always complains when starting WebKitGTK+ because we always pass uninitialized memory into the kernel at startup via NetworkProcessCreationParameters. I guess/hope this patch should fix that. See bug #146729#c5.

You have a point that it might be useful to force callers to initialize all data members, but if so then it should be enforced at compile time by adding a constructor. It's much easier and safer to placate the static analyzer by ensuring objects cannot be created with uninitialized data members than it is to check for such issues at runtime. But the counterargument is that having a constructor with tons of parameters for a Parameters object might be crazy.

[Tomas Popela]

Created attachment 304763 [details]
Patch

Moving patch here from bug 169604, where is was reviwed by Alex Christensen.

[Tomas Popela]

Comment on attachment 304763 [details]
Patch

Clearing flags on attachment: 304763

Committed r214101: <http://trac.webkit.org/changeset/214101>

[Tomas Popela]

All reviewed patches have been landed.  Closing bug.