[SYCL] Diagnose uses of zero length arrays

Fznamznon · Fznamznon · commit 52e8f58d49e6 · 2021-12-29T15:30:18.000+03:00
Adds diagnosing on attempt to use zero length arrays, pointers, refs, arrays of them and structs/classes containing all of it. In case a struct/class with zero length array is used this emits a set of notes pointing out how zero length array got into used struct, like this: ``` struct ContainsArr { int A[0]; // note: field of illegal type declared here }; struct Wrapper { ContainsArr F; // note: within field of type ContainsArr declared here // ... } // Device code Wrapper W; W.use(); // error: zero-length arrays are not permitted ``` Total deep check of each used declaration may result in double diagnosing at the same location. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D114080
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5785,7 +5785,7 @@ def err_typecheck_invalid_restrict_invalid_pointee : Error<
 def ext_typecheck_zero_array_size : Extension<
   "zero size arrays are an extension">, InGroup<ZeroLengthArray>;
 def err_typecheck_zero_array_size : Error<
-  "zero-length arrays are not permitted in C++">;
+  "zero-length arrays are not permitted in %select{C++|SYCL device code}0">;
 def err_array_size_non_int : Error<"size of array has non-integer type %0">;
 def err_init_element_not_constant : Error<
   "initializer element is not a compile-time constant">;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
@@ -13142,6 +13142,9 @@ class Sema final {
   /// Adds Callee to DeviceCallGraph if we don't know if its caller will be
   /// codegen'ed yet.
   bool checkSYCLDeviceFunction(SourceLocation Loc, FunctionDecl *Callee);
+  void deepTypeCheckForSYCLDevice(SourceLocation UsedAt,
+                                  llvm::DenseSet<QualType> Visited,
+                                  ValueDecl *DeclToCheck);
 };
 
 /// RAII object that enters a new expression evaluation context.
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
@@ -1858,6 +1858,15 @@ void Sema::checkTypeSupport(QualType Ty, SourceLocation Loc, ValueDecl *D) {
   if (isUnevaluatedContext() || Ty.isNull())
     return;
 
+  // The original idea behind checkTypeSupport function is that unused
+  // declarations can be replaced with an array of bytes of the same size during
+  // codegen, such replacement doesn't seem to be possible for types without
+  // constant byte size like zero length arrays. So, do a deep check for SYCL.
+  if (D && LangOpts.SYCLIsDevice) {
+    llvm::DenseSet<QualType> Visited;
+    deepTypeCheckForSYCLDevice(Loc, Visited, D);
+  }
+
   Decl *C = cast<Decl>(getCurLexicalContext());
 
   // Memcpy operations for structs containing a member with unsupported type
diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp
@@ -48,3 +48,101 @@ bool Sema::checkSYCLDeviceFunction(SourceLocation Loc, FunctionDecl *Callee) {
   return DiagKind != SemaDiagnosticBuilder::K_Immediate &&
          DiagKind != SemaDiagnosticBuilder::K_ImmediateWithCallStack;
 }
+
+static bool isZeroSizedArray(Sema &SemaRef, QualType Ty) {
+  if (const auto *CAT = SemaRef.getASTContext().getAsConstantArrayType(Ty))
+    return CAT->getSize() == 0;
+  return false;
+}
+
+void Sema::deepTypeCheckForSYCLDevice(SourceLocation UsedAt,
+                                      llvm::DenseSet<QualType> Visited,
+                                      ValueDecl *DeclToCheck) {
+  assert(getLangOpts().SYCLIsDevice &&
+         "Should only be called during SYCL compilation");
+  // Emit notes only for the first discovered declaration of unsupported type
+  // to avoid mess of notes. This flag is to track that error already happened.
+  bool NeedToEmitNotes = true;
+
+  auto Check = [&](QualType TypeToCheck, const ValueDecl *D) {
+    bool ErrorFound = false;
+    if (isZeroSizedArray(*this, TypeToCheck)) {
+      SYCLDiagIfDeviceCode(UsedAt, diag::err_typecheck_zero_array_size) << 1;
+      ErrorFound = true;
+    }
+    // Checks for other types can also be done here.
+    if (ErrorFound) {
+      if (NeedToEmitNotes) {
+        if (auto *FD = dyn_cast<FieldDecl>(D))
+          SYCLDiagIfDeviceCode(FD->getLocation(),
+                               diag::note_illegal_field_declared_here)
+              << FD->getType()->isPointerType() << FD->getType();
+        else
+          SYCLDiagIfDeviceCode(D->getLocation(), diag::note_declared_at);
+      }
+    }
+
+    return ErrorFound;
+  };
+
+  // In case we have a Record used do the DFS for a bad field.
+  SmallVector<const ValueDecl *, 4> StackForRecursion;
+  StackForRecursion.push_back(DeclToCheck);
+
+  // While doing DFS save how we get there to emit a nice set of notes.
+  SmallVector<const FieldDecl *, 4> History;
+  History.push_back(nullptr);
+
+  do {
+    const ValueDecl *Next = StackForRecursion.pop_back_val();
+    if (!Next) {
+      assert(!History.empty());
+      // Found a marker, we have gone up a level.
+      History.pop_back();
+      continue;
+    }
+    QualType NextTy = Next->getType();
+
+    if (!Visited.insert(NextTy).second)
+      continue;
+
+    auto EmitHistory = [&]() {
+      // The first element is always nullptr.
+      for (uint64_t Index = 1; Index < History.size(); ++Index) {
+        SYCLDiagIfDeviceCode(History[Index]->getLocation(),
+                             diag::note_within_field_of_type)
+            << History[Index]->getType();
+      }
+    };
+
+    if (Check(NextTy, Next)) {
+      if (NeedToEmitNotes)
+        EmitHistory();
+      NeedToEmitNotes = false;
+    }
+
+    // In case pointer/array/reference type is met get pointee type, then
+    // proceed with that type.
+    while (NextTy->isAnyPointerType() || NextTy->isArrayType() ||
+           NextTy->isReferenceType()) {
+      if (NextTy->isArrayType())
+        NextTy = QualType{NextTy->getArrayElementTypeNoTypeQual(), 0};
+      else
+        NextTy = NextTy->getPointeeType();
+      if (Check(NextTy, Next)) {
+        if (NeedToEmitNotes)
+          EmitHistory();
+        NeedToEmitNotes = false;
+      }
+    }
+
+    if (const auto *RecDecl = NextTy->getAsRecordDecl()) {
+      if (auto *NextFD = dyn_cast<FieldDecl>(Next))
+        History.push_back(NextFD);
+      // When nullptr is discovered, this means we've gone back up a level, so
+      // the history should be cleaned.
+      StackForRecursion.push_back(nullptr);
+      llvm::copy(RecDecl->fields(), std::back_inserter(StackForRecursion));
+    }
+  } while (!StackForRecursion.empty());
+}
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
@@ -2515,7 +2515,7 @@ QualType Sema::BuildArrayType(QualType T, ArrayType::ArraySizeModifier ASM,
         Diag(ArraySize->getBeginLoc(),
              isSFINAEContext() ? diag::err_typecheck_zero_array_size
                                : diag::ext_typecheck_zero_array_size)
-            << ArraySize->getSourceRange();
+            << 0 << ArraySize->getSourceRange();
       }
 
       // Is the array too large?
diff --git a/clang/test/SemaSYCL/zero-length-arrays.cpp b/clang/test/SemaSYCL/zero-length-arrays.cpp
@@ -0,0 +1,125 @@
+// RUN: %clang_cc1 -fsycl-is-device -triple spir64 -fsyntax-only -verify %s
+//
+// This test checks if compiler reports compilation error on an attempt to use
+// a zero-length array inside device code.
+
+template <typename Name, typename Func>
+__attribute__((sycl_kernel)) void kernel(const Func &kernelFunc) {
+  // expected-note@+1 5{{called by 'kernel}}
+  kernelFunc(); // #KernelObjCall
+}
+
+typedef float ZEROARR[0];
+
+struct Wrapper {
+  int A;
+  int BadArray[0]; // expected-note 3{{field of illegal type 'int[0]' declared here}}
+};
+
+struct WrapperOfWrapper { // expected-error 2{{zero-length arrays are not permitted in SYCL device code}}
+  Wrapper F;              // expected-note 2{{within field of type 'Wrapper' declared here}}
+  ZEROARR *Ptr;           //expected-note 5{{field of illegal pointer type 'ZEROARR *' (aka 'float (*)[0]') declared here}}
+};
+
+template <unsigned Size> struct InnerTemplated {
+  double Array[Size]; // expected-note 8{{field of illegal type 'double[0]' declared here}}
+};
+
+template <unsigned Size, typename Ty> struct Templated {
+  unsigned A;
+  Ty Arr[Size];
+  InnerTemplated<Size> Array[Size + 1]; // expected-note 8{{within field of type 'InnerTemplated<0U>[1]' declared here}}
+};
+
+struct KernelSt {
+  int A;
+  int BadArray[0]; // expected-note {{field of illegal type 'int[0]' declared here}}
+  void operator()() const {}
+};
+
+WrapperOfWrapper offendingFoo() {
+  // expected-note@+1 {{called by 'offendingFoo'}}
+  return WrapperOfWrapper{};
+}
+
+template <unsigned Size>
+void templatedContext() {
+  Templated<Size, float> Var;
+  // expected-error@#KernelObjCall 2{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@#KernelObjCall {{called by 'kernel<TempContext, (lambda at}}
+  // expected-note@+1 {{in instantiation of function template specialization}}
+  kernel<class TempContext>([=] {
+    // expected-note@+1 {{within field of type 'Templated<0U, float>' declared here}}
+    (void)Var; // expected-error 2{{zero-length arrays are not permitted in SYCL device code}}
+  });
+  // expected-error@#KernelObjCall {{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+2 {{in instantiation of function template specialization}}
+  // expected-note@+1 {{within field of type 'Templated<0U, float>' declared here}}
+  kernel<class TempContext1>([Var] {
+  });
+}
+
+void foo(const unsigned X) {
+  int Arr[0];      // expected-note 2{{declared here}}
+  ZEROARR TypeDef; // expected-note {{declared here}}
+  ZEROARR *Ptr;    // expected-note {{declared here}}
+                   // expected-error@#KernelObjCall 3{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+1 {{in instantiation of function template specialization}}
+  kernel<class Simple>([=]() {
+    (void)Arr;     // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+    (void)TypeDef; // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+    // expected-note@+1 {{field of illegal pointer type 'ZEROARR *' (aka 'float (*)[0]') declared here}}
+    (void)Ptr; // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+  });
+  // expected-error@#KernelObjCall {{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+2 {{in instantiation of function template specialization}}
+  // expected-note@+1 {{field of illegal type 'int[0]' declared here}}
+  kernel<class Simple1>([Arr] { // expected-error {{zero-length arrays are not permitted in SYCL device code}}
+  });
+  WrapperOfWrapper St;
+  // expected-error@#KernelObjCall 2{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+1 {{in instantiation of function template specialization}}
+  kernel<class SimpleStruct>([=] {
+    // expected-note@+1 {{within field of type 'WrapperOfWrapper' declared here}}
+    (void)St.F.BadArray; // expected-error 4{{zero-length arrays are not permitted in SYCL device code}}
+  });
+  // expected-error@#KernelObjCall 2{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+2 {{in instantiation of function template specialization}}
+  // expected-note@+1 {{within field of type 'WrapperOfWrapper' declared here}}
+  kernel<class SimpleStruct1>([St] { // expected-error 2{{zero-length arrays are not permitted in SYCL device code}}
+  });
+
+  Templated<1, int> OK;
+  Templated<1 - 1, double> Weirdo;
+  Templated<0, float> Zero;
+  // expected-error@#KernelObjCall 4{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+1 {{in instantiation of function template specialization}}
+  kernel<class UseTemplated>([=] {
+    (void)OK;   // No errors expected
+    (void)Zero; // expected-error 2{{zero-length arrays are not permitted in SYCL device code}}
+    // expected-note@+1 {{within field of type 'Templated<1 - 1, double>' declared here}}
+    int A = Weirdo.A; // expected-error 2{{zero-length arrays are not permitted in SYCL device code}}
+  });
+
+  // expected-note@#KernelObjCall {{called by 'kernel<UseTemplated1, (lambda at}}
+  // expected-error@#KernelObjCall 2{{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+2 {{in instantiation of function template specialization}}
+  // expected-note@+1 {{within field of type 'Templated<0, float>' declared here}}
+  kernel<class UseTemplated1>([Zero] { // expected-error 2{{zero-length arrays are not permitted in SYCL device code}}
+  });
+
+  templatedContext<10>();
+  // expected-note@+1 2{{in instantiation of function template specialization}}
+  templatedContext<0>();
+
+  KernelSt K;
+  // expected-error@#KernelObjCall {{zero-length arrays are not permitted in SYCL device code}}
+  // expected-note@+1 {{in instantiation of function template specialization}}
+  kernel<class UseFunctor>(K);
+
+  // expected-note@#KernelObjCall {{called by 'kernel<ReturnFromFunc, (lambda at}}
+  kernel<class ReturnFromFunc>([=] {
+    // expected-note@+1 {{called by 'operator()'}}
+    offendingFoo();
+  });
+}

Original file line number	Diff line number	Diff line change
`@@ -2515,7 +2515,7 @@ QualType Sema::BuildArrayType(QualType T, ArrayType::ArraySizeModifier ASM,`
`2515`	`2515`	`Diag(ArraySize->getBeginLoc(),`
`2516`	`2516`	`isSFINAEContext() ? diag::err_typecheck_zero_array_size`
`2517`	`2517`	`: diag::ext_typecheck_zero_array_size)`
`2518`		`- << ArraySize->getSourceRange();`
	`2518`	`+ << 0 << ArraySize->getSourceRange();`
`2519`	`2519`	`}`
`2520`	`2520`
`2521`	`2521`	`// Is the array too large?`