[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

In D102273#2866532 , @vsavchenko 
wrote:

> Great!  Thanks for addressing all of the comments!

Thank you for the review! Can you take care of merging it? I don't have the 
required permission.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

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

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

@vsavchenko any update on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

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

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

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

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

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

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

In D102517#2789450 , @goncharov wrote:

> Sorry, had to revert it as this fails under sanitizer : 
> https://lab.llvm.org/buildbot/#/builders/5/builds/8150

Thanks!

I missed adding "Ident_abstract" in the Parser initialize function. Should be 
fixed now.  CC: @hans


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102517

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

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 348806.
AbbasSabra added a comment.

Updating D102517 : [clang]Fix 
use-of-uninitialized-value detected by the sanitizer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102517

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp

Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -462,6 +462,77 @@
 virtual ~SealedDestructor() sealed; // expected-warning {{class with destructor marked 'sealed' cannot be inherited from}}
 };
 
+// expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+class AbstractClass abstract {
+  int i;
+};
+
+// expected-error@+1 {{variable type 'AbstractClass' is an abstract class}}
+AbstractClass abstractInstance;
+
+// expected-warning@+4 {{abstract class is marked 'sealed'}}
+// expected-note@+3 {{'AbstractAndSealedClass' declared here}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}
+class AbstractAndSealedClass abstract sealed {}; // Does no really make sense, but allowed
+
+// expected-error@+1 {{variable type 'AbstractAndSealedClass' is an abstract class}}
+AbstractAndSealedClass abstractAndSealedInstance;
+// expected-error@+1 {{base 'AbstractAndSealedClass' is marked 'sealed'}}
+class InheritFromAbstractAndSealed : AbstractAndSealedClass {};
+
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+// expected-warning@+3 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers1 final final {};
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+2 {{'sealed' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'sealed'}}
+class TooManyVirtSpecifiers2 final sealed {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+// expected-warning@+5 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+3 {{abstract class is marked 'final'}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers3 final abstract final {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+4 {{abstract class is marked 'final'}}
+// expected-warning@+3 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'abstract'}}
+class TooManyVirtSpecifiers4 abstract final abstract {};
+
+class Base {
+  virtual void i();
+};
+class AbstractFunctionInClass : public Base {
+  // expected-note@+2 {{unimplemented pure virtual method 'f' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void f() abstract;
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  void g() abstract; // expected-error {{'g' is not virtual and cannot be declared pure}}
+  // expected-note@+2 {{unimplemented pure virtual method 'h' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void h() abstract = 0; // expected-error {{class member already marked 'abstract'}}
+#if __cplusplus <= 199711L
+  // expected-warning@+4 {{'override' keyword is a C++11 extension}}
+#endif
+  // expected-note@+2 {{unimplemented pure virtual method 'i' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void i() abstract override;
+};
+
+// expected-error@+1 {{variable type 'AbstractFunctionInClass' is an abstract class}}
+AbstractFunctionInClass abstractFunctionInClassInstance;
+
 void AfterClassBody() {
   // expected-warning@+1 {{attribute 'deprecated' is ignored, place it after "struct" to apply attribute to type declaration}}
   struct D {} __declspec(deprecated);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16436,6 +16436,7 @@
 void Sema::ActOnStartCXXMemberDeclarations(Scope *S, Decl *TagD

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

Thanks for the review! Can you take care of merging the patch? I don't have 
access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102517

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

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 347955.
AbbasSabra added a comment.

Updating D102517 : [clang] Apply code review: 
while loop instead of for loop


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102517

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp

Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -462,6 +462,77 @@
 virtual ~SealedDestructor() sealed; // expected-warning {{class with destructor marked 'sealed' cannot be inherited from}}
 };
 
+// expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+class AbstractClass abstract {
+  int i;
+};
+
+// expected-error@+1 {{variable type 'AbstractClass' is an abstract class}}
+AbstractClass abstractInstance;
+
+// expected-warning@+4 {{abstract class is marked 'sealed'}}
+// expected-note@+3 {{'AbstractAndSealedClass' declared here}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}
+class AbstractAndSealedClass abstract sealed {}; // Does no really make sense, but allowed
+
+// expected-error@+1 {{variable type 'AbstractAndSealedClass' is an abstract class}}
+AbstractAndSealedClass abstractAndSealedInstance;
+// expected-error@+1 {{base 'AbstractAndSealedClass' is marked 'sealed'}}
+class InheritFromAbstractAndSealed : AbstractAndSealedClass {};
+
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+// expected-warning@+3 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers1 final final {};
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+2 {{'sealed' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'sealed'}}
+class TooManyVirtSpecifiers2 final sealed {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+// expected-warning@+5 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+3 {{abstract class is marked 'final'}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers3 final abstract final {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+4 {{abstract class is marked 'final'}}
+// expected-warning@+3 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'abstract'}}
+class TooManyVirtSpecifiers4 abstract final abstract {};
+
+class Base {
+  virtual void i();
+};
+class AbstractFunctionInClass : public Base {
+  // expected-note@+2 {{unimplemented pure virtual method 'f' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void f() abstract;
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  void g() abstract; // expected-error {{'g' is not virtual and cannot be declared pure}}
+  // expected-note@+2 {{unimplemented pure virtual method 'h' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void h() abstract = 0; // expected-error {{class member already marked 'abstract'}}
+#if __cplusplus <= 199711L
+  // expected-warning@+4 {{'override' keyword is a C++11 extension}}
+#endif
+  // expected-note@+2 {{unimplemented pure virtual method 'i' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void i() abstract override;
+};
+
+// expected-error@+1 {{variable type 'AbstractFunctionInClass' is an abstract class}}
+AbstractFunctionInClass abstractFunctionInClassInstance;
+
 void AfterClassBody() {
   // expected-warning@+1 {{attribute 'deprecated' is ignored, place it after "struct" to apply attribute to type declaration}}
   struct D {} __declspec(deprecated);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16436,6 +16436,7 @@
 void Sema::ActOnStartCXXMemberDeclarations(Scope *S, Decl *TagD,
 

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

> Does MS use this in any library headers or such

I didn't check if they do. But if any codebase does use "abstract" and they are 
trying to use clang-cl they would face a parsing error.
My use case was running static analysis on code that uses this feature. Such an 
error had a great impact on the quality of the analysis.




Comment at: clang/lib/Parse/ParseDeclCXX.cpp:3314
   if (getLangOpts().CPlusPlus && Tok.is(tok::identifier)) {
-VirtSpecifiers::Specifier Specifier = isCXX11VirtSpecifier(Tok);
-assert((Specifier == VirtSpecifiers::VS_Final ||
-Specifier == VirtSpecifiers::VS_GNU_Final ||
-Specifier == VirtSpecifiers::VS_Sealed) &&
+for (VirtSpecifiers::Specifier Specifier = isCXX11VirtSpecifier(Tok);
+ Specifier != VirtSpecifiers::VS_None;

hans wrote:
> This 'for' construct is hard to follow. I think it might be easier to follow 
> as a while loop, maybe
> 
> ```
> while ((Specifier = isCXX11VirtSpecifier(Tok)) != VirtSpecifiers::VS_None) {
> ```
> 
> Also the "is" prefix in isCXX11VirtSpecifier is confusing given that it 
> doesn't return a bool.
For "for" vs "while" if you use while you will have to declare the variable 
before the loop giving it the wrong scope which makes it less readable in my 
opinion. 
For  !!isCXX11VirtSpecifier!! I agree but I wouldn't separate the rename from 
this patch(I'm not sure what is the guidelines here)




Comment at: clang/lib/Sema/DeclSpec.cpp:1479
+  case VS_Abstract:
+VS_abstractLoc = Loc;
+break;

hans wrote:
> nit: the other cases just use one line
clang-format doesn't like it. Should I ignore it?



Comment at: clang/lib/Sema/DeclSpec.cpp:1493
   case VS_Sealed: return "sealed";
+  case VS_Abstract:
+return "abstract";

hans wrote:
> nit: skip the newline for consistency here too
same clang-format splits the line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102517

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

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 347018.
AbbasSabra marked 3 inline comments as done.
AbbasSabra added a comment.

Updating D102517 : [clang]Apply code review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102517

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp

Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -462,6 +462,77 @@
 virtual ~SealedDestructor() sealed; // expected-warning {{class with destructor marked 'sealed' cannot be inherited from}}
 };
 
+// expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+class AbstractClass abstract {
+  int i;
+};
+
+// expected-error@+1 {{variable type 'AbstractClass' is an abstract class}}
+AbstractClass abstractInstance;
+
+// expected-warning@+4 {{abstract class is marked 'sealed'}}
+// expected-note@+3 {{'AbstractAndSealedClass' declared here}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}
+class AbstractAndSealedClass abstract sealed {}; // Does no really make sense, but allowed
+
+// expected-error@+1 {{variable type 'AbstractAndSealedClass' is an abstract class}}
+AbstractAndSealedClass abstractAndSealedInstance;
+// expected-error@+1 {{base 'AbstractAndSealedClass' is marked 'sealed'}}
+class InheritFromAbstractAndSealed : AbstractAndSealedClass {};
+
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+// expected-warning@+3 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers1 final final {};
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+2 {{'sealed' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'sealed'}}
+class TooManyVirtSpecifiers2 final sealed {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+// expected-warning@+5 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+3 {{abstract class is marked 'final'}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers3 final abstract final {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+4 {{abstract class is marked 'final'}}
+// expected-warning@+3 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'abstract'}}
+class TooManyVirtSpecifiers4 abstract final abstract {};
+
+class Base {
+  virtual void i();
+};
+class AbstractFunctionInClass : public Base {
+  // expected-note@+2 {{unimplemented pure virtual method 'f' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void f() abstract;
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  void g() abstract; // expected-error {{'g' is not virtual and cannot be declared pure}}
+  // expected-note@+2 {{unimplemented pure virtual method 'h' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void h() abstract = 0; // expected-error {{class member already marked 'abstract'}}
+#if __cplusplus <= 199711L
+  // expected-warning@+4 {{'override' keyword is a C++11 extension}}
+#endif
+  // expected-note@+2 {{unimplemented pure virtual method 'i' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void i() abstract override;
+};
+
+// expected-error@+1 {{variable type 'AbstractFunctionInClass' is an abstract class}}
+AbstractFunctionInClass abstractFunctionInClassInstance;
+
 void AfterClassBody() {
   // expected-warning@+1 {{attribute 'deprecated' is ignored, place it after "struct" to apply attribute to type declaration}}
   struct D {} __declspec(deprecated);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16436,6 +16436,7 @@
 void Sema::ActOnStartCXXMemberDeclarations(Scope *S, Decl *TagD,
  

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

Note: I don't have the right to re-run the failed build/test. I assume that it 
is not related to my change since the test compile and runs omp code(Static 
analyzer doesn't run on that test)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

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

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.
Herald added a subscriber: manas.

In D102273#2766531 , @NoQ wrote:

> I've just been patching up clang-tidy's infinite loop checker and the problem 
> sounds s similar. Maybe we should move clang-tidy's alias analysis into 
> `libAnalysis` and re-use it?

I'm not familiar with clang-tidy's alias analysis. Do you think it should be 
part of this patch? or a follow-up one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

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

[PATCH] D102280: [analyzer] Engine: fix crash with SEH __leave keyword

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.



In D102280#2762760 , @steakhal wrote:

> I think it's good to go. Thank you!

Great! can you take care of merging it? I don't have accesss.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102280

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

[PATCH] D102280: [analyzer] Engine: fix crash with SEH __leave keyword

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.



> Well, and how can I build them on Linux xD

Ah, I don't think they are meant to be built on Linux :D  Especially if they 
rely on Window specific features like SEH. If they do they will disable these 
features on Linux build.

> How do you use clang exactly?

I work on the C++ static analyzer at sonarsource  
 and we use Clang front-end. We rely on CSA for some of our rules. So for these 
projects, we build them on windows. We extract the compilation command from the 
windows build and we map them to an equivalent clang command before running the 
analysis.

Is it a requirement for you to build them on Linux? Maybe you can try to build 
them on windows with !!clang-cl!!?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102280

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

[PATCH] D102517: [clang] Add support for the "abstract" contextual keyword of MSVC

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra created this revision.
AbbasSabra requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

https://docs.microsoft.com/en-us/cpp/extensions/abstract-cpp-component-extensions?view=msvc-160


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102517

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp

Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -462,6 +462,77 @@
 virtual ~SealedDestructor() sealed; // expected-warning {{class with destructor marked 'sealed' cannot be inherited from}}
 };
 
+// expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+class AbstractClass abstract {
+  int i;
+};
+
+// expected-error@+1 {{variable type 'AbstractClass' is an abstract class}}
+AbstractClass abstractInstance;
+
+// expected-warning@+4 {{abstract class is marked 'sealed'}}
+// expected-note@+3 {{'AbstractAndSealedClass' declared here}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}
+class AbstractAndSealedClass abstract sealed {}; // Does no really make sense, but allowed
+
+// expected-error@+1 {{variable type 'AbstractAndSealedClass' is an abstract class}}
+AbstractAndSealedClass abstractAndSealedInstance;
+// expected-error@+1 {{base 'AbstractAndSealedClass' is marked 'sealed'}}
+class InheritFromAbstractAndSealed : AbstractAndSealedClass {};
+
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+// expected-warning@+3 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers1 final final {};
+#if __cplusplus <= 199711L
+// expected-warning@+4 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+2 {{'sealed' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'sealed'}}
+class TooManyVirtSpecifiers2 final sealed {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+// expected-warning@+5 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+3 {{abstract class is marked 'final'}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'final'}}
+class TooManyVirtSpecifiers3 final abstract final {};
+#if __cplusplus <= 199711L
+// expected-warning@+6 {{'final' keyword is a C++11 extension}}
+#endif
+// expected-warning@+4 {{abstract class is marked 'final'}}
+// expected-warning@+3 {{'abstract' keyword is a Microsoft extension}}
+// expected-warning@+2 {{'abstract' keyword is a Microsoft extension}}
+// expected-error@+1 {{class already marked 'abstract'}}
+class TooManyVirtSpecifiers4 abstract final abstract {};
+
+class Base {
+  virtual void i();
+};
+class AbstractFunctionInClass : public Base {
+  // expected-note@+2 {{unimplemented pure virtual method 'f' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void f() abstract;
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  void g() abstract; // expected-error {{'g' is not virtual and cannot be declared pure}}
+  // expected-note@+2 {{unimplemented pure virtual method 'h' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void h() abstract = 0; // expected-error {{class member already marked 'abstract'}}
+#if __cplusplus <= 199711L
+  // expected-warning@+4 {{'override' keyword is a C++11 extension}}
+#endif
+  // expected-note@+2 {{unimplemented pure virtual method 'i' in 'AbstractFunctionInClass'}}
+  // expected-warning@+1 {{'abstract' keyword is a Microsoft extension}}
+  virtual void i() abstract override;
+};
+
+// expected-error@+1 {{variable type 'AbstractFunctionInClass' is an abstract class}}
+AbstractFunctionInClass abstractFunctionInClassInstance;
+
 void AfterClassBody() {
   // expected-warning@+1 {{attribute 'deprecated' is ignored, place it after "struct" to apply attribute to type declaration}}
   struct D {} __declspec(deprecated);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16436,6 +16436,7 @@
 void Sema::ActOnStartCXXMemberDeclarations(Scope *S, Decl *TagD,
  

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 345482.
AbbasSabra added a comment.

Updating D102273 : [analyzer] Update comments 
+ fix typos


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

Files:
  clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  clang/test/Analysis/loop-unrolling.cpp

Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify -std=c++11 -analyzer-config exploration_strategy=unexplored_first_queue %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true,exploration_strategy=dfs -verify -std=c++11 -DDFS=1 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify -std=c++14 -analyzer-config exploration_strategy=unexplored_first_queue %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true,exploration_strategy=dfs -verify -std=c++14 -DDFS=1 %s
 
 void clang_analyzer_numTimesReached();
 void clang_analyzer_warnIfReached();
@@ -511,3 +511,39 @@
 clang_analyzer_numTimesReached(); // expected-warning {{4}}
   }
 }
+
+void capture_by_value_as_loop_counter() {
+  int out = 0;
+  auto l = [i = out]() mutable {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{10}}
+}
+  };
+}
+
+void capture_by_ref_as_loop_counter() {
+  int out = 0;
+  auto l = [&i = out]() {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{4}}
+}
+  };
+}
+
+void capture_implicitly_by_value_as_loop_counter() {
+  int i = 0;
+  auto l = [=]() mutable {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{10}}
+}
+  };
+}
+
+void capture_implicitly_by_ref_as_loop_counter() {
+  int i = 0;
+  auto l = [&]() mutable {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{4}}
+}
+  };
+}
Index: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -79,14 +79,17 @@
   return State;
 }
 
-static internal::Matcher simpleCondition(StringRef BindName) {
-  return binaryOperator(anyOf(hasOperatorName("<"), hasOperatorName(">"),
-  hasOperatorName("<="), hasOperatorName(">="),
-  hasOperatorName("!=")),
-hasEitherOperand(ignoringParenImpCasts(declRefExpr(
-to(varDecl(hasType(isInteger())).bind(BindName),
-hasEitherOperand(ignoringParenImpCasts(
-integerLiteral().bind("boundNum"
+static internal::Matcher simpleCondition(StringRef BindName,
+   StringRef RefName) {
+  return binaryOperator(
+ anyOf(hasOperatorName("<"), hasOperatorName(">"),
+   hasOperatorName("<="), hasOperatorName(">="),
+   hasOperatorName("!=")),
+ hasEitherOperand(ignoringParenImpCasts(
+ declRefExpr(to(varDecl(hasType(isInteger())).bind(BindName)))
+ .bind(RefName))),
+ hasEitherOperand(
+ ignoringParenImpCasts(integerLiteral().bind("boundNum"
   .bind("conditionOperator");
 }
 
@@ -138,7 +141,7 @@
 
 static internal::Matcher forLoopMatcher() {
   return forStmt(
- hasCondition(simpleCondition("initVarName")),
+ hasCondition(simpleCondition("initVarName", "initVarRef")),
  // Initialization should match the form: 'int i = 6' or 'i = 42'.
  hasLoopInit(
  anyOf(declStmt(hasSingleDecl(
@@ -156,17 +159,52 @@
  hasUnaryOperand(declRefExpr(
  to(varDecl(allOf(equalsBoundNode("initVarName"),
   hasType(isInteger(),
- unless(hasBody(hasSuspiciousStmt("initVarName".bind("forLoop");
+ unless(hasBody(hasSuspiciousStmt("initVarName"
+  .bind("forLoop");
 }
 
-static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) {
-  // Global variables assumed as escaped variables.
+static bool isCapturedByReference(ExplodedNode *N, const DeclRefExpr *DR) {
+
+  // Get the lambda CXXRecordDecl
+  assert(DR->refersToEnclosingVariableOrCapture());
+  const LocationContext *LocCtxt = N->getLocationC

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:185-186
+  return FD->getType()->isReferenceType();
+} else {
+  assert(false && "Unknown captured variable");
+}

vsavchenko wrote:
> AbbasSabra wrote:
> > vsavchenko wrote:
> > > But actually, it's a bit different with this one. I don't know exact 
> > > details and rules how clang sema fills in the class for lambda.
> > > According to [[ https://en.cppreference.com/w/cpp/language/lambda | 
> > > cppreference ]]:
> > > > For the entities that are captured by reference (with the default 
> > > > capture [&] or when using the character &, e.g. [&a, &b, &c]), it is 
> > > > unspecified if additional data members are declared in the closure type
> > > 
> > > It can be pretty much specified in clang, that's true, but it looks like 
> > > in `DeadStoreChecker` we have a very similar situation and we do not 
> > > assume that captured variable always have a corresponding field.
> > Yes, I based this on the fact that DeadStoreChecker considers it possible, 
> > but I have never faced a case where it does not have a corresponding field.
> It still would be good to have some proof that it is indeed like this or 
> simply fallback into returning true (which you already do when in doubt).
As I said, I believe it cannot happen but I assumed based on the other checker 
that there is something I don't know.
I think based on !!getCaptureFields!! the implementation we are iterating over 
all captures. For that algorithm to work number of captures should be equal to 
number of fields
```
void CXXRecordDecl::getCaptureFields(
   llvm::DenseMap &Captures,
   FieldDecl *&ThisCapture) const {
  Captures.clear();
  ThisCapture = nullptr;

  LambdaDefinitionData &Lambda = getLambdaData();
  RecordDecl::field_iterator Field = field_begin();
  for (const LambdaCapture *C = Lambda.Captures, *CEnd = C + Lambda.NumCaptures;
   C != CEnd; ++C, ++Field) {
if (C->capturesThis())
  ThisCapture = *Field;
else if (C->capturesVariable())
  Captures[C->getCapturedVar()] = *Field;
  }
  assert(Field == field_end());
}
```

I dropped the defensive code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

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

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 345471.
AbbasSabra marked 7 inline comments as done.
AbbasSabra added a comment.

Updating D102273 : [analyzer] Apply code 
review part 2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

Files:
  clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  clang/test/Analysis/loop-unrolling.cpp

Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify -std=c++11 -analyzer-config exploration_strategy=unexplored_first_queue %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true,exploration_strategy=dfs -verify -std=c++11 -DDFS=1 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify -std=c++14 -analyzer-config exploration_strategy=unexplored_first_queue %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true,exploration_strategy=dfs -verify -std=c++14 -DDFS=1 %s
 
 void clang_analyzer_numTimesReached();
 void clang_analyzer_warnIfReached();
@@ -511,3 +511,39 @@
 clang_analyzer_numTimesReached(); // expected-warning {{4}}
   }
 }
+
+void capture_by_value_as_loop_counter() {
+  int out = 0;
+  auto l = [i = out]() mutable {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{10}}
+}
+  };
+}
+
+void capture_by_ref_as_loop_counter() {
+  int out = 0;
+  auto l = [&i = out]() {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{4}}
+}
+  };
+}
+
+void capture_implicitly_by_value_as_loop_counter() {
+  int i = 0;
+  auto l = [=]() mutable {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{10}}
+}
+  };
+}
+
+void capture_implicitly_by_ref_as_loop_counter() {
+  int i = 0;
+  auto l = [&]() mutable {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{4}}
+}
+  };
+}
Index: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -79,14 +79,17 @@
   return State;
 }
 
-static internal::Matcher simpleCondition(StringRef BindName) {
-  return binaryOperator(anyOf(hasOperatorName("<"), hasOperatorName(">"),
-  hasOperatorName("<="), hasOperatorName(">="),
-  hasOperatorName("!=")),
-hasEitherOperand(ignoringParenImpCasts(declRefExpr(
-to(varDecl(hasType(isInteger())).bind(BindName),
-hasEitherOperand(ignoringParenImpCasts(
-integerLiteral().bind("boundNum"
+static internal::Matcher simpleCondition(StringRef BindName,
+   StringRef RefName) {
+  return binaryOperator(
+ anyOf(hasOperatorName("<"), hasOperatorName(">"),
+   hasOperatorName("<="), hasOperatorName(">="),
+   hasOperatorName("!=")),
+ hasEitherOperand(ignoringParenImpCasts(
+ declRefExpr(to(varDecl(hasType(isInteger())).bind(BindName)))
+ .bind(RefName))),
+ hasEitherOperand(
+ ignoringParenImpCasts(integerLiteral().bind("boundNum"
   .bind("conditionOperator");
 }
 
@@ -138,7 +141,7 @@
 
 static internal::Matcher forLoopMatcher() {
   return forStmt(
- hasCondition(simpleCondition("initVarName")),
+ hasCondition(simpleCondition("initVarName", "initVarRef")),
  // Initialization should match the form: 'int i = 6' or 'i = 42'.
  hasLoopInit(
  anyOf(declStmt(hasSingleDecl(
@@ -156,17 +159,54 @@
  hasUnaryOperand(declRefExpr(
  to(varDecl(allOf(equalsBoundNode("initVarName"),
   hasType(isInteger(),
- unless(hasBody(hasSuspiciousStmt("initVarName".bind("forLoop");
+ unless(hasBody(hasSuspiciousStmt("initVarName"
+  .bind("forLoop");
 }
 
-static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) {
-  // Global variables assumed as escaped variables.
+static bool isCapturedByReference(ExplodedNode *N, const DeclRefExpr *DR) {
+
+  // Get the lambda CXXRecordDecl
+  assert(DR->refersToEnclosingVariableOrCapture());
+  const 

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:185-186
+  return FD->getType()->isReferenceType();
+} else {
+  assert(false && "Unknown captured variable");
+}

vsavchenko wrote:
> But actually, it's a bit different with this one. I don't know exact details 
> and rules how clang sema fills in the class for lambda.
> According to [[ https://en.cppreference.com/w/cpp/language/lambda | 
> cppreference ]]:
> > For the entities that are captured by reference (with the default capture 
> > [&] or when using the character &, e.g. [&a, &b, &c]), it is unspecified if 
> > additional data members are declared in the closure type
> 
> It can be pretty much specified in clang, that's true, but it looks like in 
> `DeadStoreChecker` we have a very similar situation and we do not assume that 
> captured variable always have a corresponding field.
Yes, I based this on the fact that DeadStoreChecker considers it possible, but 
I have never faced a case where it does not have a corresponding field.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

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

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 345435.
AbbasSabra marked 4 inline comments as done.
AbbasSabra added a comment.

Updating D102273 : [analyzer] Apply code 
review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

Files:
  clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  clang/test/Analysis/loop-unrolling.cpp

Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify -std=c++11 -analyzer-config exploration_strategy=unexplored_first_queue %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true,exploration_strategy=dfs -verify -std=c++11 -DDFS=1 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify -std=c++14 -analyzer-config exploration_strategy=unexplored_first_queue %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true,exploration_strategy=dfs -verify -std=c++14 -DDFS=1 %s
 
 void clang_analyzer_numTimesReached();
 void clang_analyzer_warnIfReached();
@@ -511,3 +511,39 @@
 clang_analyzer_numTimesReached(); // expected-warning {{4}}
   }
 }
+
+void capture_by_value_as_loop_counter() {
+  int out = 0;
+  auto l = [i = out]() mutable {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{10}}
+}
+  };
+}
+
+void capture_by_ref_as_loop_counter() {
+  int out = 0;
+  auto l = [&i = out]() {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{4}}
+}
+  };
+}
+
+void capture_implicitly_by_value_as_loop_counter() {
+  int i = 0;
+  auto l = [=]() mutable {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{10}}
+}
+  };
+}
+
+void capture_implicitly_by_ref_as_loop_counter() {
+  int i = 0;
+  auto l = [&]() mutable {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{4}}
+}
+  };
+}
Index: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -79,14 +79,17 @@
   return State;
 }
 
-static internal::Matcher simpleCondition(StringRef BindName) {
-  return binaryOperator(anyOf(hasOperatorName("<"), hasOperatorName(">"),
-  hasOperatorName("<="), hasOperatorName(">="),
-  hasOperatorName("!=")),
-hasEitherOperand(ignoringParenImpCasts(declRefExpr(
-to(varDecl(hasType(isInteger())).bind(BindName),
-hasEitherOperand(ignoringParenImpCasts(
-integerLiteral().bind("boundNum"
+static internal::Matcher simpleCondition(StringRef BindName,
+   StringRef RefName) {
+  return binaryOperator(
+ anyOf(hasOperatorName("<"), hasOperatorName(">"),
+   hasOperatorName("<="), hasOperatorName(">="),
+   hasOperatorName("!=")),
+ hasEitherOperand(ignoringParenImpCasts(
+ declRefExpr(to(varDecl(hasType(isInteger())).bind(BindName)))
+ .bind(RefName))),
+ hasEitherOperand(
+ ignoringParenImpCasts(integerLiteral().bind("boundNum"
   .bind("conditionOperator");
 }
 
@@ -138,7 +141,7 @@
 
 static internal::Matcher forLoopMatcher() {
   return forStmt(
- hasCondition(simpleCondition("initVarName")),
+ hasCondition(simpleCondition("initVarName", "initVarRef")),
  // Initialization should match the form: 'int i = 6' or 'i = 42'.
  hasLoopInit(
  anyOf(declStmt(hasSingleDecl(
@@ -156,17 +159,53 @@
  hasUnaryOperand(declRefExpr(
  to(varDecl(allOf(equalsBoundNode("initVarName"),
   hasType(isInteger(),
- unless(hasBody(hasSuspiciousStmt("initVarName".bind("forLoop");
+ unless(hasBody(hasSuspiciousStmt("initVarName"
+  .bind("forLoop");
 }
 
-static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) {
+static bool isCapturedByReference(ExplodedNode *N, const DeclRefExpr *DR) {
+  assert(DR->refersToEnclosingVariableOrCapture());
+
+  const LocationContext *LocCtxt = N->getLocationContext();
+  const Decl *D = LocCtxt->getDecl();
+  c

[PATCH] D102280: [analyzer] Engine: fix crash with SEH __leave keyword

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

In D102280#2759167 , @steakhal wrote:

> Do you have any good (mature, big enough) open-source projects for these msvc 
> constructs?

https://github.com/microsoft/winfile and 
https://github.com/chakra-core/ChakraCore seem to be using SEH but not heavily.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102280

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

[PATCH] D102280: [analyzer] Engine: fix crash with SEH __leave keyword

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 345422.
AbbasSabra added a comment.

Updating D102280 : [analyzer] rename test 
file + make sure that statements after "__leave" are not reached


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102280

Files:
  clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
  clang/test/Analysis/ms-seh.cpp


Index: clang/test/Analysis/ms-seh.cpp
===
--- /dev/null
+++ clang/test/Analysis/ms-seh.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple 
x86_64-pc-windows-msvc19.11.0 -fms-extensions -verify %s
+
+void clang_analyzer_warnIfReached();
+int filter();
+
+void try_except_leave() {
+  __try {
+__leave;// no-crash
+clang_analyzer_warnIfReached(); // no-warning
+  } __except (filter()) {
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
Index: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -349,6 +349,7 @@
 HandleBranch(cast(Term)->getCond(), Term, B, Pred);
 return;
 
+  case Stmt::SEHLeaveStmtClass:
   case Stmt::ContinueStmtClass:
   case Stmt::BreakStmtClass:
   case Stmt::GotoStmtClass:


Index: clang/test/Analysis/ms-seh.cpp
===
--- /dev/null
+++ clang/test/Analysis/ms-seh.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions -verify %s
+
+void clang_analyzer_warnIfReached();
+int filter();
+
+void try_except_leave() {
+  __try {
+__leave;// no-crash
+clang_analyzer_warnIfReached(); // no-warning
+  } __except (filter()) {
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
Index: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -349,6 +349,7 @@
 HandleBranch(cast(Term)->getCond(), Term, B, Pred);
 return;
 
+  case Stmt::SEHLeaveStmtClass:
   case Stmt::ContinueStmtClass:
   case Stmt::BreakStmtClass:
   case Stmt::GotoStmtClass:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102280: [analyzer] Engine: fix crash with SEH __leave keyword

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra marked 2 inline comments as done.
AbbasSabra added a comment.

> Please, add these reviewers for your upcoming [analyzer] patches.

Thanks, I was planning to add reviewers after making sure that the pre-checks 
are green.

> Nice to see some fixes for visual c++ stuff.

Thanks for the quick review!




Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:352
 
+  case Stmt::SEHLeaveStmtClass:
   case Stmt::ContinueStmtClass:

steakhal wrote:
> You should probably extend the `ExprEngine.cpp:1312` in a similar fashion.
isn't it already handled in  !!ExprEngine.cpp:1239!!?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102280

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

[PATCH] D102280: [analyzer] Engine: fix crash with SEH __leave keyword

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 345381.
AbbasSabra added a comment.

Updating D102280 : [analyzer] Apply code 
review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102280

Files:
  clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
  clang/test/Analysis/misc-ms-leave.cpp


Index: clang/test/Analysis/misc-ms-leave.cpp
===
--- /dev/null
+++ clang/test/Analysis/misc-ms-leave.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple 
x86_64-pc-windows-msvc19.11.0 -fms-extensions -verify %s
+
+void clang_analyzer_warnIfReached();
+int filter();
+
+void try_except_leave() {
+  __try {
+__leave; // no-crash
+  } __except (filter()) {
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
Index: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -349,6 +349,7 @@
 HandleBranch(cast(Term)->getCond(), Term, B, Pred);
 return;
 
+  case Stmt::SEHLeaveStmtClass:
   case Stmt::ContinueStmtClass:
   case Stmt::BreakStmtClass:
   case Stmt::GotoStmtClass:


Index: clang/test/Analysis/misc-ms-leave.cpp
===
--- /dev/null
+++ clang/test/Analysis/misc-ms-leave.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions -verify %s
+
+void clang_analyzer_warnIfReached();
+int filter();
+
+void try_except_leave() {
+  __try {
+__leave; // no-crash
+  } __except (filter()) {
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
Index: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -349,6 +349,7 @@
 HandleBranch(cast(Term)->getCond(), Term, B, Pred);
 return;
 
+  case Stmt::SEHLeaveStmtClass:
   case Stmt::ContinueStmtClass:
   case Stmt::BreakStmtClass:
   case Stmt::GotoStmtClass:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102280: [analyzer] Engine: fix crash with SEH __leave keyword

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra created this revision.
Herald added subscribers: steakhal, ASDenysPetrov, martong, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
AbbasSabra requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102280

Files:
  clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
  clang/test/Analysis/misc-ms-leave.cpp


Index: clang/test/Analysis/misc-ms-leave.cpp
===
--- /dev/null
+++ clang/test/Analysis/misc-ms-leave.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -triple 
x86_64-pc-windows-msvc19.11.0 -fms-extensions -verify %s
+
+void test() {
+  int *p = 0;
+  int x = *p; // expected-warning {{Dereference of null pointer (loaded from 
variable 'p')}}
+}
+
+int filter();
+
+void try_except_leave() {
+  __try {
+__leave;
+  } __except (filter()) {
+  }
+  int *p = 0;
+  int x = *p; // expected-warning {{Dereference of null pointer (loaded from 
variable 'p')}}
+}
Index: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -349,6 +349,7 @@
 HandleBranch(cast(Term)->getCond(), Term, B, Pred);
 return;
 
+  case Stmt::SEHLeaveStmtClass:
   case Stmt::ContinueStmtClass:
   case Stmt::BreakStmtClass:
   case Stmt::GotoStmtClass:


Index: clang/test/Analysis/misc-ms-leave.cpp
===
--- /dev/null
+++ clang/test/Analysis/misc-ms-leave.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions -verify %s
+
+void test() {
+  int *p = 0;
+  int x = *p; // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+}
+
+int filter();
+
+void try_except_leave() {
+  __try {
+__leave;
+  } __except (filter()) {
+  }
+  int *p = 0;
+  int x = *p; // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+}
Index: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -349,6 +349,7 @@
 HandleBranch(cast(Term)->getCond(), Term, B, Pred);
 return;
 
+  case Stmt::SEHLeaveStmtClass:
   case Stmt::ContinueStmtClass:
   case Stmt::BreakStmtClass:
   case Stmt::GotoStmtClass:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra created this revision.
Herald added subscribers: steakhal, ASDenysPetrov, martong, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, zzheng, szepet, baloghadamsoftware, 
xazax.hun.
AbbasSabra requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102273

Files:
  clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  clang/test/Analysis/loop-unrolling.cpp

Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify -std=c++11 -analyzer-config exploration_strategy=unexplored_first_queue %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true,exploration_strategy=dfs -verify -std=c++11 -DDFS=1 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify -std=c++14 -analyzer-config exploration_strategy=unexplored_first_queue %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true,exploration_strategy=dfs -verify -std=c++14 -DDFS=1 %s
 
 void clang_analyzer_numTimesReached();
 void clang_analyzer_warnIfReached();
@@ -511,3 +511,39 @@
 clang_analyzer_numTimesReached(); // expected-warning {{4}}
   }
 }
+
+void capture_by_value_as_loop_counter() {
+  int out = 0;
+  auto l = [i = out]() mutable {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{10}}
+}
+  };
+}
+
+void capture_by_ref_as_loop_counter() {
+  int out = 0;
+  auto l = [&i = out]() {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{4}}
+}
+  };
+}
+
+void capture_implicitly_by_value_as_loop_counter() {
+  int i = 0;
+  auto l = [=]() mutable {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{10}}
+}
+  };
+}
+
+void capture_implicitly_by_ref_as_loop_counter() {
+  int i = 0;
+  auto l = [&]() mutable {
+for (i = 0; i < 10; ++i) {
+  clang_analyzer_numTimesReached(); // expected-warning {{4}}
+}
+  };
+}
Index: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -79,14 +79,17 @@
   return State;
 }
 
-static internal::Matcher simpleCondition(StringRef BindName) {
-  return binaryOperator(anyOf(hasOperatorName("<"), hasOperatorName(">"),
-  hasOperatorName("<="), hasOperatorName(">="),
-  hasOperatorName("!=")),
-hasEitherOperand(ignoringParenImpCasts(declRefExpr(
-to(varDecl(hasType(isInteger())).bind(BindName),
-hasEitherOperand(ignoringParenImpCasts(
-integerLiteral().bind("boundNum"
+static internal::Matcher simpleCondition(StringRef BindName,
+   StringRef RefName) {
+  return binaryOperator(
+ anyOf(hasOperatorName("<"), hasOperatorName(">"),
+   hasOperatorName("<="), hasOperatorName(">="),
+   hasOperatorName("!=")),
+ hasEitherOperand(ignoringParenImpCasts(
+ declRefExpr(to(varDecl(hasType(isInteger())).bind(BindName)))
+ .bind(RefName))),
+ hasEitherOperand(
+ ignoringParenImpCasts(integerLiteral().bind("boundNum"
   .bind("conditionOperator");
 }
 
@@ -138,7 +141,7 @@
 
 static internal::Matcher forLoopMatcher() {
   return forStmt(
- hasCondition(simpleCondition("initVarName")),
+ hasCondition(simpleCondition("initVarName", "initVarRef")),
  // Initialization should match the form: 'int i = 6' or 'i = 42'.
  hasLoopInit(
  anyOf(declStmt(hasSingleDecl(
@@ -156,17 +159,52 @@
  hasUnaryOperand(declRefExpr(
  to(varDecl(allOf(equalsBoundNode("initVarName"),
   hasType(isInteger(),
- unless(hasBody(hasSuspiciousStmt("initVarName".bind("forLoop");
+ unless(hasBody(hasSuspiciousStmt("initVarName"
+  .bind("forLoop");
 }
 
-static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) {
+static bool isCapturedByReference(const VarDecl *VD, ExplodedNode *N,
+  const DeclRefExpr *DR) {
+  assert(DR->refersToEnclosingVariableOrCapture());
+
+  co

[PATCH] D80669: [analyzer] LoopWidening: fix crash by avoiding aliased references invalidation

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

Gentle reminder


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80669



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

[PATCH] D80669: [analyzer] LoopWidening: fix crash by avoiding aliased references invalidation

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

In D80669#2058824 , @NoQ wrote:

> Fair!


Thanks for the review! May you please take care of merging this one too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80669



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

[PATCH] D80669: [analyzer] LoopWidening: fix crash by avoiding aliased references invalidation

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 266689.
AbbasSabra added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80669

Files:
  clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
  clang/test/Analysis/loop-widening-preserve-reference-type.cpp


Index: clang/test/Analysis/loop-widening-preserve-reference-type.cpp
===
--- clang/test/Analysis/loop-widening-preserve-reference-type.cpp
+++ clang/test/Analysis/loop-widening-preserve-reference-type.cpp
@@ -12,3 +12,11 @@
   for (int i = 0; i < 10; ++i) { }
   clang_analyzer_eval(&x != 0); // expected-warning{{TRUE}}
 }   // expected-warning@-1{{reference cannot be 
bound to dereferenced null pointer in well-defined C++ code; comparison may be 
assumed to always evaluate to true}}
+
+using AR = const A &;
+void invalid_type_alias_region_access() {
+  AR x = B();
+  for (int i = 0; i < 10; ++i) {
+  }
+  clang_analyzer_eval(&x != 0); // expected-warning{{TRUE}}
+} // expected-warning@-1{{reference cannot be bound to dereferenced null 
pointer in well-defined C++ code; comparison may be assumed to always evaluate 
to true}}
Index: clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -67,8 +67,10 @@
   }
 
   // References should not be invalidated.
-  auto Matches = 
match(findAll(stmt(hasDescendant(varDecl(hasType(referenceType())).bind(MatchRef,
-   *LCtx->getDecl()->getBody(), ASTCtx);
+  auto Matches = match(
+  findAll(stmt(hasDescendant(
+  
varDecl(hasType(hasCanonicalType(referenceType(.bind(MatchRef,
+  *LCtx->getDecl()->getBody(), ASTCtx);
   for (BoundNodes Match : Matches) {
 const VarDecl *VD = Match.getNodeAs(MatchRef);
 assert(VD);


Index: clang/test/Analysis/loop-widening-preserve-reference-type.cpp
===
--- clang/test/Analysis/loop-widening-preserve-reference-type.cpp
+++ clang/test/Analysis/loop-widening-preserve-reference-type.cpp
@@ -12,3 +12,11 @@
   for (int i = 0; i < 10; ++i) { }
   clang_analyzer_eval(&x != 0); // expected-warning{{TRUE}}
 }   // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to true}}
+
+using AR = const A &;
+void invalid_type_alias_region_access() {
+  AR x = B();
+  for (int i = 0; i < 10; ++i) {
+  }
+  clang_analyzer_eval(&x != 0); // expected-warning{{TRUE}}
+} // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to true}}
Index: clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -67,8 +67,10 @@
   }
 
   // References should not be invalidated.
-  auto Matches = match(findAll(stmt(hasDescendant(varDecl(hasType(referenceType())).bind(MatchRef,
-   *LCtx->getDecl()->getBody(), ASTCtx);
+  auto Matches = match(
+  findAll(stmt(hasDescendant(
+  varDecl(hasType(hasCanonicalType(referenceType(.bind(MatchRef,
+  *LCtx->getDecl()->getBody(), ASTCtx);
   for (BoundNodes Match : Matches) {
 const VarDecl *VD = Match.getNodeAs(MatchRef);
 assert(VD);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80671: [analyzer] LoopWidening: fix crash by avoiding aliased references invalidation

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra created this revision.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, 
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a project: clang.
AbbasSabra abandoned this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80671

Files:
  clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
  clang/test/Analysis/loop-widening-preserve-reference-type.cpp


Index: clang/test/Analysis/loop-widening-preserve-reference-type.cpp
===
--- clang/test/Analysis/loop-widening-preserve-reference-type.cpp
+++ clang/test/Analysis/loop-widening-preserve-reference-type.cpp
@@ -12,3 +12,11 @@
   for (int i = 0; i < 10; ++i) { }
   clang_analyzer_eval(&x != 0); // expected-warning{{TRUE}}
 }   // expected-warning@-1{{reference cannot be 
bound to dereferenced null pointer in well-defined C++ code; comparison may be 
assumed to always evaluate to true}}
+
+using AR = const A &;
+void invalid_type_alias_region_access() {
+  AR x = B();
+  for (int i = 0; i < 10; ++i) {
+  }
+  clang_analyzer_eval(&x != 0); // expected-warning{{TRUE}}
+} // expected-warning@-1{{reference cannot be bound to dereferenced null 
pointer in well-defined C++ code; comparison may be assumed to always evaluate 
to true}}
Index: clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -67,8 +67,10 @@
   }
 
   // References should not be invalidated.
-  auto Matches = 
match(findAll(stmt(hasDescendant(varDecl(hasType(referenceType())).bind(MatchRef,
-   *LCtx->getDecl()->getBody(), ASTCtx);
+  auto Matches = match(
+  findAll(stmt(hasDescendant(
+  
varDecl(hasType(hasCanonicalType(referenceType(.bind(MatchRef,
+  *LCtx->getDecl()->getBody(), ASTCtx);
   for (BoundNodes Match : Matches) {
 const VarDecl *VD = Match.getNodeAs(MatchRef);
 assert(VD);


Index: clang/test/Analysis/loop-widening-preserve-reference-type.cpp
===
--- clang/test/Analysis/loop-widening-preserve-reference-type.cpp
+++ clang/test/Analysis/loop-widening-preserve-reference-type.cpp
@@ -12,3 +12,11 @@
   for (int i = 0; i < 10; ++i) { }
   clang_analyzer_eval(&x != 0); // expected-warning{{TRUE}}
 }   // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to true}}
+
+using AR = const A &;
+void invalid_type_alias_region_access() {
+  AR x = B();
+  for (int i = 0; i < 10; ++i) {
+  }
+  clang_analyzer_eval(&x != 0); // expected-warning{{TRUE}}
+} // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to true}}
Index: clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -67,8 +67,10 @@
   }
 
   // References should not be invalidated.
-  auto Matches = match(findAll(stmt(hasDescendant(varDecl(hasType(referenceType())).bind(MatchRef,
-   *LCtx->getDecl()->getBody(), ASTCtx);
+  auto Matches = match(
+  findAll(stmt(hasDescendant(
+  varDecl(hasType(hasCanonicalType(referenceType(.bind(MatchRef,
+  *LCtx->getDecl()->getBody(), ASTCtx);
   for (BoundNodes Match : Matches) {
 const VarDecl *VD = Match.getNodeAs(MatchRef);
 assert(VD);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80669: [analyzer] LoopWidening: fix crash by avoiding aliased references invalidation

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra created this revision.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, 
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a project: clang.
AbbasSabra edited the summary of this revision.
AbbasSabra added reviewers: xazax.hun, vsavchenko, NoQ.
Herald added a subscriber: rnkovacs.
AbbasSabra retitled this revision from "[analyzer] LoopWidening: fix crash by 
avoiding aliased references invalidationSummary: LoopWidening is invalidating 
references comming from type aliases which lead to a crashReviewers: xazax.hun 
vsavchenko NoQSubscribers:" to "[analyzer] LoopWidening: fix crash by avoiding 
aliased references invalidation".

LoopWidening is invalidating references coming from type aliases which lead to 
a crash.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80669

Files:
  clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
  clang/test/Analysis/loop-widening-preserve-reference-type.cpp


Index: clang/test/Analysis/loop-widening-preserve-reference-type.cpp
===
--- clang/test/Analysis/loop-widening-preserve-reference-type.cpp
+++ clang/test/Analysis/loop-widening-preserve-reference-type.cpp
@@ -12,3 +12,10 @@
   for (int i = 0; i < 10; ++i) { }
   clang_analyzer_eval(&x != 0); // expected-warning{{TRUE}}
 }   // expected-warning@-1{{reference cannot be 
bound to dereferenced null pointer in well-defined C++ code; comparison may be 
assumed to always evaluate to true}}
+
+using AR = const A&;
+void invalid_type_alias_region_access() {
+  AR x = B();
+  for (int i = 0; i < 10; ++i) { }
+  clang_analyzer_eval(&x != 0); // expected-warning{{TRUE}}
+}   // expected-warning@-1{{reference cannot be 
bound to dereferenced null pointer in well-defined C++ code; comparison may be 
assumed to always evaluate to true}}
Index: clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -67,8 +67,10 @@
   }
 
   // References should not be invalidated.
-  auto Matches = 
match(findAll(stmt(hasDescendant(varDecl(hasType(referenceType())).bind(MatchRef,
-   *LCtx->getDecl()->getBody(), ASTCtx);
+  auto Matches = match(
+  findAll(stmt(hasDescendant(
+  
varDecl(hasType(hasCanonicalType(referenceType(.bind(MatchRef,
+  *LCtx->getDecl()->getBody(), ASTCtx);
   for (BoundNodes Match : Matches) {
 const VarDecl *VD = Match.getNodeAs(MatchRef);
 assert(VD);


Index: clang/test/Analysis/loop-widening-preserve-reference-type.cpp
===
--- clang/test/Analysis/loop-widening-preserve-reference-type.cpp
+++ clang/test/Analysis/loop-widening-preserve-reference-type.cpp
@@ -12,3 +12,10 @@
   for (int i = 0; i < 10; ++i) { }
   clang_analyzer_eval(&x != 0); // expected-warning{{TRUE}}
 }   // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to true}}
+
+using AR = const A&;
+void invalid_type_alias_region_access() {
+  AR x = B();
+  for (int i = 0; i < 10; ++i) { }
+  clang_analyzer_eval(&x != 0); // expected-warning{{TRUE}}
+}   // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to true}}
Index: clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -67,8 +67,10 @@
   }
 
   // References should not be invalidated.
-  auto Matches = match(findAll(stmt(hasDescendant(varDecl(hasType(referenceType())).bind(MatchRef,
-   *LCtx->getDecl()->getBody(), ASTCtx);
+  auto Matches = match(
+  findAll(stmt(hasDescendant(
+  varDecl(hasType(hasCanonicalType(referenceType(.bind(MatchRef,
+  *LCtx->getDecl()->getBody(), ASTCtx);
   for (BoundNodes Match : Matches) {
 const VarDecl *VD = Match.getNodeAs(MatchRef);
 assert(VD);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80171: [analyzer] LoopUnrolling: fix crash when a parameter is a loop counter

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

Great! Can someone take care of merging it? I believe I don't have access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80171



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

[PATCH] D80171: [analyzer] LoopUnrolling: fix crash when a parameter is a loop counter

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 265253.
AbbasSabra added a comment.

Fix code review 2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80171

Files:
  clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  clang/test/Analysis/loop-unrolling.cpp


Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -499,3 +499,15 @@
 clang_analyzer_numTimesReached(); // expected-warning {{6}}
   }
 }
+
+void parm_by_value_as_loop_counter(int i) {
+  for (i = 0; i < 10; ++i) {
+clang_analyzer_numTimesReached(); // expected-warning {{10}}
+  }
+}
+
+void parm_by_ref_as_loop_counter(int &i) {
+  for (i = 0; i < 10; ++i) {
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -164,6 +164,11 @@
   if (VD->hasGlobalStorage())
 return true;
 
+  const bool isParm = isa(VD);
+  // Reference parameters are assumed as escaped variables.
+  if (isParm && VD->getType()->isReferenceType())
+return true;
+
   while (!N->pred_empty()) {
 // FIXME: getStmtForDiagnostics() does nasty things in order to provide
 // a valid statement for body farms, do we need this behavior here?
@@ -193,6 +198,11 @@
 
 N = N->getFirstPred();
   }
+
+  // Parameter declaration will not be found.
+  if (isParm)
+return false;
+
   llvm_unreachable("Reached root without finding the declaration of VD");
 }
 


Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -499,3 +499,15 @@
 clang_analyzer_numTimesReached(); // expected-warning {{6}}
   }
 }
+
+void parm_by_value_as_loop_counter(int i) {
+  for (i = 0; i < 10; ++i) {
+clang_analyzer_numTimesReached(); // expected-warning {{10}}
+  }
+}
+
+void parm_by_ref_as_loop_counter(int &i) {
+  for (i = 0; i < 10; ++i) {
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -164,6 +164,11 @@
   if (VD->hasGlobalStorage())
 return true;
 
+  const bool isParm = isa(VD);
+  // Reference parameters are assumed as escaped variables.
+  if (isParm && VD->getType()->isReferenceType())
+return true;
+
   while (!N->pred_empty()) {
 // FIXME: getStmtForDiagnostics() does nasty things in order to provide
 // a valid statement for body farms, do we need this behavior here?
@@ -193,6 +198,11 @@
 
 N = N->getFirstPred();
   }
+
+  // Parameter declaration will not be found.
+  if (isParm)
+return false;
+
   llvm_unreachable("Reached root without finding the declaration of VD");
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80171: [analyzer] LoopUnrolling: fix crash when a parameter is a loop counter

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra marked 2 inline comments as done.
AbbasSabra added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:167
 
+  const bool isParm = VD->getKind() == Decl::ParmVar;
+  // Reference parameters are assumed as escaped variables.

vsavchenko wrote:
> `getKind` function is only an implementation detail for 
> `isa`/`cast`/`dan_cast` functions ([[ 
> https://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates
>  | docs ]]).  So, this condition would be better in the following form: 
> `isa(VD)`.
> 
> NOTE: One good motivation here is that //maybe// in the future there will be 
> some sort of new type of function parameters and it will be derived from 
> `ParmVarDecl`.  In this situation, direct comparison with the kind of node 
> will not work and probably won't be fixed by developers who introduced the 
> new node, but `isa` approach will stay correct.
Thanks for the detailed explanation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80171



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

[PATCH] D80171: [analyzer] LoopUnrolling: fix crash when a parameter is a loop counter

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 265095.
AbbasSabra added a comment.

update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80171

Files:
  clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  clang/test/Analysis/loop-unrolling.cpp


Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -499,3 +499,15 @@
 clang_analyzer_numTimesReached(); // expected-warning {{6}}
   }
 }
+
+void parm_by_value_as_loop_counter(int i) {
+  for (i = 0; i < 10; ++i) {
+clang_analyzer_numTimesReached(); // expected-warning {{10}}
+  }
+}
+
+void parm_by_ref_as_loop_counter(int &i) {
+  for (i = 0; i < 10; ++i) {
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -164,6 +164,11 @@
   if (VD->hasGlobalStorage())
 return true;
 
+  const bool isParm = VD->getKind() == Decl::ParmVar;
+  // Reference parameters are assumed as escaped variables.
+  if (isParm && VD->getType()->isReferenceType())
+return true;
+
   while (!N->pred_empty()) {
 // FIXME: getStmtForDiagnostics() does nasty things in order to provide
 // a valid statement for body farms, do we need this behavior here?
@@ -193,6 +198,11 @@
 
 N = N->getFirstPred();
   }
+
+  // Parameter declaration will not be found.
+  if (isParm)
+return false;
+
   llvm_unreachable("Reached root without finding the declaration of VD");
 }
 


Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -499,3 +499,15 @@
 clang_analyzer_numTimesReached(); // expected-warning {{6}}
   }
 }
+
+void parm_by_value_as_loop_counter(int i) {
+  for (i = 0; i < 10; ++i) {
+clang_analyzer_numTimesReached(); // expected-warning {{10}}
+  }
+}
+
+void parm_by_ref_as_loop_counter(int &i) {
+  for (i = 0; i < 10; ++i) {
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -164,6 +164,11 @@
   if (VD->hasGlobalStorage())
 return true;
 
+  const bool isParm = VD->getKind() == Decl::ParmVar;
+  // Reference parameters are assumed as escaped variables.
+  if (isParm && VD->getType()->isReferenceType())
+return true;
+
   while (!N->pred_empty()) {
 // FIXME: getStmtForDiagnostics() does nasty things in order to provide
 // a valid statement for body farms, do we need this behavior here?
@@ -193,6 +198,11 @@
 
 N = N->getFirstPred();
   }
+
+  // Parameter declaration will not be found.
+  if (isParm)
+return false;
+
   llvm_unreachable("Reached root without finding the declaration of VD");
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80171: [analyzer] LoopUnrolling: fix crash when a parameter is a loop counter

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra requested review of this revision.
AbbasSabra added a comment.

What you both said makes sense.  Now, reference parameters are escaped and 
value parameters are treated as the local variables.
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80171



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

[PATCH] D80171: [analyzer] LoopUnrolling: fix crash when a parameter is a loop counter

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 265090.
AbbasSabra added a comment.

Fix code review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80171

Files:
  clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  clang/test/Analysis/loop-unrolling.cpp


Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -500,8 +500,14 @@
   }
 }
 
-void arg_as_loop_counter(int i) {
+void parm_by_value_as_loop_counter(int i) {
   for (i = 0; i < 10; ++i) {
-(void)i;
+clang_analyzer_numTimesReached(); // expected-warning {{10}}
+  }
+}
+
+void parm_by_ref_as_loop_counter(int &i) {
+  for (i = 0; i < 10; ++i) {
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
   }
 }
Index: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -160,8 +160,13 @@
 }
 
 static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) {
-  // Global variables and parameters assumed as escaped variables.
-  if (VD->hasGlobalStorage() || VD->getKind() == Decl::ParmVar)
+  // Global variables assumed as escaped variables.
+  if (VD->hasGlobalStorage())
+return true;
+
+  const bool isParm = VD->getKind() == Decl::ParmVar;
+  // Reference parameters are assumed as escaped variables.
+  if (isParm && VD->getType()->isReferenceType())
 return true;
 
   while (!N->pred_empty()) {
@@ -193,6 +198,11 @@
 
 N = N->getFirstPred();
   }
+
+  // Parameter declaration will not be found.
+  if (isParm)
+return false;
+
   llvm_unreachable("Reached root without finding the declaration of VD");
 }
 


Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -500,8 +500,14 @@
   }
 }
 
-void arg_as_loop_counter(int i) {
+void parm_by_value_as_loop_counter(int i) {
   for (i = 0; i < 10; ++i) {
-(void)i;
+clang_analyzer_numTimesReached(); // expected-warning {{10}}
+  }
+}
+
+void parm_by_ref_as_loop_counter(int &i) {
+  for (i = 0; i < 10; ++i) {
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
   }
 }
Index: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -160,8 +160,13 @@
 }
 
 static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) {
-  // Global variables and parameters assumed as escaped variables.
-  if (VD->hasGlobalStorage() || VD->getKind() == Decl::ParmVar)
+  // Global variables assumed as escaped variables.
+  if (VD->hasGlobalStorage())
+return true;
+
+  const bool isParm = VD->getKind() == Decl::ParmVar;
+  // Reference parameters are assumed as escaped variables.
+  if (isParm && VD->getType()->isReferenceType())
 return true;
 
   while (!N->pred_empty()) {
@@ -193,6 +198,11 @@
 
 N = N->getFirstPred();
   }
+
+  // Parameter declaration will not be found.
+  if (isParm)
+return false;
+
   llvm_unreachable("Reached root without finding the declaration of VD");
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80171: [analyzer] LoopUnrolling: fix crash when a parameter is a loop counter

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra created this revision.
AbbasSabra added a reviewer: szepet.
Herald added subscribers: ASDenysPetrov, martong, Charusso, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, zzheng, rnkovacs, baloghadamsoftware, 
xazax.hun.
Herald added a project: clang.

When loop counter is a function parameter "isPossiblyEscaped" will not find the 
variable declaration "VD" which lead to "llvm_unreachable". Parameters should 
be escaped like global variables to avoid a crash.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80171

Files:
  clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  clang/test/Analysis/loop-unrolling.cpp


Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -499,3 +499,9 @@
 clang_analyzer_numTimesReached(); // expected-warning {{6}}
   }
 }
+
+void arg_as_loop_counter(int i) {
+  for (i = 0; i < 10; ++i) {
+(void)i;
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -160,8 +160,8 @@
 }
 
 static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) {
-  // Global variables assumed as escaped variables.
-  if (VD->hasGlobalStorage())
+  // Global variables and parameters assumed as escaped variables.
+  if (VD->hasGlobalStorage() || VD->getKind() == Decl::ParmVar)
 return true;
 
   while (!N->pred_empty()) {


Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -499,3 +499,9 @@
 clang_analyzer_numTimesReached(); // expected-warning {{6}}
   }
 }
+
+void arg_as_loop_counter(int i) {
+  for (i = 0; i < 10; ++i) {
+(void)i;
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -160,8 +160,8 @@
 }
 
 static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) {
-  // Global variables assumed as escaped variables.
-  if (VD->hasGlobalStorage())
+  // Global variables and parameters assumed as escaped variables.
+  if (VD->hasGlobalStorage() || VD->getKind() == Decl::ParmVar)
 return true;
 
   while (!N->pred_empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits