Coding errors in chromium not detected by clang
Reported by
c...@time4t.net,
Aug 29 2017
|
||
Issue description
UserAgent: Mozilla/5.0 (X11; Linux x86_64; rv:51.0) Gecko/20100101 Firefox/51.0 Iceape/2.48
Steps to reproduce the problem:
Build chromium with a compiler other than clang, e.g. gcc 5.4.0
What is the expected behavior?
No obvious coding errors flagged by the compiler.
What went wrong?
1a. In chrome/browser/ui/webui/welcome_handler.cc - HandleUserDecline():
result_ = (result_ == WelcomeResult::ATTEMPTED)
? WelcomeResult::ATTEMPTED_DECLINED
: result_ = WelcomeResult::DECLINED;
This is not valid C++ code because the behavior is undefined. The compiler should warn about such code. Since chromium is built with warnings-as-errors this code then results in a failed build.
1b. In content/renderer/media/webrtc/media_stream_video_webrtc_sink.cc
bool is_screencast = is_screencast = video_track->is_screencast();
is another instance.
2. In third_party/WebKit/Source/platform/wtf/typed_arrays/ArrayBufferContents.h
There are 3 unqualified references to AllocationKind::X which IIUC according to C++ name resolution rules resolve to the member function AllocationKind(), not to the enum declared earlier. Clang accepts this code, however.
3. In third_party/webrtc/modules/audio_processing/aec3/aec_state.cc
This file #includes the C header file <math.h> but then uses std::abs() on a float. This is not guaranteed to work since cstdlib defines certain std::abs() functions while cmath defines some others, including std::abs(float). I guess <math.h> should be replaced by the C++ header <cmath>.
Did this work before? Yes chromium 59
Chrome version: 61.0.3163.59 Channel: beta
OS Version:
Flash Version:
,
Aug 29 2017
Thanks for the detailed report!
,
Aug 29 2017
Filed https://bugs.llvm.org/show_bug.cgi?id=34363 for 1a) and 1b).
,
Aug 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b934ddbdbc152437191c774aea91093b224f1d3b commit b934ddbdbc152437191c774aea91093b224f1d3b Author: Hans Wennborg <hans@chromium.org> Date: Tue Aug 29 21:27:39 2017 Fix typo in WelcomeHandler::HandleUserDecline Bug: 760070 Change-Id: I8669aafd5c5d6448288c7dd2827603f9100b4b2d Reviewed-on: https://chromium-review.googlesource.com/642128 Reviewed-by: Nico Weber <thakis@chromium.org> Reviewed-by: Tommy Martino <tmartino@chromium.org> Commit-Queue: Hans Wennborg <hans@chromium.org> Cr-Commit-Position: refs/heads/master@{#498235} [modify] https://crrev.com/b934ddbdbc152437191c774aea91093b224f1d3b/chrome/browser/ui/webui/welcome_handler.cc
,
Aug 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/41b720cdeb347da3ce002d8f674641f0a15d09a2 commit 41b720cdeb347da3ce002d8f674641f0a15d09a2 Author: Hans Wennborg <hans@chromium.org> Date: Tue Aug 29 22:54:43 2017 Fix typo in MediaStreamVideoWebRtcSink Bug: 760070 Change-Id: I044493fad388f8279171ea0bbed8aee3e1916092 Reviewed-on: https://chromium-review.googlesource.com/641971 Commit-Queue: Sergey Ulanov <sergeyu@chromium.org> Reviewed-by: Sergey Ulanov <sergeyu@chromium.org> Cr-Commit-Position: refs/heads/master@{#498269} [modify] https://crrev.com/41b720cdeb347da3ce002d8f674641f0a15d09a2/content/renderer/media/webrtc/media_stream_video_webrtc_sink.cc
,
Aug 31 2017
Another gcc warning that clang apparently does not have is 'variable set but not used'. In third_party/pdfium/xfa/fgas/layout/cfx_txtbreak.cpp - CFX_TxtBreak::GetDisplayPos() : fCharHeight and iCharHeight are not used. In third_party/WebKit/Source/core/events/WebInputEventConversion.cpp - FrameTranslation() : scale is not used. In third_party/pdfium/xfa/fde/cfde_textout.cpp - CFDE_TextOut::CalcLogicSize() : wPreChar is not used. In content/common/unique_name_helper.cc - UniqueNameHelper::UpdateLegacyNameFromV24() : index is not used. (from chromium 62.0.3192.0)
,
Aug 31 2017
,
Sep 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ec0d7abe5c1520464ff0ea2405b2c59d51b0b775 commit ec0d7abe5c1520464ff0ea2405b2c59d51b0b775 Author: Piotr Tworek <ptworek@opera.com> Date: Fri Sep 01 12:20:03 2017 Rename ArrayBufferContents::AllocationKind to GetAllocationKind. Current name clashes with the name of the enum class the function is supposed to return. Clang does not care, but gcc fails to compile the code. Bug: 760070 Change-Id: I935d91493a3fc7b289d03f776c14b6ec4f3f622a Reviewed-on: https://chromium-review.googlesource.com/645549 Reviewed-by: Yuta Kitamura <yutak@chromium.org> Commit-Queue: Yuta Kitamura <yutak@chromium.org> Cr-Commit-Position: refs/heads/master@{#499171} [modify] https://crrev.com/ec0d7abe5c1520464ff0ea2405b2c59d51b0b775/third_party/WebKit/Source/platform/wtf/typed_arrays/ArrayBufferContents.h
,
Sep 18 2017
The following revision refers to this bug: https://pdfium.googlesource.com/pdfium/+/91c2f7cdcf8545d764a8b6543b42f0819f8d4ad3 commit 91c2f7cdcf8545d764a8b6543b42f0819f8d4ad3 Author: Lei Zhang <thestig@chromium.org> Date: Mon Sep 18 19:15:36 2017 Remove some unused variables. BUG=chromium:760070 Change-Id: Icb2e46da86b6b7e71dae309a015ae1c5301fc931 Reviewed-on: https://pdfium-review.googlesource.com/14230 Reviewed-by: Henrique Nakashima <hnakashima@chromium.org> Commit-Queue: Lei Zhang <thestig@chromium.org> [modify] https://crrev.com/91c2f7cdcf8545d764a8b6543b42f0819f8d4ad3/xfa/fgas/layout/cfx_txtbreak.cpp
,
Sep 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8fc20355a6369b3316e381c514d0ff2fafe0243d commit 8fc20355a6369b3316e381c514d0ff2fafe0243d Author: pdfium-deps-roller@chromium.org <pdfium-deps-roller@chromium.org> Date: Mon Sep 18 22:25:51 2017 Roll src/third_party/pdfium/ 275e260a6..5278cebc4 (2 commits) https://pdfium.googlesource.com/pdfium.git/+log/275e260a6cd4..5278cebc468e $ git log 275e260a6..5278cebc4 --date=short --no-merges --format='%ad %ae %s' 2017-09-18 dsinclair Cleanup word break properties 2017-09-18 thestig Remove some unused variables. Created with: roll-dep src/third_party/pdfium BUG=760070 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, see: http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls TBR=dsinclair@chromium.org Change-Id: Ia3db049e281121caedecafca5a6122c18d4b644c Reviewed-on: https://chromium-review.googlesource.com/671822 Reviewed-by: <pdfium-deps-roller@chromium.org> Commit-Queue: <pdfium-deps-roller@chromium.org> Cr-Commit-Position: refs/heads/master@{#502694} [modify] https://crrev.com/8fc20355a6369b3316e381c514d0ff2fafe0243d/DEPS |
||
►
Sign in to add a comment |
||
Comment 1 by mosescu@chromium.org
, Aug 29 2017