[analyzer] Fix StdLibraryFunctionsChecker crash on macOS

[vsavchenko]

Summary

EOF macro token coming from a PCH file on macOS while marked as literal,
doesn't contain any literal data. This causes crash on every project
using PCHs.

This commit doesn't resolve the problem with PCH (maybe it was
designed like this for a purpose) or with tryExpandAsInteger, but
rather simply shoots off a crash itself.

[NoQ]

Thanks!

@martong, thoughts on this? :)

Also i guess in this test we're unable to obtain the value for EOF, what would be a good fixme-test to demonstrate that?

clang/test/Analysis/pch_crash.cpp

2

Just curious - did you double-check that only macos triples are affected? Maybe we should add a pair of run-lines without the triple, so that to automatically get coverage from buildbots on all platforms on which we have buildbots. Because the test should obviously pass on all platforms.

4

Can we enable core as well? Because we keep telling everybody that running without core is unsupported, so we tend to enable it on tests in order to demonstrate that.

22

Let's add sone comment like // no-crash so that to indicate what is it that we're testing for.

[NoQ]

clang/test/Analysis/pch_crash.cpp

22

*some

[vsavchenko]

Fix review comments

[vsavchenko]

clang/test/Analysis/pch_crash.cpp

2

No, I didn't check that :(. I just know that this problem is not reproduced in docker (aka Ubuntu).

[martong]

In D81916#2095106, @NoQ wrote:

Thanks!

@martong, thoughts on this? :)

Also i guess in this test we're unable to obtain the value for EOF, what would be a good fixme-test to demonstrate that?

I accept this as a very quick fix, but this seems to be an error rather in ASTReader and ASTWriter. And this could affect other modules related logic in macOS, not just the CSA. I wouldn't be surprised if LLDB develeopers were the next to struggle with the same ASTReader/Writer issue.

clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp

131–133

Could you please add a FIXME above the line with the real reasons: "EOF macro token coming from a PCH file on macOS while marked as literal,
doesn't contain any literal data."

[vsavchenko]

clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp

131–133

Sure

[vsavchenko]

Add FIXME comment

clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp

llvm::Optional<int> tryExpandAsInteger(StringRef Macro,
   std::vector<Token> FilteredTokens;
   FilteredTokens.reserve(MI->tokens().size());
   for (auto &T : MI->tokens())
     if (!T.isOneOf(tok::l_paren, tok::r_paren))
       FilteredTokens.push_back(T);
 
   // Parse an integer at the end of the macro definition.
   const Token &T = FilteredTokens.back();
-  if (!T.isLiteral())
+  // FIXME: EOF macro token coming from a PCH file on macOS while marked as
+  //        literal, doesn't contain any literal data
+  if (!T.isLiteral() || !T.getLiteralData())
     return llvm::None;
   StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
   llvm::APInt IntValue;
   constexpr unsigned AutoSenseRadix = 0;
   if (ValueStr.getAsInteger(AutoSenseRadix, IntValue))
     return llvm::None;
 
   // Parse an optional minus sign.

clang/test/Analysis/pch_crash.cpp

+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.15.0 -emit-pch -o %t %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-macosx10.15.0 -include-pch %t \
+// RUN:   -analyzer-checker=core,apiModeling -verify %s
+//
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_analyze_cc1 -include-pch %t \
+// RUN:   -analyzer-checker=core,apiModeling -verify %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+// Pre-compiled header
+
+int foo();
+
+// Literal data for this macro value will be null
+#define EOF -1
+
+#else
+// Source file
+
+int test() {
+  // we need a function call here to initiate erroneous routine
+  return foo(); // no-crash
+}
+
+#endif