Bug 186763 – bmalloc: Coverity scan issues

• Component: bmalloc
• Status: RESOLVED FIXED

[Tomas Popela]

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)

[Tomas Popela]

Created attachment 342932 [details]
Patch

[EWS Watchlist]

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

[EWS Watchlist]

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

[Geoffrey Garen]

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.

[Tomas Popela]

(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.

[Tomas Popela]

Created attachment 343022 [details]
Patch

[EWS Watchlist]

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

[EWS Watchlist]

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

[Tomas Popela]

Geoffrey ca you please look on the updated patch?

[Saam Barati]

Comment on attachment 343022 [details]
Patch

This seems reasonable to me.

[Tomas Popela]

Comment on attachment 343022 [details]
Patch

Clearing flags on attachment: 343022

Committed r234913: <https://trac.webkit.org/changeset/234913>

[Tomas Popela]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/43370264>