Add sanitized builds to Kokoro (#10706)

* Adding build configs for sanitization

* Update bazel runner to accept configs to loop over

* Fix Bazel query from to googletest upgrade

* Fix pre-existing ODR violation

* Clean up bazel configs

* Fix UBSAN issues in tests

* Upgrade zlib to pull in UBSAN fix

* Fix conformance test UB

* Add *san builds to Bazel tests

* Add dbg to *san builds

* Extend timeout for Bazel build

* Enable ODR checks again by using static linkage in ASAN

* Cleanup kokoro setup

* Disable MSAN for now

* Enable MSAN in kokoro build

* Fix msan failure

* Remove broken bazel clean

* Cleanup

* Fix false leaks

* Fix cap-add argument

* Fix asan config name

* Remove LSAN verbosity

* Expand size of big test, add verbose failures

* Skip slow test in TSAN

* Workaround for bazel issue with ubsan

.bazelrc

@@ -1 +1,35 @@
 build --cxxopt=-std=c++14 --host_cxxopt=-std=c++14
+
+build:dbg --compilation_mode=dbg
+
+build:opt --compilation_mode=opt
+
+build:san-common --config=dbg --strip=never --copt=-O0 --copt=-fno-omit-frame-pointer
+
+build:asan --config=san-common --copt=-fsanitize=address --linkopt=-fsanitize=address
+build:asan --copt=-DADDRESS_SANITIZER=1
+# ASAN hits ODR violations with shared linkage due to rules_proto.
+build:asan --dynamic_mode=off
+
+build:msan --config=san-common --copt=-fsanitize=memory --linkopt=-fsanitize=memory
+build:msan --copt=-fsanitize-memory-track-origins
+build:msan --copt=-fsanitize-memory-use-after-dtor
+build:msan --action_env=MSAN_OPTIONS=poison_in_dtor=1
+build:msan --copt=-DMEMORY_SANITIZER=1
+
+# Use our instrumented LLVM libc++ in Kokoro.
+build:kokoro-msan --config=msan
+build:kokoro-msan --linkopt=-L/opt/libcxx_msan/lib
+build:kokoro-msan --linkopt=-Wl,-rpath,/opt/libcxx_msan/lib
+build:kokoro-msan --cxxopt=-stdlib=libc++ --linkopt=-stdlib=libc++
+
+
+build:tsan --config=san-common --copt=-fsanitize=thread --linkopt=-fsanitize=thread
+build:tsan --copt=-DTHREAD_SANITIZER=1
+
+build:ubsan --config=san-common --copt=-fsanitize=undefined --linkopt=-fsanitize=undefined
+build:ubsan --action_env=UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1
+build:ubsan --copt=-DUNDEFINED_SANITIZER=1
+# Workaround for the fact that Bazel links with $CC, not $CXX
+# https://github.com/bazelbuild/bazel/issues/11122#issuecomment-613746748
+build:ubsan --copt=-fno-sanitize=function --copt=-fno-sanitize=vptr

WORKSPACE

@@ -16,6 +16,13 @@ http_archive(
     ],
 )
 
+http_archive(
+    name = "com_googlesource_code_re2",
+    sha256 = "906d0df8ff48f8d3a00a808827f009a840190f404559f649cb8e4d7143255ef9",
+    strip_prefix = "re2-a276a8c738735a0fe45a6ee590fe2df69bcf4502",
+    urls = ["https://github.com/google/re2/archive/a276a8c738735a0fe45a6ee590fe2df69bcf4502.zip"],  # 2022-04-08
+)
+
 # Bazel platform rules.
 http_archive(
     name = "platforms",

kokoro/linux/bazel.sh

@@ -8,7 +8,6 @@ fi
 
 cd $(dirname $0)/../..
 GIT_REPO_ROOT=`pwd`
-rm -rf $GIT_REPO_ROOT/logs
 
 ENVS=()
 
@@ -25,20 +24,38 @@ if [ -n "$BAZEL_ENV" ]; then
   done
 fi
 
-tmpfile=$(mktemp -u)
+function run {
-
+  local CONFIG=$1
-docker run \
+  local BAZEL_CONFIG=$2
-  --cidfile $tmpfile \
+
-  -v $GIT_REPO_ROOT:/workspace \
+  tmpfile=$(mktemp -u)
-  $CONTAINER_IMAGE \
+
-  test \
+  rm -rf $GIT_REPO_ROOT/bazel-out $GIT_REPO_ROOT/bazel-bin
-  --keep_going \
+  rm -rf $GIT_REPO_ROOT/logs
-  --test_output=streamed \
+
-  ${ENVS[@]} \
+  docker run \
-  $PLATFORM_CONFIG \
+    --cidfile $tmpfile \
-  $BAZEL_EXTRA_FLAGS \
+    --cap-add=SYS_PTRACE \
-  $BAZEL_TARGETS
+    -v $GIT_REPO_ROOT:/workspace \
-
+    $CONTAINER_IMAGE \
-# Save logs for Kokoro
+    test \
-docker cp \
+    --keep_going \
-  `cat $tmpfile`:/workspace/logs $KOKORO_ARTIFACTS_DIR
+    --test_output=streamed \
+    ${ENVS[@]} \
+    $PLATFORM_CONFIG \
+    $BAZEL_CONFIG \
+    $BAZEL_EXTRA_FLAGS \
+    $BAZEL_TARGETS
+
+  # Save logs for Kokoro
+  docker cp \
+    `cat $tmpfile`:/workspace/logs $KOKORO_ARTIFACTS_DIR/$CONFIG
+}
+
+if [ -n "$BAZEL_CONFIGS" ]; then
+  for config in $BAZEL_CONFIGS; do
+    run $config "--config=$config"
+  done
+else
+    run
+fi

kokoro/linux/bazel/common.cfg

@@ -2,13 +2,28 @@
 
 # Location of the build script in repository
 build_file: "protobuf/kokoro/linux/bazel.sh"
-timeout_mins: 15
+timeout_mins: 120
+
+env_vars {
+  key: "CONTAINER_IMAGE"
+  value: "gcr.io/protobuf-build/bazel/linux-san:b6bfa3bb505e83f062af0cb0ed23abf1e89b9edb"
+}
 
 env_vars {
   key: "BAZEL_TARGETS"
   value: "//src/... @com_google_protobuf_examples//..."
 }
 
+env_vars {
+  key: "BAZEL_CONFIGS"
+  value: "opt dbg asan kokoro-msan tsan ubsan"
+}
+
+env_vars {
+  key: "BAZEL_EXTRA_FLAGS"
+  value: "--distinct_host_configuration=false"
+}
+
 action {
   define_artifacts {
     regex: "**/sponge_log.*"

protobuf_deps.bzl

@@ -49,9 +49,9 @@ def protobuf_deps():
         http_archive(
             name = "zlib",
             build_file = "@com_google_protobuf//:third_party/zlib.BUILD",
-            sha256 = "629380c90a77b964d896ed37163f5c3a34f6e6d897311f1df2a7016355c45eff",
+            sha256 = "d8688496ea40fb61787500e863cc63c9afcbc524468cedeb478068924eb54932",
-            strip_prefix = "zlib-1.2.11",
+            strip_prefix = "zlib-1.2.12",
-            urls = ["https://github.com/madler/zlib/archive/v1.2.11.tar.gz"],
+            urls = ["https://github.com/madler/zlib/archive/v1.2.12.tar.gz"],
         )
 
     if not native.existing_rule("rules_cc"):

src/google/protobuf/BUILD.bazel

@@ -189,7 +189,6 @@ cc_library(
     name = "protobuf_lite",
     srcs = [
         "any_lite.cc",
-        "arena_config.cc",
         "arenastring.cc",
         "arenaz_sampler.cc",
         "extension_set.cc",

src/google/protobuf/compiler/parser.cc

@@ -1336,19 +1336,19 @@ bool Parser::ParseDefaultAssignment(
     }
 
     case FieldDescriptorProto::TYPE_FLOAT:
-    case FieldDescriptorProto::TYPE_DOUBLE:
+    case FieldDescriptorProto::TYPE_DOUBLE: {
       // These types can be negative.
       if (TryConsume("-")) {
         default_value->append("-");
       }
       // Parse the integer because we have to convert hex integers to decimal
       // floats.
-      double value;
+      double value = 0.0;
       DO(ConsumeNumber(&value, "Expected number."));
       // And stringify it again.
       default_value->append(SimpleDtoa(value));
       break;
-
+    }
     case FieldDescriptorProto::TYPE_BOOL:
       if (TryConsume("true")) {
         default_value->assign("true");

src/google/protobuf/io/zero_copy_stream_unittest.cc

@@ -720,9 +720,9 @@ TEST_F(IoTest, StringIo) {
 
 // Verifies that outputs up to kint32max can be created.
 TEST_F(IoTest, LargeOutput) {
-  // Filter out this test on 32-bit architectures.
+  // Filter out this test on 32-bit architectures and tsan builds.
   if(sizeof(void*) < 8) return;
-
+#ifndef THREAD_SANITIZER
   std::string str;
   StringOutputStream output(&str);
   void* unused_data;
@@ -734,6 +734,7 @@ TEST_F(IoTest, LargeOutput) {
   // Further increases should be possible.
   output.Next(&unused_data, &size);
   EXPECT_GT(size, 0);
+#endif  // THREAD_SANITIZER
 }
 
 

src/google/protobuf/map_field_test.cc

@@ -72,14 +72,6 @@ class MapFieldBaseStub : public MapFieldBase {
   MapFieldBaseStub() {}
   virtual ~MapFieldBaseStub() { MapFieldBase::Destruct(); }
   explicit MapFieldBaseStub(Arena* arena) : MapFieldBase(arena) {}
-  // Get underlined repeated field without synchronizing map.
-  RepeatedPtrField<Message>* InternalRepeatedField() { return repeated_field_; }
-  bool IsMapClean() {
-    return state_.load(std::memory_order_relaxed) != STATE_MODIFIED_MAP;
-  }
-  bool IsRepeatedClean() {
-    return state_.load(std::memory_order_relaxed) != STATE_MODIFIED_REPEATED;
-  }
   void SetMapDirty() {
     state_.store(STATE_MODIFIED_MAP, std::memory_order_relaxed);
   }
@@ -293,26 +285,34 @@ class MapFieldStateTest
 
   void Expect(MapFieldType* map_field, State state, int map_size,
               int repeated_size, bool is_repeated_null) {
-    MapFieldBase* map_field_base = map_field;
-    MapFieldBaseStub* stub =
-        reinterpret_cast<MapFieldBaseStub*>(map_field_base);
-
     // We use MutableMap on impl_ because we don't want to disturb the syncing
     Map<int32_t, int32_t>* map = map_field->impl_.MutableMap();
-    RepeatedPtrField<Message>* repeated_field = stub->InternalRepeatedField();
+    RepeatedPtrField<Message>* repeated_field = map_field->repeated_field_;
 
     switch (state) {
       case MAP_DIRTY:
-        EXPECT_FALSE(stub->IsMapClean());
+        EXPECT_FALSE(
-        EXPECT_TRUE(stub->IsRepeatedClean());
+          map_field->state_.load(std::memory_order_relaxed) !=
+          MapFieldType::STATE_MODIFIED_MAP);
+        EXPECT_TRUE(
+          map_field->state_.load(std::memory_order_relaxed) !=
+          MapFieldType::STATE_MODIFIED_REPEATED);
         break;
       case REPEATED_DIRTY:
-        EXPECT_TRUE(stub->IsMapClean());
+        EXPECT_TRUE(
-        EXPECT_FALSE(stub->IsRepeatedClean());
+          map_field->state_.load(std::memory_order_relaxed) !=
+          MapFieldType::STATE_MODIFIED_MAP);
+        EXPECT_FALSE(
+          map_field->state_.load(std::memory_order_relaxed) !=
+          MapFieldType::STATE_MODIFIED_REPEATED);
         break;
       case CLEAN:
-        EXPECT_TRUE(stub->IsMapClean());
+        EXPECT_TRUE(
-        EXPECT_TRUE(stub->IsRepeatedClean());
+          map_field->state_.load(std::memory_order_relaxed) !=
+          MapFieldType::STATE_MODIFIED_MAP);
+        EXPECT_TRUE(
+          map_field->state_.load(std::memory_order_relaxed) !=
+          MapFieldType::STATE_MODIFIED_REPEATED);
         break;
       default:
         FAIL();

src/google/protobuf/parse_context.h

@@ -901,11 +901,13 @@ const char* EpsCopyInputStream::ReadPackedFixed(const char* ptr, int size,
     nbytes = static_cast<int>(buffer_end_ + kSlopBytes - ptr);
   }
   int num = size / sizeof(T);
+  int block_size = num * sizeof(T);
+  if(num == 0) return size == block_size ? ptr : nullptr;
   int old_entries = out->size();
   out->Reserve(old_entries + num);
-  int block_size = num * sizeof(T);
   auto dst = out->AddNAlreadyReserved(num);
 #ifdef PROTOBUF_LITTLE_ENDIAN
+  GOOGLE_CHECK(dst != nullptr) << out << "," << num;
   std::memcpy(dst, ptr, block_size);
 #else
   for (int i = 0; i < num; i++) dst[i] = UnalignedLoad<T>(ptr + i * sizeof(T));

src/google/protobuf/stubs/bytestream.cc

@@ -124,8 +124,10 @@ char* GrowingArrayByteSink::GetBuffer(size_t* nbytes) {
 void GrowingArrayByteSink::Expand(size_t amount) {  // Expand by at least 50%.
   size_t new_capacity = std::max(capacity_ + amount, (3 * capacity_) / 2);
   char* bigger = new char[new_capacity];
-  memcpy(bigger, buf_, size_);
+  if(buf_ != nullptr) {
-  delete[] buf_;
+    memcpy(bigger, buf_, size_);
+    delete[] buf_;
+  }
   buf_ = bigger;
   capacity_ = new_capacity;
 }