llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28k stars 11.56k forks source link

clang-analyzer: false positive destructing a union containing a union #59109

Open Pehrsons opened 1 year ago

Pehrsons commented 1 year ago

TL;DR https://godbolt.org/z/6nW54nTh5

Consider this bit:

template<typename T>
union U {
  U() {}
  ~U() {}
  T t;
};

template<typename T>
struct A {
  ~A() {
    if(b) {
      u.t.T::~T();
    }
  }
  U<T> u;
  bool b = false;
};

int main() {
  A<A<int>> nested;
  return 0;
}

clang-analyzer thinks U<A<int>>::~U() will implicitly call A<int>::~A(), therefore accessing an uninitialized A<int>::b:

[<source>:11:8: warning: Branch condition evaluates to a garbage value [clang-analyzer-core.uninitialized.Branch]]
    if(b) {
       ^
[<source>:21:13: note: Calling implicit default constructor for 'A<A<int>>']
  A<A<int>> nested;
            ^~~~~~
[<source>:21:13: note: Calling default constructor for 'U<A<int>>']
  A<A<int>> nested;
            ^~~~~~
[<source>:3:8: note: Returning without writing to 'this->t.b']
  U() {}
       ^
[<source>:21:13: note: Returning from default constructor for 'U<A<int>>']
  A<A<int>> nested;
            ^~~~~~
[<source>:21:13: note: Returning from default constructor for 'A<A<int>>']
  A<A<int>> nested;
            ^~~~~~
[<source>:22:10: note: Calling '~A']
  return 0;
         ^
[<source>:11:8: note: Field 'b' is false]
    if(b) {
       ^
[<source>:11:5: note: Taking false branch]
    if(b) {
    ^
[<source>:14:3: note: Calling '~U']
  }
  ^
[<source>:4:9: note: Calling '~A']
  ~U() {}
        ^
[<source>:11:8: note: Branch condition evaluates to a garbage value]
    if(b) {
       ^
1 warning generated.

However, assembly shows there is no implicit A<int>::~A() called from U<A<int>>::~U().

llvmbot commented 1 year ago

@llvm/issue-subscribers-clang-static-analyzer

steakhal commented 1 year ago

I don't think it's related to nested unions. Here a simplified example demonstrating the root cause, which is that we model a call to ~Clazz() to an object which never existed (no Clazz() constructor was called). https://godbolt.org/z/KqKx13c1W

int sink;
struct Clazz {
    Clazz() {
        sink = 12 / 0; // no-warning bc. we never call this.
    }
    ~Clazz() {
        sink = 12 / 0; // For some reason we reach this from `main()`, which is really weird.
    }
};
union U {
    U() {}
    ~U() {}
    Clazz t; // This is left uninitialized, hence any member of this is uninitialized!
};

int main() {
    U u;
}