New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 697659 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Fix issues found by building Chrome with PVS-Studio

Project Member Reported by brucedaw...@chromium.org, Mar 1 2017

Issue description

A PVS-Studio run over Chrome gave ~2,500 warnings and a first-pass over them suggests that there are ~75 warnings that warrant fixes. It's not clear how many are likely to have practical effects on the code. Some point out inefficiency, some point out construct which are illegal C++ but unlikely to cause problems in practice, and some may indicate real security issues.
 
Cc: stanisc@chromium.org bsep@chromium.org chengx@chromium.org
For all of these bugs the secondary question is why they weren't caught by /analyze or by clang. That will evaluated on a bug-by-bug basis.
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/09e7c0a4a6f84f323b1676c172af3fcd1b8bd52a

commit 09e7c0a4a6f84f323b1676c172af3fcd1b8bd52a
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Thu Mar 02 05:19:02 2017

Avoid redundant INSERT_PROC_ADDRESS calls

PVS-Studio pointed out that seven GL functions were being initialized
twice. This isn't critical but is slightly inefficient.

R=jmadill@chromium.org
BUG=697659

Change-Id: I199bd06ae1136bc3b8efd519787d89f4447f326d
Reviewed-on: https://chromium-review.googlesource.com/448639
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>

[modify] https://crrev.com/09e7c0a4a6f84f323b1676c172af3fcd1b8bd52a/src/libGLESv2/entry_points_egl.cpp

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4180e5edb4b0963fc235891cc75f0411855e3d43

commit 4180e5edb4b0963fc235891cc75f0411855e3d43
Author: cwallez <cwallez@chromium.org>
Date: Thu Mar 02 17:47:20 2017

Roll ANGLE 0a17c92..185e32b

https://chromium.googlesource.com/angle/angle.git/+log/0a17c92..185e32b

BUG= chromium:694877 ,697659

TBR=geofflang@chromium.org

TEST=bots

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2732463002
Cr-Commit-Position: refs/heads/master@{#454300}

[modify] https://crrev.com/4180e5edb4b0963fc235891cc75f0411855e3d43/DEPS

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f12d3c9c6e6d3b26750b4e897cdd3d7fb5972b0

commit 9f12d3c9c6e6d3b26750b4e897cdd3d7fb5972b0
Author: chengx <chengx@chromium.org>
Date: Fri Mar 03 01:42:10 2017

Remove a redundant clear() call

PVS-Studio pointed out that the "clear" function is called twice for
deallocation of the same resource in query_manager.cc. The redundant
one should be removed.

BUG=697659

Review-Url: https://codereview.chromium.org/2732533002
Cr-Commit-Position: refs/heads/master@{#454459}

[modify] https://crrev.com/9f12d3c9c6e6d3b26750b4e897cdd3d7fb5972b0/gpu/command_buffer/service/query_manager.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e9dbe4d1324caf643ae7808a875c6fad254555a3

commit e9dbe4d1324caf643ae7808a875c6fad254555a3
Author: brucedawson <brucedawson@chromium.org>
Date: Fri Mar 03 10:06:19 2017

Fix incorrect use of unique_ptr found by PVS-Studio

A run of PVS-Studio on Chrome reported various warnings including:
    V554 Incorrect use of unique_ptr. The memory allocated with 'new []'
    will be cleaned using 'delete'. install_util.cc 500

R=grt@chromium.org
BUG=697659

Review-Url: https://codereview.chromium.org/2727593003
Cr-Commit-Position: refs/heads/master@{#454555}

[modify] https://crrev.com/e9dbe4d1324caf643ae7808a875c6fad254555a3/chrome/install_static/install_util.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/15343248cec70d6da70eafc4cb87c1626d040349

commit 15343248cec70d6da70eafc4cb87c1626d040349
Author: chengx <chengx@chromium.org>
Date: Fri Mar 03 17:15:07 2017

Declare a variable before using it

PVS-Studio pointed out that an uninitialized variable 'limit' is used to initialize
itself, which may cause undefined behavior. Other calls to StringToSizeT() don't
have this issue.

BUG=697659

Review-Url: https://codereview.chromium.org/2724153004
Cr-Commit-Position: refs/heads/master@{#454604}

[modify] https://crrev.com/15343248cec70d6da70eafc4cb87c1626d040349/components/autofill/core/browser/personal_data_manager.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76905b16d9f5ba9533a7e8d41724fcd383fa4d37

commit 76905b16d9f5ba9533a7e8d41724fcd383fa4d37
Author: chengx <chengx@chromium.org>
Date: Sat Mar 04 05:38:29 2017

Fix the conditional operator's usage

PVS-Studio pointed out that for the code touched in this CL, the '?:'
operator, regardless of its conditional expression, always returns one
and the same value: std::numeric_limits <int>::min(). The second min()
should be fixed to max().

This CL fixed this bug and added unittests.

BUG=697659

Review-Url: https://codereview.chromium.org/2733503002
Cr-Commit-Position: refs/heads/master@{#454763}

[modify] https://crrev.com/76905b16d9f5ba9533a7e8d41724fcd383fa4d37/third_party/WebKit/Source/platform/geometry/FloatQuad.cpp
[modify] https://crrev.com/76905b16d9f5ba9533a7e8d41724fcd383fa4d37/third_party/WebKit/Source/platform/geometry/FloatQuadTest.cpp

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ed17a3d795edc5af91cd6074a7c299425b1fe9d7

commit ed17a3d795edc5af91cd6074a7c299425b1fe9d7
Author: chengx <chengx@chromium.org>
Date: Mon Mar 06 20:49:38 2017

Post fix of CL "declare a variable before using it"

PVS-Studio pointed out that an uninitialized variable 'limit' is used to
initialize itself, which may cause undefined behavior. Other calls to
StringToSizeT() don't have this issue.

After my landing crrev.com/2724153004 for the fix, the declaration and
assignment of variable |limit| are separate. If somebody adds code
between them, there will be a non-trivial interval of |limit| having an
undefined value.

This CL fixed this by initializing |limit| right after its declaration.

BUG=697659

Review-Url: https://codereview.chromium.org/2732913004
Cr-Commit-Position: refs/heads/master@{#454950}

[modify] https://crrev.com/ed17a3d795edc5af91cd6074a7c299425b1fe9d7/components/autofill/core/browser/personal_data_manager.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9d187859c424c34fc77448d5fa56515f30b8ceed

commit 9d187859c424c34fc77448d5fa56515f30b8ceed
Author: bsep <bsep@chromium.org>
Date: Wed Jun 21 14:23:31 2017

Fix BOOLEAN truncation warning in sync_policy.cc.

PVS-Studio gave this warning:
sandbox\win\src\sync_policy.cc(198): warning V724: Converting type
'uint32_t' to type 'BOOLEAN' can lead to a loss of high-order bits.
Non-zero value can become 'FALSE'.

Since initial_state comes from an IPC it seems like this might actually
happen in practice.

BUG=697659
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2918583003
Cr-Commit-Position: refs/heads/master@{#481200}

[modify] https://crrev.com/9d187859c424c34fc77448d5fa56515f30b8ceed/sandbox/win/src/sync_policy.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/749ba3dbe97a100b5b09b6ce1d2829dd85b43c9d

commit 749ba3dbe97a100b5b09b6ce1d2829dd85b43c9d
Author: bsep <bsep@chromium.org>
Date: Fri Jun 23 21:01:32 2017

Fix unique_ptr usage warning in bluetooth_task_manager_win.cc.

PVS-Studio gave this warning:
device\bluetooth\bluetooth_task_manager_win.cc(880): warning V554:
Incorrect use of unique_ptr. The memory allocated with 'new []' will be
cleaned using 'delete'.

BUG=697659

Review-Url: https://codereview.chromium.org/2941193002
Cr-Commit-Position: refs/heads/master@{#482025}

[modify] https://crrev.com/749ba3dbe97a100b5b09b6ce1d2829dd85b43c9d/device/bluetooth/bluetooth_task_manager_win.cc

Sign in to add a comment