Saturday, September 16, 2017

Useful GCC warning options not enabled by -Wall -Wextra

Note: This blog post was written for GCC 7. Later versions of the compiler may have added some of these warnings to -Wall or -Wextra.

GCC can warn about questionable constructs in the source code, but most such warnings are not enabled by default – developers need to use the options -Wall and -Wextra to get all generally useful warnings. There are many additional warning options that are not enabled by -Wall -Wextra as they may produce too many false positive warnings or be targeted to a specific obscure use case, but I think a few of them (listed below) may be useful for general use.

-Wduplicated-cond

Warn about duplicated condition in if-else-if chains, such as
int foo(int a)
{
  int b;
  if (a == 0)
    b = 42;
  else if (a == 0)
    b = 43;
  return b;
}
Note: -Wduplicated-cond was added in GCC 6.


-Wduplicated-branches

Warn when an if-else has identical branches, such as
int foo(int a)
{
  int b;
  if (a == 0)
    b = 42;
  else
    b = 42;
  return b;
}
It also warns for conditional operators having identical second and third expressions
int foo(int a)
{
  int b;
  b = (a == 0) ? 42 : 42;
  return b;
}
Note: -Wduplicated-branches was added in GCC 7.


-Wlogical-op

Warn about use of logical operations where a bitwise operation probably was intended, such as
int foo(int a)
{
  a = a || 0xf0;
  return a;
}
It also warns when the operands of logical operations are the same
int foo(int a)
{
  if (a < 0 && a < 0)
    return 0;
  return 1;
}
Note: -Wlogical-op was added in GCC 4.3.


-Wrestrict

Warn when the compiler detects that an argument passed to a restrict or __restrict qualified parameter alias with another parameter.
void bar(char * __restrict, char * __restrict);

void foo(char *p)
{
  bar(p, p);
}
Note: -Wrestrict was added in GCC 7.


-Wnull-dereference

Warn when the compiler detects paths that dereferences a null pointer.
void foo(int *p, int a)
{
  int *q = 0;
  if (0 <= a && a < 10)
    q = p + a;
  *q = 1;  // q may be NULL
}
Note: -Wnull-dereference was added in GCC 6.


-Wold-style-cast

Warn if a C-style cast to a non-void type is used within a C++ program.
int *foo(void *p)
{
  return (int *)p;
}
Note: -Wold-style-cast was added before GCC 3.
Note: -Wold-style-cast is only available for C++.


-Wuseless-cast

Warn when an expression is cast to its own type within a C++ program.
int *foo(int *p)
{
  return static_cast<int *>(p);
}
Note: -Wuseless-cast was added in GCC 4.8.
Note: -Wuseless-cast is only available for C++.


-Wjump-misses-init

Warn if a goto statement or a switch statement jumps forward across the initialization of a variable, or jumps backward to a label after the variable has been initialized.
int foo(int a)
{
  int b;
  switch (a)
  {
  case 0:
    b = 0;
    int c = 42;
    break;
  default:
    b = c;  // c not initialized here
  }
  return b;
}
Note: -Wjump-misses-init was added in GCC 4.5.
Note: -Wjump-misses-init is only available for C – jumping over variable initialization is an error in C++.


-Wdouble-promotion

Warn when a value of type float is implicitly promoted to double.

Floating point constants have the type double, which makes it easy to accidentally compute in a higher precision than intended. For example,
float area(float radius)
{
  return 3.14159 * radius * radius;
}
does all the computation in double precision instead of float. There is normally no difference in performance between float and double for scalar x86 code (although there may be a big difference for small, embedded, CPUs), but double may be much slower after vectorization as only half the number of elements fit in the vectors compared to float values.

Note: -Wdouble-promotion was added in GCC 4.6.


-Wshadow

Warn when a local variable or type declaration shadows another variable, parameter, type, or class member.
int result;

int foo(int *p, int len)
{
  int result = 0;  // Shadows the global variable
  for (int i = 0; i < len; i++)
    result += p[i];
  return result;
}
Note: -Wshadow was added before GCC 3.


-Wformat=2

The -Wformat option warns when calls to printf, scanf, and similar functions have an incorrect format string or when the arguments do not have the correct type for the format string. The option is enabled by -Wall, but it can be made more aggressive by adding -Wformat=2 which adds security-related warnings. For example, it warns for
#include <stdio.h>

void foo(char *p)
{
  printf(p);
}
that may be a security hole if the format string came from untrusted input and contains ‘%n’.

Note: -Wformat=2 was added in GCC 3.0.

Friday, September 8, 2017

Follow-up on “Why undefined behavior may call a never-called function”

I have recieved several questions on the previous blog post about what happens for more complex cases, such as
#include <cstdlib>

typedef int (*Function)();

static Function Do;

static int EraseAll() {
  return system("rm -rf /");
}

static int LsAll() {
  return system("ls /");
}

void NeverCalled() {
  Do = EraseAll;
}

void NeverCalled2() {
  Do = LsAll;
}

int main() {
  return Do();
}
where the compiler will find three possible values for Do: EraseAll, LsAll, and 0.

The value 0 is eliminated from the set of possible values for the call in main, in the same way as for the simpler case, but the compiler cannot change the indirect call to a direct call as there are still two possible values for the function pointer, and clang generates the expected
main:
        jmpq    *Do(%rip)
But a compiler could transform the line
return Do();
to
if (Do == LsAll)
  return LsAll();
else
  return EraseAll();
that has the same surprising effect of calling a never-called function. This transformation would be silly in this case as the cost of the extra comparison is similar to the cost of the eliminated indirect call, but it may be a good optimization when the compiler can determine that the result will be faster (for example, if the functions can be simplified after inlining). I don’t know if this is implemented in clang/LLVM — I could not get this to happen when writing some small test-programs. But, for example, GCC’s implementation of devirtualization can do it if -fdevirtualize-speculatively is enabled, so this is not a hypothetical optimization (GCC does, however, not take advantage of undefined behavior in this case, so it will not insert calls to never-called functions).

Monday, September 4, 2017

Why undefined behavior may call a never-called function

My twitter feed has recently been filled with discussions about the following program
#include <cstdlib>

typedef int (*Function)();

static Function Do;

static int EraseAll() {
  return system("rm -rf /");
}

void NeverCalled() {
  Do = EraseAll;  
}

int main() {
  return Do();
}
that clang compiles to
main:
        movl    $.L.str, %edi
        jmp     system

.L.str:
        .asciz  "rm -rf /"
That is, the compiled program executes “rm -rf /” even though the original program never calls EraseAll!

Clang is allowed to do this – the function pointer Do is initialized to 0 as it is a static variable, and calling 0 invokes undefined behavior – but it may seem strange that the compiler chooses to generate this code. It does, however, follow naturally from how compilers analyze programs...

Eliminating function pointers can give big performance improvements – especially for C++ as virtual functions are generated as function pointers and changing these to direct calls enable optimizations such as inlining. It is in general hard to track the possible pointer values through the code, but it is easy in this program – Do is static and its address is not taken, so the compiler can trivially see all writes to it and determines that Do must have either the value 0 or the value EraseAll (as NeverCalled may have been called from, for example, a global constructor in another file before main is run). The compiler can remove 0 from the set of possible values when processing the call to Do as it would invoke undefined behavior, so the only possible value is EraseAll and the compiler changes
return Do();
to
return EraseAll();

I’m not too happy with taking advantage of undefined behavior in order to eliminate possible pointer values as this has a tendency to affect unrelated code, but there may be good reasons for clang/LLVM doing this (for example, it may be common that devirtualization is prevented as the set of possible pointer values contain a 0 because the compiler finds a spurious pure virtual function).

Update: I wrote a follow-up post discussing a slightly more complex case.