[clang] [clang][analyzer] Fix the false positive ArgInitializedness warning on unnamed bit-field (PR #145066)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-static-analyzer-1

Author: None (Tedlion)

<details>
<summary>Changes</summary>

For the following code:
 struct B{
   int i  :2;
   int    :30;  // unnamed bit-field
 };

 extern void consume_B(B);

 void bitfield_B_init(void) {
   B b1;
   b1.i = 1; // b1 is initialized
   consume_B(b1);
 }

The current clang static analyzer gives false positive warning "Passed-by-value struct argument contains uninitialized data (e.g., field: '') [core.CallAndMessage]" when taking the source as C code. However, no such warning is generated when clang takes the source as C++ code.

After comparing the CallAndMessageChecker's different behaviors between C and C++, the reason is found: When FindUninitializedField::Find(const TypedValueRegion *R) is invoked, the concrete type of R is different. In C, 'b1' is considered to be a 'StackLocalsSpaceRegion', which makes 'StoreMgr.getBinding(store, loc::MemRegionVal(FR))' return an 'UndefinedVal'. While in c++, 'b1' is considered to be a 'tackArgumentsSpaceRegion', which finally makes the 'getBinding' return a SymbolVal. **I am not quite sure about the region difference, maybe in C++ there is an implicit copy constructor function?**

 Anyway, the unnamed bit-field is undefined, for it cannot be written unless using memory operation such
as 'memset'. So a special check FD->isUnnamedBitField() is added in RegionStoreManager::getBindingForField in file RegionStore.cpp.

To handle the false warning, a check isUnnamedBitField is also added in FindUninitializedField::Find in file CallAndMessageChecker.cpp.

Testcases of unnamed bit-field are added in file call-and-message.c and call-and-message.cpp. **I do not know what to do on the hash, so it may be updated?**

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


4 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp (+1-1) 
- (modified) clang/lib/StaticAnalyzer/Core/RegionStore.cpp (+14-1) 
- (modified) clang/test/Analysis/call-and-message.c (+26-1) 
- (modified) clang/test/Analysis/call-and-message.cpp (+16) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
index 86476b32309c3..677cc6ee57ad2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -259,7 +259,7 @@ class FindUninitializedField {
         if (T->getAsStructureType()) {
           if (Find(FR))
             return true;
-        } else {
+        } else if (!I->isUnnamedBitField()){
           SVal V = StoreMgr.getBinding(store, loc::MemRegionVal(FR));
           if (V.isUndef())
             return true;
diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 388034b087789..1208036700f32 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2122,8 +2122,21 @@ SVal RegionStoreManager::getBindingForField(RegionBindingsConstRef B,
   if (const std::optional<SVal> &V = B.getDirectBinding(R))
     return *V;
 
-  // If the containing record was initialized, try to get its constant value.
+  // UnnamedBitField is always Undefined unless using memory operation such
+  // as 'memset'.
+  // For example, for code
+  //    typedef struct {
+  //      int i  :2;
+  //      int    :30;  // unnamed bit-field
+  //    } A;
+  //    A a = {1};
+  // The bits of the unnamed bit-field in local variable a can be anything.
   const FieldDecl *FD = R->getDecl();
+  if (FD->isUnnamedBitField()) {
+      return UndefinedVal();
+  }
+
+  // If the containing record was initialized, try to get its constant value.
   QualType Ty = FD->getType();
   const MemRegion* superR = R->getSuperRegion();
   if (const auto *VR = dyn_cast<VarRegion>(superR)) {
diff --git a/clang/test/Analysis/call-and-message.c b/clang/test/Analysis/call-and-message.c
index b79ec8c344b6c..e2fba55d3343d 100644
--- a/clang/test/Analysis/call-and-message.c
+++ b/clang/test/Analysis/call-and-message.c
@@ -1,12 +1,19 @@
 // RUN: %clang_analyze_cc1 %s -verify \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-config core.CallAndMessage:ArgPointeeInitializedness=true \
+// RUN:   -analyzer-config core.CallAndMessage:ArgInitializedness=false \
 // RUN:   -analyzer-output=plist -o %t.plist
 // RUN: cat %t.plist | FileCheck %s
 
 // RUN: %clang_analyze_cc1 %s -verify=no-pointee \
 // RUN:   -analyzer-checker=core \
-// RUN:   -analyzer-config core.CallAndMessage:ArgPointeeInitializedness=false
+// RUN:   -analyzer-config core.CallAndMessage:ArgPointeeInitializedness=false \
+// RUN:   -analyzer-config core.CallAndMessage:ArgInitializedness=false
+
+// RUN: %clang_analyze_cc1 %s -verify=arg-init \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config core.CallAndMessage:ArgPointeeInitializedness=false \
+// RUN:   -analyzer-config core.CallAndMessage:ArgInitializedness=true
 
 // no-pointee-no-diagnostics
 
@@ -22,3 +29,21 @@ void pointee_uninit(void) {
 // checker, as described in the CallAndMessage comments!
 // CHECK: <key>issue_hash_content_of_line_in_context</key>
 // CHECK-SAME: <string>97a74322d64dca40aa57303842c745a1</string>
+
+typedef struct {
+  int i  :2;
+  int    :30;  // unnamed bit-field
+} B;
+
+extern void consume_B(B);
+
+void bitfield_B_init(void) {
+  B b1;
+  b1.i = 1; // b1 is initialized
+  consume_B(b1);
+}
+
+void bitfield_B_uninit(void) {
+  B b2;
+  consume_B(b2); // arg-init-warning{{Passed-by-value struct argument contains uninitialized data (e.g., field: 'i') [core.CallAndMessage]}}
+}
\ No newline at end of file
diff --git a/clang/test/Analysis/call-and-message.cpp b/clang/test/Analysis/call-and-message.cpp
index 25ae23833478b..1e76973f49e13 100644
--- a/clang/test/Analysis/call-and-message.cpp
+++ b/clang/test/Analysis/call-and-message.cpp
@@ -169,4 +169,20 @@ void record_uninit() {
 // CHECK-SAME: <string>a46bb5c1ee44d4611ffeb13f7f499605</string>
 // CHECK: <key>issue_hash_content_of_line_in_context</key>
 // CHECK-SAME: <string>e0e0d30ea5a7b2e3a71e1931fa0768a5</string>
+
+struct B{
+  int i  :2;
+  int    :30;  // unnamed bit-field
+};
+
+void bitfield_B_init(void) {
+  B b1;
+  b1.i = 1; // b1 is initialized
+  consume(b1);
+}
+
+void bitfield_B_uninit(void) {
+  B b2;
+  consume(b2); // arg-init-warning{{Passed-by-value struct argument contains uninitialized data (e.g., field: 'i') [core.CallAndMessage]}}
+}
 } // namespace uninit_arg

``````````

</details>


https://github.com/llvm/llvm-project/pull/145066