Fix issues reported by PVS-Studio's 5th Chromium analysis |
||
Issue descriptionThe PVS-Studio author published a new analysis of Chromium: http://www.viva64.com/en/b/0442/ We should fix the issues that were found. Below are the issues that were called out on the blog post. It might be worth running the tool itself to find more. 1. Identical sub-expressions: http_stream_parser.cc:1222 bool HttpStreamParser::SendRequestBuffersEmpty() { return request_headers_ == nullptr && request_body_send_buf_ == nullptr && request_body_send_buf_ == nullptr; // <= } 2. Adding item with the same key twice: ntp_resource_cache.cc:581 void NTPResourceCache::CreateNewTabCSS() { .... substitutions["colorSectionBorder"] = // <= SkColorToRGBAString(color_section_border); .... substitutions["colorSectionBorder"] = // <= SkColorToRGBComponents(color_section_border); .... } 3. Possible null-ptr dereference: action_wait.cc:41 (seems fixed/rewritten already) auto* item = FindUpdateItemById(update_context->queue.front()); if (!item) { item->error_category = static_cast<int>(ErrorCategory::kServiceError); 4. Bad pointer arithmetic: string_conversion.cc:62 int UTF8ToUTF16Char(const char *in, int in_length, uint16_t out[2]) { const UTF8 *source_ptr = reinterpret_cast<const UTF8 *>(in); const UTF8 *source_end_ptr = source_ptr + sizeof(char); uint16_t *target_ptr = out; uint16_t *target_end_ptr = target_ptr + 2 * sizeof(uint16_t); // <= Falling off the edge of non-void function: (Clang has a warning; is it not enabled?) memory_allocator.h:39, sandbox_bpf.cc:115, events_x.cc:73 5. Ternary operator has same value on both sides: configurator_impl.cc:133 int ConfiguratorImpl::StepDelay() const { return fast_update_ ? 1 : 1; } 6. Redundant sub-expression in logical expression: udp_socket_posix.cc:735 if (rv == OK || rv != ERR_ADDRESS_IN_USE) // <= return rv; Identical then and else branches: (This code doesn't seem to exist anymore) if (request->was_cached()) FinishRequest(request); // <= else FinishRequest(request); // <= 7. Potential division by zero: addr.h:159 static int BlockSizeForFileType(FileType file_type) { switch (file_type) { .... default: return 0; // <= } } static int RequiredBlocks(int size, FileType file_type) { int block_size = BlockSizeForFileType(file_type); return (size + block_size - 1) / block_size; // <= } 8. Successive assignments to the same value (accidental case fallthrough): util.cc:138 case MODULEWHITELIST: *list = kModuleWhitelist; // <= case RESOURCEBLACKLIST: *list = kResourceBlacklist;
,
Oct 27 2016
,
Oct 27 2016
,
Oct 27 2016
,
Oct 27 2016
,
Oct 27 2016
Case 4 was also supposedly detected at string_conversion.cc:106. Be careful fixing these; the most obvious code correction is not always the right one.
,
Oct 27 2016
,
Oct 27 2016
Looks like grt fixed 8) already with https://codereview.chromium.org/2451163002
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/breakpad/breakpad/+/26ed3386af6d03de20510b0132cc37c4a10d9b97 commit 26ed3386af6d03de20510b0132cc37c4a10d9b97 Author: Hans Wennborg <hans@chromium.org> Date: Thu Oct 27 22:49:06 2016 Fix pointer arithmetic in UTF8ToUTF16Char Found by PVS-Studio! BUG= chromium:660198 Change-Id: I2605de2b1499f85c6e01d19e87e9eeb6af8486f3 Reviewed-on: https://chromium-review.googlesource.com/404552 Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/26ed3386af6d03de20510b0132cc37c4a10d9b97/src/common/string_conversion.cc
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/breakpad/breakpad/+/26ed3386af6d03de20510b0132cc37c4a10d9b97 commit 26ed3386af6d03de20510b0132cc37c4a10d9b97 Author: Hans Wennborg <hans@chromium.org> Date: Thu Oct 27 22:49:06 2016 Fix pointer arithmetic in UTF8ToUTF16Char Found by PVS-Studio! BUG= chromium:660198 Change-Id: I2605de2b1499f85c6e01d19e87e9eeb6af8486f3 Reviewed-on: https://chromium-review.googlesource.com/404552 Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/26ed3386af6d03de20510b0132cc37c4a10d9b97/src/common/string_conversion.cc
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/breakpad/breakpad/+/26ed3386af6d03de20510b0132cc37c4a10d9b97 commit 26ed3386af6d03de20510b0132cc37c4a10d9b97 Author: Hans Wennborg <hans@chromium.org> Date: Thu Oct 27 22:49:06 2016 Fix pointer arithmetic in UTF8ToUTF16Char Found by PVS-Studio! BUG= chromium:660198 Change-Id: I2605de2b1499f85c6e01d19e87e9eeb6af8486f3 Reviewed-on: https://chromium-review.googlesource.com/404552 Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/26ed3386af6d03de20510b0132cc37c4a10d9b97/src/common/string_conversion.cc
,
Oct 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d8c29e03c348bdb7ce8522f22515523de1c20df9 commit d8c29e03c348bdb7ce8522f22515523de1c20df9 Author: hans <hans@chromium.org> Date: Fri Oct 28 04:07:06 2016 Clarify that BlockSizeForFileType can't be called with EXTERNAL FileType PVS-Studio analysis pointed out that if it were called with such a file type, that would lead to division by zero in RequiredBlocks. Also, remove the default: case since the enum is now covered. Clang warns if the switch doesn't cover all enum values. BUG= 660198 Review-Url: https://codereview.chromium.org/2458773003 Cr-Commit-Position: refs/heads/master@{#428279} [modify] https://crrev.com/d8c29e03c348bdb7ce8522f22515523de1c20df9/net/disk_cache/blockfile/addr.h
,
Oct 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d235d4a00cabbb2e017450fb5e64302544e39d89 commit d235d4a00cabbb2e017450fb5e64302544e39d89 Author: hans <hans@chromium.org> Date: Fri Oct 28 17:47:14 2016 Simplify if condition in UDPSocket::RandomBind Found by PVS-Studio! BUG= 660198 Review-Url: https://codereview.chromium.org/2453873004 Cr-Commit-Position: refs/heads/master@{#428414} [modify] https://crrev.com/d235d4a00cabbb2e017450fb5e64302544e39d89/net/udp/udp_socket_posix.cc [modify] https://crrev.com/d235d4a00cabbb2e017450fb5e64302544e39d89/net/udp/udp_socket_win.cc
,
Nov 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8f240d5ca7cad065a2dd72fb819537914319c806 commit 8f240d5ca7cad065a2dd72fb819537914319c806 Author: hans <hans@chromium.org> Date: Tue Nov 01 00:28:19 2016 Fix a redundant expression Found by static analysis with PVS-Studio! BUG= 660198 Review-Url: https://codereview.chromium.org/2448193007 Cr-Commit-Position: refs/heads/master@{#428882} [modify] https://crrev.com/8f240d5ca7cad065a2dd72fb819537914319c806/net/http/http_stream_parser.cc
,
Nov 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6ddf87591b0227947d16d415d3401aabb26ebef commit c6ddf87591b0227947d16d415d3401aabb26ebef Author: hans <hans@chromium.org> Date: Tue Nov 01 02:35:07 2016 NTPResourceCache don't add substitution for colorSectionBorder twice Found through static analysis by PVS-Studio! Also remove .thumbnail-wrapper style, which referred to colorSectionBorder but wasn't used. BUG= 660198 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2456193003 Cr-Commit-Position: refs/heads/master@{#428917} [modify] https://crrev.com/c6ddf87591b0227947d16d415d3401aabb26ebef/chrome/browser/resources/ntp4/new_tab_theme.css [modify] https://crrev.com/c6ddf87591b0227947d16d415d3401aabb26ebef/chrome/browser/ui/webui/ntp/ntp_resource_cache.cc
,
Nov 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/54a5efa8bb07fd1fe99ad9cf3505c584611b0f6c commit 54a5efa8bb07fd1fe99ad9cf3505c584611b0f6c Author: hans <hans@chromium.org> Date: Tue Nov 01 17:24:03 2016 Simplify return expression in ConfiguratorImpl::StepDelay() Found by PVS-Studio! BUG= 660198 Review-Url: https://codereview.chromium.org/2446273006 Cr-Commit-Position: refs/heads/master@{#429032} [modify] https://crrev.com/54a5efa8bb07fd1fe99ad9cf3505c584611b0f6c/components/component_updater/configurator_impl.cc
,
Nov 1 2016
|
||
►
Sign in to add a comment |
||
Comment 1 by h...@chromium.org
, Oct 27 2016