Bug 186763 - bmalloc: Coverity scan issues
Summary: bmalloc: Coverity scan issues
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: bmalloc (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tomas Popela
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-06-18 06:57 PDT by Tomas Popela
Modified: 2018-08-16 00:42 PDT (History)
4 users (show)

See Also:


Attachments
Patch (2.59 KB, patch)
2018-06-18 07:01 PDT, Tomas Popela
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews206 for win-future (12.75 MB, application/zip)
2018-06-18 08:58 PDT, EWS Watchlist
no flags Details
Patch (2.61 KB, patch)
2018-06-18 22:54 PDT, Tomas Popela
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews123 for ios-simulator-wk2 (2.35 MB, application/zip)
2018-06-19 02:16 PDT, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tomas Popela 2018-06-18 06:57:39 PDT
Defect type: NEGATIVE_RETURNS
1. webkitgtk-2.20.3/Source/bmalloc/bmalloc/bmalloc.cpp:46: negative_return_fn: Function "bmalloc::vmPageSize()" returns a negative number.
3. webkitgtk-2.20.3/Source/bmalloc/bmalloc/VMAllocate.h:60:9: negative_return: Calling "sysconf", which might return a negative value.
4. webkitgtk-2.20.3/Source/bmalloc/bmalloc/VMAllocate.h:60:9: var_assign: Assigning: "cached" = "sysconf(_SC_PAGESIZE)", which might be negative.
5. webkitgtk-2.20.3/Source/bmalloc/bmalloc/VMAllocate.h:61:5: return_negative_variable: Explicitly returning negative variable "cached".
6. webkitgtk-2.20.3/Source/bmalloc/bmalloc/bmalloc.cpp:46: var_assign: Assigning: unsigned variable "pageSize" = "vmPageSize".
7. webkitgtk-2.20.3/Source/bmalloc/bmalloc/bmalloc.cpp:47: negative_returns: "pageSize" is passed to a parameter that cannot be negative.
8. webkitgtk-2.20.3/Source/bmalloc/bmalloc/Algorithm.h:73:58: sizet: "divisor" is a size_t parameter.
#    45|   
#    46|       size_t pageSize = vmPageSize();
#    47|->     alignment = roundUpToMultipleOf(pageSize, alignment);
#    48|       size = roundUpToMultipleOf(pageSize, size);
#    49|   

Defect type: NEGATIVE_RETURNS 
20. webkitgtk-2.20.3/Source/bmalloc/bmalloc/IsoTLS.cpp:98: negative_return_fn: Function "bmalloc::vmPageSize()" returns a negative number.
22. webkitgtk-2.20.3/Source/bmalloc/bmalloc/VMAllocate.h:60:9: negative_return: Calling "sysconf", which might return a negative value.
23. webkitgtk-2.20.3/Source/bmalloc/bmalloc/VMAllocate.h:60:9: var_assign: Assigning: "cached" = "sysconf(_SC_PAGESIZE)", which might be negative.
24. webkitgtk-2.20.3/Source/bmalloc/bmalloc/VMAllocate.h:61:5: return_negative_variable: Explicitly returning negative variable "cached".
25. webkitgtk-2.20.3/Source/bmalloc/bmalloc/IsoTLS.cpp:98: negative_returns: "bmalloc::vmPageSize()" is passed to a parameter that cannot be negative.
26. webkitgtk-2.20.3/Source/bmalloc/bmalloc/Algorithm.h:73:58: sizet: "divisor" is a size_t parameter.
#    96|       if (!tls || requiredCapacity > tls->m_capacity) {
#    97|           size_t requiredSize = sizeForCapacity(requiredCapacity);
#    98|->         size_t goodSize = roundUpToMultipleOf(vmPageSize(), requiredSize);
#    99|           size_t goodCapacity = capacityForSize(goodSize);
#   100|           void* memory = vmAllocate(goodSize);

Defect type: CHECKED_RETURN 
1. webkitgtk-2.20.3/Source/bmalloc/bmalloc/IsoTLS.cpp:69: check_return: Calling "pthread_key_create" without checking return value (as is done elsewhere 5 out of 6 times).
2. webkitgtk-2.20.3/Source/ThirdParty/ANGLE/src/common/tls.cpp:59: example_checked: Example 1: "pthread_key_create(&index, NULL)" has its value checked in "pthread_key_create(&index, NULL) != 0".
3. webkitgtk-2.20.3/Source/WTF/wtf/ThreadSpecific.h:139: example_assign: Example 2: Assigning: "error" = return value from "pthread_key_create(key, destructor)".
4. webkitgtk-2.20.3/Source/WTF/wtf/ThreadSpecific.h:140: example_checked: Example 2 (cont.): "error" has its value checked in "error".
5. webkitgtk-2.20.3/Source/WTF/wtf/ThreadSpecific.h:164: example_assign: Example 3: Assigning: "error" = return value from "pthread_key_create(&this->m_key, destroy)".
6. webkitgtk-2.20.3/Source/WTF/wtf/ThreadSpecific.h:165: example_checked: Example 3 (cont.): "error" has its value checked in "error".
7. webkitgtk-2.20.3/Source/bmalloc/bmalloc/PerThread.h:102: example_assign: Example 4: Assigning: "error" = return value from "pthread_key_create(&bmalloc::PerThreadStorage<bmalloc::PerHeapKind<bmalloc::Cache> >::s_key, this->destructor)".
8. webkitgtk-2.20.3/Source/bmalloc/bmalloc/PerThread.h:103: example_checked: Example 4 (cont.): "error" has its value checked in "error".
9. webkitgtk-2.20.3/x86_64-redhat-linux-gnu/DerivedSources/ForwardingHeaders/wtf/ThreadSpecific.h:164: example_assign: Example 5: Assigning: "error" = return value from "pthread_key_create(&this->m_key, destroy)".
10. webkitgtk-2.20.3/x86_64-redhat-linux-gnu/DerivedSources/ForwardingHeaders/wtf/ThreadSpecific.h:165: example_checked: Example 5 (cont.): "error" has its value checked in "error".
#    67|               pthread_key_init_np(tlsKey, destructor);
#    68|   #else
#    69|->             pthread_key_create(&s_tlsKey, destructor);
#    70|               s_didInitialize = true;
#    71|   #endif

Defect type: UNINIT_CTOR 
1. webkitgtk-2.20.3/Source/bmalloc/bmalloc/DebugHeap.h:56: member_decl: Class member declaration for "m_pageSize".
2. webkitgtk-2.20.3/Source/bmalloc/bmalloc/DebugHeap.cpp:79: uninit_member: Non-static class member "m_pageSize" is not initialized in this constructor nor in any functions that it calls.
#    77|   DebugHeap::DebugHeap(std::lock_guard<StaticMutex>&)
#    78|   {
#    79|-> }
#    80|   
#    81|   void* DebugHeap::malloc(size_t size)
Comment 1 Tomas Popela 2018-06-18 07:01:23 PDT
Created attachment 342932 [details]
Patch
Comment 2 EWS Watchlist 2018-06-18 08:58:21 PDT
Comment on attachment 342932 [details]
Patch

Attachment 342932 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/8232114

New failing tests:
http/tests/preload/onload_event.html
Comment 3 EWS Watchlist 2018-06-18 08:58:33 PDT
Created attachment 342942 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 4 Geoffrey Garen 2018-06-18 12:50:35 PDT
Comment on attachment 342932 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342932&action=review

> Source/bmalloc/bmalloc/VMAllocate.h:61
> +        cached = pageSize < 0 ? smallPageSize : pageSize;

This is not a meaningful default. You probably need to crash here too.
Comment 5 Tomas Popela 2018-06-18 22:50:32 PDT
(In reply to Geoffrey Garen from comment #4)
> > Source/bmalloc/bmalloc/VMAllocate.h:61
> > +        cached = pageSize < 0 ? smallPageSize : pageSize;
> 
> This is not a meaningful default. You probably need to crash here too.

Ok, makes sense.
Comment 6 Tomas Popela 2018-06-18 22:54:53 PDT
Created attachment 343022 [details]
Patch
Comment 7 EWS Watchlist 2018-06-19 02:16:09 PDT
Comment on attachment 343022 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/FileAPI/blob/Blob-constructor.html
Comment 8 EWS Watchlist 2018-06-19 02:16:10 PDT
Created attachment 343032 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 9 Tomas Popela 2018-07-25 00:13:13 PDT
Geoffrey ca you please look on the updated patch?
Comment 10 Saam Barati 2018-08-15 15:33:39 PDT
Comment on attachment 343022 [details]
Patch

This seems reasonable to me.
Comment 11 Tomas Popela 2018-08-16 00:41:52 PDT
Comment on attachment 343022 [details]
Patch

Clearing flags on attachment: 343022

Committed r234913: <https://trac.webkit.org/changeset/234913>
Comment 12 Tomas Popela 2018-08-16 00:41:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Radar WebKit Bug Importer 2018-08-16 00:42:24 PDT
<rdar://problem/43370264>