Re: [PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Nico Weber via cfe-commits]

No worries. If it takes a while to analyze, please revert while you you
investigate, to keep trunk green.

On Wed, Aug 21, 2019 at 10:29 PM Csaba Dabis via Phabricator via
cfe-commits  wrote:

> Charusso added a comment.
>
>  return C.getNoteTag(
>   -  [=] {
>   +  [=]() -> std::string {
>SmallString<128> Msg;
>
> That was the fix by rL369609 . Somehow
> it converted to a temporary object therefore that was an issue:
>
>   [175/176] Running the Clang regression tests
>   llvm-lit:
> /b/sanitizer-x86_64-linux-fast/build/llvm/utils/lit/lit/llvm/config.py:340:
> note: using clang:
> /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/clang
>   -- Testing: 15399 tests, 64 threads --
>   Testing: 0
>   FAIL: Clang :: Analysis/cast-value-notes.cpp (355 of 15399)
>    TEST 'Clang :: Analysis/cast-value-notes.cpp'
> FAILED 
>   Script:
>   --
>   : 'RUN: at line 1';
>  /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/clang -cc1
> -internal-isystem
> /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/lib/clang/10.0.0/include
> -nostdsysteminc -analyze -analyzer-constraints=range
>  -analyzer-checker=core,apiModeling.llvm.CastValue   -analyzer-output=text
> -verify
> /b/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/test/Analysis/cast-value-notes.cpp
>   --
>   Exit Code: 1
>
>   Command Output (stderr):
>   --
>   =
>   ==43337==ERROR: AddressSanitizer: stack-use-after-scope on address
> 0x7fa639ecfa30 at pc 0x00c7ac85 bp 0x7fff83887490 sp 0x7fff83886c40
>   READ of size 19 at 0x7fa639ecfa30 thread T0
>   #0 0xc7ac84 in __asan_memcpy
> /b/sanitizer-x86_64-linux-fast/build/llvm/projects/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cc:22
>   #1 0xa328415 in copy
> /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/__string:225:50
>   #2 0xa328415 in __init
> /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/string:1792
>   #3 0xa328415 in basic_string
> /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/string:1813
>   #4 0xa328415 in str
> /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/ADT/StringRef.h:220
>   #5 0xa328415 in operator basic_string
> /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/ADT/StringRef.h:247
>   #6 0xa328415 in __call<(lambda at
> /b/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:113:7)
> &>
> /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/__functional_base:317
>   #7 0xa328415 in operator()
> /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/functional:1540
>   #8 0xa328415 in
> std::__1::__function::__func clang::ento::DynamicCastInfo const*, clang::QualType, clang::Expr const*,
> bool, bool)::$_0,
> std::__1::allocator clang::ento::DynamicCastInfo const*, clang::QualType, clang::Expr const*,
> bool, bool)::$_0>, std::__1::basic_string std::__1::char_traits, std::__1::allocator > ()>::operator()()
> /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/functional:1714
>   #9 0xa32751d in operator()
> /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/functional:1867:16
>   #10 0xa32751d in operator()
> /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/functional:2473
>   #11 0xa32751d in operator()
> /b/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:259
>   #12 0xa32751d in __invoke<(lambda at
> /b/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:259:23)
> &, clang::ento::BugReporterContext &, clang::ento::BugReport &>
> /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/type_traits:3501
>   #13 0xa32751d in __call<(lambda at
> /b/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:259:23)
> &, clang::ento::BugReporterContext &, clang::ento::BugReport &>
> /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/__functional_base:317
>   #14 0xa32751d in operator()
> /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/functional:1540
>   #15 0xa32751d in
> std::__1::__function::__func std::__1::char_traits, std::__1::allocator > ()>&&,
> bool)::'lambda'(clang::ento::BugReporterContext&, clang::ento::BugReport&),
> std::__1::allocator std::__1::char_traits, std::__1::allocator > ()>&&,
> bool)::'lambda'(clang::ento::BugReporterContext&,
> clang::ento::BugReport&)>, std::__1::basic_string std::__1::char_traits, std::__1::allocator >
> (clang::ento::BugReporterContext&,
> clang::ento::BugReport&)>::operator()(clang::ento::BugReporterContext&,
> clang::ento::BugReport&)
> 

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

 return C.getNoteTag(
  -  [=] {
  +  [=]() -> std::string {
   SmallString<128> Msg;

That was the fix by rL369609 . Somehow it 
converted to a temporary object therefore that was an issue:

  [175/176] Running the Clang regression tests
  llvm-lit: 
/b/sanitizer-x86_64-linux-fast/build/llvm/utils/lit/lit/llvm/config.py:340: 
note: using clang: 
/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/clang
  -- Testing: 15399 tests, 64 threads --
  Testing: 0 
  FAIL: Clang :: Analysis/cast-value-notes.cpp (355 of 15399)
   TEST 'Clang :: Analysis/cast-value-notes.cpp' FAILED 

  Script:
  --
  : 'RUN: at line 1';   
/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/clang -cc1 
-internal-isystem 
/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/lib/clang/10.0.0/include 
-nostdsysteminc -analyze -analyzer-constraints=range   
-analyzer-checker=core,apiModeling.llvm.CastValue   -analyzer-output=text 
-verify 
/b/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/test/Analysis/cast-value-notes.cpp
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  =
  ==43337==ERROR: AddressSanitizer: stack-use-after-scope on address 
0x7fa639ecfa30 at pc 0x00c7ac85 bp 0x7fff83887490 sp 0x7fff83886c40
  READ of size 19 at 0x7fa639ecfa30 thread T0
  #0 0xc7ac84 in __asan_memcpy 
/b/sanitizer-x86_64-linux-fast/build/llvm/projects/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cc:22
  #1 0xa328415 in copy 
/b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/__string:225:50
  #2 0xa328415 in __init 
/b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/string:1792
  #3 0xa328415 in basic_string 
/b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/string:1813
  #4 0xa328415 in str 
/b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/ADT/StringRef.h:220
  #5 0xa328415 in operator basic_string 
/b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/ADT/StringRef.h:247
  #6 0xa328415 in __call<(lambda at 
/b/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:113:7)
 &> 
/b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/__functional_base:317
  #7 0xa328415 in operator() 
/b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/functional:1540
  #8 0xa328415 in 
std::__1::__function::__func, std::__1::basic_string, 
std::__1::allocator > ()>::operator()() 
/b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/functional:1714
  #9 0xa32751d in operator() 
/b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/functional:1867:16
  #10 0xa32751d in operator() 
/b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/functional:2473
  #11 0xa32751d in operator() 
/b/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:259
  #12 0xa32751d in __invoke<(lambda at 
/b/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:259:23)
 &, clang::ento::BugReporterContext &, clang::ento::BugReport &> 
/b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/type_traits:3501
  #13 0xa32751d in __call<(lambda at 
/b/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:259:23)
 &, clang::ento::BugReporterContext &, clang::ento::BugReport &> 
/b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/__functional_base:317
  #14 0xa32751d in operator() 
/b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/functional:1540
  #15 0xa32751d in 
std::__1::__function::__func, std::__1::allocator > ()>&&, 
bool)::'lambda'(clang::ento::BugReporterContext&, clang::ento::BugReport&), 
std::__1::allocator, std::__1::allocator > ()>&&, 
bool)::'lambda'(clang::ento::BugReporterContext&, clang::ento::BugReport&)>, 
std::__1::basic_string, 
std::__1::allocator > (clang::ento::BugReporterContext&, 
clang::ento::BugReport&)>::operator()(clang::ento::BugReporterContext&, 
clang::ento::BugReport&) 
/b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/functional:1714
  #16 0xa990926 in operator() 
/b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/functional:1867:16
  #17 0xa990926 in operator() 
/b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/functional:2473
  #18 0xa990926 in generateMessage 
/b/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:572
  #19 0xa990926 in 
clang::ento::TagVisitor::VisitNode(clang::ento::ExplodedNode const*, 

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D66325#1640483 , @thakis wrote:

> Thanks! Looks like it builds fine now, but the tests are failing: 
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/19297/steps/test-check-all/logs/stdio
>  Failing Tests (2):
>
>   Clang :: Analysis/cast-value-notes.cpp
>   Clang :: Analysis/cast-value-state-dump.cpp


I have noticed something is broken other than the first fix, just I am not that 
professional C++ developer to immediately catch the error. I am totally on it, 
thanks!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks! Looks like it builds fine now, but the tests are failing: 
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/19297/steps/test-check-all/logs/stdio

  FAIL: Clang :: Analysis/cast-value-state-dump.cpp (4352 of 48515)
   TEST 'Clang :: Analysis/cast-value-state-dump.cpp' 
FAILED 
  Script:
  --
  : 'RUN: at line 1';   
c:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\clang.exe 
-cc1 -internal-isystem 
c:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\lib\clang\10.0.0\include
 -nostdsysteminc -analyze -analyzer-constraints=range   
-analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection  
-analyzer-output=text -verify 
C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Analysis\cast-value-state-dump.cpp
 2>&1 | 
c:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\filecheck.exe
 
C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Analysis\cast-value-state-dump.cpp
  --
  Exit Code: 1
  
  Command Output (stdout):
  --
  $ ":" "RUN: at line 1"
  $ 
"c:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\clang.exe" 
"-cc1" "-internal-isystem" 
"c:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\lib\clang\10.0.0\include"
 "-nostdsysteminc" "-analyze" "-analyzer-constraints=range" 
"-analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection" 
"-analyzer-output=text" "-verify" 
"C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Analysis\cast-value-state-dump.cpp"
  note: command had no output on stdout or stderr
  error: command failed with exit status: 1
  $ 
"c:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\filecheck.exe"
 
"C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Analysis\cast-value-state-dump.cpp"
  
  --
  
  
  PASS: Clang :: Analysis/castexpr-callback.c (4353 of 48515)
  PASS: Clang :: Analysis/bool-assignment.c (4354 of 48515)
  PASS: Clang :: Analysis/builtin-functions.cpp (4355 of 48515)
  PASS: Clang :: Analysis/cfg-indirect-goto-determinism.cpp (4356 of 48515)
  PASS: Clang :: Analysis/casts.m (4357 of 48515)
  FAIL: Clang :: Analysis/cast-value-notes.cpp (4358 of 48515)
   TEST 'Clang :: Analysis/cast-value-notes.cpp' FAILED 

  Script:
  --
  : 'RUN: at line 1';   
c:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\clang.exe 
-cc1 -internal-isystem 
c:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\lib\clang\10.0.0\include
 -nostdsysteminc -analyze -analyzer-constraints=range   
-analyzer-checker=core,apiModeling.llvm.CastValue   -analyzer-output=text 
-verify 
C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Analysis\cast-value-notes.cpp
  --
  Exit Code: 1
  
  Command Output (stdout):
  --
  $ ":" "RUN: at line 1"
  $ 
"c:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\clang.exe" 
"-cc1" "-internal-isystem" 
"c:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\lib\clang\10.0.0\include"
 "-nostdsysteminc" "-analyze" "-analyzer-constraints=range" 
"-analyzer-checker=core,apiModeling.llvm.CastValue" "-analyzer-output=text" 
"-verify" 
"C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Analysis\cast-value-notes.cpp"
  # command stderr:
  error: 'note' diagnostics expected but not seen: 
  
File 
C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Analysis\cast-value-notes.cpp
 Line 23 (directive at 
C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Analysis\cast-value-notes.cpp:24):
 Assuming 'S' is not a 'Circle'
  
File 
C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Analysis\cast-value-notes.cpp
 Line 30 (directive at 
C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Analysis\cast-value-notes.cpp:31):
 Assuming 'S' is a 'Circle'
  
File 
C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Analysis\cast-value-notes.cpp
 Line 34 (directive at 
C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Analysis\cast-value-notes.cpp:35):
 'C' is a 'Circle'
  
File 
C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Analysis\cast-value-notes.cpp
 Line 40 (directive at 
C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Analysis\cast-value-notes.cpp:41):
 Assuming 'C' is not a 'Triangle'
  
File 
C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Analysis\cast-value-notes.cpp
 Line 46 (directive at 

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D66325#1640426 , @thakis wrote:

> It looks like this renamed DynamicTypeMap.h but didn't update all users, see 
> all the files in Checkers at 
> http://llvm-cs.pcc.me.uk/?q=include.*dynamictypemap.h for example.
>
> Which targets did you try to build locally?


Well, the errors are in the Static Analyzer, and I have not got any warnings, 
but now I realized that this header has plenty of injections. Thanks for the 
fast response, I have fixed it!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

See e.g. 
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-gn/builds/4344/steps/ninja%20/logs/stdio
 for errors; I'm guessing most other bots on http://lab.llvm.org:8011/console 
will turn red in a bit too.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

It looks like this renamed DynamicTypeMap.h but didn't update all users, see 
all the files in Checkers at 
http://llvm-cs.pcc.me.uk/?q=include.*dynamictypemap.h for example.

Which targets did you try to build locally?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Charusso marked an inline comment as done.
Closed by commit rL369605: [analyzer] CastValueChecker: Store the dynamic types 
and casts (authored by Charusso, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66325?vs=216513=216530#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66325

Files:
  cfe/trunk/include/clang/AST/Type.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h
  cfe/trunk/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
  cfe/trunk/lib/StaticAnalyzer/Core/DynamicType.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/DynamicTypeMap.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp
  cfe/trunk/test/Analysis/Inputs/llvm.h
  cfe/trunk/test/Analysis/cast-value-logic.cpp
  cfe/trunk/test/Analysis/cast-value-notes.cpp
  cfe/trunk/test/Analysis/cast-value-state-dump.cpp
  cfe/trunk/test/Analysis/cast-value.cpp
  cfe/trunk/test/Analysis/dump_egraph.cpp
  cfe/trunk/test/Analysis/expr-inspection.c

Index: cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
===
--- cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
+++ cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
@@ -16,7 +16,7 @@
   CommonBugCategories.cpp
   ConstraintManager.cpp
   CoreEngine.cpp
-  DynamicTypeMap.cpp
+  DynamicType.cpp
   Environment.cpp
   ExplodedGraph.cpp
   ExprEngine.cpp
Index: cfe/trunk/lib/StaticAnalyzer/Core/DynamicType.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/DynamicType.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/DynamicType.cpp
@@ -0,0 +1,223 @@
+//===- DynamicType.cpp - Dynamic type related APIs --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+//  This file defines APIs that track and query dynamic type information. This
+//  information can be used to devirtualize calls during the symbolic execution
+//  or do type checking.
+//
+//===--===//
+
+#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h"
+#include "clang/Basic/JsonSupport.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+
+/// The GDM component containing the dynamic type info. This is a map from a
+/// symbol to its most likely type.
+REGISTER_MAP_WITH_PROGRAMSTATE(DynamicTypeMap, const clang::ento::MemRegion *,
+   clang::ento::DynamicTypeInfo)
+
+/// A set factory of dynamic cast informations.
+REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(CastSet, clang::ento::DynamicCastInfo)
+
+/// A map from symbols to cast informations.
+REGISTER_MAP_WITH_PROGRAMSTATE(DynamicCastMap, const clang::ento::MemRegion *,
+   CastSet)
+
+namespace clang {
+namespace ento {
+
+DynamicTypeInfo getDynamicTypeInfo(ProgramStateRef State, const MemRegion *MR) {
+  MR = MR->StripCasts();
+
+  // Look up the dynamic type in the GDM.
+  if (const DynamicTypeInfo *DTI = State->get(MR))
+return *DTI;
+
+  // Otherwise, fall back to what we know about the region.
+  if (const auto *TR = dyn_cast(MR))
+return DynamicTypeInfo(TR->getLocationType(), /*CanBeSub=*/false);
+
+  if (const auto *SR = dyn_cast(MR)) {
+SymbolRef Sym = SR->getSymbol();
+return DynamicTypeInfo(Sym->getType());
+  }
+
+  return {};
+}
+
+const DynamicTypeInfo *getRawDynamicTypeInfo(ProgramStateRef State,
+ const MemRegion *MR) {
+  return State->get(MR);
+}
+
+const DynamicCastInfo *getDynamicCastInfo(ProgramStateRef State,
+  const MemRegion *MR,
+  QualType CastFromTy,
+  QualType CastToTy) {
+  const auto *Lookup = State->get().lookup(MR);
+  if (!Lookup)
+return nullptr;

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso marked 6 inline comments as done.
Charusso added a comment.

Thanks for the review! The build-bots will fire with that `QualType` fix (1028 
TU on its own). I will look into the exploded-graph-rewriter.py after GSoC to 
fix every stuff like that patch and also invoke my HTML simplification idea.




Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:83-93
+static QualType getRecordType(QualType Ty) {
+  Ty = Ty.getCanonicalType();
+
+  if (Ty->isPointerType())
+return getRecordType(Ty->getPointeeType());
 
+  if (Ty->isReferenceType())

NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > NoQ wrote:
> > > > Most of the time we should know exactly how many pointer/reference 
> > > > types we need to unwrap. I really prefer we hard-assert this knowledge 
> > > > instead of blindly unwrapping as many pointer/reference types as we 
> > > > want. Because every time we have an unexpected number of unwraps it's 
> > > > an indication that something went horribly wrong. So it's good to have 
> > > > the extra sanity checking.
> > > I think this one is still forgotten. Maybe a FIXME?
> > Now I pattern-match that rule in the `evalCall()`, therefore we cannot try 
> > to evaluate nested references. Would you like to see more checks?
> Oh, i see, you removed the recursion. Great.
I was dumb enough to do not publish my comment six times, sorry.


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Let's land this then!~~




Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:83-93
+static QualType getRecordType(QualType Ty) {
+  Ty = Ty.getCanonicalType();
+
+  if (Ty->isPointerType())
+return getRecordType(Ty->getPointeeType());
 
+  if (Ty->isReferenceType())

Charusso wrote:
> NoQ wrote:
> > NoQ wrote:
> > > Most of the time we should know exactly how many pointer/reference types 
> > > we need to unwrap. I really prefer we hard-assert this knowledge instead 
> > > of blindly unwrapping as many pointer/reference types as we want. Because 
> > > every time we have an unexpected number of unwraps it's an indication 
> > > that something went horribly wrong. So it's good to have the extra sanity 
> > > checking.
> > I think this one is still forgotten. Maybe a FIXME?
> Now I pattern-match that rule in the `evalCall()`, therefore we cannot try to 
> evaluate nested references. Would you like to see more checks?
Oh, i see, you removed the recursion. Great.


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 216513.
Charusso added a comment.

- Fix printing.


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

https://reviews.llvm.org/D66325

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/DynamicType.cpp
  clang/lib/StaticAnalyzer/Core/DynamicTypeMap.cpp
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp
  clang/test/Analysis/Inputs/llvm.h
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/Analysis/cast-value.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -38,6 +38,7 @@
 // CHECK-NEXT: { "symbol": "reg_$0", "range": "{ [-2147483648, 13] }" }
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "dynamic_types": null,
+// CHECK-NEXT:   "dynamic_casts": null,
 // CHECK-NEXT:   "constructing_objects": null,
 // CHECK-NEXT:   "checker_messages": null
 // CHECK-NEXT: }
Index: clang/test/Analysis/dump_egraph.cpp
===
--- clang/test/Analysis/dump_egraph.cpp
+++ clang/test/Analysis/dump_egraph.cpp
@@ -24,4 +24,5 @@
 
 // CHECK: \"cluster\": \"t\", \"pointer\": \"{{0x[0-9a-f]+}}\", \"items\": [\l\{ \"kind\": \"Default\", \"offset\": 0, \"value\": \"conj_$2\{int, LC5, no stmt, #1\}\"
 
-// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false\}\l
+// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false \}\l
+
Index: clang/test/Analysis/cast-value.cpp
===
--- clang/test/Analysis/cast-value.cpp
+++ /dev/null
@@ -1,239 +0,0 @@
-// RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN:  -verify=logic %s
-// RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue \
-// RUN:  -analyzer-output=text -verify %s
-
-void clang_analyzer_numTimesReached();
-void clang_analyzer_warnIfReached();
-void clang_analyzer_eval(bool);
-
-namespace llvm {
-template 
-const X *cast(Y Value);
-
-template 
-const X *dyn_cast(Y *Value);
-template 
-const X _cast(Y );
-
-template 
-const X *cast_or_null(Y Value);
-
-template 
-const X *dyn_cast_or_null(Y *Value);
-template 
-const X *dyn_cast_or_null(Y );
-} // namespace llvm
-
-namespace clang {
-struct Shape {
-  template 
-  const T *castAs() const;
-
-  template 
-  const T *getAs() const;
-};
-class Triangle : public Shape {};
-class Circle : public Shape {};
-} // namespace clang
-
-using namespace llvm;
-using namespace clang;
-
-namespace test_cast {
-void evalLogic(const Shape *S) {
-  const Circle *C = cast(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{1}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // no-warning
-
-  if (!S)
-clang_analyzer_warnIfReached(); // no-warning
-}
-} // namespace test_cast
-
-namespace test_dyn_cast {
-void evalLogic(const Shape *S) {
-  const Circle *C = dyn_cast(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{2}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // logic-warning {{REACHABLE}}
-
-  if (!S)
-clang_analyzer_warnIfReached(); // no-warning
-}
-} // namespace test_dyn_cast
-
-namespace test_cast_or_null {
-void evalLogic(const Shape *S) {
-  const Circle *C = cast_or_null(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{2}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // no-warning
-
-  if (!S)
-clang_analyzer_eval(!C); // logic-warning {{TRUE}}
-}
-} // namespace test_cast_or_null
-
-namespace test_dyn_cast_or_null {
-void evalLogic(const Shape *S) {
-  const Circle *C = dyn_cast_or_null(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{3}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // 

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso marked an inline comment as done.
Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:83-93
+static QualType getRecordType(QualType Ty) {
+  Ty = Ty.getCanonicalType();
+
+  if (Ty->isPointerType())
+return getRecordType(Ty->getPointeeType());
 
+  if (Ty->isReferenceType())

NoQ wrote:
> NoQ wrote:
> > Most of the time we should know exactly how many pointer/reference types we 
> > need to unwrap. I really prefer we hard-assert this knowledge instead of 
> > blindly unwrapping as many pointer/reference types as we want. Because 
> > every time we have an unexpected number of unwraps it's an indication that 
> > something went horribly wrong. So it's good to have the extra sanity 
> > checking.
> I think this one is still forgotten. Maybe a FIXME?
Now I pattern-match that rule in the `evalCall()`, therefore we cannot try to 
evaluate nested references. Would you like to see more checks?


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:126
+} else {
+  Out << "the object";
+}

"The" should be capitalized if there's no "Assuming" before it.


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 216509.
Charusso marked 4 inline comments as done and an inline comment as not done.
Charusso added a comment.

- Fix


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

https://reviews.llvm.org/D66325

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/DynamicType.cpp
  clang/lib/StaticAnalyzer/Core/DynamicTypeMap.cpp
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp
  clang/test/Analysis/Inputs/llvm.h
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/Analysis/cast-value.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -38,6 +38,7 @@
 // CHECK-NEXT: { "symbol": "reg_$0", "range": "{ [-2147483648, 13] }" }
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "dynamic_types": null,
+// CHECK-NEXT:   "dynamic_casts": null,
 // CHECK-NEXT:   "constructing_objects": null,
 // CHECK-NEXT:   "checker_messages": null
 // CHECK-NEXT: }
Index: clang/test/Analysis/dump_egraph.cpp
===
--- clang/test/Analysis/dump_egraph.cpp
+++ clang/test/Analysis/dump_egraph.cpp
@@ -24,4 +24,5 @@
 
 // CHECK: \"cluster\": \"t\", \"pointer\": \"{{0x[0-9a-f]+}}\", \"items\": [\l\{ \"kind\": \"Default\", \"offset\": 0, \"value\": \"conj_$2\{int, LC5, no stmt, #1\}\"
 
-// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false\}\l
+// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false \}\l
+
Index: clang/test/Analysis/cast-value.cpp
===
--- clang/test/Analysis/cast-value.cpp
+++ /dev/null
@@ -1,239 +0,0 @@
-// RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN:  -verify=logic %s
-// RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue \
-// RUN:  -analyzer-output=text -verify %s
-
-void clang_analyzer_numTimesReached();
-void clang_analyzer_warnIfReached();
-void clang_analyzer_eval(bool);
-
-namespace llvm {
-template 
-const X *cast(Y Value);
-
-template 
-const X *dyn_cast(Y *Value);
-template 
-const X _cast(Y );
-
-template 
-const X *cast_or_null(Y Value);
-
-template 
-const X *dyn_cast_or_null(Y *Value);
-template 
-const X *dyn_cast_or_null(Y );
-} // namespace llvm
-
-namespace clang {
-struct Shape {
-  template 
-  const T *castAs() const;
-
-  template 
-  const T *getAs() const;
-};
-class Triangle : public Shape {};
-class Circle : public Shape {};
-} // namespace clang
-
-using namespace llvm;
-using namespace clang;
-
-namespace test_cast {
-void evalLogic(const Shape *S) {
-  const Circle *C = cast(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{1}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // no-warning
-
-  if (!S)
-clang_analyzer_warnIfReached(); // no-warning
-}
-} // namespace test_cast
-
-namespace test_dyn_cast {
-void evalLogic(const Shape *S) {
-  const Circle *C = dyn_cast(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{2}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // logic-warning {{REACHABLE}}
-
-  if (!S)
-clang_analyzer_warnIfReached(); // no-warning
-}
-} // namespace test_dyn_cast
-
-namespace test_cast_or_null {
-void evalLogic(const Shape *S) {
-  const Circle *C = cast_or_null(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{2}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // no-warning
-
-  if (!S)
-clang_analyzer_eval(!C); // logic-warning {{TRUE}}
-}
-} // namespace test_cast_or_null
-
-namespace test_dyn_cast_or_null {
-void evalLogic(const Shape *S) {
-  const Circle *C = dyn_cast_or_null(S);
-  clang_analyzer_numTimesReached(); // 

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Thanks!! Here's the last couple of nits that i have.




Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:83-93
+static QualType getRecordType(QualType Ty) {
+  Ty = Ty.getCanonicalType();
+
+  if (Ty->isPointerType())
+return getRecordType(Ty->getPointeeType());
 
+  if (Ty->isReferenceType())

NoQ wrote:
> Most of the time we should know exactly how many pointer/reference types we 
> need to unwrap. I really prefer we hard-assert this knowledge instead of 
> blindly unwrapping as many pointer/reference types as we want. Because every 
> time we have an unexpected number of unwraps it's an indication that 
> something went horribly wrong. So it's good to have the extra sanity checking.
I think this one is still forgotten. Maybe a FIXME?



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:96-100
+static std::string getObjectName(const Expr *E, CheckerContext ) {
+  return Lexer::getSourceText(
+  CharSourceRange::getTokenRange(E->getSourceRange()), 
C.getSourceManager(),
+  C.getASTContext().getLangOpts());
+}

I'm worried that this may occasionally get too long and/or contain line breaks. 
Let's do the usual pattern matching instead: say 'X' for a `DeclRefExpr` to 
'X', say 'field Y' for a `MemberExpr` made with field 'Y', say "the object" in 
other cases.


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/test/Analysis/cast-value-state-dump.cpp:30
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Circle' to 'Triangle' 
fails}}
+// expected-note@-2 {{Taking false branch}}

Charusso wrote:
> NoQ wrote:
> > We're not assuming it, right? We already know it's gonna fail because we 
> > already know that it's a circle.
> We have no knowledge what is what. At the top we see that: `Shape -> Circle`, 
> and then `Triangle -> Circle` is something new, so we have to assume that. It 
> is an implementation detail we are allowing only one cast to be succeed. From 
> the user point of view it is an assumption as the current state of the 
> checker could not provide more/better information. It is the same logic from 
> the `ConditionBRVisitor`.
Yeah, but it's not the observable behavior we'll be trying to preserve, so i 
suggest a FIXME.


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 216317.
Charusso added a comment.

- Make the check-clang pass and simplify a test case.


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

https://reviews.llvm.org/D66325

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/DynamicType.cpp
  clang/lib/StaticAnalyzer/Core/DynamicTypeMap.cpp
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp
  clang/test/Analysis/Inputs/llvm.h
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/Analysis/cast-value.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -38,6 +38,7 @@
 // CHECK-NEXT: { "symbol": "reg_$0", "range": "{ [-2147483648, 13] }" }
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "dynamic_types": null,
+// CHECK-NEXT:   "dynamic_casts": null,
 // CHECK-NEXT:   "constructing_objects": null,
 // CHECK-NEXT:   "checker_messages": null
 // CHECK-NEXT: }
Index: clang/test/Analysis/dump_egraph.cpp
===
--- clang/test/Analysis/dump_egraph.cpp
+++ clang/test/Analysis/dump_egraph.cpp
@@ -24,4 +24,5 @@
 
 // CHECK: \"cluster\": \"t\", \"pointer\": \"{{0x[0-9a-f]+}}\", \"items\": [\l\{ \"kind\": \"Default\", \"offset\": 0, \"value\": \"conj_$2\{int, LC5, no stmt, #1\}\"
 
-// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false\}\l
+// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false \}\l
+
Index: clang/test/Analysis/cast-value.cpp
===
--- clang/test/Analysis/cast-value.cpp
+++ /dev/null
@@ -1,239 +0,0 @@
-// RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN:  -verify=logic %s
-// RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue \
-// RUN:  -analyzer-output=text -verify %s
-
-void clang_analyzer_numTimesReached();
-void clang_analyzer_warnIfReached();
-void clang_analyzer_eval(bool);
-
-namespace llvm {
-template 
-const X *cast(Y Value);
-
-template 
-const X *dyn_cast(Y *Value);
-template 
-const X _cast(Y );
-
-template 
-const X *cast_or_null(Y Value);
-
-template 
-const X *dyn_cast_or_null(Y *Value);
-template 
-const X *dyn_cast_or_null(Y );
-} // namespace llvm
-
-namespace clang {
-struct Shape {
-  template 
-  const T *castAs() const;
-
-  template 
-  const T *getAs() const;
-};
-class Triangle : public Shape {};
-class Circle : public Shape {};
-} // namespace clang
-
-using namespace llvm;
-using namespace clang;
-
-namespace test_cast {
-void evalLogic(const Shape *S) {
-  const Circle *C = cast(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{1}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // no-warning
-
-  if (!S)
-clang_analyzer_warnIfReached(); // no-warning
-}
-} // namespace test_cast
-
-namespace test_dyn_cast {
-void evalLogic(const Shape *S) {
-  const Circle *C = dyn_cast(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{2}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // logic-warning {{REACHABLE}}
-
-  if (!S)
-clang_analyzer_warnIfReached(); // no-warning
-}
-} // namespace test_dyn_cast
-
-namespace test_cast_or_null {
-void evalLogic(const Shape *S) {
-  const Circle *C = cast_or_null(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{2}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // no-warning
-
-  if (!S)
-clang_analyzer_eval(!C); // logic-warning {{TRUE}}
-}
-} // namespace test_cast_or_null
-
-namespace test_dyn_cast_or_null {
-void evalLogic(const Shape *S) {
-  const Circle *C = dyn_cast_or_null(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{3}}
-
-  if (S && C)

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 216314.
Charusso added a comment.

- Remove `CastFromTy` finally from the `getNoteTag()` API as it was 
uninformative.


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

https://reviews.llvm.org/D66325

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/DynamicType.cpp
  clang/lib/StaticAnalyzer/Core/DynamicTypeMap.cpp
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp
  clang/test/Analysis/Inputs/llvm.h
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/Analysis/cast-value.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -38,6 +38,7 @@
 // CHECK-NEXT: { "symbol": "reg_$0", "range": "{ [-2147483648, 13] }" }
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "dynamic_types": null,
+// CHECK-NEXT:   "dynamic_casts": null,
 // CHECK-NEXT:   "constructing_objects": null,
 // CHECK-NEXT:   "checker_messages": null
 // CHECK-NEXT: }
Index: clang/test/Analysis/dump_egraph.cpp
===
--- clang/test/Analysis/dump_egraph.cpp
+++ clang/test/Analysis/dump_egraph.cpp
@@ -24,4 +24,5 @@
 
 // CHECK: \"cluster\": \"t\", \"pointer\": \"{{0x[0-9a-f]+}}\", \"items\": [\l\{ \"kind\": \"Default\", \"offset\": 0, \"value\": \"conj_$2\{int, LC5, no stmt, #1\}\"
 
-// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false\}\l
+// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false \}\l
+
Index: clang/test/Analysis/cast-value.cpp
===
--- clang/test/Analysis/cast-value.cpp
+++ /dev/null
@@ -1,239 +0,0 @@
-// RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN:  -verify=logic %s
-// RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue \
-// RUN:  -analyzer-output=text -verify %s
-
-void clang_analyzer_numTimesReached();
-void clang_analyzer_warnIfReached();
-void clang_analyzer_eval(bool);
-
-namespace llvm {
-template 
-const X *cast(Y Value);
-
-template 
-const X *dyn_cast(Y *Value);
-template 
-const X _cast(Y );
-
-template 
-const X *cast_or_null(Y Value);
-
-template 
-const X *dyn_cast_or_null(Y *Value);
-template 
-const X *dyn_cast_or_null(Y );
-} // namespace llvm
-
-namespace clang {
-struct Shape {
-  template 
-  const T *castAs() const;
-
-  template 
-  const T *getAs() const;
-};
-class Triangle : public Shape {};
-class Circle : public Shape {};
-} // namespace clang
-
-using namespace llvm;
-using namespace clang;
-
-namespace test_cast {
-void evalLogic(const Shape *S) {
-  const Circle *C = cast(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{1}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // no-warning
-
-  if (!S)
-clang_analyzer_warnIfReached(); // no-warning
-}
-} // namespace test_cast
-
-namespace test_dyn_cast {
-void evalLogic(const Shape *S) {
-  const Circle *C = dyn_cast(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{2}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // logic-warning {{REACHABLE}}
-
-  if (!S)
-clang_analyzer_warnIfReached(); // no-warning
-}
-} // namespace test_dyn_cast
-
-namespace test_cast_or_null {
-void evalLogic(const Shape *S) {
-  const Circle *C = cast_or_null(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{2}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // no-warning
-
-  if (!S)
-clang_analyzer_eval(!C); // logic-warning {{TRUE}}
-}
-} // namespace test_cast_or_null
-
-namespace test_dyn_cast_or_null {
-void evalLogic(const Shape *S) {
-  const Circle *C = dyn_cast_or_null(S);
-  clang_analyzer_numTimesReached(); // 

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso marked an inline comment as done.
Charusso added inline comments.



Comment at: clang/test/Analysis/cast-value-notes.cpp:34
+  if (dyn_cast_or_null(C)) {
+// no-note: 'Assuming 'C' is a 'Circle', not a 'Circle''
+return;

NoQ wrote:
> A circle is always a circle.
I have removed that contradiction test case as being silly, but yes, that was 
the `no-warning` test properly.


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h:19
+public:
+  enum CastKind { Success, Fail };
+

NoQ wrote:
> I suggest `enum CastResult { Success, Failure }` ("a fail" is slang, also 
> "result" because it's basically a result).
> 
> Also TODO: The success information should ideally go straight into the 
> dynamic type map.
The successful cast is going to the type map, and the failure will not. Why 
would we need to store the failing casts in the type map? Therefore no 
differentiation is necessary I believe so that nor `TODO`.



Comment at: clang/test/Analysis/cast-value-state-dump.cpp:30
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Circle' to 'Triangle' 
fails}}
+// expected-note@-2 {{Taking false branch}}

NoQ wrote:
> We're not assuming it, right? We already know it's gonna fail because we 
> already know that it's a circle.
We have no knowledge what is what. At the top we see that: `Shape -> Circle`, 
and then `Triangle -> Circle` is something new, so we have to assume that. It 
is an implementation detail we are allowing only one cast to be succeed. From 
the user point of view it is an assumption as the current state of the checker 
could not provide more/better information. It is the same logic from the 
`ConditionBRVisitor`.


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 216312.
Charusso marked 3 inline comments as done.
Charusso added a comment.

- Fix more and publish the previously forgotten comments.


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

https://reviews.llvm.org/D66325

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/DynamicType.cpp
  clang/lib/StaticAnalyzer/Core/DynamicTypeMap.cpp
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp
  clang/test/Analysis/Inputs/llvm.h
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/Analysis/cast-value.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -38,6 +38,7 @@
 // CHECK-NEXT: { "symbol": "reg_$0", "range": "{ [-2147483648, 13] }" }
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "dynamic_types": null,
+// CHECK-NEXT:   "dynamic_casts": null,
 // CHECK-NEXT:   "constructing_objects": null,
 // CHECK-NEXT:   "checker_messages": null
 // CHECK-NEXT: }
Index: clang/test/Analysis/dump_egraph.cpp
===
--- clang/test/Analysis/dump_egraph.cpp
+++ clang/test/Analysis/dump_egraph.cpp
@@ -24,4 +24,5 @@
 
 // CHECK: \"cluster\": \"t\", \"pointer\": \"{{0x[0-9a-f]+}}\", \"items\": [\l\{ \"kind\": \"Default\", \"offset\": 0, \"value\": \"conj_$2\{int, LC5, no stmt, #1\}\"
 
-// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false\}\l
+// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false \}\l
+
Index: clang/test/Analysis/cast-value.cpp
===
--- clang/test/Analysis/cast-value.cpp
+++ /dev/null
@@ -1,239 +0,0 @@
-// RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN:  -verify=logic %s
-// RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue \
-// RUN:  -analyzer-output=text -verify %s
-
-void clang_analyzer_numTimesReached();
-void clang_analyzer_warnIfReached();
-void clang_analyzer_eval(bool);
-
-namespace llvm {
-template 
-const X *cast(Y Value);
-
-template 
-const X *dyn_cast(Y *Value);
-template 
-const X _cast(Y );
-
-template 
-const X *cast_or_null(Y Value);
-
-template 
-const X *dyn_cast_or_null(Y *Value);
-template 
-const X *dyn_cast_or_null(Y );
-} // namespace llvm
-
-namespace clang {
-struct Shape {
-  template 
-  const T *castAs() const;
-
-  template 
-  const T *getAs() const;
-};
-class Triangle : public Shape {};
-class Circle : public Shape {};
-} // namespace clang
-
-using namespace llvm;
-using namespace clang;
-
-namespace test_cast {
-void evalLogic(const Shape *S) {
-  const Circle *C = cast(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{1}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // no-warning
-
-  if (!S)
-clang_analyzer_warnIfReached(); // no-warning
-}
-} // namespace test_cast
-
-namespace test_dyn_cast {
-void evalLogic(const Shape *S) {
-  const Circle *C = dyn_cast(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{2}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // logic-warning {{REACHABLE}}
-
-  if (!S)
-clang_analyzer_warnIfReached(); // no-warning
-}
-} // namespace test_dyn_cast
-
-namespace test_cast_or_null {
-void evalLogic(const Shape *S) {
-  const Circle *C = cast_or_null(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{2}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // no-warning
-
-  if (!S)
-clang_analyzer_eval(!C); // logic-warning {{TRUE}}
-}
-} // namespace test_cast_or_null
-
-namespace test_dyn_cast_or_null {
-void evalLogic(const Shape *S) {
-  const Circle *C = dyn_cast_or_null(S);
-  

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 216250.
Charusso added a comment.

- Added a new test case and refactored that test file.


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

https://reviews.llvm.org/D66325

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/DynamicType.cpp
  clang/lib/StaticAnalyzer/Core/DynamicTypeMap.cpp
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp
  clang/test/Analysis/Inputs/llvm.h
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/Analysis/cast-value.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -38,6 +38,7 @@
 // CHECK-NEXT: { "symbol": "reg_$0", "range": "{ [-2147483648, 13] }" }
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "dynamic_types": null,
+// CHECK-NEXT:   "dynamic_casts": null,
 // CHECK-NEXT:   "constructing_objects": null,
 // CHECK-NEXT:   "checker_messages": null
 // CHECK-NEXT: }
Index: clang/test/Analysis/dump_egraph.cpp
===
--- clang/test/Analysis/dump_egraph.cpp
+++ clang/test/Analysis/dump_egraph.cpp
@@ -24,4 +24,5 @@
 
 // CHECK: \"cluster\": \"t\", \"pointer\": \"{{0x[0-9a-f]+}}\", \"items\": [\l\{ \"kind\": \"Default\", \"offset\": 0, \"value\": \"conj_$2\{int, LC5, no stmt, #1\}\"
 
-// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false\}\l
+// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false \}\l
+
Index: clang/test/Analysis/cast-value.cpp
===
--- clang/test/Analysis/cast-value.cpp
+++ /dev/null
@@ -1,239 +0,0 @@
-// RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN:  -verify=logic %s
-// RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue \
-// RUN:  -analyzer-output=text -verify %s
-
-void clang_analyzer_numTimesReached();
-void clang_analyzer_warnIfReached();
-void clang_analyzer_eval(bool);
-
-namespace llvm {
-template 
-const X *cast(Y Value);
-
-template 
-const X *dyn_cast(Y *Value);
-template 
-const X _cast(Y );
-
-template 
-const X *cast_or_null(Y Value);
-
-template 
-const X *dyn_cast_or_null(Y *Value);
-template 
-const X *dyn_cast_or_null(Y );
-} // namespace llvm
-
-namespace clang {
-struct Shape {
-  template 
-  const T *castAs() const;
-
-  template 
-  const T *getAs() const;
-};
-class Triangle : public Shape {};
-class Circle : public Shape {};
-} // namespace clang
-
-using namespace llvm;
-using namespace clang;
-
-namespace test_cast {
-void evalLogic(const Shape *S) {
-  const Circle *C = cast(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{1}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // no-warning
-
-  if (!S)
-clang_analyzer_warnIfReached(); // no-warning
-}
-} // namespace test_cast
-
-namespace test_dyn_cast {
-void evalLogic(const Shape *S) {
-  const Circle *C = dyn_cast(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{2}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // logic-warning {{REACHABLE}}
-
-  if (!S)
-clang_analyzer_warnIfReached(); // no-warning
-}
-} // namespace test_dyn_cast
-
-namespace test_cast_or_null {
-void evalLogic(const Shape *S) {
-  const Circle *C = cast_or_null(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{2}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // no-warning
-
-  if (!S)
-clang_analyzer_eval(!C); // logic-warning {{TRUE}}
-}
-} // namespace test_cast_or_null
-
-namespace test_dyn_cast_or_null {
-void evalLogic(const Shape *S) {
-  const Circle *C = dyn_cast_or_null(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{3}}
-
-  if (S && C)

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Something went wrong with message generation, can you take a look?




Comment at: clang/test/Analysis/cast-value-notes.cpp:24
+  const auto  = dyn_cast(S);
+  // expected-note@-1 {{Assuming 'S' is a 'Circle', not a 'Shape'}}
+  // expected-note@-2 {{Dereference of null pointer}}

A circle is always a shape.

`Assuming 'S' is a 'Shape' that is not a 'Circle'` sounds about right.

Or just `Assuming 'S' is not a 'Circle'`.



Comment at: clang/test/Analysis/cast-value-notes.cpp:34
+  if (dyn_cast_or_null(C)) {
+// no-note: 'Assuming 'C' is a 'Circle', not a 'Circle''
+return;

A circle is always a circle.



Comment at: clang/test/Analysis/cast-value-notes.cpp:55
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming 'C' is a 'Triangle', not a 'Circle'}}
+// expected-note@-2 {{Taking false branch}}

The `not a 'Circle'` part is suspiciously specific.


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 216246.
Charusso marked 15 inline comments as done.
Charusso added a comment.
Herald added a subscriber: mgorny.

- Fix.


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

https://reviews.llvm.org/D66325

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/DynamicType.cpp
  clang/lib/StaticAnalyzer/Core/DynamicTypeMap.cpp
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp
  clang/test/Analysis/Inputs/llvm.h
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/Analysis/cast-value.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -38,6 +38,7 @@
 // CHECK-NEXT: { "symbol": "reg_$0", "range": "{ [-2147483648, 13] }" }
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "dynamic_types": null,
+// CHECK-NEXT:   "dynamic_casts": null,
 // CHECK-NEXT:   "constructing_objects": null,
 // CHECK-NEXT:   "checker_messages": null
 // CHECK-NEXT: }
Index: clang/test/Analysis/dump_egraph.cpp
===
--- clang/test/Analysis/dump_egraph.cpp
+++ clang/test/Analysis/dump_egraph.cpp
@@ -24,4 +24,5 @@
 
 // CHECK: \"cluster\": \"t\", \"pointer\": \"{{0x[0-9a-f]+}}\", \"items\": [\l\{ \"kind\": \"Default\", \"offset\": 0, \"value\": \"conj_$2\{int, LC5, no stmt, #1\}\"
 
-// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false\}\l
+// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false \}\l
+
Index: clang/test/Analysis/cast-value.cpp
===
--- clang/test/Analysis/cast-value.cpp
+++ /dev/null
@@ -1,239 +0,0 @@
-// RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN:  -verify=logic %s
-// RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue \
-// RUN:  -analyzer-output=text -verify %s
-
-void clang_analyzer_numTimesReached();
-void clang_analyzer_warnIfReached();
-void clang_analyzer_eval(bool);
-
-namespace llvm {
-template 
-const X *cast(Y Value);
-
-template 
-const X *dyn_cast(Y *Value);
-template 
-const X _cast(Y );
-
-template 
-const X *cast_or_null(Y Value);
-
-template 
-const X *dyn_cast_or_null(Y *Value);
-template 
-const X *dyn_cast_or_null(Y );
-} // namespace llvm
-
-namespace clang {
-struct Shape {
-  template 
-  const T *castAs() const;
-
-  template 
-  const T *getAs() const;
-};
-class Triangle : public Shape {};
-class Circle : public Shape {};
-} // namespace clang
-
-using namespace llvm;
-using namespace clang;
-
-namespace test_cast {
-void evalLogic(const Shape *S) {
-  const Circle *C = cast(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{1}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // no-warning
-
-  if (!S)
-clang_analyzer_warnIfReached(); // no-warning
-}
-} // namespace test_cast
-
-namespace test_dyn_cast {
-void evalLogic(const Shape *S) {
-  const Circle *C = dyn_cast(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{2}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // logic-warning {{REACHABLE}}
-
-  if (!S)
-clang_analyzer_warnIfReached(); // no-warning
-}
-} // namespace test_dyn_cast
-
-namespace test_cast_or_null {
-void evalLogic(const Shape *S) {
-  const Circle *C = cast_or_null(S);
-  clang_analyzer_numTimesReached(); // logic-warning {{2}}
-
-  if (S && C)
-clang_analyzer_eval(C == S); // logic-warning {{TRUE}}
-
-  if (S && !C)
-clang_analyzer_warnIfReached(); // no-warning
-
-  if (!S)
-clang_analyzer_eval(!C); // logic-warning {{TRUE}}
-}
-} // namespace test_cast_or_null
-
-namespace test_dyn_cast_or_null {
-void evalLogic(const Shape *S) {
-  const Circle *C = dyn_cast_or_null(S);
-  clang_analyzer_numTimesReached(); // 

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Also thanks, everything makes sense now!

Do we already have a test that will cover the necessity for having a map from 
regions to cast results? Eg.:

  void foo(Shape *C, Shape *T) {
if (isa(S) && !isa(T))
  clang_analyzer_warnIfReached(); // expected-warning{{TRUE}}
  }




Comment at: clang/test/Analysis/cast-value-state-dump.cpp:30
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Circle' to 'Triangle' 
fails}}
+// expected-note@-2 {{Taking false branch}}

We're not assuming it, right? We already know it's gonna fail because we 
already know that it's a circle.


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h:19
+public:
+  enum CastKind { Success, Fail };
+

I suggest `enum CastResult { Success, Failure }` ("a fail" is slang, also 
"result" because it's basically a result).

Also TODO: The success information should ideally go straight into the dynamic 
type map.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h:31-32
+
+  bool isSucceeds() const { return Kind == CastKind::Success; }
+  bool isFails() const { return Kind == CastKind::Fail; }
+

`succeeds()`, `fails()` (valid English).



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h:28
 /// symbol to its most likely type.
-struct DynamicTypeMap {};
+REGISTER_MAP_WITH_PROGRAMSTATE(DynamicTypeMap, const clang::ento::MemRegion *,
+   clang::ento::DynamicTypeInfo)

Can we move these macros into the cpp file so that they were only accessed by 
the fancy accessors?

Also i don't remember whether these macros do actually work correctly in 
headers. The original code was doing the trait manually because it had 
`GDMIndex()` defined in the cpp file, but if we put these macros into the 
header we'll have a static variable defined in multiple translation units, 
which may cause it to have different addresses in different dynamically loaded 
libraries that include this header.

You might need to merge your `checkDeadSymbols()` into the existing 
`checkDeadSymbols()`.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:83-93
+static QualType getRecordType(QualType Ty) {
+  Ty = Ty.getCanonicalType();
+
+  if (Ty->isPointerType())
+return getRecordType(Ty->getPointeeType());
 
+  if (Ty->isReferenceType())

Most of the time we should know exactly how many pointer/reference types we 
need to unwrap. I really prefer we hard-assert this knowledge instead of 
blindly unwrapping as many pointer/reference types as we want. Because every 
time we have an unexpected number of unwraps it's an indication that something 
went horribly wrong. So it's good to have the extra sanity checking.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:115
+
+  const DynamicCastInfo *Cast = getDynamicCastInfo(State, CastFromTy, 
CastToTy);
+

It would have been much easier for me to read if this variable was called 
`CastInfo`.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:118
+  // We assume that every checked cast succeeds.
+  bool IsCastSucceeds;
+  if (Cast)

Just `CastSucceeds` (valid English).



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:145-152
+if (!IsCheckedCast)
+  Out << (IsKnownCast ? "Dynamic cast" : "Assuming dynamic cast");
+else
+  Out << "Checked cast";
 
-Out << "to '" << CastToName << "' "
-<< (!IsNullReturn ? "succeeds" : "fails");
+Out << " from '" << CastFromTy->getAsCXXRecordDecl()->getNameAsString()
+<< "' to '" << CastToTy->getAsCXXRecordDecl()->getNameAsString()

To think: in D66423 you added this fancy feature when you dump the current 
dynamic type of the object instead of the static type. I think this is super 
cool and we should do it here as well, because the dynamic type is pretty much 
the only thing that you won't be able to figure out by looking at the source.

We could give the same message as for `isa`, say, `Assuming the object is a 
'Circle'` etc.



Comment at: clang/lib/StaticAnalyzer/Core/DynamicTypeMap.cpp:83-86
+  CastSet Set = F.getEmptySet();
+
+  if (const CastSet *TempSet = State->get(MR))
+Set = *TempSet;

My favorite way of writing this stuff:
```lang=c++
const CastSet *SetPtr = State->get(MR);
CastSet Set = SetPtr ? *SetPTr : F.getEmptySet();
```


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D66325#1636091 , @NoQ wrote:

> Yay! I understand the rough idea and it looks perfectly reasonable to start 
> with. Please add FIXMEs/TODOs on how we eventually want to support more 
> complicated inheritance hierarchies.


I have not decided yet. This only one successful cast allowance is the smallest 
possible solution, then we need a very great design to do better without 
exponential path-splitting. I have added a tiny TODO section.


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 216023.
Charusso edited the summary of this revision.
Charusso added a comment.

- Use a set factory to store a dynamic cast information set per memory region.


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

https://reviews.llvm.org/D66325

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Core/DynamicTypeMap.cpp
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/Analysis/cast-value.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -38,6 +38,7 @@
 // CHECK-NEXT: { "symbol": "reg_$0", "range": "{ [-2147483648, 13] }" }
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "dynamic_types": null,
+// CHECK-NEXT:   "dynamic_casts": null,
 // CHECK-NEXT:   "constructing_objects": null,
 // CHECK-NEXT:   "checker_messages": null
 // CHECK-NEXT: }
Index: clang/test/Analysis/dump_egraph.cpp
===
--- clang/test/Analysis/dump_egraph.cpp
+++ clang/test/Analysis/dump_egraph.cpp
@@ -24,4 +24,5 @@
 
 // CHECK: \"cluster\": \"t\", \"pointer\": \"{{0x[0-9a-f]+}}\", \"items\": [\l\{ \"kind\": \"Default\", \"offset\": 0, \"value\": \"conj_$2\{int, LC5, no stmt, #1\}\"
 
-// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false\}\l
+// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false \}\l
+
Index: clang/test/Analysis/cast-value-state-dump.cpp
===
--- /dev/null
+++ clang/test/Analysis/cast-value-state-dump.cpp
@@ -0,0 +1,68 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
+// RUN:  -analyzer-output=text -verify %s 2>&1 | FileCheck %s
+
+void clang_analyzer_printState();
+
+namespace llvm {
+template 
+const X *dyn_cast_or_null(Y *Value);
+template 
+const X *dyn_cast_or_null(Y );
+} // namespace llvm
+
+namespace clang {
+struct Shape {};
+class Triangle : public Shape {};
+class Circle : public Shape {};
+class Square : public Shape {};
+} // namespace clang
+
+using namespace llvm;
+using namespace clang;
+
+void evalNonNullParamNonNullReturnReference(const Shape ) {
+  const auto *C = dyn_cast_or_null(S);
+  // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' succeeds}}
+  // expected-note@-2 {{'C' initialized here}}
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Circle' to 'Triangle' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Dynamic cast from 'Circle' to 'Triangle' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Circle' to 'Square' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(S)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Square' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  clang_analyzer_printState();
+
+  // CHECK:  "dynamic_types": [
+  // CHECK-NEXT:   { "region": "SymRegion{reg_$0}", "dyn_type": "const class clang::Circle", "sub_classable": true }
+  // CHECK-NEXT: ],
+  // CHECK-NEXT: "dynamic_casts": [
+  // CHECK:{ "region": "SymRegion{reg_$0}", "casts": [
+  // CHECK-NEXT: { "from": "struct clang::Shape", "to": "class clang::Circle", "kind": "success" },
+  // CHECK-NEXT: { "from": "struct clang::Shape", "to": "class clang::Square", "kind": "fail" }
+  // CHECK-NEXT:   ]}
+
+  (void)(1 / !C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+}
Index: clang/test/Analysis/cast-value-notes.cpp
===
--- clang/test/Analysis/cast-value-notes.cpp
+++ clang/test/Analysis/cast-value-notes.cpp
@@ -1,14 +1,7 @@
 // RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN:  -verify=logic %s
-// RUN: %clang_analyze_cc1 \
 // RUN:  

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h:32
+/// A set of dynamic cast informations.
+REGISTER_SET_WITH_PROGRAMSTATE(DynamicCastSet, clang::ento::DynamicCastInfo)
 

NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Emm, so you're saving successes and failures of all casts //regardless of 
> > > which object is getting casted//? That's definitely not sufficient. If 
> > > `X` is a `Stmt` that isn't an `Expr`, you can't automatically infer that 
> > > `Y` is a `Stmt` that isn't an `Expr` for any object `Y` other than `X` . 
> > > This information needs to be per-object. Then you'll need to clean it up 
> > > in `checkDeadSymbols`.
> > I have two implementations, the other is set factory based on memory 
> > regions. Is it would be a good idea to go through all the regions every 
> > time and all their casts to know every possible cast-assumption? I have 
> > made it, and I felt like it is overcomplicated as the DynamicTypeMap could 
> > be used for asking what is the type now and whether we have better 
> > precision and update that information therefore we could update the 
> > cast-set as well.
> It's about correctness, we don't have much choice here. The current data 
> structure simply cannot work correctly because there's a "one vs many" 
> problem in it: for every pair of types (T₁, T₂) there are *many* possible 
> outcomes of a cast from T₁ to T₂ (depending on the object that's being 
> casted) but the current data structure only has room for *one* such outcome. 
> Your data structure is basically saying "Oh, this shape turned out to be a 
> circle, let's from now on forever believe that triangles don't exist" (?)
Well, let me swap then with the set factory. I think I could handle that in the 
current shape, but I cannot say no to your clear idea. Thanks for the factory 
idea! It does the same at the moment.


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h:32
+/// A set of dynamic cast informations.
+REGISTER_SET_WITH_PROGRAMSTATE(DynamicCastSet, clang::ento::DynamicCastInfo)
 

Charusso wrote:
> NoQ wrote:
> > Emm, so you're saving successes and failures of all casts //regardless of 
> > which object is getting casted//? That's definitely not sufficient. If `X` 
> > is a `Stmt` that isn't an `Expr`, you can't automatically infer that `Y` is 
> > a `Stmt` that isn't an `Expr` for any object `Y` other than `X` . This 
> > information needs to be per-object. Then you'll need to clean it up in 
> > `checkDeadSymbols`.
> I have two implementations, the other is set factory based on memory regions. 
> Is it would be a good idea to go through all the regions every time and all 
> their casts to know every possible cast-assumption? I have made it, and I 
> felt like it is overcomplicated as the DynamicTypeMap could be used for 
> asking what is the type now and whether we have better precision and update 
> that information therefore we could update the cast-set as well.
It's about correctness, we don't have much choice here. The current data 
structure simply cannot work correctly because there's a "one vs many" problem 
in it: for every pair of types (T₁, T₂) there are *many* possible outcomes of a 
cast from T₁ to T₂ (depending on the object that's being casted) but the 
current data structure only has room for *one* such outcome. Your data 
structure is basically saying "Oh, this shape turned out to be a circle, let's 
from now on forever believe that triangles don't exist" (?)


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h:32
+/// A set of dynamic cast informations.
+REGISTER_SET_WITH_PROGRAMSTATE(DynamicCastSet, clang::ento::DynamicCastInfo)
 

NoQ wrote:
> Emm, so you're saving successes and failures of all casts //regardless of 
> which object is getting casted//? That's definitely not sufficient. If `X` is 
> a `Stmt` that isn't an `Expr`, you can't automatically infer that `Y` is a 
> `Stmt` that isn't an `Expr` for any object `Y` other than `X` . This 
> information needs to be per-object. Then you'll need to clean it up in 
> `checkDeadSymbols`.
I have two implementations, the other is set factory based on memory regions. 
Is it would be a good idea to go through all the regions every time and all 
their casts to know every possible cast-assumption? I have made it, and I felt 
like it is overcomplicated as the DynamicTypeMap could be used for asking what 
is the type now and whether we have better precision and update that 
information therefore we could update the cast-set as well.


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Yay! I understand the rough idea and it looks perfectly reasonable to start 
with. Please add FIXMEs/TODOs on how we eventually want to support more 
complicated inheritance hierarchies.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h:32
+/// A set of dynamic cast informations.
+REGISTER_SET_WITH_PROGRAMSTATE(DynamicCastSet, clang::ento::DynamicCastInfo)
 

Emm, so you're saving successes and failures of all casts //regardless of which 
object is getting casted//? That's definitely not sufficient. If `X` is a 
`Stmt` that isn't an `Expr`, you can't automatically infer that `Y` is a `Stmt` 
that isn't an `Expr` for any object `Y` other than `X` . This information needs 
to be per-object. Then you'll need to clean it up in `checkDeadSymbols`.


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

https://reviews.llvm.org/D66325



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

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 215893.
Charusso retitled this revision from "[analyzer] CastValueChecker: Store the 
dynamic types in DynamicTypeMap" to "[analyzer] CastValueChecker: Store the 
dynamic types and casts".
Charusso added a comment.

This patch introduces `DynamicCastInfo` similar to `DynamicTypeInfo` which
is stored in `DynamicCastSet` similar to `DynamicTypeMap`. It could be used
to store and check the casts and prevent infeasible paths.


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

https://reviews.llvm.org/D66325

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Core/DynamicTypeMap.cpp
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/Analysis/cast-value.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -38,6 +38,7 @@
 // CHECK-NEXT: { "symbol": "reg_$0", "range": "{ [-2147483648, 13] }" }
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "dynamic_types": null,
+// CHECK-NEXT:   "dynamic_casts": null,
 // CHECK-NEXT:   "constructing_objects": null,
 // CHECK-NEXT:   "checker_messages": null
 // CHECK-NEXT: }
Index: clang/test/Analysis/dump_egraph.cpp
===
--- clang/test/Analysis/dump_egraph.cpp
+++ clang/test/Analysis/dump_egraph.cpp
@@ -24,4 +24,5 @@
 
 // CHECK: \"cluster\": \"t\", \"pointer\": \"{{0x[0-9a-f]+}}\", \"items\": [\l\{ \"kind\": \"Default\", \"offset\": 0, \"value\": \"conj_$2\{int, LC5, no stmt, #1\}\"
 
-// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false\}\l
+// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false \}\l
+
Index: clang/test/Analysis/cast-value-state-dump.cpp
===
--- /dev/null
+++ clang/test/Analysis/cast-value-state-dump.cpp
@@ -0,0 +1,69 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
+// RUN:  -analyzer-output=text -verify %s 2>&1 | FileCheck %s
+
+void clang_analyzer_printState();
+
+namespace llvm {
+template 
+const X *dyn_cast_or_null(Y *Value);
+template 
+const X *dyn_cast_or_null(Y );
+} // namespace llvm
+
+namespace clang {
+struct Shape {};
+class Triangle : public Shape {};
+class Circle : public Shape {};
+class Square : public Shape {};
+} // namespace clang
+
+using namespace llvm;
+using namespace clang;
+
+void evalNonNullParamNonNullReturnReference(const Shape ) {
+  const auto *C = dyn_cast_or_null(S);
+  // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' succeeds}}
+  // expected-note@-2 {{'C' initialized here}}
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Circle' to 'Triangle' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Dynamic cast from 'Circle' to 'Triangle' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Circle' to 'Square' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(S)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Square' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  clang_analyzer_printState();
+
+  // CHECK:  "dynamic_types": [
+  // CHECK-NEXT:   { "region": "SymRegion{reg_$0}", "dyn_type": "const class clang::Circle", "sub_classable": true }
+  // CHECK-NEXT: ],
+  // CHECK-NEXT: "dynamic_casts": [
+  // CHECK-NEXT:   { "from": "struct clang::Shape", "to": "class clang::Circle", "kind": "success" },
+  // CHECK-NEXT:   { "from": "class clang::Circle", "to": "class clang::Triangle", "kind": "fail" },
+  // CHECK-NEXT:   { "from": "class clang::Circle", "to": "class clang::Square", "kind": "fail" },
+  // CHECK-NEXT:   { "from": "struct clang::Shape", "to": "class clang::Square", "kind": "fail" }
+  // CHECK-NEXT: ],
+
+  (void)(1 / !C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+}
Index: