[Sema] Improve a -Warray-bounds diagnostic

[erik.pilkington]

Summary

Fix a bug where we would compare array sizes with incompatible element types, and look through explicit casts.

rdar://44800168

Thanks for taking a look!

[aaron.ballman]

LGTM with a testing request and some small nits.

clang/include/clang/AST/ASTContext.h

2092 ↗

(On Diff #182960)

Can you add a dependent type test as well?

clang/test/Sema/static-array.c

25 ↗

(On Diff #182960)

Add an extra space after expected-warning and the { for the second warning.

29 ↗

(On Diff #182960)

Same here.

[erik.pilkington]

clang/include/clang/AST/ASTContext.h

2092 ↗

(On Diff #182960)

I don't think its possible to get a dependent type here, since this feature is only supported in C mode.

[aaron.ballman]

clang/include/clang/AST/ASTContext.h

2092 ↗

(On Diff #182960)

That makes sense; I couldn't figure out a way to trigger it either. It seems sensible to leave it in should we ever need this for C++, though.

cfe/trunk/include/clang/AST/ASTContext.h

public:
   /// Convert a size in characters to a size in bits.
   int64_t toBits(CharUnits CharSize) const;
 
   /// Return the size of the specified (complete) type \p T, in
   /// characters.
   CharUnits getTypeSizeInChars(QualType T) const;
   CharUnits getTypeSizeInChars(const Type *T) const;
 
+  Optional<CharUnits> getTypeSizeInCharsIfKnown(QualType Ty) const {
+    if (Ty->isIncompleteType() || Ty->isDependentType())
+      return None;
+    return getTypeSizeInChars(Ty);
+  }
+
+  Optional<CharUnits> getTypeSizeInCharsIfKnown(const Type *Ty) const {
+    return getTypeSizeInCharsIfKnown(QualType(Ty, 0));
+  }
+
   /// Return the ABI-specified alignment of a (complete) type \p T, in
   /// bits.
   unsigned getTypeAlign(QualType T) const { return getTypeInfo(T).Align; }
   unsigned getTypeAlign(const Type *T) const { return getTypeInfo(T).Align; }
 
   /// Return the ABI-specified natural alignment of a (complete) type \p T,
   /// before alignment adjustments, in bits.
   ///

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def warn_format_zero_positional_specifier : Warning<
   InGroup<Format>;
 def warn_format_invalid_positional_specifier : Warning<
   "invalid position specified for %select{field width|field precision}0">,
   InGroup<Format>;
 def warn_format_mix_positional_nonpositional_args : Warning<
   "cannot mix positional and non-positional arguments in format string">,
   InGroup<Format>;
 def warn_static_array_too_small : Warning<
-  "array argument is too small; contains %0 elements, callee requires at least %1">,
+  "array argument is too small; %select{contains %0 elements|is of size %0}2,"
+  " callee requires at least %1">,
   InGroup<ArrayBounds>;
 def note_callee_static_array : Note<
   "callee declares array parameter as static here">;
 def warn_empty_format_string : Warning<
   "format string is empty">, InGroup<FormatZeroLength>;
 def warn_format_string_is_wide_literal : Warning<
   "format string should not be a wide string">, InGroup<Format>;
 def warn_printf_format_string_contains_null_char : Warning<

cfe/trunk/lib/Sema/SemaExpr.cpp

if (ArgExpr->isNullPointerConstant(Context,
     return;
   }
 
   const ConstantArrayType *CAT = dyn_cast<ConstantArrayType>(AT);
   if (!CAT)
     return;
 
   const ConstantArrayType *ArgCAT =
-    Context.getAsConstantArrayType(ArgExpr->IgnoreParenImpCasts()->getType());
+    Context.getAsConstantArrayType(ArgExpr->IgnoreParenCasts()->getType());
   if (!ArgCAT)
     return;
 
+  if (getASTContext().hasSameUnqualifiedType(CAT->getElementType(),
+                                             ArgCAT->getElementType())) {
     if (ArgCAT->getSize().ult(CAT->getSize())) {
       Diag(CallLoc, diag::warn_static_array_too_small)
           << ArgExpr->getSourceRange()
-      << (unsigned) ArgCAT->getSize().getZExtValue()
-      << (unsigned) CAT->getSize().getZExtValue();
+          << (unsigned)ArgCAT->getSize().getZExtValue()
+          << (unsigned)CAT->getSize().getZExtValue() << 0;
+      DiagnoseCalleeStaticArrayParam(*this, Param);
+    }
+    return;
+  }
+
+  Optional<CharUnits> ArgSize =
+      getASTContext().getTypeSizeInCharsIfKnown(ArgCAT);
+  Optional<CharUnits> ParmSize = getASTContext().getTypeSizeInCharsIfKnown(CAT);
+  if (ArgSize && ParmSize && *ArgSize < *ParmSize) {
+    Diag(CallLoc, diag::warn_static_array_too_small)
+        << ArgExpr->getSourceRange() << (unsigned)ArgSize->getQuantity()
+        << (unsigned)ParmSize->getQuantity() << 1;
     DiagnoseCalleeStaticArrayParam(*this, Param);
   }
 }
 
 /// Given a function expression of unknown-any type, try to rebuild it
 /// to have a function type.
 static ExprResult rebuildUnknownAnyFunction(Sema &S, Expr *fn);
 

cfe/trunk/test/Sema/static-array.c

-// RUN: %clang_cc1 -fsyntax-only -fblocks -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsyntax-only -fblocks -verify %s
 
 void cat0(int a[static 0]) {} // expected-warning {{'static' has no effect on zero-length arrays}}
 
-void cat(int a[static 3]) {} // expected-note 2 {{callee declares array parameter as static here}}
+void cat(int a[static 3]) {} // expected-note 4 {{callee declares array parameter as static here}} expected-note 2 {{passing argument to parameter 'a' here}}
 
 void vat(int i, int a[static i]) {} // expected-note {{callee declares array parameter as static here}}
 
 void f(int *p) {
   int a[2], b[3], c[4];
 
   cat0(0);
 
   cat(0); // expected-warning {{null passed to a callee that requires a non-null argument}}
   cat(a); // expected-warning {{array argument is too small; contains 2 elements, callee requires at least 3}}
   cat(b);
   cat(c);
   cat(p);
 
   vat(1, 0); // expected-warning {{null passed to a callee that requires a non-null argument}}
   vat(3, b);
+
+  char d[4];
+  cat((int *)d); // expected-warning {{array argument is too small; is of size 4, callee requires at least 12}}
+  cat(d); // expected-warning {{array argument is too small; is of size 4, callee requires at least 12}} expected-warning {{incompatible pointer types}}
+
+  char e[12];
+  cat((int *)e);
+  cat(e); // expected-warning {{incompatible pointer types}}
 }
 
 
 typedef int td[static 3]; // expected-error {{'static' used in array declarator outside of function prototype}}
 typedef void(*fp)(int[static 42]); // no-warning
 
 void g(void) {
   int a[static 42]; // expected-error {{'static' used in array declarator outside of function prototype}}