Title: [250325] trunk/Source/bmalloc
- Revision
- 250325
- Author
- [email protected]
- Date
- 2019-09-24 17:13:59 -0700 (Tue, 24 Sep 2019)
Log Message
Address static analysis warning in Allocator.cpp: Null pointer argument in call to memory copy function
https://bugs.webkit.org/show_bug.cgi?id=202152
<rdar://problem/55671444>
Reviewed by Geoffrey Garen.
Xcode's static analysis facility flags the following:
.../OpenSource/Source/bmalloc/bmalloc/Allocator.cpp:98:5: warning: Null pointer argument in call to memory copy function
memcpy(result, object, copySize);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
https://en.cppreference.com/w/cpp/string/byte/memcpy explains that
this is undefined behavior:
"If either dest or src is a null pointer, the behavior is
undefined, even if count is zero."
I suppose that passing in a null source pointer could be bad if the
implementation fetched the first source byte to copy before actually
checking the number of bytes to copy. So far, it doesn’t seem to be an
issue, but we should clean this up.
Simply adding "if (result && object)" before the memcpy will add tests
and branches in the hot path of this function and so might not be the
best solution. Instead, straighten out the code a little bit by
putting an early check and return on "object". This also allows us to
remove some intermediate code.
* bmalloc/Allocator.cpp:
(bmalloc::Allocator::reallocateImpl):
Modified Paths
Diff
Modified: trunk/Source/bmalloc/ChangeLog (250324 => 250325)
--- trunk/Source/bmalloc/ChangeLog 2019-09-24 23:54:48 UTC (rev 250324)
+++ trunk/Source/bmalloc/ChangeLog 2019-09-25 00:13:59 UTC (rev 250325)
@@ -1,3 +1,37 @@
+2019-09-24 Keith Rollin <[email protected]>
+
+ Address static analysis warning in Allocator.cpp: Null pointer argument in call to memory copy function
+ https://bugs.webkit.org/show_bug.cgi?id=202152
+ <rdar://problem/55671444>
+
+ Reviewed by Geoffrey Garen.
+
+ Xcode's static analysis facility flags the following:
+
+ .../OpenSource/Source/bmalloc/bmalloc/Allocator.cpp:98:5: warning: Null pointer argument in call to memory copy function
+ memcpy(result, object, copySize);
+ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+ https://en.cppreference.com/w/cpp/string/byte/memcpy explains that
+ this is undefined behavior:
+
+ "If either dest or src is a null pointer, the behavior is
+ undefined, even if count is zero."
+
+ I suppose that passing in a null source pointer could be bad if the
+ implementation fetched the first source byte to copy before actually
+ checking the number of bytes to copy. So far, it doesn’t seem to be an
+ issue, but we should clean this up.
+
+ Simply adding "if (result && object)" before the memcpy will add tests
+ and branches in the hot path of this function and so might not be the
+ best solution. Instead, straighten out the code a little bit by
+ putting an early check and return on "object". This also allows us to
+ remove some intermediate code.
+
+ * bmalloc/Allocator.cpp:
+ (bmalloc::Allocator::reallocateImpl):
+
2019-09-17 Mark Lam <[email protected]>
Use constexpr instead of const in symbol definitions that are obviously constexpr.
Modified: trunk/Source/bmalloc/bmalloc/Allocator.cpp (250324 => 250325)
--- trunk/Source/bmalloc/bmalloc/Allocator.cpp 2019-09-24 23:54:48 UTC (rev 250324)
+++ trunk/Source/bmalloc/bmalloc/Allocator.cpp 2019-09-25 00:13:59 UTC (rev 250325)
@@ -65,13 +65,12 @@
void* Allocator::reallocateImpl(void* object, size_t newSize, FailureAction action)
{
+ if (!object)
+ return allocateImpl(newSize, action);
+
size_t oldSize = 0;
switch (objectType(m_heap, object)) {
case ObjectType::Small: {
- BASSERT(objectType(m_heap, nullptr) == ObjectType::Small);
- if (!object)
- break;
-
size_t sizeClass = Object(object).page()->sizeClass();
oldSize = objectSize(sizeClass);
break;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
