[analyzer] Evaluate PredefinedExpressions

[steakhal]

Summary

We did not evaluate such expressions, just returned unknown for such cases.
After this patch, we will be able to access a unique value identifying a template instantiation.

[Szelethus]

LGTM! Nice!

clang/test/Analysis/eval-predefined-exprs.cpp

1

Isn't it -std=c++17?

7–21

Why not put the expected warning right below the function call?

[vsavchenko]

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

311

I'm not super familiar with predefined expressions, but it looks like it this commit doesn't cover all kinds of those. And for some this can return nullptr.

Can you, please, add tests for other kinds as well?

[steakhal]

• Added tests for Microsoft extensions.
• Added an assert requiring the PredefinedExpression to have a function name.

---

I don't know how could a PredefinedExpression lack the function name, probably @riccibruno or @rjmccall can help with this - according to D53605.

[steakhal]

clang/test/Analysis/eval-predefined-exprs.cpp

1

Thanks, typo.

7–21

That would seriously violate the column limit.
This way it is easier to see and validate the pattern IMO.

[martong]

clang/test/Analysis/eval-predefined-exprs.cpp

7–21

This could be like:

clang_analyzer_dump(__FUNCDNAME__); // \
// expected-warning@-4 {{&Element{"??$func@UClass@?1??foo@@YAXXZ@$0CK@D@@YAXD@Z",0 S64b,char}}}
clang_analyzer_dump(L__FUNCTION__); // \
// expected-warning@-4 {{&Element{L"func",0 S64b,wchar_t}}}
...

So, this way you can keep the line limit, I think this is what @Szelethus refers to.

[martong]

clang/test/Analysis/eval-predefined-exprs.cpp

7–21

This could be like: ...

There is no need for the @-4 of course with that approach.

[riccibruno]

I don't know how could a PredefinedExpression lack the function name, probably @riccibruno or @rjmccall can help with this - according to D53605.

A PredefinedExpr whose declaration context is dependent has no name (see Sema::BuildPredefinedExpr). Example:

void f0() { __func__; }
-PredefinedExpr 0x6a76ba8 <col:13> 'const char [3]' lvalue __func__
`-StringLiteral 0x6a76b88 <col:13> 'const char [3]' lvalue "f0"

template <typename> void f1() { __func__; }
`-PredefinedExpr 0x6a76e78 <col:35> '<dependent type>' lvalue __func__

template void f1<int>();
-PredefinedExpr 0x6a77060 <col:35> 'const char [3]' lvalue __func__
`-StringLiteral 0x6a77040 <col:35> 'const char [3]' lvalue "f1"

clang/test/Analysis/eval-predefined-exprs.cpp

7–21

There is no column limit in test/.

[steakhal]

We only analyze instantiated functions, which are not dependently typed.
Safe to assume that every encountered PredefinedExpression has a defined (non-null) function name.

Just to be sure I added a clang_analyzer_warnIfReached in a test-case.

clang/test/Analysis/eval-predefined-exprs.cpp

7–21

Do you think it would be more readable that way? I'm still not convinced.

[vsavchenko]

Awesome!
Can we cover __builtin_unique_stable_name as well?

[vsavchenko]

clang/test/Analysis/eval-predefined-exprs.cpp

102

Also can you please add // no-warning as a matter of conventions?

[riccibruno]

clang/test/Analysis/eval-predefined-exprs.cpp

7–21

I have no opinion on whether it is more readable. I was just saying that test/ is an exception to the column limit policy, and indeed that *many* tests have lines longer that the limit.

[steakhal]

• Added no-warning.
• Added test-case for __builtin_unique_stable_name as well.

[steakhal]

Thank you all for the comments!

clang/test/Analysis/eval-predefined-exprs.cpp

102

Sure, thanks.

[vsavchenko]

Thanks!
Great job!

clang/lib/StaticAnalyzer/Core/Environment.cpp

SVal Environment::getSVal(const EnvironmentEntry &Entry,
   case Stmt::ImplicitValueInitExprClass:
   case Stmt::IntegerLiteralClass:
   case Stmt::ObjCBoolLiteralExprClass:
   case Stmt::CXXNullPtrLiteralExprClass:
   case Stmt::ObjCStringLiteralClass:
   case Stmt::StringLiteralClass:
   case Stmt::TypeTraitExprClass:
   case Stmt::SizeOfPackExprClass:
+  case Stmt::PredefinedExprClass:
     // Known constants; defer to SValBuilder.
     return svalBuilder.getConstantVal(cast<Expr>(S)).getValue();
 
   case Stmt::ReturnStmtClass: {
     const auto *RS = cast<ReturnStmt>(S);
     if (const Expr *RE = RS->getRetValue())
       return getSVal(EnvironmentEntry(RE, LCtx), svalBuilder);
     return UndefinedVal();

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

case Stmt::ObjCStringLiteralClass: {
     return makeLoc(getRegionManager().getObjCStringRegion(SL));
   }
 
   case Stmt::StringLiteralClass: {
     const auto *SL = cast<StringLiteral>(E);
     return makeLoc(getRegionManager().getStringRegion(SL));
   }
 
+  case Stmt::PredefinedExprClass: {
+    const auto *PE = cast<PredefinedExpr>(E);
+    return makeLoc(getRegionManager().getStringRegion(PE->getFunctionName()));
+  }
+
   // Fast-path some expressions to avoid the overhead of going through the AST's
   // constant evaluator
   case Stmt::CharacterLiteralClass: {
     const auto *C = cast<CharacterLiteral>(E);
     return makeIntVal(C->getValue(), C->getType());
   }
 
   case Stmt::CXXBoolLiteralExprClass:

clang/test/Analysis/eval-predefined-exprs.cpp

+// RUN: %clang_analyze_cc1 --std=c++17 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_dump(const char *);
+
+template <typename T, auto Value, typename U>
+void func(U param) {
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void func(U) [T = Class, Value = 42, U = char]",0 S64b,char}}}
+}
+
+void foo() {
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"foo",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"foo",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void foo()",0 S64b,char}}}
+
+  func<struct Class, 42ull>('b'); // instantiate template
+}