================
@@ -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.
----------------
Tedlion wrote:

@steakhal I think I've found the reason.

When the source is C++, there is **an additional copy constructor invocation**. 
`b1` is created at `StackLocalsSpaceRegion`, while the  function `consume_B` 
takes the object being copy-constructed, so I suppose 
`StackArgumentsSpaceRegion` is corrected.

If the copy-constructor is non-trivial and not visible in the TU, the argument 
function `consume_B` takes maybe well-initialized in the custom 
copy-constructor, then no warnings should be raised. That's a scenario never 
happens in C.

When the copy-constructor is trivial, the analyzer's ExprEngine copies the 
binding from `StackLocalsSpaceRegion` to  `StackArgumentsSpaceRegion`.  I get 
the frame stacks handling this  
(Note the frame `8: ExprEngine::handleConstructor` and 
`ExprEngine::performTrivialCopy`):
```
#0: RegionStoreManager::getBindingForField @ RegionStore.cpp:2123
#1: RegionStoreManager::tryBindSmallStruct @ RegionStore.cpp:2826
#2: RegionStoreManager::bindStruct @ RegionStore.cpp:2855
#3: RegionStoreManager::bind @ RegionStore.cpp:2550
#4: RegionStoreManager::Bind @ RegionStore.cpp:597
#5: ProgramState::bindLoc @ ProgramState.cpp:119
#6: ExprEngine::evalBind @ ExprEngine.cpp:3741
#7: ExprEngine::performTrivialCopy @ ExprEngineCXX.cpp:88
#8: ExprEngine::handleConstructor @ ExprEngineCXX.cpp:746
#9: ExprEngine::Visit @ ExprEngine.cpp:2192
#10: ExprEngine::ProcessStmt @ ExprEngine.cpp:1135
#11: CoreEngine::HandlePostStmt @ CoreEngine.cpp:531
#12: CoreEngine::dispatchWorkItem @ CoreEngine.cpp:255
#13: `CoreEngine::ExecuteWorkList'::`2'::<lambda_1>::operator() @ 
CoreEngine.cpp:159
#14: CoreEngine::ExecuteWorkList @ CoreEngine.cpp:163
#15: AnalysisConsumer::RunPathSensitiveChecks @ ExprEngine.h:192
#16: AnalysisConsumer::RunPathSensitiveChecks @ AnalysisConsumer.cpp:768
#17: AnalysisConsumer::HandleCode @ AnalysisConsumer.cpp:737
#18: AnalysisConsumer::HandleDeclsCallGraph @ AnalysisConsumer.cpp:508
#19: AnalysisConsumer::runAnalysisOnTranslationUnit @ AnalysisConsumer.cpp:580
#20: AnalysisConsumer::HandleTranslationUnit @ AnalysisConsumer.cpp:643
#21: ParseAST @ ParseAST.cpp:183
```
The above procedure happens when parsing the statement `consume_B(b1)` and 
before the `CallAndMessageChecker::PreVisitProcessArg` is invoked.

**In function `RegionStoreManager::tryBindSmallStruct`, there are statements 
which intentionally skip the unnamed bit-fields**:
```cpp
......
  for (const auto *FD : RD->fields()) {
    if (FD->isUnnamedBitField())
      continue;
......
``` 
That's the problem when `CallAndMessageChecker::PreVisitProcessArg` is 
invoking, a direct binding for named field `i` can be found , while the binding 
for unnamed not, then a default value is returned. After commenting the 
continue lines,  the `UndefinedVal` for the unnamed bit-field is returned  in  
`FindUninitializedField::find()` and the warning "Passed-by-value struct 
argument contains uninitialized data (e.g., field: '') [core.CallAndMessage]" 
is raised, if my patch is not applied.

PS:  Since the intentionally skipping is in `tryBindSmallStruct`, so I try to 
make the struct larger:
 ```cpp
typedef struct {
    int x;
    int y;

    int   i : 2; // 1 bit
    int     :30;
} B;

extern void consume_B(B);

void bitfield_B_init(void) {
    B b1;
    b1.x = 0;
    b1.y = 0;
    b1.i = 1; // b1 is initialized
    consume_B(b1);
}
```
Now the binding for the unnamed bit-field in   `FindUninitializedField::find()` 
is the correct `SVal` `UndefinedVal`, and checking for c++ also raise a warning 
"warning: Passed-by-value struct argument contains uninitialized data (e.g., 
field: '') [core.CallAndMessage]".


https://github.com/llvm/llvm-project/pull/145066
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to