New issue
Advanced search Search tips

Issue 812686 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug

Blocking:
issue 807632



Sign in to add a comment

wimplicit-fallthrough for v8

Project Member Reported by thakis@chromium.org, Feb 15 2018

Issue description

Project Member

Comment 1 by bugdroid1@chromium.org, Feb 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac

commit 8ac70e8663d7d34649f82514f6ca8a2b08eea3ac
Author: Jakob Kummerow <jkummerow@chromium.org>
Date: Fri Feb 16 13:32:25 2018

Work on -Wimplicit-fallthrough for v8

This doesn't enable the warning yet, but adds V8_FALLTHROUGH annotations
in enough places so that v8 can build with the warning on on my linux box.

Found one real bug
(in effect-control-linearizer.cc,
https://chromium-review.googlesource.com/c/v8/v8/+/850392/3/src/compiler/effect-control-linearizer.cc#825
).

Bug:  chromium:812686 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I3542550b9c24b545641d0f0fc43f28f2780b0ab3
Reviewed-on: https://chromium-review.googlesource.com/911731
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51322}
[add] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/base/v8-fallthrough.h
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/compiler/load-elimination.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/compiler/register-allocator.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/compiler/representation-change.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/compiler/simd-scalar-lowering.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/compiler/simplified-lowering.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/compiler/verifier.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/compiler/wasm-compiler.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/debug/debug.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/fast-dtoa.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/heap/spaces-inl.h
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/heap/spaces.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/ic/ic.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/inspector/string-16.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/interpreter/bytecode-generator.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/lookup.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/objects.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/parsing/parser-base.h
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/parsing/parser.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/regexp/jsregexp.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/regexp/regexp-parser.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/snapshot/deserializer.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/utils.h
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/wasm/function-body-decoder-impl.h
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/wasm/wasm-text.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/x64/assembler-x64.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/src/x64/disasm-x64.cc
[modify] https://crrev.com/8ac70e8663d7d34649f82514f6ca8a2b08eea3ac/test/unittests/object-unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Feb 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/19e0e69a9726d47963579d8caf8f1e941c96b15d

commit 19e0e69a9726d47963579d8caf8f1e941c96b15d
Author: Nico Weber <thakis@chromium.org>
Date: Wed Feb 21 15:37:03 2018

Make v8 build with -Wimplicit-fallthrough in x86, arm, arm64, mips, mips64 configs.

x86, arm, arm64: no change in behavior
mips, mips64: disasm-mips(64).cc grows an UNREACHABLE that's
              maybe optimistic (but if it's not true, then that
              looks like a current unintentional fallthrough at
              that spot)
test-js-typed-lowering.cc: looks like a clear bug, but test-only code

Follow-up to https://chromium-review.googlesource.com/c/v8/v8/+/911731 which
did this for x64.

Doesn't turn on the warning yet.

Bug:  chromium:812686 
Change-Id: I7dd79c9885c90f41dd7e3a595256a954ab0ae643
Reviewed-on: https://chromium-review.googlesource.com/923528
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51437}
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/src/arm64/decoder-arm64-inl.h
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/src/arm64/disasm-arm64.cc
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/src/arm64/simulator-arm64.cc
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/src/compiler/arm/instruction-selector-arm.cc
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/src/compiler/arm64/instruction-selector-arm64.cc
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/src/compiler/ia32/instruction-selector-ia32.cc
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/src/compiler/load-elimination.cc
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/src/compiler/mips64/instruction-selector-mips64.cc
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/src/ia32/assembler-ia32.cc
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/src/mips/disasm-mips.cc
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/src/mips/simulator-mips.cc
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/src/mips64/disasm-mips64.cc
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/src/mips64/simulator-mips64.cc
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/src/wasm/baseline/ia32/liftoff-assembler-ia32.h
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/test/cctest/compiler/test-code-generator.cc
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/test/cctest/compiler/test-js-typed-lowering.cc
[modify] https://crrev.com/19e0e69a9726d47963579d8caf8f1e941c96b15d/test/cctest/test-api.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 21 2018

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

commit dab696e7ae4c8c12ac9813dd356186a1deb805ba
Author: Nico Weber <thakis@chromium.org>
Date: Wed Feb 21 18:33:33 2018

v8: Turn on -Wimplicit-fallthrough.

https://chromium-review.googlesource.com/911731 made things build with
this on x64, and https://chromium-review.googlesource.com/923528 added
x86, arm, arm64, mips, mips64.  This are all the configs covered by
v8's trybots.  If this breaks yet another config I don't know about,
these two CLs should give you a good idea how to fix them.

Bug:  chromium:812686 
Change-Id: Ib9a9714a070dd876a8f5911a1bc974ffd7aa3995
Reviewed-on: https://chromium-review.googlesource.com/928842
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51446}
[modify] https://crrev.com/dab696e7ae4c8c12ac9813dd356186a1deb805ba/BUILD.gn

Comment 4 by thakis@chromium.org, Feb 21 2018

Status: Fixed (was: Assigned)

Sign in to add a comment