[PATCH] D119601: [analyzer] Refactor makeNull to makeNullWithWidth (NFC)

Vince Bridgers via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Mar 19 13:41:22 PDT 2022


vabridgers added inline comments.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:378
+    type = type->isReferenceType()
+               ? Context.getPointerType(type->getPointeeType())
+               : type;
----------------
vabridgers wrote:
> NoQ wrote:
> > vabridgers wrote:
> > > NoQ wrote:
> > > > How do you discover the pointer width by looking only at the pointee type? Are objects of specific type always restricted to a specific address space? I.e., does every `int *` have the same width everywhere in the program? If not, how is this code supposed to work?
> > > The case this code addresses is represented in the test case clang/test/Analysis/cast-value-notes.cpp, this function ...
> > > 
> > > ```
> > >  24 void evalReferences(const Shape &S) {
> > >  25   const auto &C = dyn_cast<Circle>(S);
> > >  26   // expected-note at -1 {{Assuming 'S' is not a 'Circle'}}
> > >  27   // expected-note at -2 {{Dereference of null pointer}}
> > >  28   // expected-warning at -3 {{Dereference of null pointer}}
> > >  29 }
> > > 
> > > ```
> > > The crash occurs from line 25 above. 
> > > 
> > > Debug printing what I see at this point in the code for this case ...
> > > 
> > > ```
> > > QualType type : LValueReferenceType 0xffea820 'const class clang::Circle &'
> > > `-QualType 0xffea7c1 'const class clang::Circle' const
> > >   `-SubstTemplateTypeParmType 0xffea7c0 'class clang::Circle' sugar
> > >     |-TemplateTypeParmType 0xffe4f90 'X' dependent depth 0 index 0
> > >     | `-TemplateTypeParm 0xffe4f40 'X'
> > >     `-RecordType 0xffea090 'class clang::Circle'
> > >       `-CXXRecord 0xffea000 'Circle'
> > > 
> > > Context.getPointerType(type) : PointerType 0x10006ab0 'const class clang::Circle &*'
> > > `-LValueReferenceType 0xffea820 'const class clang::Circle &'
> > >   `-QualType 0xffea7c1 'const class clang::Circle' const
> > >     `-SubstTemplateTypeParmType 0xffea7c0 'class clang::Circle' sugar
> > >       |-TemplateTypeParmType 0xffe4f90 'X' dependent depth 0 index 0
> > >       | `-TemplateTypeParm 0xffe4f40 'X'
> > >       `-RecordType 0xffea090 'class clang::Circle'
> > >         `-CXXRecord 0xffea000 'Circle'
> > > 
> > > Context.getPointerType(type->getPointeeType()) : PointerType 0x10006ae0 'const class clang::Circle *'
> > > `-QualType 0xffea7c1 'const class clang::Circle' const
> > >   `-SubstTemplateTypeParmType 0xffea7c0 'class clang::Circle' sugar
> > >     |-TemplateTypeParmType 0xffe4f90 'X' dependent depth 0 index 0
> > >     | `-TemplateTypeParm 0xffe4f40 'X'
> > >     `-RecordType 0xffea090 'class clang::Circle'
> > >       `-CXXRecord 0xffea000 'Circle'
> > > 
> > > ```
> > > The LValueReferenceType causes a crash if I do not get a pointer type, looks like ... 
> > > 
> > > ```
> > > clang:  <root>/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h:219: const llvm::APSInt& clang::ento::BasicValueFactory::getZeroWithTypeSize(clang::QualType): Assertion `T->isScalarType()' failed.
> > > 
> > > ```
> > > I'm assuming the pointer type retains the address space attribute of the LValueReferenceType. 
> > > 
> > > Context.getPointerType(type->getPointeeType()) produces a QualType :  PointerType 'const class clang::Circle *' from an original QualType : LValueReferenceType 'const class clang::Circle &'
> > > 
> > > Is this not what's wanted - a pointer type instead of a reference, with the same address space qualifiers, or am I missing something in the query? 
> > > 
> > > Thanks
> > Do I understand correctly that `__attribute__((address_space))` is a type attribute that gets applied to values rather than to pointers, so `__attribute__((address_space(3))) int *x` defines `x` as "pointer to (int that lives in address_space(3))", so when you unwrap the pointer type you get an "int that lives in address_space(3)"?
> > 
> > Can you share some dumps to demonstrate that the attribute is correctly preserved by this procedure? Damn, a test case could be great if we could have them.
> I'll see what can be done about a test case, and provide the dumps for evaluation. Best
I developed "a" test methodology to demonstrate this at work. I modified the LIT case to use "dyn_cast<Circle>(S)" and "dyn_cast<__attribute__((address_space(3))) Circle>(S)", checking the program state using clang_analyzer_printState(). Here's the info I see from the program state and from debug prints in each case. 

First, the "dyn_cast<Circle>(S)" case.


```
void evalReferences(const Shape &S) {
  const auto &C = dyn_cast<Circle>(S);
  clang_analyzer_printState();
  ...
}

  "dynamic_types": [
    { "region": "SymRegion{reg_$0<const struct clang::Shape & S>}", "dyn_type": "const class clang::Circle &", "sub_classable": true }


QualType type : LValueReferenceType 0x1118ef60 'const class clang::Circle &'
`-QualType 0x1118ef01 'const class clang::Circle' const
  `-SubstTemplateTypeParmType 0x1118ef00 'class clang::Circle' sugar
    |-TemplateTypeParmType 0x111887d0 'X' dependent depth 0 index 0
    | `-TemplateTypeParm 0x11188780 'X'
    `-RecordType 0x1118e710 'class clang::Circle'
      `-CXXRecord 0x1118e680 'Circle'

Context.getPointerType(type) : PointerType 0x111a94d0 'const class clang::Circle &*'
`-LValueReferenceType 0x1118ef60 'const class clang::Circle &'
  `-QualType 0x1118ef01 'const class clang::Circle' const
    `-SubstTemplateTypeParmType 0x1118ef00 'class clang::Circle' sugar
      |-TemplateTypeParmType 0x111887d0 'X' dependent depth 0 index 0
      | `-TemplateTypeParm 0x11188780 'X'
      `-RecordType 0x1118e710 'class clang::Circle'
        `-CXXRecord 0x1118e680 'Circle'

Context.getPointerType(type->getPointeeType()) : PointerType 0x111a9500 'const class clang::Circle *'
`-QualType 0x1118ef01 'const class clang::Circle' const
  `-SubstTemplateTypeParmType 0x1118ef00 'class clang::Circle' sugar
    |-TemplateTypeParmType 0x111887d0 'X' dependent depth 0 index 0
    | `-TemplateTypeParm 0x11188780 'X'
    `-RecordType 0x1118e710 'class clang::Circle'
      `-CXXRecord 0x1118e680 'Circle'
```

then the "dyn_cast<__attribute__((address_space(3))) Circle>(S)" case


```
void evalReferences(const Shape &S) {
  const auto &C = dyn_cast<__attribute__((address_space(3))) Circle>(S);
  clang_analyzer_printState();
  ...
}

 "dynamic_types": [
    { "region": "SymRegion{reg_$0<const struct clang::Shape & S>}", "dyn_type": "const __attribute__((address_space(3))) class clang::Circle &", "sub_classable": true }


QualType type : LValueReferenceType 0x11527030 'const __attribute__((address_space(3))) class clang::Circle &'
`-QualType 0x11526fd1 'const __attribute__((address_space(3))) class clang::Circle' const
  `-SubstTemplateTypeParmType 0x11526fd0 '__attribute__((address_space(3))) class clang::Circle' sugar
    |-TemplateTypeParmType 0x115207d0 'X' dependent depth 0 index 0
    | `-TemplateTypeParm 0x11520780 'X'
    `-QualType 0x11526ce8 '__attribute__((address_space(3))) class clang::Circle' __attribute__((address_space(3)))
      `-RecordType 0x11526710 'class clang::Circle'
        `-CXXRecord 0x11526680 'Circle'

Context.getPointerType(type) : PointerType 0x115424e0 'const __attribute__((address_space(3))) class clang::Circle &*'
`-LValueReferenceType 0x11527030 'const __attribute__((address_space(3))) class clang::Circle &'
  `-QualType 0x11526fd1 'const __attribute__((address_space(3))) class clang::Circle' const
    `-SubstTemplateTypeParmType 0x11526fd0 '__attribute__((address_space(3))) class clang::Circle' sugar
      |-TemplateTypeParmType 0x115207d0 'X' dependent depth 0 index 0
      | `-TemplateTypeParm 0x11520780 'X'
      `-QualType 0x11526ce8 '__attribute__((address_space(3))) class clang::Circle' __attribute__((address_space(3)))
        `-RecordType 0x11526710 'class clang::Circle'
          `-CXXRecord 0x11526680 'Circle'

Context.getPointerType(type->getPointeeType()) : PointerType 0x11542510 'const __attribute__((address_space(3))) class clang::Circle *'
`-QualType 0x11526fd1 'const __attribute__((address_space(3))) class clang::Circle' const
  `-SubstTemplateTypeParmType 0x11526fd0 '__attribute__((address_space(3))) class clang::Circle' sugar
    |-TemplateTypeParmType 0x115207d0 'X' dependent depth 0 index 0
    | `-TemplateTypeParm 0x11520780 'X'
    `-QualType 0x11526ce8 '__attribute__((address_space(3))) class clang::Circle' __attribute__((address_space(3)))
      `-RecordType 0x11526710 'class clang::Circle'
        `-CXXRecord 0x11526680 'Circle'
```

So it looks to me like the address space attribute is retained through both the "Context.getPointerType(type)" and "Context.getPointerType(type->getPointeeType())" invocations. 

This also revealed the core.NullDereference checker is ignoring pointers with address space attributes - so the test cases need to be different for this change (at least for now). 

I think this is enough to prove this is a NFC. Please let me know if there's anything else I need to do here. 

Best




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119601



More information about the cfe-commits mailing list