Bug 169598 - Fix uninitialized public members in NetworkProcess
Summary: Fix uninitialized public members in NetworkProcess
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tomas Popela
URL:
Keywords:
Depends on:
Blocks: 104114
  Show dependency treegraph
 
Reported: 2017-03-14 06:33 PDT by Tomas Popela
Modified: 2017-03-17 02:46 PDT (History)
11 users (show)

See Also:


Attachments
Patch (6.19 KB, patch)
2017-03-14 06:37 PDT, Tomas Popela
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (1.52 MB, application/zip)
2017-03-14 07:37 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (139.28 MB, application/zip)
2017-03-14 08:03 PDT, Build Bot
no flags Details
Patch (3.91 KB, patch)
2017-03-17 00:31 PDT, Tomas Popela
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tomas Popela 2017-03-14 06:33:50 PDT
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;
Comment 1 Tomas Popela 2017-03-14 06:37:08 PDT
Created attachment 304374 [details]
Patch
Comment 2 Build Bot 2017-03-14 07:37:19 PDT
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.
Comment 3 Build Bot 2017-03-14 07:37:24 PDT
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
Comment 4 Build Bot 2017-03-14 08:03:04 PDT
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.
Comment 5 Build Bot 2017-03-14 08:03:12 PDT
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
Comment 6 Michael Catanzaro 2017-03-14 08:10:25 PDT
LGTM, needs an owner.
Comment 7 WebKit Commit Bot 2017-03-14 10:39:31 PDT
Comment on attachment 304374 [details]
Patch

Clearing flags on attachment: 304374

Committed r213915: <http://trac.webkit.org/changeset/213915>
Comment 8 WebKit Commit Bot 2017-03-14 10:39:36 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Ryan Haddad 2017-03-14 12:16:55 PDT
Reverted r213915 for reason:

Caused WK2 LayoutTests to exit early with timeouts.

Committed r213927: <http://trac.webkit.org/changeset/213927>
Comment 11 Michael Catanzaro 2017-03-15 08:29:34 PDT
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.
Comment 12 Tomas Popela 2017-03-17 00:31:01 PDT
Created attachment 304763 [details]
Patch

Moving patch here from bug 169604, where is was reviwed by Alex Christensen.
Comment 13 Tomas Popela 2017-03-17 02:46:03 PDT
Comment on attachment 304763 [details]
Patch

Clearing flags on attachment: 304763

Committed r214101: <http://trac.webkit.org/changeset/214101>
Comment 14 Tomas Popela 2017-03-17 02:46:15 PDT
All reviewed patches have been landed.  Closing bug.