Forbid non-const static variables in kernels.

Bug: 7688011
Change-Id: If03084a9ece76478e0ed0847b75f09c41b89a500

8 files changed, 87 insertions, 28 deletions

diff --git a/slang_rs_check_ast.cpp b/slang_rs_check_ast.cpp
index 118bda7..e9a49e6 100644
--- a/slang_rs_check_ast.cpp
+++ b/slang_rs_check_ast.cpp
@@ -18,6 +18,7 @@
 
 #include "slang_assert.h"
 #include "slang_rs.h"
+#include "slang_rs_export_foreach.h"
 #include "slang_rs_export_type.h"
 
 namespace slang {
@@ -40,37 +41,37 @@ void RSCheckAST::ValidateFunctionDecl(clang::FunctionDecl *FD) {
     return;
   }
 
-  if (!mIsFilterscript) {
-    // No additional validation for non-Filterscript functions.
-    if (clang::Stmt *Body = FD->getBody()) {
-      Visit(Body);
-    }
-    return;
-  }
-
-  size_t numParams = FD->getNumParams();
+  if (mIsFilterscript) {
+    // Validate parameters for Filterscript.
+    size_t numParams = FD->getNumParams();
 
-  clang::QualType resultType = FD->getResultType().getCanonicalType();
+    clang::QualType resultType = FD->getResultType().getCanonicalType();
 
-  // We use FD as our NamedDecl in the case of a bad return type.
-  if (!RSExportType::ValidateType(C, resultType, FD,
-                                  FD->getLocStart(), mTargetAPI,
-                                  mIsFilterscript)) {
-    mValid = false;
-  }
-
-  for (size_t i = 0; i < numParams; i++) {
-    clang::ParmVarDecl *PVD = FD->getParamDecl(i);
-    clang::QualType QT = PVD->getType().getCanonicalType();
-    if (!RSExportType::ValidateType(C, QT, PVD, PVD->getLocStart(),
-                                    mTargetAPI, mIsFilterscript)) {
+    // We use FD as our NamedDecl in the case of a bad return type.
+    if (!RSExportType::ValidateType(C, resultType, FD,
+                                    FD->getLocStart(), mTargetAPI,
+                                    mIsFilterscript)) {
       mValid = false;
     }
+
+    for (size_t i = 0; i < numParams; i++) {
+      clang::ParmVarDecl *PVD = FD->getParamDecl(i);
+      clang::QualType QT = PVD->getType().getCanonicalType();
+      if (!RSExportType::ValidateType(C, QT, PVD, PVD->getLocStart(),
+                                      mTargetAPI, mIsFilterscript)) {
+        mValid = false;
+      }
+    }
   }
 
+  bool saveKernel = mInKernel;
+  mInKernel = RSExportForEach::isRSForEachFunc(mTargetAPI, &mDiagEngine, FD);
+
   if (clang::Stmt *Body = FD->getBody()) {
     Visit(Body);
   }
+
+  mInKernel = saveKernel;
 }
 
 
@@ -79,15 +80,29 @@ void RSCheckAST::ValidateVarDecl(clang::VarDecl *VD) {
     return;
   }
 
-  const clang::Type *T = VD->getType().getTypePtr();
+  clang::QualType QT = VD->getType();
 
   if (VD->getLinkage() == clang::ExternalLinkage) {
     llvm::StringRef TypeName;
+    const clang::Type *T = QT.getTypePtr();
     if (!RSExportType::NormalizeType(T, TypeName, &mDiagEngine, VD)) {
       mValid = false;
     }
   }
 
+  // We don't allow static (non-const) variables within kernels.
+  if (mInKernel && VD->isStaticLocal()) {
+    if (!QT.isConstQualified()) {
+      mDiagEngine.Report(
+        clang::FullSourceLoc(VD->getLocation(), mSM),
+        mDiagEngine.getCustomDiagID(
+          clang::DiagnosticsEngine::Error,
+          "Non-const static variables are not allowed in kernels: '%0'"))
+          << VD->getName();
+      mValid = false;
+    }
+  }
+
   if (!RSExportType::ValidateVarDecl(VD, mTargetAPI, mIsFilterscript)) {
     mValid = false;
   } else if (clang::Expr *Init = VD->getInit()) {
diff --git a/slang_rs_check_ast.h b/slang_rs_check_ast.h
index 6a76360..855d22f 100644
--- a/slang_rs_check_ast.h
+++ b/slang_rs_check_ast.h
@@ -34,12 +34,14 @@ class RSCheckAST : public clang::StmtVisitor<RSCheckAST> {
   bool mValid;
   unsigned int mTargetAPI;
   bool mIsFilterscript;
+  bool mInKernel;
 
  public:
   explicit RSCheckAST(clang::ASTContext &Con, unsigned int TargetAPI,
                       bool IsFilterscript)
       : C(Con), mDiagEngine(Con.getDiagnostics()), mSM(C.getSourceManager()),
-        mValid(true), mTargetAPI(TargetAPI), mIsFilterscript(IsFilterscript) {
+        mValid(true), mTargetAPI(TargetAPI), mIsFilterscript(IsFilterscript),
+        mInKernel(false) {
     return;
   }
 
diff --git a/slang_rs_context.cpp b/slang_rs_context.cpp
index a400b12..caa3955 100644
--- a/slang_rs_context.cpp
+++ b/slang_rs_context.cpp
@@ -114,11 +114,12 @@ bool RSContext::processExportFunc(const clang::FunctionDecl *FD) {
     return false;
   }
 
+  clang::DiagnosticsEngine *DiagEngine = getDiagnostics();
   if (RSExportForEach::isSpecialRSFunc(mTargetAPI, FD)) {
     // Do not reflect specialized functions like init, dtor, or graphics root.
     return RSExportForEach::validateSpecialFuncDecl(mTargetAPI,
-                                                    getDiagnostics(), FD);
-  } else if (RSExportForEach::isRSForEachFunc(mTargetAPI, FD)) {
+                                                    DiagEngine, FD);
+  } else if (RSExportForEach::isRSForEachFunc(mTargetAPI, DiagEngine, FD)) {
     RSExportForEach *EFE = RSExportForEach::Create(this, FD);
     if (EFE == NULL)
       return false;
diff --git a/slang_rs_export_foreach.cpp b/slang_rs_export_foreach.cpp
index 5754b41..8462099 100644
--- a/slang_rs_export_foreach.cpp
+++ b/slang_rs_export_foreach.cpp
@@ -516,9 +516,26 @@ bool RSExportForEach::isGraphicsRootRSFunc(int targetAPI,
 }
 
 bool RSExportForEach::isRSForEachFunc(int targetAPI,
+    clang::DiagnosticsEngine *DiagEngine,
     const clang::FunctionDecl *FD) {
+  slangAssert(DiagEngine && FD);
+  bool hasKernelAttr = FD->hasAttr<clang::KernelAttr>();
+
+  if (FD->getStorageClass() == clang::SC_Static) {
+    if (hasKernelAttr) {
+      DiagEngine->Report(
+        clang::FullSourceLoc(FD->getLocation(),
+                             DiagEngine->getSourceManager()),
+        DiagEngine->getCustomDiagID(clang::DiagnosticsEngine::Error,
+                                    "Invalid use of attribute kernel with "
+                                    "static function declaration: %0"))
+        << FD->getName();
+    }
+    return false;
+  }
+
   // Anything tagged as a kernel is definitely used with ForEach.
-  if (FD->hasAttr<clang::KernelAttr>()) {
+  if (hasKernelAttr) {
     return true;
   }
 
diff --git a/slang_rs_export_foreach.h b/slang_rs_export_foreach.h
index 9bc185e..6cacc60 100644
--- a/slang_rs_export_foreach.h
+++ b/slang_rs_export_foreach.h
@@ -170,7 +170,9 @@ class RSExportForEach : public RSExportable {
   static bool isGraphicsRootRSFunc(int targetAPI,
                                    const clang::FunctionDecl *FD);
 
-  static bool isRSForEachFunc(int targetAPI, const clang::FunctionDecl *FD);
+  static bool isRSForEachFunc(int targetAPI, 
+                              clang::DiagnosticsEngine *DiagEngine,
+                              const clang::FunctionDecl *FD);
 
   inline static bool isSpecialRSFunc(int targetAPI,
                                      const clang::FunctionDecl *FD) {
diff --git a/tests/F_kernel_static/kernel_static.rs b/tests/F_kernel_static/kernel_static.rs
new file mode 100644
index 0000000..c6ea226
--- /dev/null
+++ b/tests/F_kernel_static/kernel_static.rs
@@ -0,0 +1,20 @@
+#pragma version(1)
+#pragma rs java_package_name(foo)
+
+static int gi;
+
+static void not_a_kernel(int i) {
+    static int j;
+    int k;
+    j = i;
+}
+
+int __attribute__((kernel)) root(uint32_t ain) {
+  static const int ci;
+  static int i;
+  return 0;
+}
+
+static int __attribute__((kernel)) static_kernel() {
+  return 0;
+}
diff --git a/tests/F_kernel_static/stderr.txt.expect b/tests/F_kernel_static/stderr.txt.expect
new file mode 100644
index 0000000..1cb7221
--- /dev/null
+++ b/tests/F_kernel_static/stderr.txt.expect
@@ -0,0 +1,2 @@
+kernel_static.rs:14:14: error: Non-const static variables are not allowed in kernels: 'i'
+kernel_static.rs:18:36: error: Invalid use of attribute kernel with static function declaration: static_kernel
diff --git a/tests/F_kernel_static/stdout.txt.expect b/tests/F_kernel_static/stdout.txt.expect
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tests/F_kernel_static/stdout.txt.expect