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@-1 {{Assuming 'S' is not a 'Circle'}} > > > 27 // expected-note@-2 {{Dereference of null pointer}} > > > 28 // expected-warning@-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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits