[Sema] Fix a crash when nonnull checking

[hliao]

Summary

• Non-null checking is triggered during prototype substitution from a template instantiation, if expressions in decltype contains nullptr chcking. Skip non-null checking in that case as the protype is not finalized yet. Also, the nullptr checking in decltype is only used for type inspection instead of codegen. Ignoring that is harmless.

[tra]

@rsmith, @jlebar I'm out of my depth here and could use some language lawyering help.

[jlebar]

I uh... I also think this is an @rsmith question, I have no idea.

[hliao]

just explain what's the issue is and hope that help reviewers to get this fix quick.

• In Sema::DiagnoseAlwaysNonNullPointer, we issue warnings if a nonnull parameter is used in null checking. It need the function declaration to check parameter attributes. That usually works but fails if that function decl is not ready yet.
• In template instantiation, we first create the function prototype followed by instantiating the function body. When the function prototype is being formed, we may create binary or other expressions for semantic checking. But, in that phase, i.e. Sema::SubstFunctionDeclType, we don't have a fully specialized function prototype yet to check the parameter number and run into the assertion @ line 11596.

Hope that help to get the picture of this fix. Thanks.

[Rakete1111]

Otherwise LGTM.

clang/test/SemaTemplate/decltype.cpp

1

test/SemaCXX/nonnull.cpp would be a better place to put this test.

2

This is redundant :)

4

If you move the test as above you can drop this line.

[tra]

clang/test/SemaTemplate/decltype.cpp

1

test/SemaCXX/nonnull.cpp tests attribute((nonnull)). Placing this test there would be... odd.

The test could use a more descriptive name, though. Just decltype is way too generic. early-func-template-decltype? Or PR-somthing-something, if there's a bug for this crasher.

[hliao]

just search bugzilla and, fortunately, found this issue is reported 2+ years ago @ https://bugs.llvm.org/show_bug.cgi?id=30559. I will revise the test case to PR30559 and move it into test/SemaCXX/nonnull.cpp

clang/lib/Sema/SemaChecking.cpp

if (const auto* PV = dyn_cast<ParmVarDecl>(D)) {
     if (getCurFunction() &&
         !getCurFunction()->ModifiedNonNullParams.count(PV)) {
       if (const Attr *A = PV->getAttr<NonNullAttr>()) {
         ComplainAboutNonnullParamOrCall(A);
         return;
       }
 
       if (const auto *FD = dyn_cast<FunctionDecl>(PV->getDeclContext())) {
+        // Skip function template not specialized yet.
+        if (FD->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate)
+          return;
         auto ParamIter = llvm::find(FD->parameters(), PV);
         assert(ParamIter != FD->param_end());
         unsigned ParamNo = std::distance(FD->param_begin(), ParamIter);
 
         for (const auto *NonNull : FD->specific_attrs<NonNullAttr>()) {
           if (!NonNull->args_size()) {
               ComplainAboutNonnullParamOrCall(NonNull);
               return;

clang/test/SemaTemplate/decltype.cpp

+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// no crash & no diag
+
+// expected-no-diagnostics
+template <typename T>
+auto foo(T x) -> decltype((x == nullptr), *x) {
+  return *x;
+}
+
+void bar() {
+  foo(new int);
+}