isuckatcs added inline comments.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, gamesh411, 
dkrupp, donat.nagy, baloghadamsoftware.
Herald added a project: All.


================
Comment at: 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:222
 
-    if (isVoidPointer(DynT)) {
+  while (const MemRegion *Tmp = State->getSVal(R, DynT).getAsRegion()) {
+
----------------
@Szelethus @NoQ  @xazax.hun I ran into an issue related to this line.

Consider this test case:
```lang=c++
struct CyclicPointerTest1 {
  int *ptr; // expected-note{{object references itself 'this->ptr'}}
  int dontGetFilteredByNonPedanticMode = 0;

  CyclicPointerTest1() : ptr(reinterpret_cast<int *>(&ptr)) {} // 
expected-warning{{1 uninitialized field}}
};
```

At this point, with the example above, the store looks like this:
```
{"kind": "Direct", "offset": 0, "extent": 64, value: 
&Element{temp_object{CyclicPointerTest1, S915}.ptr,0 S64b,int}}
{"kind": "Direct", "offset": 64, "extent": 32, value: 0 S32b}
```

which means the values are the following:
```
V = &Element{temp_object{CyclicPointerTest1, S915}.ptr,0 S64b,int}
R = Element{temp_object{CyclicPointerTest1, S915}.ptr,0 S64b,int}
DynT = PointerType ... 'int *'
```
when `State->getSVal(R, DynT)` is called, eventually 
`RegionStoreManager::getBinding()` will also be invoked, where because `R` is 
an `ElementRegion` we reach these lines:
```lang=c++
  if (const ElementRegion *ER = dyn_cast<ElementRegion>(R)) {
    // FIXME: Here we actually perform an implicit conversion from the loaded
    // value to the element type.  Eventually we want to compose these values
    // more intelligently.  For example, an 'element' can encompass multiple
    // bound regions (e.g., several bound bytes), or could be a subset of
    // a larger value.
    return svalBuilder.evalCast(getBindingForElement(B, ER), T, QualType{});
  }
```
What happens here is that we call `getBindingForElement(B, ER)` and whatever 
value it returns, we cast to `T`, which is `int *` in this example. The type of 
the element inside the `ER` however is an `int`, so the following lookup will 
be performed:
```
Binding being looked up: 
  {"kind": "Direct", "offset": 0, "extent": 32}
------------------------------------------------------------------------------------------------------------------------
Store:
  {"kind": "Direct", "offset": 0, "extent": 64 value: 
&Element{temp_object{CyclicPointerTest1, S915}.ptr,0 S64b,int}}
  {"kind": "Direct", "offset": 64, "extent": 32 value: 0 S32b}
```
Since extents are ignored by default, the returned value will be 
`&Element{temp_object{CyclicPointerTest1, S915}.ptr,0 S64b,int}}`, which is 
indeed a pointer, however I'm not sure that the lookup here is actually 
correct. 

As far as I understand, we want to lookup an `int` element based on it's region 
and we receive a pointer value. Is it because I misunderstand something, or 
it's an actual issue that is hid by how region store works right now? 


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D51057/new/

https://reviews.llvm.org/D51057

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to