r179395 - [analyzer]Print field region even when the base region is not printable

[Anna Zaks]

On Apr 12, 2013, at 12:35 PM, Jordan Rose <jordan_rose at apple.com> wrote:

> 
> On Apr 12, 2013, at 11:40 , Anna Zaks <ganna at apple.com> wrote:
> 
>> Author: zaks
>> Date: Fri Apr 12 13:40:21 2013
>> New Revision: 179395
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=179395&view=rev
>> Log:
>> [analyzer]Print field region even when the base region is not printable
>> 
>> Modified:
>>    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
>>    cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
>>    cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
>>    cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
>>    cfe/trunk/test/Analysis/inlining/path-notes.c
>> 
>> Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h?rev=179395&r1=179394&r2=179395&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h (original)
>> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h Fri Apr 12 13:40:21 2013
>> @@ -169,6 +169,8 @@ public:
>>   /// \brief Print the region for use in diagnostics.
>>   virtual void printPretty(raw_ostream &os) const;
>> 
>> +  virtual void printPrettyNoQuotes(raw_ostream &os) const;
> 
> Can this be named more clearly? This is being used when printing part of a larger expression, so how about something like 'printPrettyPartial' or 'printPiecePretty'?
> 
>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=179395&r1=179394&r2=179395&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Fri Apr 12 13:40:21 2013
>> @@ -559,6 +559,15 @@ bool MemRegion::canPrintPretty() const {
>> }
>> 
>> void MemRegion::printPretty(raw_ostream &os) const {
>> +  assert(canPrintPretty() && "This region cannot be printed pretty.");
>> +  os << "'";
>> +  printPrettyNoQuotes(os);
>> +  os << "'";
>> +  return;
>> +}
> 
>> +void MemRegion::printPrettyNoQuotes(raw_ostream &os) const {
>> +  assert(canPrintPretty() && "This region cannot be printed pretty.");
>>   return;
>> }
> 
> This should just be llvm_unreachable. If a region really is pretty-printed by not printing anything, it should override both methods.
> 
OK.
> Oh shoot, CXXBaseObjectRegion is such a region. Here is the test case:
> 
> struct Base { int *x; };
> struct Derived : public Base {};
> 
> void test(Derived d) {
>   d.x = 0;
>   *d.x = 1;
> }
> 
I don't follow. What is this example demonstrating?
> 
>> @@ -566,7 +575,7 @@ bool VarRegion::canPrintPretty() const {
>>   return true;
>> }
>> 
>> -void VarRegion::printPretty(raw_ostream &os) const {
>> +void VarRegion::printPrettyNoQuotes(raw_ostream &os) const {
>>   os << getDecl()->getName();
>> }
>> 
>> @@ -574,17 +583,32 @@ bool ObjCIvarRegion::canPrintPretty() co
>>   return true;
>> }
>> 
>> -void ObjCIvarRegion::printPretty(raw_ostream &os) const {
>> +void ObjCIvarRegion::printPrettyNoQuotes(raw_ostream &os) const {
>>   os << getDecl()->getName();
>> }
>> 
>> bool FieldRegion::canPrintPretty() const {
>> -  return superRegion->canPrintPretty();
>> +  return true;
>> +}
>> +
>> +void FieldRegion::printPrettyNoQuotes(raw_ostream &os) const {
>> +  if (superRegion->canPrintPretty()) {
>> +    superRegion->printPrettyNoQuotes(os);
>> +    os << "." << getDecl()->getName();
>> +  } else {
>> +    os << "field " << "\'" << getDecl()->getName() << "'";
>> +  }
>> }
> 
> Won't this break for nested fields?
> 

Good catch!


> struct Outer {
>   struct Inner {
>     int *p;
>   } inner;
> };
> 
> void test(Outer *wrapperPtr) {
>   wrapperPtr->inner.p = 0;
>   *wrapperPtr->inner.p = 1;
> }
> 

What do we want to print in this case? Options:
A: "nothing"
B: "field 'inner' of field 'p'"

B looks OK, but might get a bit too complex with a lot of nesting.

> I think you're going to need to have different levels of 'canPrintPretty' to fix this one.
> 
> 
>> void FieldRegion::printPretty(raw_ostream &os) const {
>> -  superRegion->printPretty(os);
>> -  os << "." << getDecl()->getName();
>> +  if (superRegion->canPrintPretty()) {
>> +    os << "\'";
>> +    printPrettyNoQuotes(os);
>> +    os << "'";
>> +  } else {
>> +    printPrettyNoQuotes(os);
>> +  }
>> +  return;
>> }
> 
> I feel like this is indicative of the problems above—the same check has to be repeated here and in FieldRegion::printPrettyNoQuotes

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130412/7fd8a99d/attachment.html>