[PATCH] D144136: Add a "remark" to report on array accesses

[Kees Cook via Phabricator via cfe-commits]

kees added a comment.

This gets me all 6 reports. The details about the array and the index don't really matter for the basic metrics:

  diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/Diagnostic
  SemaKinds.td
  index ba831c026342..29d2167b504b 100644
  --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
  +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
  @@ -9451,7 +9451,7 @@ def note_array_declared_here : Note<                                       
     "array %0 declared here">;
                                                    
   def remark_array_access : Remark<
  -  "accessing %select{fixed|dynamic}0 sized array %1 by %2">,
  +  "accessing %select{fixed|unknown|dynamic}0 sized array %1 by %2">,
     InGroup<ArrayBoundsRemarks>;                                                                     
                                                    
   def warn_inconsistent_array_form : Warning<
  diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
  index 9ced29a5f5d0..1c6aa7f05c7f 100644   
  --- a/clang/lib/Sema/SemaChecking.cpp                                                               
  +++ b/clang/lib/Sema/SemaChecking.cpp
  @@ -16207,8 +16207,26 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,   
       return;                                      
                                                    
     Expr::EvalResult Result;
  -  if (!IndexExpr->EvaluateAsInt(Result, Context, Expr::SE_AllowSideEffects))
  +  if (!IndexExpr->EvaluateAsInt(Result, Context, Expr::SE_AllowSideEffects)) {
  +    SmallString<128> sizeString;
  +    llvm::raw_svector_ostream OS(sizeString);
  +
  +    OS << "'";
  +    IndexExpr->printPretty(OS, nullptr, getPrintingPolicy());
  +    OS << "'";
  +
  +    if (!IsUnboundedArray) {
  +      Context.getDiagnostics().Report(
  +          BaseExpr->getBeginLoc(), diag::remark_array_access)
  +              << 0 << ArrayTy->desugar() << OS.str();
  +    } else {
  +      Context.getDiagnostics().Report(
  +          BaseExpr->getBeginLoc(), diag::remark_array_access)
  +              << 1 << "something" << OS.str();
  +    }
  +
       return;
  +  }
    
     llvm::APSInt index = Result.Val.getInt();
     if (IndexNegated) {
  @@ -16219,6 +16237,11 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
     if (IsUnboundedArray) {
       if (EffectiveType->isFunctionType())
         return;
  +
  +    Context.getDiagnostics().Report(
  +        BaseExpr->getBeginLoc(), diag::remark_array_access)
  +            << 1 << "something" << "whatever";
  +
       if (index.isUnsigned() || !index.isNegative()) {
         const auto &ASTC = getASTContext();
         unsigned AddrBits = ASTC.getTargetInfo().getPointerWidth(

Using "desugar" on a flexible array appears to blow up. :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144136/new/

https://reviews.llvm.org/D144136