remove use of variable length arrays

bazel/envoy_build_system.bzl

@@ -13,6 +13,7 @@ def envoy_copts(repository, test = False):
         "-Wnon-virtual-dtor",
         "-Woverloaded-virtual",
         "-Wold-style-cast",
+        "-Wvla",
         "-std=c++14",
     ]
 

include/envoy/common/platform.h

@@ -2,16 +2,11 @@
 
 // NOLINT(namespace-envoy)
 #if !defined(_MSC_VER)
-#define STACK_ALLOC_ARRAY(var, type, num) type var[num]
-
 #define PACKED_STRUCT(definition, ...) definition, ##__VA_ARGS__ __attribute__((packed))
 
 #else
 #include <malloc.h>
 
-#define STACK_ALLOC_ARRAY(var, type, num)                                                          \
-  type* var = static_cast<type*>(::_alloca(sizeof(type) * num))
-
 #define PACKED_STRUCT(definition, ...)                                                             \
   __pragma(pack(push, 1)) definition, ##__VA_ARGS__;                                               \
   __pragma(pack(pop))

source/common/buffer/BUILD

@@ -26,6 +26,7 @@ envoy_cc_library(
         "//include/envoy/buffer:buffer_interface",
         "//source/common/api:os_sys_calls_lib",
         "//source/common/common:non_copyable",
+        "//source/common/common:stack_array",
         "//source/common/event:libevent_lib",
     ],
 )

source/common/buffer/buffer_impl.cc

@@ -3,10 +3,9 @@
 #include <cstdint>
 #include <string>
 
-#include "envoy/common/platform.h"
-
 #include "common/api/os_sys_calls_impl.h"
 #include "common/common/assert.h"
+#include "common/common/stack_array.h"
 
 #include "event2/buffer.h"
 
@@ -36,10 +35,9 @@ void OwnedImpl::add(const std::string& data) {
 
 void OwnedImpl::add(const Instance& data) {
   uint64_t num_slices = data.getRawSlices(nullptr, 0);
-  STACK_ALLOC_ARRAY(slices, RawSlice, num_slices);
-  data.getRawSlices(slices, num_slices);
-  for (uint64_t i = 0; i < num_slices; i++) {
-    RawSlice& slice = slices[i];
+  STACK_ARRAY(slices, RawSlice, num_slices);
+  data.getRawSlices(slices.begin(), num_slices);
+  for (const RawSlice& slice : slices) {
     add(slice.mem_, slice.len_);
   }
 }
@@ -116,7 +114,7 @@ Api::SysCallIntResult OwnedImpl::read(int fd, uint64_t max_length) {
   constexpr uint64_t MaxSlices = 2;
   RawSlice slices[MaxSlices];
   const uint64_t num_slices = reserve(max_length, slices, MaxSlices);
-  STACK_ALLOC_ARRAY(iov, iovec, num_slices);
+  STACK_ARRAY(iov, iovec, num_slices);
   uint64_t num_slices_to_read = 0;
   uint64_t num_bytes_to_read = 0;
   for (; num_slices_to_read < num_slices && num_bytes_to_read < max_length; num_slices_to_read++) {
@@ -130,7 +128,7 @@ Api::SysCallIntResult OwnedImpl::read(int fd, uint64_t max_length) {
   ASSERT(num_bytes_to_read <= max_length);
   auto& os_syscalls = Api::OsSysCallsSingleton::get();
   const Api::SysCallSizeResult result =
-      os_syscalls.readv(fd, iov, static_cast<int>(num_slices_to_read));
+      os_syscalls.readv(fd, iov.begin(), static_cast<int>(num_slices_to_read));
   if (result.rc_ < 0) {
     return {static_cast<int>(result.rc_), result.errno_};
   }
@@ -171,7 +169,7 @@ Api::SysCallIntResult OwnedImpl::write(int fd) {
   constexpr uint64_t MaxSlices = 16;
   RawSlice slices[MaxSlices];
   const uint64_t num_slices = std::min(getRawSlices(slices, MaxSlices), MaxSlices);
-  STACK_ALLOC_ARRAY(iov, iovec, num_slices);
+  STACK_ARRAY(iov, iovec, num_slices);
   uint64_t num_slices_to_write = 0;
   for (uint64_t i = 0; i < num_slices; i++) {
     if (slices[i].mem_ != nullptr && slices[i].len_ != 0) {
@@ -184,7 +182,7 @@ Api::SysCallIntResult OwnedImpl::write(int fd) {
     return {0, 0};
   }
   auto& os_syscalls = Api::OsSysCallsSingleton::get();
-  const Api::SysCallSizeResult result = os_syscalls.writev(fd, iov, num_slices_to_write);
+  const Api::SysCallSizeResult result = os_syscalls.writev(fd, iov.begin(), num_slices_to_write);
   if (result.rc_ > 0) {
     drain(static_cast<uint64_t>(result.rc_));
   }
@@ -201,17 +199,15 @@ OwnedImpl::OwnedImpl(const void* data, uint64_t size) : OwnedImpl() { add(data,
 
 std::string OwnedImpl::toString() const {
   uint64_t num_slices = getRawSlices(nullptr, 0);
-  STACK_ALLOC_ARRAY(slices, RawSlice, num_slices);
-  getRawSlices(slices, num_slices);
+  STACK_ARRAY(slices, RawSlice, num_slices);
+  getRawSlices(slices.begin(), num_slices);
   size_t len = 0;
-  for (uint64_t i = 0; i < num_slices; i++) {
-    RawSlice& slice = slices[i];
+  for (const RawSlice& slice : slices) {
     len += slice.len_;
   }
   std::string output;
   output.reserve(len);
-  for (uint64_t i = 0; i < num_slices; i++) {
-    RawSlice& slice = slices[i];
+  for (const RawSlice& slice : slices) {
     output.append(static_cast<const char*>(slice.mem_), slice.len_);
   }
 

source/common/common/BUILD

@@ -33,6 +33,7 @@ envoy_cc_library(
     hdrs = ["base64.h"],
     deps = [
         ":empty_string",
+        ":stack_array",
         "//include/envoy/buffer:buffer_interface",
     ],
 )
@@ -150,6 +151,14 @@ envoy_cc_library(
     hdrs = ["stl_helpers.h"],
 )
 
+envoy_cc_library(
+    name = "stack_array",
+    hdrs = ["stack_array.h"],
+    deps = [
+        ":assert_lib",
+    ],
+)
+
 envoy_cc_library(
     name = "thread_annotations",
     hdrs = ["thread_annotations.h"],

source/common/common/base64.cc

@@ -3,9 +3,8 @@
 #include <cstdint>
 #include <string>
 
-#include "envoy/common/platform.h"
-
 #include "common/common/empty_string.h"
+#include "common/common/stack_array.h"
 
 namespace Envoy {
 namespace {
@@ -180,13 +179,12 @@ std::string Base64::encode(const Buffer::Instance& buffer, uint64_t length) {
   ret.reserve(output_length);
 
   uint64_t num_slices = buffer.getRawSlices(nullptr, 0);
-  STACK_ALLOC_ARRAY(slices, Buffer::RawSlice, num_slices);
-  buffer.getRawSlices(slices, num_slices);
+  STACK_ARRAY(slices, Buffer::RawSlice, num_slices);
+  buffer.getRawSlices(slices.begin(), num_slices);
 
   uint64_t j = 0;
   uint8_t next_c = 0;
-  for (uint64_t i = 0; i < num_slices; i++) {
-    Buffer::RawSlice& slice = slices[i];
+  for (const Buffer::RawSlice& slice : slices) {
     const uint8_t* slice_mem = static_cast<const uint8_t*>(slice.mem_);
 
     for (uint64_t i = 0; i < slice.len_ && j < length; ++i, ++j) {

source/common/common/stack_array.h

@@ -0,0 +1,56 @@
+#pragma once
+
+#if !defined(WIN32)
+#include <alloca.h>
+
+#else
+#include <malloc.h>
+#endif
+
+#include <stddef.h>
+
+#include "common/common/assert.h"
+
+namespace Envoy {
+
+// This macro is intended to be used as a replacement for variable-length arrays.
+// Note that the StackArray wrapper object will be destructed and each element's
+// destructor will be called when it leaves scope. However, the memory containing
+// the array won't be deallocated until the function containing the macro returns.
+// We can't call alloca in the StackArray constructor since the memory would
+// be freed when the constructor returns.
+#define STACK_ARRAY(var, type, num) StackArray<type> var(::alloca(sizeof(type) * num), num)
+
+template <typename T> class StackArray {
+public:
+  StackArray(void* buf, size_t num_items) {
+    ASSERT(buf != nullptr, "StackArray received null pointer");
+    begin_ = static_cast<T*>(buf);
+    end_ = static_cast<T*>(buf) + num_items;
+    for (T& ref : *this) {
+      new (&ref) T;
+    }
+  }
+
+  ~StackArray() {
+    for (T& ref : *this) {
+      ref.~T();
+    }
+  }
+
+  T* begin() { return begin_; }
+
+  T* end() { return end_; }
+
+  T& operator[](size_t idx) { return begin_[idx]; }
+
+  void* operator new(size_t) = delete;
+
+  void* operator new[](size_t) = delete;
+
+private:
+  T* begin_;
+  T* end_;
+};
+
+} // namespace Envoy

source/common/compressor/BUILD

@@ -17,5 +17,6 @@ envoy_cc_library(
         "//include/envoy/compressor:compressor_interface",
         "//source/common/buffer:buffer_lib",
         "//source/common/common:assert_lib",
+        "//source/common/common:stack_array",
     ],
 )

source/common/compressor/zlib_compressor_impl.cc

@@ -1,9 +1,9 @@
 #include "common/compressor/zlib_compressor_impl.h"
 
 #include "envoy/common/exception.h"
-#include "envoy/common/platform.h"
 
 #include "common/common/assert.h"
+#include "common/common/stack_array.h"
 
 namespace Envoy {
 namespace Compressor {
@@ -36,11 +36,10 @@ uint64_t ZlibCompressorImpl::checksum() { return zstream_ptr_->adler; }
 
 void ZlibCompressorImpl::compress(Buffer::Instance& buffer, State state) {
   const uint64_t num_slices = buffer.getRawSlices(nullptr, 0);
-  STACK_ALLOC_ARRAY(slices, Buffer::RawSlice, num_slices);
-  buffer.getRawSlices(slices, num_slices);
+  STACK_ARRAY(slices, Buffer::RawSlice, num_slices);
+  buffer.getRawSlices(slices.begin(), num_slices);
 
-  for (uint64_t i = 0; i < num_slices; i++) {
-    const Buffer::RawSlice& input_slice = slices[i];
+  for (const Buffer::RawSlice& input_slice : slices) {
     zstream_ptr_->avail_in = input_slice.len_;
     zstream_ptr_->next_in = static_cast<Bytef*>(input_slice.mem_);
     // Z_NO_FLUSH tells the compressor to take the data in and compresses it as much as possible

source/common/decompressor/zlib_decompressor_impl.cc

@@ -1,9 +1,9 @@
 #include "common/decompressor/zlib_decompressor_impl.h"
 
 #include "envoy/common/exception.h"
-#include "envoy/common/platform.h"
 
 #include "common/common/assert.h"
+#include "common/common/stack_array.h"
 
 namespace Envoy {
 namespace Decompressor {
@@ -35,11 +35,10 @@ uint64_t ZlibDecompressorImpl::checksum() { return zstream_ptr_->adler; }
 void ZlibDecompressorImpl::decompress(const Buffer::Instance& input_buffer,
                                       Buffer::Instance& output_buffer) {
   const uint64_t num_slices = input_buffer.getRawSlices(nullptr, 0);
-  STACK_ALLOC_ARRAY(slices, Buffer::RawSlice, num_slices);
-  input_buffer.getRawSlices(slices, num_slices);
+  STACK_ARRAY(slices, Buffer::RawSlice, num_slices);
+  input_buffer.getRawSlices(slices.begin(), num_slices);
 
-  for (uint64_t i = 0; i < num_slices; i++) {
-    Buffer::RawSlice& input_slice = slices[i];
+  for (const Buffer::RawSlice& input_slice : slices) {
     zstream_ptr_->avail_in = input_slice.len_;
     zstream_ptr_->next_in = static_cast<Bytef*>(input_slice.mem_);
     while (inflateNext()) {

source/common/filesystem/filesystem_impl.cc

@@ -11,14 +11,14 @@
 #include <string>
 
 #include "envoy/common/exception.h"
-#include "envoy/common/platform.h"
 #include "envoy/common/time.h"
 #include "envoy/event/dispatcher.h"
 
 #include "common/api/os_sys_calls_impl.h"
 #include "common/common/assert.h"
 #include "common/common/fmt.h"
 #include "common/common/lock_guard.h"
+#include "common/common/stack_array.h"
 #include "common/common/thread.h"
 
 #include "absl/strings/match.h"
@@ -142,8 +142,8 @@ FileImpl::~FileImpl() {
 
 void FileImpl::doWrite(Buffer::Instance& buffer) {
   uint64_t num_slices = buffer.getRawSlices(nullptr, 0);
-  STACK_ALLOC_ARRAY(slices, Buffer::RawSlice, num_slices);
-  buffer.getRawSlices(slices, num_slices);
+  STACK_ARRAY(slices, Buffer::RawSlice, num_slices);
+  buffer.getRawSlices(slices.begin(), num_slices);
 
   // We must do the actual writes to disk under lock, so that we don't intermix chunks from
   // different FileImpl pointing to the same underlying file. This can happen either via hot
@@ -155,8 +155,7 @@ void FileImpl::doWrite(Buffer::Instance& buffer) {
   //            process lock or had multiple locks.
   {
     Thread::LockGuard lock(file_lock_);
-    for (uint64_t i = 0; i < num_slices; i++) {
-      Buffer::RawSlice& slice = slices[i];
+    for (const Buffer::RawSlice& slice : slices) {
       const Api::SysCallSizeResult result = os_sys_calls_.write(fd_, slice.mem_, slice.len_);
       ASSERT(result.rc_ == static_cast<ssize_t>(slice.len_));
       stats_.write_completed_.inc();

source/common/grpc/BUILD

@@ -43,6 +43,7 @@ envoy_cc_library(
         "//include/envoy/buffer:buffer_interface",
         "//source/common/buffer:buffer_lib",
         "//source/common/common:minimal_logger_lib",
+        "//source/common/common:stack_array",
     ],
 )
 

source/common/grpc/codec.cc

@@ -4,9 +4,8 @@
 #include <cstdint>
 #include <vector>
 
-#include "envoy/common/platform.h"
-
 #include "common/buffer/buffer_impl.h"
+#include "common/common/stack_array.h"
 
 namespace Envoy {
 namespace Grpc {
@@ -25,10 +24,9 @@ Decoder::Decoder() : state_(State::FH_FLAG) {}
 
 bool Decoder::decode(Buffer::Instance& input, std::vector<Frame>& output) {
   uint64_t count = input.getRawSlices(nullptr, 0);
-  STACK_ALLOC_ARRAY(slices, Buffer::RawSlice, count);
-  input.getRawSlices(slices, count);
-  for (uint64_t i = 0; i < count; i++) {
-    Buffer::RawSlice& slice = slices[i];
+  STACK_ARRAY(slices, Buffer::RawSlice, count);
+  input.getRawSlices(slices.begin(), count);
+  for (const Buffer::RawSlice& slice : slices) {
     uint8_t* mem = reinterpret_cast<uint8_t*>(slice.mem_);
     for (uint64_t j = 0; j < slice.len_;) {
       uint8_t c = *mem;

source/common/http/BUILD

@@ -203,6 +203,7 @@ envoy_cc_library(
         "//include/envoy/http:message_interface",
         "//source/common/buffer:buffer_lib",
         "//source/common/common:non_copyable",
+        "//source/common/common:stack_array",
     ],
 )