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

[Nick Desaulniers via cfe-commits]

Radovan =?utf-8?q?Božić?= <radovan.bozic at htecgroup.com>,
Radovan =?utf-8?q?Božić?= <radovan.bozic at htecgroup.com>,
Radovan =?utf-8?q?Božić?= <radovan.bozic at htecgroup.com>,
Radovan =?utf-8?q?Božić?= <radovan.bozic at htecgroup.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/204337 at github.com>


================
@@ -1174,18 +1174,32 @@ class FortifiedBufferChecker {
     return NewIndex;
   }
 
-  std::optional<llvm::APSInt>
-  ComputeExplicitObjectSizeArgument(unsigned Index) {
+  /// Evaluate the argument at Index as an integer constant while preserving
+  /// its signedness, or return std::nullopt if it cannot be evaluated.
+  std::optional<llvm::APSInt> EvaluateIntegerArgument(unsigned Index) {
     std::optional<unsigned> IndexOptional = TranslateIndex(Index);
     if (!IndexOptional)
       return std::nullopt;
     unsigned NewIndex = *IndexOptional;
+
+    if (NewIndex >= TheCall->getNumArgs())
+      return std::nullopt;
----------------
nickdesaulniers wrote:

I think this check should be hoisted into `TranslateIndex`:

```diff
       std::optional<unsigned> TranslateIndex(unsigned Index) {
         // If we refer to a diagnose_as_builtin attribute, we need to change the
         // argument index to refer to the arguments of the called function. Unless
         // the index is out of bounds, which presumably means it's a variadic
         // function.
    -    if (!DABAttr)
    -      return Index;
    -    unsigned DABIndices = DABAttr->argIndices_size();
    -    unsigned NewIndex = Index < DABIndices
    -                            ? DABAttr->argIndices_begin()[Index]
    -                            : Index - DABIndices + FD->getNumParams();
    +    unsigned NewIndex = Index;
    +    if (DABAttr) {
    +      unsigned DABIndices = DABAttr->argIndices_size();
    +      NewIndex = Index < DABIndices
    +                     ? DABAttr->argIndices_begin()[Index]
    +                     : Index - DABIndices + FD->getNumParams();
    +    }
```
```diff
       std::optional<llvm::APSInt> EvaluateIntegerArgument(unsigned Index) {
         std::optional<unsigned> IndexOptional = TranslateIndex(Index);
         if (!IndexOptional)
           return std::nullopt;
    -    unsigned NewIndex = *IndexOptional;
    -
    -    if (NewIndex >= TheCall->getNumArgs())
    -      return std::nullopt;
    -
         Expr::EvalResult Result;
    -    Expr *Arg = TheCall->getArg(NewIndex);
    +    Expr *Arg = TheCall->getArg(*IndexOptional);
```
(and similar simplification in `ComputeSizeArgument`).

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