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

[via flang-commits]

Author: Krzysztof Parzyszek
Date: 2024-03-12T07:53:57-05:00
New Revision: 871086bf7fad42d610bfe02224662bdc71494a70

URL: https://github.com/llvm/llvm-project/commit/871086bf7fad42d610bfe02224662bdc71494a70
DIFF: https://github.com/llvm/llvm-project/commit/871086bf7fad42d610bfe02224662bdc71494a70.diff

LOG: [flang] Avoid passing null pointers to nonnull parameters (#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.

Added: 
    

Modified: 
    flang/include/flang/Parser/char-block.h
    flang/runtime/buffer.h
    flang/runtime/temporary-stack.cpp

Removed: 
    


################################################################################
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;
 }