[clang] [Clang][Sema] Add fortify warnings for fread, fwrite, and fgets (PR #204337)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Radovan Božić (bozicrHT)

<details>
<summary>Changes</summary>

Add missing `-Wforitfy-source` diagnostics for `fread`, `fwrite`, and `fgets`.
This continues the work from #<!-- -->142230.

---
Full diff: https://github.com/llvm/llvm-project/pull/204337.diff


4 Files Affected:

- (modified) clang/include/clang/Basic/Builtins.td (+5) 
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+5) 
- (modified) clang/lib/Sema/SemaChecking.cpp (+37) 
- (modified) clang/test/Sema/warn-fortify-source.c (+14) 


``````````diff
diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td
index 053a257ba6d4a..81f1d1011911e 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -3510,6 +3510,11 @@ def Fwrite : LibBuiltin<"stdio.h"> {
   let Prototype = "size_t(void const*, size_t, size_t, FILE*)";
 }
 
+def Fgets : LibBuiltin<"stdio.h"> {
+  let Spellings = ["fgets"];
+  let Prototype = "char*(char*, int, FILE*)";
+}
+
 // C99 ctype.h
 
 def IsAlNum : LibBuiltin<"ctype.h"> {
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f84cd8dca6d4c..6daa810488242 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1002,6 +1002,11 @@ def warn_fortify_scanf_overflow : Warning<
   "%2, but the corresponding specifier may require size %3">,
   InGroup<FortifySource>;
 
+def warn_fortify_source_overread : Warning<
+  "'%0' will always read past the source buffer; source buffer has "
+  "size %1, but size argument is %2">,
+  InGroup<FortifySource>;
+
 def err_function_start_invalid_type: Error<
   "argument must be a function">;
 
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index b8a3f48a32f24..c57733034b10b 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1232,6 +1232,25 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
     return std::nullopt;
   };
 
+  auto ComputeExplicitObjectSizeArgumentProduct =
+      [&](unsigned LIndex, unsigned RIndex) -> std::optional<llvm::APSInt> {
+    auto L = ComputeExplicitObjectSizeArgument(LIndex);
+    auto R = ComputeExplicitObjectSizeArgument(RIndex);
+    if (!L || !R)
+      return std::nullopt;
+
+    unsigned W =
+        2 * std::max({L->getBitWidth(), R->getBitWidth(), SizeTypeWidth});
+
+    llvm::APSInt LE = L->extOrTrunc(W);
+    llvm::APSInt RE = R->extOrTrunc(W);
+
+    LE.setIsUnsigned(true);
+    RE.setIsUnsigned(true);
+
+    return LE * RE;
+  };
+
   auto ComputeStrLenArgument =
       [&](unsigned Index) -> std::optional<llvm::APSInt> {
     std::optional<unsigned> IndexOptional = TranslateIndex(Index);
@@ -1433,6 +1452,24 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
     DestinationSize = ComputeSizeArgument(1);
     break;
   }
+  case Builtin::BIfread: {
+    DiagID = diag::warn_fortify_source_overflow;
+    SourceSize = ComputeExplicitObjectSizeArgumentProduct(1, 2);
+    DestinationSize = ComputeSizeArgument(0);
+    break;
+  }
+  case Builtin::BIfwrite: {
+    DiagID = diag::warn_fortify_source_overread;
+    SourceSize = ComputeExplicitObjectSizeArgumentProduct(1, 2);
+    DestinationSize = ComputeSizeArgument(0);
+    break;
+  }
+  case Builtin::BIfgets: {
+    DiagID = diag::warn_fortify_source_size_mismatch;
+    SourceSize = ComputeExplicitObjectSizeArgument(1);
+    DestinationSize = ComputeSizeArgument(0);
+    break;
+  }
   case Builtin::BIsnprintf:
   case Builtin::BI__builtin_snprintf:
   case Builtin::BIvsnprintf:
diff --git a/clang/test/Sema/warn-fortify-source.c b/clang/test/Sema/warn-fortify-source.c
index d0b519a516545..89f1f25773514 100644
--- a/clang/test/Sema/warn-fortify-source.c
+++ b/clang/test/Sema/warn-fortify-source.c
@@ -9,6 +9,8 @@
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS -fexperimental-new-constant-interpreter
 
 typedef unsigned long size_t;
+typedef struct _IO_FILE FILE;
+
 
 #ifdef __cplusplus
 extern "C" {
@@ -23,6 +25,10 @@ void *memcpy(void *dst, const void *src, size_t c);
 #endif
 void bcopy(const void *src, void *dst, size_t n);
 void bzero(void *dst, size_t n);
+size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream);
+size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream);
+char *fgets(char *s, int size, FILE *stream);
+
 
 #ifdef __cplusplus
 }
@@ -116,6 +122,14 @@ void call_bcopy_bzero(void) {
   __builtin_bzero(dst, 11); // expected-warning {{'bzero' will always overflow; destination buffer has size 10, but size argument is 11}}
 }
 
+void call_fread_fwrite_fgets(FILE *fp) {
+  char src[4];
+  fread(src, 2, 3, fp); // expected-warning {{'fread' will always overflow; destination buffer has size 4, but size argument is 6}}
+  fwrite(src, 2, 3, fp); // expected-warning {{'fwrite' will always read past the source buffer; source buffer has size 4, but size argument is 6}}
+  fgets(src, 5, fp); // expected-warning {{'fgets' size argument is too large; destination buffer has size 4, but size argument is 5}}
+
+}
+
 void call_snprintf(double d, int n) {
   char buf[10];
   __builtin_snprintf(buf, 10, "merp");

``````````

</details>


https://github.com/llvm/llvm-project/pull/204337