Add fallthrough annotations on new C++ switches

A recent commit converted some source from C to C++. Chrome has
checks against unannotated switch case fallthroughs in C++ code, so the
converted source needs annotations.

Bug: 990190
Change-Id: Ib92435b4877be936f837928a70b552ec4975d42a
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1898429
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
This commit is contained in:
Christopher Grant 2019-11-04 21:34:20 -05:00 committed by Mark Mentovai
parent 8c62281f8b
commit 5915ea929c
3 changed files with 100 additions and 41 deletions

View file

@ -60,6 +60,8 @@ See the header file "ConvertUTF.h" for complete documentation.
#include <stdio.h> #include <stdio.h>
#endif #endif
#include "common/macros.h"
namespace google_breakpad { namespace google_breakpad {
namespace { namespace {
@ -295,10 +297,20 @@ ConversionResult ConvertUTF16toUTF8 (const UTF16** sourceStart, const UTF16* sou
target -= bytesToWrite; result = targetExhausted; break; target -= bytesToWrite; result = targetExhausted; break;
} }
switch (bytesToWrite) { /* note: everything falls through. */ switch (bytesToWrite) { /* note: everything falls through. */
case 4: *--target = (UTF8)((ch | byteMark) & byteMask); ch >>= 6; case 4:
case 3: *--target = (UTF8)((ch | byteMark) & byteMask); ch >>= 6; *--target = (UTF8)((ch | byteMark) & byteMask);
case 2: *--target = (UTF8)((ch | byteMark) & byteMask); ch >>= 6; ch >>= 6;
case 1: *--target = (UTF8)(ch | firstByteMark[bytesToWrite]); BP_FALLTHROUGH;
case 3:
*--target = (UTF8)((ch | byteMark) & byteMask);
ch >>= 6;
BP_FALLTHROUGH;
case 2:
*--target = (UTF8)((ch | byteMark) & byteMask);
ch >>= 6;
BP_FALLTHROUGH;
case 1:
*--target = (UTF8)(ch | firstByteMark[bytesToWrite]);
} }
target += bytesToWrite; target += bytesToWrite;
} }
@ -327,9 +339,14 @@ Boolean isLegalUTF8(const UTF8 *source, int length) {
switch (length) { switch (length) {
default: return false; default: return false;
/* Everything else falls through when "true"... */ /* Everything else falls through when "true"... */
case 4: if ((a = (*--srcptr)) < 0x80 || a > 0xBF) return false; case 4:
case 3: if ((a = (*--srcptr)) < 0x80 || a > 0xBF) return false; if ((a = (*--srcptr)) < 0x80 || a > 0xBF) return false;
case 2: if ((a = (*--srcptr)) > 0xBF) return false; BP_FALLTHROUGH;
case 3:
if ((a = (*--srcptr)) < 0x80 || a > 0xBF) return false;
BP_FALLTHROUGH;
case 2:
if ((a = (*--srcptr)) > 0xBF) return false;
switch (*source) { switch (*source) {
/* no fall-through in this inner switch */ /* no fall-through in this inner switch */
@ -339,8 +356,8 @@ Boolean isLegalUTF8(const UTF8 *source, int length) {
case 0xF4: if (a > 0x8F) return false; break; case 0xF4: if (a > 0x8F) return false; break;
default: if (a < 0x80) return false; default: if (a < 0x80) return false;
} }
BP_FALLTHROUGH;
case 1: if (*source >= 0x80 && *source < 0xC2) return false; case 1: if (*source >= 0x80 && *source < 0xC2) return false;
} }
if (*source > 0xF4) return false; if (*source > 0xF4) return false;
return true; return true;
@ -384,12 +401,14 @@ ConversionResult ConvertUTF8toUTF16 (const UTF8** sourceStart, const UTF8* sourc
* The cases all fall through. See "Note A" below. * The cases all fall through. See "Note A" below.
*/ */
switch (extraBytesToRead) { switch (extraBytesToRead) {
case 5: ch += *source++; ch <<= 6; /* remember, illegal UTF-8 */ /* remember, illegal UTF-8 */
case 4: ch += *source++; ch <<= 6; /* remember, illegal UTF-8 */ case 5: ch += *source++; ch <<= 6; BP_FALLTHROUGH;
case 3: ch += *source++; ch <<= 6; /* remember, illegal UTF-8 */
case 2: ch += *source++; ch <<= 6; case 4: ch += *source++; ch <<= 6; BP_FALLTHROUGH;
case 1: ch += *source++; ch <<= 6; case 3: ch += *source++; ch <<= 6; BP_FALLTHROUGH;
case 0: ch += *source++; case 2: ch += *source++; ch <<= 6; BP_FALLTHROUGH;
case 1: ch += *source++; ch <<= 6; BP_FALLTHROUGH;
case 0: ch += *source++;
} }
ch -= offsetsFromUTF8[extraBytesToRead]; ch -= offsetsFromUTF8[extraBytesToRead];
@ -474,10 +493,20 @@ ConversionResult ConvertUTF32toUTF8 (const UTF32** sourceStart, const UTF32* sou
target -= bytesToWrite; result = targetExhausted; break; target -= bytesToWrite; result = targetExhausted; break;
} }
switch (bytesToWrite) { /* note: everything falls through. */ switch (bytesToWrite) { /* note: everything falls through. */
case 4: *--target = (UTF8)((ch | byteMark) & byteMask); ch >>= 6; case 4:
case 3: *--target = (UTF8)((ch | byteMark) & byteMask); ch >>= 6; *--target = (UTF8)((ch | byteMark) & byteMask);
case 2: *--target = (UTF8)((ch | byteMark) & byteMask); ch >>= 6; ch >>= 6;
case 1: *--target = (UTF8) (ch | firstByteMark[bytesToWrite]); BP_FALLTHROUGH;
case 3:
*--target = (UTF8)((ch | byteMark) & byteMask);
ch >>= 6;
BP_FALLTHROUGH;
case 2:
*--target = (UTF8)((ch | byteMark) & byteMask);
ch >>= 6;
BP_FALLTHROUGH;
case 1:
*--target = (UTF8) (ch | firstByteMark[bytesToWrite]);
} }
target += bytesToWrite; target += bytesToWrite;
} }
@ -508,12 +537,12 @@ ConversionResult ConvertUTF8toUTF32 (const UTF8** sourceStart, const UTF8* sourc
* The cases all fall through. See "Note A" below. * The cases all fall through. See "Note A" below.
*/ */
switch (extraBytesToRead) { switch (extraBytesToRead) {
case 5: ch += *source++; ch <<= 6; case 5: ch += *source++; ch <<= 6; BP_FALLTHROUGH;
case 4: ch += *source++; ch <<= 6; case 4: ch += *source++; ch <<= 6; BP_FALLTHROUGH;
case 3: ch += *source++; ch <<= 6; case 3: ch += *source++; ch <<= 6; BP_FALLTHROUGH;
case 2: ch += *source++; ch <<= 6; case 2: ch += *source++; ch <<= 6; BP_FALLTHROUGH;
case 1: ch += *source++; ch <<= 6; case 1: ch += *source++; ch <<= 6; BP_FALLTHROUGH;
case 0: ch += *source++; case 0: ch += *source++;
} }
ch -= offsetsFromUTF8[extraBytesToRead]; ch -= offsetsFromUTF8[extraBytesToRead];

45
src/common/macros.h Normal file
View file

@ -0,0 +1,45 @@
// Copyright (c) 2019, Google Inc.
// All rights reserved.
//
// Redistribution and use in source and binary forms, with or without
// modification, are permitted provided that the following conditions are
// met:
//
// * Redistributions of source code must retain the above copyright
// notice, this list of conditions and the following disclaimer.
// * Redistributions in binary form must reproduce the above
// copyright notice, this list of conditions and the following disclaimer
// in the documentation and/or other materials provided with the
// distribution.
// * Neither the name of Google Inc. nor the names of its
// contributors may be used to endorse or promote products derived from
// this software without specific prior written permission.
//
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#ifndef BREAKPAD_COMMON_MACROS_H_
#define BREAKPAD_COMMON_MACROS_H_
// Ensure that this macro definition stays in a private header file: clang
// suggests the first macro expanding to [[clang::fallthrough]] in its
// diagnostics, so if BP_FALLTHROUGH is visible in code depending on breakpad,
// clang would suggest BP_FALLTHROUGH for code depending on breakpad, instead of
// the client code's own fallthrough macro.
// TODO(thakis): Once everyone uses C++17, use its [[fallthrough]] instead.
#if defined(__clang__)
#define BP_FALLTHROUGH [[clang::fallthrough]]
#else
#define BP_FALLTHROUGH
#endif
#endif // BREAKPAD_COMMON_MACROS_H_

View file

@ -54,6 +54,7 @@
#include "processor/range_map-inl.h" #include "processor/range_map-inl.h"
#include "common/macros.h"
#include "common/scoped_ptr.h" #include "common/scoped_ptr.h"
#include "common/stdio_wrapper.h" #include "common/stdio_wrapper.h"
#include "google_breakpad/processor/dump_context.h" #include "google_breakpad/processor/dump_context.h"
@ -62,22 +63,6 @@
#include "processor/convert_old_arm64_context.h" #include "processor/convert_old_arm64_context.h"
#include "processor/logging.h" #include "processor/logging.h"
// All intentional fallthroughs in breakpad are in this file, so define
// this macro locally.
// If you ever move this to a .h file, make sure it's defined in a
// private header file: clang suggests the first macro expanding to
// [[clang::fallthrough]] in its diagnostics, so if BP_FALLTHROUGH
// is visible in code depending on breakpad, clang would suggest
// BP_FALLTHROUGH for code depending on breakpad, instead of the
// client code's own fallthrough macro.
// TODO(thakis): Once everyone uses C++17, use its [[fallthrough]] instead.
#if defined(__clang__)
#define BP_FALLTHROUGH [[clang::fallthrough]]
#else
#define BP_FALLTHROUGH
#endif
namespace google_breakpad { namespace google_breakpad {
using std::istream; using std::istream;