[clang] Fix -Wuninitialized for values passed by const pointers

[igorkudrin]

This enables producing a "variable is uninitialized" warning when a value is passed to a pointer-to-const argument:

void foo(const int *);
void test() {
  int *v;
  foo(v);
}

Fixes #37460

[llvmbot]

@llvm/pr-subscribers-libcxx
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-analysis

Author: Igor Kudrin (igorkudrin)

Changes

This enables producing a "variable is uninitialized" warning when a value is passed to a pointer-to-const argument:

void foo(const int *);
void test() {
  int *v;
  foo(v);
}

Fixes #37460

---

Full diff: https://github.com/llvm/llvm-project/pull/147221.diff

2 Files Affected:

• (modified) clang/lib/Analysis/UninitializedValues.cpp (+5-8)
• (modified) clang/test/SemaCXX/uninitialized.cpp (+6-2)

diff --git a/clang/lib/Analysis/UninitializedValues.cpp b/clang/lib/Analysis/UninitializedValues.cpp
index b2a68b6c39a7e..540838f89f20c 100644
--- a/clang/lib/Analysis/UninitializedValues.cpp
+++ b/clang/lib/Analysis/UninitializedValues.cpp
@@ -438,13 +438,10 @@ void ClassifyRefs::VisitCallExpr(CallExpr *CE) {
     return;
   }
   bool isTrivialBody = hasTrivialBody(CE);
-  // If a value is passed by const pointer to a function,
-  // we should not assume that it is initialized by the call, and we
-  // conservatively do not assume that it is used.
-  // If a value is passed by const reference to a function,
-  // it should already be initialized.
-  for (CallExpr::arg_iterator I = CE->arg_begin(), E = CE->arg_end();
-       I != E; ++I) {
+  // A value passed by const pointer or reference to a function should already
+  // be initialized.
+  for (CallExpr::arg_iterator I = CE->arg_begin(), E = CE->arg_end(); I != E;
+       ++I) {
     if ((*I)->isGLValue()) {
       if ((*I)->getType().isConstQualified())
         classify((*I), isTrivialBody ? Ignore : ConstRefUse);
@@ -453,7 +450,7 @@ void ClassifyRefs::VisitCallExpr(CallExpr *CE) {
       const auto *UO = dyn_cast<UnaryOperator>(Ex);
       if (UO && UO->getOpcode() == UO_AddrOf)
         Ex = UO->getSubExpr();
-      classify(Ex, Ignore);
+      classify(Ex, Use);
     }
   }
 }
diff --git a/clang/test/SemaCXX/uninitialized.cpp b/clang/test/SemaCXX/uninitialized.cpp
index c7b987e2172e6..4a944ba830bc3 100644
--- a/clang/test/SemaCXX/uninitialized.cpp
+++ b/clang/test/SemaCXX/uninitialized.cpp
@@ -162,12 +162,16 @@ void test_const_ptr() {
   int a;
   int b;  // expected-note {{initialize the variable 'b' to silence this warning}}
   foo(&a);
-  bar(&b);
-  b = a + b; // expected-warning {{variable 'b' is uninitialized when used here}}
+  bar(&b); // expected-warning {{variable 'b' is uninitialized when used here}}
+  b = a + b;
   int *ptr;  //expected-note {{initialize the variable 'ptr' to silence this warning}}
   const int *ptr2;
   foo(ptr); // expected-warning {{variable 'ptr' is uninitialized when used here}}
   foobar(&ptr2);
+  int *ptr3; // expected-note {{initialize the variable 'ptr3' to silence this warning}}
+  const int *ptr4; // expected-note {{initialize the variable 'ptr4' to silence this warning}}
+  bar(ptr3); // expected-warning {{variable 'ptr3' is uninitialized when used here}}
+  bar(ptr4); // expected-warning {{variable 'ptr4' is uninitialized when used here}}
 }
 }
 

[Xazax-hun]

LGTM! I have a nit inline for your consideration.

[Xazax-hun]

Could this use CE->arguments() in a range based for loop instead?

[Xazax-hun]

Actually, before merging, did you manage to run this on any non-trivial projects to see if there are any false positives? I think this change is valuable and important but these sorts of changes can expose some false positives. If that is the case (or there are some noisy patterns), we might want to do the stricter version of the analysis under a different flag.