Search code examples
c++constructorclangclang-static-analyzer

Can a constructor affect other fields of an enclosing object, or is this a static analysis false positive?


Consider this C++ code:

struct SomeStruct {
  SomeStruct() noexcept;
};

//SomeStruct::SomeStruct() noexcept {}

class SomeClass {
  const bool b;
  const SomeStruct s;

public:
  SomeClass() : b(true) {}
  operator bool() const { return b; }
};

void f() {
  int *p = new int;
  if (SomeClass())
    delete p;
}

When I run clang --analyze -Xanalyzer -analyzer-output=text on it, I get this:

q72007867.cpp:20:1: warning: Potential leak of memory pointed to by 'p' [cplusplus.NewDeleteLeaks]
}
^
q72007867.cpp:17:12: note: Memory is allocated
  int *p = new int;
           ^~~~~~~
q72007867.cpp:18:7: note: Assuming the condition is false
  if (SomeClass())
      ^~~~~~~~~~~
q72007867.cpp:18:3: note: Taking false branch
  if (SomeClass())
  ^
q72007867.cpp:20:1: note: Potential leak of memory pointed to by 'p'
}
^
1 warning generated.

Uncommenting the definition of SomeStruct's constructor makes the warning go away, though. Swapping the order of const bool b; and const SomeStruct s; also makes it go away. In the original program, is there actually some other definition of SomeStruct's constructor that would lead to the false branch being taken there, or is this a false positive in Clang's static analyzer?


Solution

  • There is no standard compliant way for a const member to be changed after initialization; any mechanism is going to be UB.

    Like

    struct foo{
      const bool b=true;
      foo(){ b=false; }
    };
    

    is illegal, as is code that const_casts b to edit it like:

    struct foo{
      const bool b=true;
      foo(){ const_cast<bool&>(b)=false; }
    };
    

    (this second version compiles, but produces UB).

    Such UB is sadly not that rare. For example, I could implement the constructor of SomeStruct to fiddle with memory before the this pointer address. It would be doubly illegal (modifying a const value after construction, and violating reachability rules), but depending on optimization settings it could work.

    On the other hand, the compiler is free to notice that the only constructor assigns true to b and then convert operator bool to just return true.

    But instead, the static code analyzer gives up on provong the state of b once a call to a function body outside of the visible source code occurs. This is a pretty reasonable thing to give up on. Here, the function even gets a pointer into the same temporary object; doing a full proof that said pointer cannot change some state regardless of what code is run is possible, but failing to do that seems also reasonable.

    Style wise, the code is also a bit of a mess. A provably true branch either should not exist, or the failure branch should semantically make sense. Neither occurs here; anyone reading this code cannot determine correctness from code structure; the code structure looks misleading.