String literals with checked types and improved support for array literals. (#396)

This adds compiler support for using string literals in checked scopes.   In C, a string literal has the type "array of character", where there are several possible character types.  In Checked C, that corresponds to an "unchecked array of character".  To be able to use string in checked scopes, we need a way that they can be typed as a "checked null-terminated array of characters."

We support this in two ways.  First we alter the type of string literals that appears in checked scopes to have null-terminated array types.   Array-to-pointer conversion converts this to having an nt_array_ptr test, and at assignments nt_array_ptr can converted array_ptr..

Second,  we allow array literals (of which strings are a special case) to be converted implicitly to a checked pointer type when a checked pointer type is expected.  The checked pointer type should have the same pointer kind as the expected pointer type.    The implicit conversions can happen at assignments, function calls, and within initializers.    The language rules haven't been added to the Checked C specification yet.  The proposed rule is described[here](checkedc/checkedc#207).

Array literals specified using initializers or compound literal expressions already worked for the most part.   The C language rules use the type of the variable being initialized or the type of the compound literal expression as guides for filling in the type.   The clang machinery for filling in arrays worked for checked arrays too.   

This commit adds one important missing case: incomplete array types.   We were not propagating the checked enum when the complete array type was formed after the number of array elements had been determined.   It also checks in a checked scope that the compound literal type is a checked type.   This closes off one more route by which an unchecked type could appear in a checked scope.

This commit adds a new method for checking types that appear syntactically in expressions in checked scopes (for example, in cast operators and compound literal expressions).    This produces error message with similar detail to those produced for declarations, calling out the problematic unchecked type within a constructed type if necessary.    The existing error message for cast operations are switched to use this method, so we get better error message for cast expressions too.

Testing:
- Added new tests in the Checked C repo in initializers.c that cover strings, array initializers, and pointers initialized via  strings or compound literal expressions with array type.
- Passes existing automated testing for Linux and Windows.  I had to modify several LNT benchmarks because the improved typing discovered that we were passing strings to methods with bounds-safe interfaces.

Author: dtarditi

include/clang/Basic/DiagnosticSemaKinds.td

@@ -9332,29 +9332,30 @@ def err_bounds_type_annotation_lost_checking : Error<
     "|argument for parameter used in function return bounds expression"
     "|argument for parameter used in function parameter bounds expression}1">;
 
-  def err_checked_scope_type: Error<
+  def err_checked_scope_decl_type: Error<
     "%select{parameter|return|local variable|global variable|member}0 %select{|used }1in a checked scope must have "
     "%select{a |a pointer, array or function type that uses only |a bounds-safe interface type that uses only }2"
     "checked %plural{0:type %plural{2:|:or a bounds-safe interface}0|:types or parameter/return types with bounds-safe interfaces}2">;
 
+  def err_checked_scope_type: Error<
+    "type in a checked scope must %plural{0:be a checked pointer or array type"
+    "|:use only checked types or parameter/return types with bounds-safe interfaces}0">;
+
   def note_checked_scope_declaration : Note<
     "%select{parameter|return|local variable|global variable|member}0 declared here">;
 
   def note_checked_scope_problem_type : Note<
      "%0 is not allowed in a checked scope">;
 
-  def err_checked_scope_type_for_casting : Error<
-    "invalid casting to unchecked type %0 in a checked scope">;
-
   def err_checked_scope_no_prototype_func : Error<
     "function without a prototype cannot be used or declared in a checked scope">;
 
-  def err_checked_scope_no_variable_args : Error<
+  def err_checked_scope_decl_variable_args : Error<
     "%select{parameter|return|local variable|global variable|member}0 %select{|used }1in a checked scope "
     "cannot have variable arguments">;
 
-  def err_checked_scope_no_variable_args_for_casting : Error<
-    "invalid casting to type having variable arguments in a checked scope">;
+ def err_checked_scope_type_variable_args : Error<
+    "type in a checked scope cannot have variable arguments">;
 
   def err_checked_scope_no_variadic_func_for_declaration : Error<
     "variable arguments function cannot be made in a checked scope">;

include/clang/Sema/Sema.h

@@ -3831,6 +3831,8 @@ class Sema {
   bool DiagnoseCheckedDecl(const ValueDecl *D,
                            SourceLocation UseLoc = SourceLocation());
 
+  bool DiagnoseTypeInCheckedScope(QualType Ty, SourceLocation Start, SourceLocation End);
+
   /// \brief Warn if we're implicitly casting from a _Nullable pointer type to a
   /// _Nonnull one.
   void diagnoseNullableToNonnullConversion(QualType DstType, QualType SrcType,

lib/Sema/SemaBounds.cpp

@@ -690,8 +690,10 @@ namespace {
           return LValueBounds(ICE->getSubExpr());
          return CreateBoundsUnknown();
       }
-      // TODO: fill in these cases.
+      // TODO: these cases need CurrentExprValue to be implemented to express
+      // the bounds.
       case Expr::CompoundLiteralExprClass:
+      case Expr::StringLiteralClass:
         return CreateBoundsAllowedButNotComputed();
       default:
         return CreateBoundsUnknown();

lib/Sema/SemaCast.cpp

@@ -2574,24 +2574,15 @@ void CastOperation::CheckCStyleCast() {
   // Checked C - No C-style casts to unchecked pointer/array type or variadic
   // type in a checked block.
   if (Self.getCurScope()->isCheckedScope()) {
-    bool IsUnchecked = DestType->isOrContainsUncheckedType();
-    bool HasVariadicType = DestType->hasVariadicType();
-    bool ConstructsNullPointer = false;
-    if (IsUnchecked)
-      if (DestType->isVoidPointerType() &&
-          !SrcExpr.isInvalid()) {
-        const IntegerLiteral *Lit = dyn_cast<IntegerLiteral>(SrcExpr.get());
-        if (Lit && !Lit->getValue())
-         ConstructsNullPointer = true;
-      }
-    if ((IsUnchecked && !ConstructsNullPointer) || HasVariadicType) {
-      if (IsUnchecked) {
-        Self.Diag(OpRange.getBegin(), diag::err_checked_scope_type_for_casting)
-          << DestType;
-      } else {
-        Self.Diag(OpRange.getBegin(),
-                  diag::err_checked_scope_no_variable_args_for_casting);
-      }
+    bool isNullPointerConstant =
+      DestType->isVoidPointerType() &&
+      DestType->isUncheckedPointerType() &&
+      !SrcExpr.isInvalid() &&
+      SrcExpr.get()->isNullPointerConstant(Self.Context,
+                                           Expr::NPC_NeverValueDependent);
+    if (!isNullPointerConstant &&
+        !Self.DiagnoseTypeInCheckedScope(DestType, OpRange.getBegin(),
+                                         OpRange.getEnd())) {
       SrcExpr = ExprError();
       return;
     }
@@ -2644,21 +2635,13 @@ void CastOperation::CheckBoundsCast(tok::TokenKind kind) {
   // Checked C - No C-style casts to unchecked pointer/array type or variadic
   // type in a checked block.
   if (Self.getCurScope()->isCheckedScope()) {
-    bool IsUnchecked = DestType->isOrContainsUncheckedType();
-    bool HasVariadicType = DestType->hasVariadicType();
     if (Kind == CK_AssumePtrBounds) {
       Self.Diag(OpRange.getBegin(),
                 diag::err_checked_scope_no_assume_bounds_casting);
       SrcExpr = ExprError();
       return;
-    } else if (IsUnchecked || HasVariadicType) {
-      if (IsUnchecked) {
-        Self.Diag(OpRange.getBegin(), diag::err_checked_scope_type_for_casting)
-          << DestType;
-      } else {
-        Self.Diag(OpRange.getBegin(),
-                  diag::err_checked_scope_no_variable_args_for_casting);
-      }
+    } else if (!Self.DiagnoseTypeInCheckedScope(DestType, OpRange.getBegin(),
+                                                OpRange.getEnd())) {
       SrcExpr = ExprError();
       return;
     }

lib/Sema/SemaChecking.cpp

@@ -11588,7 +11588,8 @@ bool Sema::AllowedInCheckedScope(QualType Ty, const BoundsExpr *Bounds,
         return false;
       }
     }
-    return AllowedInCheckedScope(Ty->getPointeeType(), nullptr, false, Loc,
+    QualType ReferentType = QualType(Ty->getPointeeOrArrayElementType(), 0);
+    return AllowedInCheckedScope(ReferentType, nullptr, false, Loc,
                                  ProblemLoc, ProblemTy);
   } else if (const FunctionProtoType *fpt = Ty->getAs<FunctionProtoType>()) {
     const BoundsExpr *ReturnBounds = fpt->getReturnBounds();
@@ -11661,7 +11662,7 @@ bool Sema::DiagnoseCheckedDecl(const ValueDecl *Decl, SourceLocation UseLoc) {
   if (!AllowedInCheckedScope(Ty, TargetDecl->getBoundsExpr(),
                              isa<ParmVarDecl>(TargetDecl), CSTL_TopLevel,
                              ProblemLoc, ProblemTy)) {
-    Diag(Loc, diag::err_checked_scope_type) << DeclKind << IsUse
+    Diag(Loc, diag::err_checked_scope_decl_type) << DeclKind << IsUse
       << ProblemLoc;
     if (IsUse) {
       Diag(TargetDecl->getLocStart(), diag::note_checked_scope_declaration)
@@ -11685,14 +11686,37 @@ bool Sema::DiagnoseCheckedDecl(const ValueDecl *Decl, SourceLocation UseLoc) {
   }
 
   if (Ty->hasVariadicType()) {
-    Diag(Loc, diag::err_checked_scope_no_variable_args) << DeclKind
+    Diag(Loc, diag::err_checked_scope_decl_variable_args) << DeclKind
       << IsUse;
     Result = false;
   }
 
   return Result;
 }
 
+bool Sema::DiagnoseTypeInCheckedScope(QualType Ty, SourceLocation StartLoc,
+                                      SourceLocation EndLoc) {
+  CheckedScopeTypeLocation ProblemLoc = CSTL_TopLevel;
+  QualType ProblemTy = Ty;
+  if (!AllowedInCheckedScope(Ty, nullptr, false, CSTL_TopLevel,
+                             ProblemLoc, ProblemTy)) {
+    Diag(StartLoc, diag::err_checked_scope_type) << ProblemLoc
+      << SourceRange(StartLoc, EndLoc);
+
+    // Print a note about the problem type if it might not be obvious.
+    if (ProblemLoc != CSTL_TopLevel || !DisplaysAsArrayOrPointer(ProblemTy))
+      Diag(StartLoc, diag::note_checked_scope_problem_type) << ProblemTy;
+    return false;
+  }
+
+  if (Ty->hasVariadicType()) {
+    Diag(StartLoc, diag::err_checked_scope_type_variable_args) <<
+      SourceRange(StartLoc, EndLoc);
+    return false;
+  }
+
+  return true;
+}
 
 //===--- Layout compatibility ----------------------------------------------//
 

lib/Sema/SemaExpr.cpp

@@ -1731,9 +1731,13 @@ Sema::ActOnStringLiteral(ArrayRef<Token> StringToks, Scope *UDLScope) {
   // Get an array type for the string, according to C99 6.4.5.  This includes
   // the nul terminator character as well as the string length for pascal
   // strings.
+  CheckedArrayKind ArrayKind = getCurScope()->isCheckedScope() ?
+    CheckedArrayKind::NtChecked : CheckedArrayKind::Unchecked;
+
   QualType StrTy = Context.getConstantArrayType(CharTyConst,
-                                 llvm::APInt(32, Literal.GetNumStringChars()+1),
-                                 ArrayType::Normal, 0);
+                                                llvm::APInt(32, Literal.GetNumStringChars() + 1),
+                                                ArrayType::Normal, 0,
+                                                ArrayKind);
 
   // OpenCL v1.1 s6.5.3: a string literal is in the constant address space.
   if (getLangOpts().OpenCL) {
@@ -5795,6 +5799,14 @@ Sema::BuildCompoundLiteralExpr(SourceLocation LParenLoc, TypeSourceInfo *TInfo,
                SourceRange(LParenLoc, LiteralExpr->getSourceRange().getEnd())))
     return ExprError();
 
+  if (getCurScope()->isCheckedScope()) {
+    if (literalType->isArrayType())
+      literalType = MakeCheckedArrayType(literalType);
+    if (!DiagnoseTypeInCheckedScope(literalType, LParenLoc, RParenLoc))
+      return ExprError();
+    literalType = RewriteBoundsSafeInterfaceTypes(literalType);
+  }
+
   InitializedEntity Entity
     = InitializedEntity::InitializeCompoundLiteralInit(TInfo);
   InitializationKind Kind
@@ -8213,6 +8225,50 @@ Sema::CheckTransparentUnionArgumentConstraints(QualType ArgType,
   return Compatible;
 }
 
+// If the LHS type is a checked pointer type and the RHS is an array literal,
+// retype the RHS to have the same kind of checked pointer.   We assume
+// array-to-pointer converison has already been done.
+static bool arrayConstantCheckedConversion(Sema &S, QualType LHSType,
+                                           ExprResult &RHS) {
+  // Recognize the syntax for the constant.
+  ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(RHS.get());
+  if (!ICE)
+    return false;
+
+  if (ICE->getCastKind() != CK_ArrayToPointerDecay)
+    return false;
+
+  Expr *Child = ICE->getSubExpr()->IgnoreParens();
+  if (!isa<InitListExpr>(Child) && !isa<StringLiteral>(Child) &&
+      !isa<CompoundLiteralExpr>(Child))
+    return false;
+
+  // See if the constant needs to be retyped.
+  QualType RHSType = RHS.get()->getType();
+  const PointerType *RHSPointerType = dyn_cast<PointerType>(RHSType);
+  const PointerType *LHSPointerType = dyn_cast<PointerType>(LHSType);
+
+  if (!LHSType->isCheckedPointerType() || !RHSPointerType ||
+      LHSPointerType->getKind() == RHSPointerType->getKind())
+    return false;
+
+  QualType RHSPointee = RHSPointerType->getPointeeType();
+
+  // Retype the constant.
+
+  // For checked null-terminated pointers, only retype the constant if the
+  // type would be a valid null-termianted ponter type.
+  if (LHSType->isCheckedPointerNtArrayType() && !RHSPointee->isIntegerType() &&
+      !RHSPointee->isPointerType())
+    return false;
+
+  RHSType = S.Context.getPointerType(RHSPointee, LHSPointerType->getKind());
+  RHS = S.ImpCastExprToType(RHS.get(), RHSType, CK_ArrayToPointerDecay,
+                            clang::VK_RValue, nullptr,
+                            Sema::CCK_ImplicitConversion, true);
+  return true;
+}
+
 Sema::AssignConvertType
 Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &CallerRHS,
                                        bool Diagnose,
@@ -8228,7 +8284,6 @@ Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &CallerRHS,
   // to put the updated value.
   ExprResult LocalRHS = CallerRHS;
   ExprResult &RHS = ConvertRHS ? CallerRHS : LocalRHS;
-
   if (getLangOpts().CPlusPlus) {
     if (!LHSType->isRecordType() && !LHSType->isAtomicType()) {
       // C++ 5.17p3: If the left operand is not of class type, the
@@ -8305,6 +8360,14 @@ Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &CallerRHS,
       return Incompatible;
   }
 
+  // Checked C: implicitly convert array constants to checked pointer types
+  // when the LHS is a checked pointer type, This is done after array-to-pointer
+  // conversion to avoid duplicating the type conversion logic there.
+  if (arrayConstantCheckedConversion(*this, LHSType, RHS)) {
+    if (RHS.isInvalid())
+       return Incompatible;
+  }
+
   Expr *PRE = RHS.get()->IgnoreParenCasts();
   if (Diagnose && isa<ObjCProtocolExpr>(PRE)) {
     ObjCProtocolDecl *PDecl = cast<ObjCProtocolExpr>(PRE)->getProtocol();

lib/Sema/SemaInit.cpp

@@ -160,7 +160,8 @@ static void CheckStringInit(Expr *Str, QualType &DeclT, const ArrayType *AT,
     // Return a new array type (C99 6.7.8p22).
     DeclT = S.Context.getConstantArrayType(IAT->getElementType(),
                                            ConstVal,
-                                           ArrayType::Normal, 0);
+                                           ArrayType::Normal, 0,
+                                           IAT->getKind());
     updateStringLiteralType(Str, DeclT);
     return;
   }
@@ -1698,7 +1699,8 @@ void InitListChecker::CheckArrayType(const InitializedEntity &Entity,
     }
 
     DeclType = SemaRef.Context.getConstantArrayType(elementType, maxElements,
-                                                     ArrayType::Normal, 0);
+                                                     ArrayType::Normal, 0,
+                                                    arrayType->getKind());
   }
   if (!hadError && VerifyOnly) {
     // If there are any members of the array that get value-initialized, check