Clang-tidy Bugprone Checks

[mwaxmonsky]

Is your feature request related to a problem? Please describe.
No

Describe the solution you'd like
A clang-tidy config file addition that enables the bugprone-* checks that are relevant to MICM development.

Describe alternatives you've considered
N\A

Additional context

.clang-tidy modifications

Checks: bugprone-*,-bugprone-signal-handler,

To Review

• bugprone-crtp-constructor-accessibility
• bugprone-easily-swappable-parameters
• bugprone-implicit-widening-of-multiplication-result
• bugprone-unhandled-exception-at-new

clang-tidy Recommended Checks Reasoning

bugprone-argument-comment ON

We can turn this on but I don't think any modern style uses functionCall(/*value=*/5) where void functionCall(int value)`.

bugprone-assert-side-effect ON

Recommend turning on as it will flag things like assert(++i = 5) or similar asserts where there are state modifications that would only be tracked in debug builds.

bugprone-assignment-in-if-condition ON

Detects things like if(i = 5) when meant to use if(i == 5).

bugprone-bad-signal-to-kill-thread ON

Prevents using pthread_kill(thread, SIGTERM); as this will stop the entire process, not just the thread.

bugprone-bool-pointer-implicit-conversion ON unless needed otherwise

I think leaving this on is a good idea as int *p = ...; if(p) {...} is more of a C-style approach.

bugprone-branch-clone ON

Prevents

if(thing)
  x+=1;
  ...
else
  ...
  x+=1;

Would flag x+=1 and anything else in both branches as repeated and can be manually moved out, reducing code.

bugprone-casting-through-void ON

Detects unsafe or redundant two-step casting operations involving void*

bugprone-chained-comparison ON

Prevents if(a < b < c) which really evaluates if ( (a<b)<c) when probably wanted if( (a<b) && (b<c))

bugprone-compare-pointer-to-member-virtual-function ON

In certain compilers, virtual function addresses are not conventional pointers but instead consist of offsets and indexes within a virtual function table (vtable). Consequently, these pointers may vary between base and derived classes, leading to unpredictable behavior when compared directly. This issue becomes particularly challenging when dealing with pointers to pure virtual functions, as they may not even have a valid address, further complicating comparisons.

bugprone-copy-constructor-init ON

Finds copy constructors where the constructor doesn’t call the copy constructor of the base class.

bugprone-crtp-constructor-accessibility MAYBE

Detects error-prone Curiously Recurring Template Pattern usage, when the CRTP can be constructed outside itself and the derived class.

Unsure of this one and additional digging is needed

bugprone-dangling-handle ON

Detect dangling references in value handles like std::string_view

Ex:

vector<string_view> V;
V.push_back(string());  // V[0] is dangling.

bugprone-dynamic-static-initializers ON

int foo() {
  static int k = bar();
  return k;
}

Might have foo called twice by different threads simultaneously resulting in k being double initialized.

bugprone-easily-swappable-parameters MAYBE

This one can be tricky to handle in every case. For example:

void draw(int x, int y)

Could be refactored to use a point struct but this is potentially an externally visible API change that might not be possible.

bugprone-empty-catch ON

Prevents catch(std::exception &e) {}.

bugprone-exception-escape ON

Prevents void something() noexcept { somethingThatCouldThrow(); }

bugprone-fold-init-type ON

Prevents

auto a = {0.5f, 0.5f, 0.5f, 0.5f};
return std::accumulate(std::begin(a), std::end(a), 0);

Which mistakenly returns 0 because the operator+ will truncate each operation to int.

bugprone-forward-declaration-namespace ON

Prevents

namespace na { struct A; }
namespace nb { struct A {}; }
nb::A a;
// warning : no definition found for 'A', but a definition with the same name
// 'A' found in another namespace 'nb::'

bugprone-forwarding-reference-overload ON

The check looks for perfect forwarding constructors that can hide copy or move constructors. (See Scott Meyers, Effective Modern C++, Item 26)

bugprone-implicit-widening-of-multiplication-result MAYBE

Flags:

void zeroinit(char* base, unsigned width, unsigned height) {
  for(unsigned row = 0; row != height; ++row) {
    for(unsigned col = 0; col != width; ++col) {
      char* ptr = base + row * width + col;
      *ptr = 0;
    }
  }
}

as (unsigned) row * (unsigned) col could be in the space of long instead of int. The fix isn't terible but there are possibly other gotchas in the fix process, especially if row and col are used in other contexts.

bugprone-inaccurate-erase ON

Prevents

std::vector<int> xs;
...
xs.erase(std::remove(xs.begin(), xs.end(), 10));

which only removes the first element and recommends

std::vector<int> xs;
...
xs.erase(std::remove(xs.begin(), xs.end(), 10), xs.end());

bugprone-inc-dec-in-conditions ON

Prevents

int i = 0;
// ...
if (i++ < 5 && i > 0) {
  // do something
}

bugprone-incorrect-enable-if ON

Detects incorrect usages of std::enable_if that don’t name the nested type type.

bugprone-incorrect-roundings ON

Prevents:

(int)(double_expression + 0.5)

It is incorrect. The number 0.499999975 (smallest representable float number below 0.5) rounds to 1.0. Even worse behavior for negative numbers where both -0.5f and -1.4f both round to 0.0.

bugprone-infinite-loop ON

Prevents

int i = 0, j = 0;
while (i < 10) {
  ++j;
}

bugprone-integer-division ON

Prevents

float floatFunc(float);
int intFunc(int);
double d;
int i = 42;

// Warn, floating-point values expected.
d = 32 * 8 / (2 + i);
d = 8 * floatFunc(1 + 7 / 2);
d = i / (1 << 4);

But does not warn on:

// OK, no integer division.
d = 32 * 8.0 / (2 + i);
d = 8 * floatFunc(1 + 7.0 / 2);
d = (double)i / (1 << 4);

// OK, there are signs of deliberateness.
d = 1 << (i / 2);
d = 9 + intFunc(6 * i / 32);
d = (int)(i / 32) - 8;

bugprone-lambda-function-name ON

Prevents

void FancyFunction() {
  [] { printf("Called from %s\n", __func__); }();
  [] { printf("Now called from %s\n", __FUNCTION__); }();
}

as this outputs:

Called from operator()
Now called from operator()

bugprone-macro-parentheses ON

Prevents

#define CUBE(X) (X) * (X) * (X)
int i = 3;

int a = 81 / CUBE(i);
-> 
int a = 81 / i * i * i;
->
int a = ((81 / i) * i) * i);  /* Evaluates to 243 */

bugprone-macro-repeated-side-effects ON

Prevents

#define badA(x,y)  ((x)+((x)+(y))+(y))

ret = badA(a++, b);

bugprone-misplaced-operator-in-strlen-in-alloc ON

Prevents

void bad_new(char *str) {
  char *c = new char[strlen(str + 1)];
}

and recommends

char *c = new char[strlen(str) + 1];
//or
void fixed_malloc(char *str) {
  char *c = (char*) malloc(strlen(str) + 1);
}

bugprone-misplaced-pointer-arithmetic-in-alloc ON

Prevents

void bad_malloc(int n) {
  char *p = (char*) malloc(n) + 10;
}

and recommends

char *p = (char*) malloc(n + 10);

bugprone-misplaced-widening-cast ON

Flags:

long f(int x) {
    return (long)(x * 1000);
}

and recommends:

long f(int x) {
    return (long)x * 1000;
}

bugprone-move-forwarding-reference ON

Flags

template <typename T>
void foo(T&& t) {
  bar(std::move(t));
}

and recommends:

bar(std::forward<T>(t));

bugprone-multi-level-implicit-pointer-conversion ON

Prevents

void foo(void* ptr);

int main() {
  int x = 42;
  int* ptr = &x;
  int** ptr_ptr = &ptr;
  foo(ptr_ptr); // warning will trigger here
  return 0;
}

Using an explicit cast is a recommended solution to prevent issues caused by implicit pointer level conversion, as it allows the developer to explicitly state their intention and show their reasoning for the type conversion. Additionally, it is recommended that developers thoroughly check and verify the safety of the conversion before using an explicit cast.

bugprone-multiple-new-in-one-expression ON

Prevents:

struct A {
  int Var;
};
struct B {
  B();
  B(A *);
  int Var;
};
struct C {
  int *X1;
  int *X2;
};

void f(A *, B *);
int f1(A *);
int f1(B *);
bool f2(A *);

void foo() {
  A *PtrA;
  B *PtrB;
  try {
    // Allocation of 'B'/'A' may fail after memory for 'A'/'B' was allocated.
    f(new A, new B); // warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception; order of these allocations is undefined

    // List (aggregate) initialization is used.
    C C1{new int, new int}; // no warning

    // Allocation of 'B'/'A' may fail after memory for 'A'/'B' was allocated but not yet passed to function 'f1'.
    int X = f1(new A) + f1(new B); // warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception; order of these allocations is undefined

    // Allocation of 'B' may fail after memory for 'A' was allocated.
    // From C++17 on memory for 'B' is allocated first but still may leak if allocation of 'A' fails.
    PtrB = new B(new A); // warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception

    // 'new A' and 'new B' may be performed in any order.
    // 'new B'/'new A' may fail after memory for 'A'/'B' was allocated but not assigned to 'PtrA'/'PtrB'.
    (PtrA = new A)->Var = (PtrB = new B)->Var; // warning: memory allocation may leak if an other allocation is sequenced after it and throws an exception; order of these allocations is undefined

    // Evaluation of 'f2(new A)' must be finished before 'f1(new B)' starts.
    // If 'new B' fails the allocated memory for 'A' is supposedly handled correctly because function 'f2' could take the ownership.
    bool Z = f2(new A) || f1(new B); // no warning

    X = (f2(new A) ? f1(new A) : f1(new B)); // no warning

    // No warning if the result of both allocations is not passed to a function
    // or stored in a variable.
    (new A)->Var = (new B)->Var; // no warning

    // No warning if at least one non-throwing allocation is used.
    f(new(std::nothrow) A, new B); // no warning
  } catch(std::bad_alloc) {
  }

  // No warning if the allocation is outside a try block (or no catch handler exists for std::bad_alloc).
  // (The fact if exceptions can escape from 'foo' is not taken into account.)
  f(new A, new B); // no warning
}

bugprone-multiple-statement-macro ON

Prevents

#define INCREMENT_TWO(x, y) (x)++; (y)++
if (do_increment)
  INCREMENT_TWO(a, b);  // (b)++ will be executed unconditionally.

bugprone-no-escape ON

Prevents

void foo(__attribute__((noescape)) int *p) {
  dispatch_async(queue, ^{
    *p = 123;
  });
});

bugprone-non-zero-enum-to-bool-conversion ON

Prevents

enum EStatus {
  OK = 1,
  NOT_OK,
  UNKNOWN
};

void process(EStatus status) {
  if (!status) {
    // this true-branch won't be executed
    return;
  }
  // proceed with "valid data"
}

bugprone-not-null-terminated-result ON

Flags

static char *stringCpy(const std::string &str) {
  char *result = reinterpret_cast<char *>(malloc(str.size()));
  memcpy(result, str.data(), str.size());
  return result;
}

and recommends

static char *stringCpy(const std::string &str) {
  char *result = reinterpret_cast<char *>(malloc(str.size() + 1));
  strcpy(result, str.data());
  return result;
}

Checks usage of memcpy, memcpy_s, memchr, memmove, memmove_s, strerror_s, strncmp, strxfrm

bugprone-optional-value-conversion ON

#include <optional>

void print(std::optional<int>);

int main()
{
  std::optional<int> opt;
  // ...

  // Unintentional conversion from std::optional<int> to int and back to
  // std::optional<int>:
  print(opt.value());

  // ...
}

becomes

#include <optional>

void print(std::optional<int>);

int main()
{
  std::optional<int> opt;
  // ...

  // Proposed code: Directly pass the std::optional<int> to the print
  // function.
  print(opt);

  // ...
}

bugprone-parent-virtual-call ON

struct A {
  int virtual foo() {...}
};

struct B: public A {
  int foo() override {...}
};

struct C: public B {
  int foo() override { A::foo(); }
//                     ^^^^^^^^
// warning: qualified name A::foo refers to a member overridden in subclass; did you mean 'B'?  [bugprone-parent-virtual-call]
};

bugprone-posix-return ON

Flags
if (posix_fadvise(...) < 0) {
and recommends changing the < to either = for success or > for failure

bugprone-redundant-branch-condition ON

Flags

bool onFire = isBurning();
if (onFire) {
  if (onFire)
    scream();
}

and

bool onFire = isBurning();
if (onFire) {
  if (onFire && peopleInTheBuilding > 0)
    scream();
}

but will fail to detect

int a = somethingReturning1();
int b = somethingReturning1();
if (peopleInTheBuilding == a) {
  if (peopleInTheBuilding == b) {
    doSomething();
  }
}

bugprone-reserved-identifier ON

Checks for usages of identifiers reserved for use by the implementation.

Prevents

namespace NS {
  void __f(); // name is not allowed in user code
  using _Int = int; // same with this
  #define cool__macro // also this
}
int _g(); // disallowed in global namespace only

bugprone-return-const-ref-from-parameter ON

Detects return statements that return a constant reference parameter as constant reference. This may cause use-after-free errors if the caller uses xvalues as arguments.

struct S {
  int v;
  S(int);
  ~S();
};

const S &fn(const S &a) {
  return a;
}

const S& s = fn(S{1});
s.v; // use after free

bugprone-shared-ptr-array-mismatch ON

Finds initializations of C++ shared pointers to non-array type that are initialized with an array.

If a shared pointer std::shared_ptr is initialized with a new-expression new T[] the memory is not deallocated correctly.

std::shared_ptr<Foo> x(new Foo[10]); // -> std::shared_ptr<Foo[]> x(new Foo[10]);
//                     ^ warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
std::shared_ptr<Foo> x1(new Foo), x2(new Foo[10]); // no replacement
//                                   ^ warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]

std::shared_ptr<Foo> x3(new Foo[10], [](const Foo *ptr) { delete[] ptr; }); // no warning

struct S {
  std::shared_ptr<Foo> x(new Foo[10]); // no replacement in this case
  //                     ^ warning: shared pointer to non-array is initialized with array [bugprone-shared-ptr-array-mismatch]
};

bugprone-signal-handler OFF

Finds specific constructs in signal handler functions that can cause undefined behavior. The rules for what is allowed differ between C++ language versions.

C++17 loosense restrictions thus this is harder to track and not effective (meaning the fix will still be problematic)

Flags

#include <csignal>
  
static void sig_handler(int sig) {
  // Implementation details elided.
}
 
void install_signal_handler() {
  if (SIG_ERR == std::signal(SIGTERM, sig_handler)) {
    // Handle error
  }
}

and expects

#include <csignal>
  
extern "C" void sig_handler(int sig) {
  // Implementation details elided.
}
 
void install_signal_handler() {
  if (SIG_ERR == std::signal(SIGTERM, sig_handler)) {
    // Handle error
  }
}

bugprone-signed-char-misuse ON

Currently, this check warns in the following cases: - signed char is assigned to an integer variable - signed char and unsigned char are compared with equality/inequality operator - signed char is converted to an integer in the array subscript

int IChar = EOF;
if (readChar(CChar)) {
  IChar = CChar;
}
->
IChar = static_cast<unsigned char>(CChar);

bugprone-sizeof-container ON

The check finds usages of sizeof on expressions of STL container types. Most likely the user wanted to use .size() instead.

std::string s;
int a = 47 + sizeof(s); // warning: sizeof() doesn't return the size of the container. Did you mean .size()?

int b = sizeof(std::string); // no warning, probably intended.

std::string array_of_strings[10];
int c = sizeof(array_of_strings) / sizeof(array_of_strings[0]); // no warning, definitely intended.

std::array<int, 3> std_array;
int d = sizeof(std_array); // no warning, probably intended.

bugprone-sizeof-expression ON

The check finds usages of sizeof expressions which are most likely errors.

#define BUFLEN 42
char buf[BUFLEN];
memset(buf, 0, sizeof(BUFLEN));  // sizeof(42) ==> sizeof(int)


class Point {
  [...]
  size_t size() { return sizeof(this); }  // should probably be sizeof(*this)
  [...]
};

...and many others

bugprone-spuriously-wake-up-functions ON

Not sure we will hit this frequently but it flags:

#include <condition_variable>
#include <mutex>
  
struct Node {
  void *node;
  struct Node *next;
};
   
static Node list;
static std::mutex m;
static std::condition_variable condition;
   
void consume_list_element(std::condition_variable &condition) {
  std::unique_lock<std::mutex> lk(m);
   
  if (list.next == nullptr) {
    condition.wait(lk);
  }
  
  // Proceed when condition holds.
}

This noncompliant code example nests the call to wait() inside an if block and consequently fails to check the condition predicate after the notification is received. If the notification was spurious or malicious, the thread would wake up prematurely.

This compliant solution calls the wait() member function from within a while loop to check the condition both before and after the call to wait().

#include <condition_variable>
#include <mutex>
  
struct Node {
  void *node;
  struct Node *next;
};
   
static Node list;
static std::mutex m;
static std::condition_variable condition;
   
void consume_list_element() {
  std::unique_lock<std::mutex> lk(m);
   
  while (list.next == nullptr) {
    condition.wait(lk);
  }
  
  // Proceed when condition holds.
}

bugprone-standalone-empty ON

std::vector<int> v;
...
v.empty(); // Returns bool not used.  Probably meant to clear.
->
v.clear(); // which returns void.

bugprone-string-constructor ON

std::string str('x', 50); // should be str(50, 'x')

std::string("test", 200);   // Will include random characters after "test".
std::string_view("test", 200);

std::string("test", 0);   // Creation of an empty string.
std::string_view("test", 0);

bugprone-string-integer-assignment ON

std::string s;
int x = 5965;
s = 6;
s = x; // implicit cast to char which is unintended
->
s = std::to_string(x);
s = static_cast<char>(6); // or an explicit cast to supress false positives

bugprone-string-literal-with-embedded-nul ON

Flags:

const char* Example[] = "Invalid character: \0x12 should be \x12";
const char* Bytes[] = "\x03\0x02\0x01\0x00\0xFF\0xFF\0xFF";
// each \0 is interpreted as null

std::string str("abc\0def");  // "def" is truncated
str += "\0";                  // This statement is doing nothing
if (str == "\0abc") return;   // This expression is always true

bugprone-stringview-nullptr ON

Flags:

std::string_view sv = nullptr;

sv = nullptr;

bool is_empty = sv == nullptr;
bool isnt_empty = sv != nullptr;

accepts_sv(nullptr);

accepts_sv({{}});  // A

accepts_sv({nullptr, 0});  // B

and prefers:

std::string_view sv = {};

sv = {};

bool is_empty = sv.empty();
bool isnt_empty = !sv.empty();

accepts_sv("");

accepts_sv("");  // A

accepts_sv({nullptr, 0});  // B

bugprone-suspicious-enum-usage ON

enum { A, B, C };
enum { D, E, F = 5 };
enum { G = 10, H = 11, I = 12 };

unsigned flag;
flag =
    A |
    H; // OK, disjoint value intervals in the enum types ->probably good use.
flag = B | F; // Warning, have common values so they are probably misused.

// Case 2:
enum Bitmask {
  A = 0,
  B = 1,
  C = 2,
  D = 4,
  E = 8,
  F = 16,
  G = 31 // OK, real bitmask.
};

enum Almostbitmask {
  AA = 0,
  BB = 1,
  CC = 2,
  DD = 4,
  EE = 8,
  FF = 16,
  GG // Problem, forgot to initialize.
};

unsigned flag = 0;
flag |= E; // OK.
flag |=
    EE; // Warning at the decl, and note that it was used here as a bitmask.

bugprone-suspicious-include ON

Flags:

#include "Dinosaur.hpp"     // OK, .hpp files tend not to have definitions.
#include "Pterodactyl.h"    // OK, .h files tend not to have definitions.
#include "Velociraptor.cpp" // Warning, filename is suspicious.
#include_next <stdio.c>     // Warning, filename is suspicious.

bugprone-suspicious-memory-comparison ON

Because std::memcmp() performs a bytewise comparison of the object representations, if the implementation uses a vtable pointer as part of the object representation, it will compare vtable pointers. If the dynamic type of either c1 or c2 is a derived class of type C, the comparison may fail despite the value representation of either object.

#include <cstring>
  
class C {
  int i;
  
public:
  virtual void f();
   
  // ...
};
  
void f(C &c1, C &c2) {
  if (!std::memcmp(&c1, &c2, sizeof(C))) {
    // ...
  }
}

->

class C {
  int i;
  
public:
  virtual void f();
   
  bool operator==(const C &rhs) const {
    return rhs.i == i;
  }
 
  // ...
};
  
void f(C &c1, C &c2) {
  if (c1 == c2) {
    // ...
  }
}

In this compliant solution, C defines an equality operator that is used instead of calling std::memcmp(). This solution ensures that only the value representation of the objects is considered when performing the comparison.

bugprone-suspicious-memset-usage ON

This check finds memset() calls with potential mistakes in their arguments.

void foo() {
  int i[5] = {1, 2, 3, 4, 5};
  int *ip = i;
  char c = '1';
  char *cp = &c;
  int v = 0;

  // Case 1
  memset(ip, '0', 1); // suspicious
  memset(cp, '0', 1); // OK

  // Case 2
  memset(ip, 0xabcd, 1); // fill value gets truncated
  memset(ip, 0x00, 1);   // OK

  // Case 3
  memset(ip, sizeof(int), v); // zero length, potentially swapped
  memset(ip, 0, 1);           // OK
}

bugprone-suspicious-missing-comma ON

String literals placed side-by-side are concatenated at translation phase 6 (after the preprocessor). This feature is used to represent long string literal on multiple lines.

const char* Test[] = {
  "line 1",
  "line 2"     // Missing comma!
  "line 3",
  "line 4",
  "line 5"
};

const char* A[] = "This is a test"; // OK
const char* B[] = "This" " is a "    "test";  // Warns

bugprone-suspicious-realloc-usage ON

This check finds usages of realloc where the return value is assigned to the same expression as passed to the first argument: p = realloc(p, size); The problem with this construct is that if realloc fails it returns a null pointer but does not deallocate the original memory.

struct A {
  void *p;
};

A &getA();

void foo(void *p, A *a, int new_size) {
  p = realloc(p, new_size); // warning: 'p' may be set to null if 'realloc' fails, which may result in a leak of the original buffer
  a->p = realloc(a->p, new_size); // warning: 'a->p' may be set to null if 'realloc' fails, which may result in a leak of the original buffer
  getA().p = realloc(getA().p, new_size); // no warning
}

void foo1(void *p, int new_size) {
  void *p1 = p;
  p = realloc(p, new_size); // no warning
}

bugprone-suspicious-semicolon ON

Finds most instances of stray semicolons that unexpectedly alter the meaning of the code. More specifically, it looks for if, while, for and for-range statements whose body is a single semicolon, and then analyzes the context of the code (e.g. indentation) in an attempt to determine whether that is intentional.

if (x < y); // warns
{
  x++;
}

while ((line = readLine(file)) != NULL); //warns
  processLine(line);

if (x >= y); //warns
x -= y;

while (readWhitespace())
  ;                          // Does not warn as drastic unexpected change implies intended to do so

  Token t = readNextToken();

bugprone-suspicious-string-compare ON

if (strcmp(...))       // Implicitly compare to zero
if (!strcmp(...))      // Won't warn
if (strcmp(...) != 0)  // Won't warn

if (strcmp(...) == -1)  // Incorrect usage of the returned value.

if (strcmp(...) < 0.)  // Incorrect usage of the returned value.

bugprone-suspicious-stringview-data-usage ON

Identifies suspicious usages of std::string_view::data() that could lead to reading out-of-bounds data due to inadequate or incorrect string null termination.

void printString(const std::string& str) {
  std::cout << "String: " << str << std::endl;
}

void something(std::string_view sv) {
  printString(sv.data());   // warns of implicit casting of char* to string
}

-> printString(std::string(sv)); // Manually create a temporary string to be memory safe or...
-> void printString(const std::string_view& sv) // Change printString API to accept string_view directly.

bugprone-swapped-arguments ON (unless too many false positives)

Flags

void printNumbers(int a, float b);

int main() {
  // Swapped arguments: float passed as int, int as float)
  printNumbers(10.0f, 5);
  return 0;
}

bugprone-switch-missing-default-case ON

// Example 1:
// warning: switching on non-enum value without default case may not cover all cases
switch (i) {
case 0:
  break;
}
// adding
// default:
//   break;
// fixes warning


// Example 2:
enum E { eE1 };
E e = eE1;
switch (e) { // no-warning
case eE1:
  break;
}

bugprone-terminating-continue ON

void f() {
do {
  // some code
  continue; // warn: terminating continue
  // some other code
} while(false);

Fixed by setting flag in while check instead of calling continue.

bugprone-throw-keyword-missing ON (but might need to turn off because of MICM error handling)

void f(int i) {
  if (i < 0) {
    // Warns exception is created but is not thrown.
    std::runtime_error("Unexpected argument");
  }
}

bugprone-too-small-loop-variable ON

int main() {
  long size = 294967296l;
  for (short i = 0; i < size; ++i) {} // warns `i` is too small
}

bugprone-unchecked-optional-access ON

Prevents

void f(std::optional<int> opt) {
  use(*opt); // unsafe: it is unclear whether `opt` has a value.
  // fixed with if (opt.has_value()) {...}
}

void f(std::optional<int> opt) {
  if (opt.has_value()) {
  } 
  else {
    use(opt.value()); // unsafe: it is clear that `opt` does *not* have a value.
  }
}

void f(Foo foo) {
  if (foo.opt().has_value()) {
    use(*foo.opt()); // unsafe: it is unclear whether `foo.opt()` has a value.
    // fixed with const auto& foo_opt = foo.opt(); foo_opt.has_value()
  }
}

bugprone-undefined-memory-manipulation ON

Finds calls of memory manipulation functions memset(), memcpy() and memmove() on non-TriviallyCopyable objects resulting in undefined behavior.

For example, when using memcpy to copy std::string, pointer data is being copied, and it can result in a double free issue.

#include <cstring>
#include <string>

int main() {
    std::string source = "Hello";
    std::string destination;

    std::memcpy(&destination, &source, sizeof(std::string));

    // Undefined behavior may occur here, during std::string destructor call.
    return 0;
}

bugprone-undelegated-constructor ON

struct Ctor {
  Ctor();
  Ctor(int);
  Ctor(int, int);
  Ctor(Ctor *i) {
    // All Ctor() calls result in a temporary object
    Ctor(); // did you intend to call a delegated constructor?
    Ctor(0); // did you intend to call a delegated constructor?
    Ctor(1, 2); // did you intend to call a delegated constructor?
    foo();
  }
};

bugprone-unhandled-exception-at-new MAYBE

Finds calls to new with missing exception handler for std::bad_alloc.

Adds significant amount of error handling that for non-mission critical systems may add unnecessary verbosity.

int *f() noexcept {
  int *p = new int[1000]; // warning: missing exception handler for allocation failure at 'new'
  // ...
  return p;
}

->

int *f1() { // not 'noexcept'
  int *p = new int[1000]; // no warning: exception can be handled outside
                          // of this function
  // ...
  return p;
}

int *f2() noexcept {
  try {
    int *p = new int[1000]; // no warning: exception is handled
    // ...
    return p;
  } catch (std::bad_alloc &) {
    // ...
  }
  // ...
}

int *f3() noexcept {
  int *p = new (std::nothrow) int[1000]; // no warning: "nothrow" is used
  // ...
  return p;
}

bugprone-unhandled-self-assignment ON

Finds user-defined copy assignment operators which do not protect the code against self-assignment either by checking self-assignment explicitly or using the copy-and-swap or the copy-and-move method.

class T {
int* p;

public:
  T(const T &rhs) : p(rhs.p ? new int(*rhs.p) : nullptr) {}
  ~T() { delete p; }

  // ...

  T& operator=(const T &rhs) { // warns here due to no self check
    delete p;
    p = new int(*rhs.p);
    return *this;
  }
};

->

class T {
int* p;

public:
  T(const T &rhs) : p(rhs.p ? new int(*rhs.p) : nullptr) {}
  ~T() { delete p; }

  // ...

  T& operator=(const T &rhs) {
    if(this == &rhs)
      return *this;

    delete p;
    p = new int(*rhs.p);
    return *this;
  }
};

bugprone-unique-ptr-array-mismatch ON

Finds initializations of C++ unique pointers to non-array type that are initialized with an array.

std::unique_ptr<Foo> x(new Foo[10]); // -> std::unique_ptr<Foo[]> x(new Foo[10]);
//                     ^ warning: unique pointer to non-array is initialized with array
std::unique_ptr<Foo> x1(new Foo), x2(new Foo[10]); // no replacement
//                                   ^ warning: unique pointer to non-array is initialized with array

D d;
std::unique_ptr<Foo, D> x3(new Foo[10], d); // no warning (custom deleter used)

struct S {
  std::unique_ptr<Foo> x(new Foo[10]); // no replacement in this case
  //                     ^ warning: unique pointer to non-array is initialized with array
};

bugprone-unsafe-functions ON

Checks for functions that have safer, more secure replacements available, or are considered deprecated due to design flaws. The check heavily relies on the functions from the Annex K. “Bounds-checking interfaces” of C11.

#ifndef __STDC_LIB_EXT1__
#error "Annex K is not supported by the current standard library implementation."
#endif

#define __STDC_WANT_LIB_EXT1__ 1

#include <string.h> // Defines functions from Annex K.
#include <stdio.h>

enum { BUFSIZE = 32 };

void Unsafe(const char *Msg) {
  static const char Prefix[] = "Error: ";
  static const char Suffix[] = "\n";
  char Buf[BUFSIZE] = {0};

  strcpy(Buf, Prefix); // warning: function 'strcpy' is not bounds-checking; 'strcpy_s' should be used instead.
  strcat(Buf, Msg);    // warning: function 'strcat' is not bounds-checking; 'strcat_s' should be used instead.
  strcat(Buf, Suffix); // warning: function 'strcat' is not bounds-checking; 'strcat_s' should be used instead.
  if (fputs(buf, stderr) < 0) {
    // error handling
    return;
  }
}

void UsingSafeFunctions(const char *Msg) {
  static const char Prefix[] = "Error: ";
  static const char Suffix[] = "\n";
  char Buf[BUFSIZE] = {0};

  if (strcpy_s(Buf, BUFSIZE, Prefix) != 0) {
    // error handling
    return;
  }

  if (strcat_s(Buf, BUFSIZE, Msg) != 0) {
    // error handling
    return;
  }

  if (strcat_s(Buf, BUFSIZE, Suffix) != 0) {
    // error handling
    return;
  }

  if (fputs(Buf, stderr) < 0) {
    // error handling
    return;
  }
}

bugprone-unused-local-non-trivial-variable ON

Warns when a local non trivial variable is unused within a function.

std::mutex my_lock;
// my_lock local variable is never used

std::future<MyObject> future1;
std::future<MyObject> future2;
// ...
MyObject foo = future1.get();
// future2 is not used.

bugprone-unused-raii ON

Finds temporaries that look like RAII objects.

{
  scoped_lock(&global_mutex);
  critical_section();
}

The destructor of the scoped_lock is called before the critical_section is entered, leaving it unprotected.

bugprone-unused-return-value ON

Warns on unused function return values. The checked functions can be configured.

bugprone-virtual-near-miss ON

Warn if a function is a near miss (i.e. the name is very similar and the function signature is the same) to a virtual function from a base class.

struct Base {
  virtual void func();
};

struct Derived : Base {
  virtual void funk();
  // warning: 'Derived::funk' has a similar name and the same signature as virtual method 'Base::func'; did you mean to override it?
};