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

Your understanding of `UndefinedVal` is correct, unlike with `SymbolVal`. 
Symbols (`SymbolVal` aka. `SymExpr`) we track values. We may or may not know 
anything about these symbols (most notably the value range that a symbol can 
hold). More importantly, we can combine such symbols into making larger 
symbols, basically embedding the history of the computation that the given 
variable holds at any given point in time. But this is likely not important 
here.

This is a critical component, so we don't accept patches without tests. Even 
tests are not enough to demonstrate correctness, thus we frequently ask for 
"measurements", or running differential analysis with and without a patch and 
observing the outcomes of many many real-world projects to have a better 
picture of what the implications are.

Frequently even doing the correct thing reveals untended other bugs that are 
actually worse than what we initially wanted to fix, thus effectively 
preventing us from doing the right thing. Don't worry, this is not the case 
with the `CallAndMessageChecker`.

You can propose a FIXME, but without more context it can do more harm than good 
if put at the wrong place with a misleading content. So to approve that, we 
will need to do some digging where the Symbol is coming from and why do we have 
that Symbol instead of Undef there?
Otherwise we are better off not having this FIXME I think.

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