1

As I understand Valgrind should report errors when code contains usage of uninitialized variables. In this toy example below printer is uninitialized, but program "happily" prints message anyway. 

#include <iostream>

class Printer {
    public:
        void print() {
            std::cout<<"I PRINT"<<std::endl;
        }
};


int main() {
    Printer* printer;
    printer->print();
};

When I test this program with Valgrind it doesn't report any errors.

Is it expected behavior? And if yes, why so?

emptysamurai
  • 161
  • 1
  • 6
  • `print()` isn't virtual, so the entire call can be bound at compile time. – user207421 Apr 28 '18 at 10:21
  • 1
    Did you compile with optimizations? You code has UB, it is entirely possible that the optimizer just called `print`. – Rakete1111 Apr 28 '18 at 10:22
  • 2
    Your `Printer::print()` doesn't modify the object, so it doesn't actually *need* a valid `this` pointer. And since dereferencing a uninitialized pointer is UB the compiler can do whatever it wants, so it probably just inlined `print()` straight into `main()` - take a look at the disassembly if you want to know what happened. – Jesper Juhl Apr 28 '18 at 10:25
  • @Rakete1111 I compiled it without optimizations, but it still prints the message – emptysamurai Apr 28 '18 at 10:53
  • First of all, this UB would have easily been detected by clang/gcc using `-Wuninitialized` (which is part of `-Wall`). And to detect this kind of UB on runtime, you may check the `-fsanitize=...` options of your compiler. – chtz Apr 28 '18 at 11:09
  • 1
    Valgrind isn't so much about C++ as it is about runtime correctness. If you want C++ correctness analysis, you might try a tool like ASAN. – Kerrek SB Apr 28 '18 at 13:00

2 Answers2

4

The variable is actually never used.

  1. The method call is inlined1, so the variable is not passed as an argument.
  2. The method itself doesn't use this in any way, so the variable is not used at all.

Above is independent of turning optimizations on or off.

As a matter of fact, in optimized code the variable will never exist at all - not even as memory allocation.

Question about a similar case: Extern variable only in header unexpectedly working, why? .

1 All methods defined in the class body are inlined by default.

Is it an Undefined Behavior?

Yes it is. Calling the method requires this to point at an actual, initialized intance of object to be well-formed. As Nir Friedman points out, compiler is free to assume that and optimize on that base (and IIRC this kind of optimizations can happen even with -O0!).

I'd personally expect the specific code in question to work in any practical conditions (as the pointer value is really irrelevant), but I would never rely on that. You should fix your code right now.

Detection

To detect usage of uninitialized variables in Clang/GCC, use option -Wuninitialized (or simply use -Wall, which includes this flag).

-Wuninitialized should mostly cover use of stack-allocated memory, though I guess some use of stack-allocated arrays may still slip. Some compilers may support including extra runtime checks for uninitialized reads with -fsanitize=... options, like -fsanitize=memory in Clang (thx, chtz). These checks should cover the edge cases as well as the use of heap-allocated memory.

Community
  • 1
  • 1
Frax
  • 5,015
  • 2
  • 17
  • 19
  • Even without optimizations it works and Valgrind doesn't report anything. But if I replace output line by `std::cout<<"I PRINT "< – emptysamurai Apr 28 '18 at 11:02
  • 1
    Yes, the variable is never used regardless of optimization. However, unoptimized code may still allocate memory for this pointer. Optimized code won't. – Frax Apr 28 '18 at 11:19
  • 1
    It is definitely ub, and in particular this has burned a lot of people who depended on this and wrote branches if this == nullptr which were optimized out. – Nir Friedman Apr 28 '18 at 13:04
  • Good point about `this == nullptr` which is relying on this behavior being consistent. I rephrased the section on UB to clearly say that this is UB and should never be used. – Frax Apr 28 '18 at 13:45
0

The main() function has undefined behaviour, since printer is uninitialised and the statement printer->print() both accesses the value of printer and dereferences it via -> and the call of the member function.

Practically, however, a compiler is permitted to handle undefined behaviour by simply assuming it is not present. Then the compiler can, if it chooses, follow a chain of logic;

  • When it sees a statement like printer->print() this means it is allowed to reason that printer has a value that can be accessed and dereferenced without introducing undefined behaviour.
  • Based on this reasoning, it is then permitted to assume that printer must have been initialised (by some means invisible to the compiler) to point at a valid object.
  • Based on this assumption, it can reason that the statement printer->print() will result in a call of Printer::print().
  • Since the compiler can see the definition of Printer::print(), it can simply inline it, and execute the statement std::cout<<"I PRINT"<<std::endl.
  • Since it doesn't need to access printer at all to produce that output, it can optimise out any reference to the variable named printer in main().

If a compiler follows the above sequence of logic, the program will simply print I PRINT and exit, without accessing any memory in a way that might trigger a report from Valgrind.

If you think the above sounds far-fetched, then you are mistaken. LLVM/Clang is one compiler that notionally follows a chain of logic very similar to what I have described. For more information have a look at the the LLVM Project Blog link to first article, second article, and third article.

Peter
  • 35,646
  • 4
  • 32
  • 74