[flang-commits] [flang] [flang] Avoid passing null pointers to nonnull parameters (PR #84785)

[Krzysztof Parzyszek via flang-commits]

https://github.com/kparzysz created https://github.com/llvm/llvm-project/pull/84785

Certain functions in glibc have "nonnull" attributes on pointer parameters (even in cases where passing a null pointer should be handled correctly). There are a few cases of such calls in flang: memcmp and memcpy with the length parameter set to 0.

Avoid passing a null pointer to these functions, since the conflict with the nonnull attribute could cause an undefined behavior.

This was detected by the undefined behavior sanitizer.

>From 747dddc97a7f521211891e23d8ed196a4a9c9df3 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 11 Mar 2024 10:15:56 -0500
Subject: [PATCH] [flang] Avoid passing null pointers to nonnull parameters

Certain functions in glibc have "nonnull" attributes on pointer parameters
(even in cases where passing a null pointer should be handled correctly).
There are a few cases of such calls in flang: memcmp and memcpy with the
length parameter set to 0.

Avoid passing a null pointer to these functions, since the conflict with
the nonnull attribute could cause an undefined behavior.

This was detected by the undefined behavior sanitizer.
---
 flang/include/flang/Parser/char-block.h |  7 +++++++
 flang/runtime/buffer.h                  | 11 ++++++++---
 flang/runtime/temporary-stack.cpp       |  9 +++++++--
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/flang/include/flang/Parser/char-block.h b/flang/include/flang/Parser/char-block.h
index fc5de2607b51b9..acd8aee98bf8db 100644
--- a/flang/include/flang/Parser/char-block.h
+++ b/flang/include/flang/Parser/char-block.h
@@ -129,6 +129,13 @@ class CharBlock {
 
 private:
   int Compare(const CharBlock &that) const {
+    // "memcmp" in glibc has "nonnull" attributes on the input pointers.
+    // Avoid passing null pointers, since it would result in an undefined
+    // behavior.
+    if (size() == 0)
+      return that.size() == 0 ? 0 : -1;
+    if (that.size() == 0)
+      return 1;
     std::size_t bytes{std::min(size(), that.size())};
     int cmp{std::memcmp(static_cast<const void *>(begin()),
         static_cast<const void *>(that.begin()), bytes)};
diff --git a/flang/runtime/buffer.h b/flang/runtime/buffer.h
index a77a5a5dda5c78..93fda36f500d31 100644
--- a/flang/runtime/buffer.h
+++ b/flang/runtime/buffer.h
@@ -148,10 +148,15 @@ template <typename STORE, std::size_t minBuffer = 65536> class FileFrame {
       buffer_ =
           reinterpret_cast<char *>(AllocateMemoryOrCrash(terminator, size_));
       auto chunk{std::min<std::int64_t>(length_, oldSize - start_)};
-      std::memcpy(buffer_, old + start_, chunk);
+      // "memcpy" in glibc has a "nonnull" attribute on the source pointer.
+      // Avoid passing a null pointer, since it would result in an undefined
+      // behavior.
+      if (old != nullptr) {
+        std::memcpy(buffer_, old + start_, chunk);
+        std::memcpy(buffer_ + chunk, old, length_ - chunk);
+        FreeMemory(old);
+      }
       start_ = 0;
-      std::memcpy(buffer_ + chunk, old, length_ - chunk);
-      FreeMemory(old);
     }
   }
 
diff --git a/flang/runtime/temporary-stack.cpp b/flang/runtime/temporary-stack.cpp
index b4d7c6064457f2..667b10e04dbd29 100644
--- a/flang/runtime/temporary-stack.cpp
+++ b/flang/runtime/temporary-stack.cpp
@@ -93,8 +93,13 @@ void DescriptorStorage<COPY_VALUES>::resize(size_type newCapacity) {
   }
   Descriptor **newData =
       static_cast<Descriptor **>(AllocateMemoryOrCrash(terminator_, bytes));
-  memcpy(newData, data_, capacity_ * sizeof(Descriptor *));
-  FreeMemory(data_);
+  // "memcpy" in glibc has a "nonnull" attribute on the source pointer.
+  // Avoid passing a null pointer, since it would result in an undefined
+  // behavior.
+  if (data_ != nullptr) {
+    memcpy(newData, data_, capacity_ * sizeof(Descriptor *));
+    FreeMemory(data_);
+  }
   data_ = newData;
   capacity_ = newCapacity;
 }