[analyzer] Engine: fix crash with SEH __leave keyword

[AbbasSabra]

Summary

https://docs.microsoft.com/en-us/cpp/cpp/try-except-statement?view=msvc-160

[steakhal]

Please, add these reviewers for your upcoming [analyzer] patches.
Inline a couple of nits.
Nice to see some fixes for visual c++ stuff.

clang/lib/StaticAnalyzer/Core/CoreEngine.cpp

352

You should probably extend the ExprEngine.cpp:1312 in a similar fashion.

clang/test/Analysis/misc-ms-leave.cpp

12

If it crashed previously at this statement, please put there a // no-crash comment.

16

I guess you could simply use the clang_analyzer_warnIfReached() here.
https://clang.llvm.org/docs/analyzer/developer-docs/DebugChecks.html

[AbbasSabra]

Updating D102280: [analyzer] Apply code review

[AbbasSabra]

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!

clang/lib/StaticAnalyzer/Core/CoreEngine.cpp

352

isn't it already handled in ExprEngine.cpp:1239?

[steakhal]

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

clang/lib/StaticAnalyzer/Core/CoreEngine.cpp

352

Good point.
At the other switch it states, that this leave statement is unsupported. I guess it will stay unsupported for a while because we don't model exceptions at all. Treat my previous comment as resolved.

clang/test/Analysis/misc-ms-leave.cpp

10

I'm curious to see if any statement after the __leave is executed. Could you place another warnIfReached there too?

Also, this file could contain all the ms try except constructs. That we might plan to support in the future.
So, a more generic file name would be more future-proof I guess.

[AbbasSabra]

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

[AbbasSabra]

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.

[steakhal]

In D102280#2759480, @AbbasSabra wrote:

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.

Well, and how can I build them on Linux xD
I tried mingw64, but failed :D

How do you use clang exactly?

[AbbasSabra]

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?

[steakhal]

In D102280#2760402, @AbbasSabra wrote:

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

It is a hard requirement. I'm gonna think more about this in the far future I guess. Thanks for the insight though.

[steakhal]

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

[AbbasSabra]

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.

clang/lib/StaticAnalyzer/Core/CoreEngine.cpp

switch (Term->getStmtClass()) {
       case Stmt::CXXForRangeStmtClass:
         HandleBranch(cast<CXXForRangeStmt>(Term)->getCond(), Term, B, Pred);
         return;
 
       case Stmt::ForStmtClass:
         HandleBranch(cast<ForStmt>(Term)->getCond(), Term, B, Pred);
         return;
 
+      case Stmt::SEHLeaveStmtClass:
       case Stmt::ContinueStmtClass:
       case Stmt::BreakStmtClass:
       case Stmt::GotoStmtClass:
         break;
 
       case Stmt::IfStmtClass:
         HandleBranch(cast<IfStmt>(Term)->getCond(), Term, B, Pred);
         return;

clang/test/Analysis/misc-ms-leave.cpp

+// 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')}}
+}