Skip to content

Commit

Permalink
Migrate sanitizer related macros from port_def into internal function…
Browse files Browse the repository at this point in the history
…s in

port.h.
This reduces the cost of port_def.inc includes.

PiperOrigin-RevId: 696931317
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Nov 15, 2024
1 parent b6b774e commit 1d55c2b
Show file tree
Hide file tree
Showing 18 changed files with 121 additions and 136 deletions.
17 changes: 0 additions & 17 deletions csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.g.cs

This file was deleted.

2 changes: 2 additions & 0 deletions src/google/protobuf/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,7 @@ cc_library(
"//third_party/utf8_range:utf8_validity",
"@com_google_absl//absl/algorithm:container",
"@com_google_absl//absl/base",
"@com_google_absl//absl/base:config",
"@com_google_absl//absl/base:core_headers",
"@com_google_absl//absl/base:dynamic_annotations",
"@com_google_absl//absl/cleanup",
Expand Down Expand Up @@ -1965,6 +1966,7 @@ cc_test(
"//src/google/protobuf/stubs",
"//src/google/protobuf/testing",
"//src/google/protobuf/testing:file",
"@com_google_absl//absl/base:config",
"@com_google_absl//absl/log:absl_check",
"@com_google_absl//absl/numeric:bits",
"@com_google_absl//absl/random",
Expand Down
12 changes: 6 additions & 6 deletions src/google/protobuf/arena.cc
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ void SerialArena::AllocateNewBlock(size_t n) {
// Previous writes must take effect before writing new head.
head_.store(new_head, std::memory_order_release);

PROTOBUF_POISON_MEMORY_REGION(ptr(), limit_ - ptr());
internal::PoisonMemoryRegion(ptr(), limit_ - ptr());
}

uint64_t SerialArena::SpaceUsed() const {
Expand Down Expand Up @@ -716,7 +716,7 @@ void ThreadSafeArena::UnpoisonAllArenaBlocks() const {
VisitSerialArena([](const SerialArena* serial) {
for (const auto* b = serial->head(); b != nullptr && !b->IsSentry();
b = b->next) {
PROTOBUF_UNPOISON_MEMORY_REGION(b, b->size);
internal::UnpoisonMemoryRegion(b, b->size);
}
});
}
Expand Down Expand Up @@ -746,7 +746,7 @@ ThreadSafeArena::~ThreadSafeArena() {
auto mem = Free();
if (alloc_policy_.is_user_owned_initial_block()) {
// Unpoison the initial block, now that it's going back to the user.
PROTOBUF_UNPOISON_MEMORY_REGION(mem.p, mem.n);
internal::UnpoisonMemoryRegion(mem.p, mem.n);
} else if (mem.n > 0) {
GetDeallocator(alloc_policy_.get())(mem);
}
Expand Down Expand Up @@ -922,9 +922,9 @@ template void*
ThreadSafeArena::AllocateAlignedFallback<AllocationClient::kArray>(size_t);

void ThreadSafeArena::CleanupList() {
#ifdef PROTOBUF_ASAN
UnpoisonAllArenaBlocks();
#endif
if constexpr (HasMemoryPoisoning()) {
UnpoisonAllArenaBlocks();
}

WalkSerialArenaChunk([](SerialArenaChunk* chunk) {
absl::Span<std::atomic<SerialArena*>> span = chunk->arenas();
Expand Down
31 changes: 16 additions & 15 deletions src/google/protobuf/arena_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1539,13 +1539,13 @@ TEST(ArenaTest, ClearOneofMessageOnArena) {
child->set_moo_int(100);
message->clear_foo_message();

#ifndef PROTOBUF_ASAN
EXPECT_NE(child->moo_int(), 100);
#else
if (internal::HasMemoryPoisoning()) {
#if GTEST_HAS_DEATH_TEST
EXPECT_DEATH(EXPECT_EQ(child->moo_int(), 0), "use-after-poison");
EXPECT_DEATH(EXPECT_EQ(child->moo_int(), 0), "use-after-poison");
#endif // !GTEST_HAS_DEATH_TEST
#endif // !PROTOBUF_ASAN
} else {
EXPECT_NE(child->moo_int(), 100);
}
}

TEST(ArenaTest, CopyValuesWithinOneof) {
Expand Down Expand Up @@ -1840,7 +1840,10 @@ TEST(ArenaTest, SpaceReuseForArraysSizeChecks) {
}

TEST(ArenaTest, SpaceReusePoisonsAndUnpoisonsMemory) {
#ifdef PROTOBUF_ASAN
if constexpr (!internal::HasMemoryPoisoning()) {
GTEST_SKIP() << "Memory poisoning not enabled.";
}

char buf[1024]{};
constexpr int kSize = 32;
{
Expand All @@ -1849,19 +1852,21 @@ TEST(ArenaTest, SpaceReusePoisonsAndUnpoisonsMemory) {
for (int i = 0; i < 100; ++i) {
void* p = Arena::CreateArray<char>(&arena, kSize);
// Simulate other ASan client managing shadow memory.
ASAN_POISON_MEMORY_REGION(p, kSize);
ASAN_UNPOISON_MEMORY_REGION(p, kSize - 4);
internal::PoisonMemoryRegion(p, kSize);
internal::UnpoisonMemoryRegion(p, kSize - 4);
pointers.push_back(p);
}
for (void* p : pointers) {
internal::ArenaTestPeer::ReturnArrayMemory(&arena, p, kSize);
// The first one is not poisoned because it becomes the freelist.
if (p != pointers[0]) EXPECT_TRUE(__asan_address_is_poisoned(p));
if (p != pointers[0]) {
EXPECT_TRUE(internal::IsMemoryPoisoned(p));
}
}

bool found_poison = false;
for (char& c : buf) {
if (__asan_address_is_poisoned(&c)) {
if (internal::IsMemoryPoisoned(&c)) {
found_poison = true;
break;
}
Expand All @@ -1871,12 +1876,8 @@ TEST(ArenaTest, SpaceReusePoisonsAndUnpoisonsMemory) {

// Should not be poisoned after destruction.
for (char& c : buf) {
ASSERT_FALSE(__asan_address_is_poisoned(&c));
ASSERT_FALSE(internal::IsMemoryPoisoned(&c));
}

#else // PROTOBUF_ASAN
GTEST_SKIP();
#endif // PROTOBUF_ASAN
}


Expand Down
2 changes: 1 addition & 1 deletion src/google/protobuf/generated_message_reflection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1374,7 +1374,7 @@ void Reflection::MaybePoisonAfterClear(Message& root) const {
for (auto it : nodes) {
(void)it;
PROTOBUF_POISON_MEMORY_REGION(it.ptr, it.size);
internal::PoisonMemoryRegion(it.ptr, it.size);
}
}
Expand Down
9 changes: 5 additions & 4 deletions src/google/protobuf/generated_message_tctable_lite_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "google/protobuf/io/coded_stream.h"
#include "google/protobuf/message_lite.h"
#include "google/protobuf/parse_context.h"
#include "google/protobuf/port.h"
#include "google/protobuf/unittest.pb.h"
#include "google/protobuf/wire_format_lite.h"

Expand Down Expand Up @@ -916,10 +917,10 @@ TEST(GeneratedMessageTctableLiteTest, PackedEnumSmallRange) {
// This test checks that the parser doesn't overflow an int32 when computing the
// array's new length.
TEST(GeneratedMessageTctableLiteTest, PackedEnumSmallRangeLargeSize) {
#ifdef PROTOBUF_MSAN
// This test attempts to allocate 8GB of memory, which OOMs MSAN.
return;
#endif
if constexpr (internal::HasAnySanitizer()) {
GTEST_SKIP() << "This test attempts to allocate 8GB of memory, which OOMs "
"in sanitizer mode.";
}

#ifdef _WIN32
// This test OOMs on Windows. I think this is because Windows is committing
Expand Down
1 change: 1 addition & 0 deletions src/google/protobuf/io/zero_copy_stream_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
#include "google/protobuf/io/coded_stream.h"
#include "google/protobuf/io/io_win32.h"
#include "google/protobuf/io/zero_copy_stream_impl.h"
#include "google/protobuf/port.h"
#include "google/protobuf/test_util2.h"

#if HAVE_ZLIB
Expand Down
3 changes: 2 additions & 1 deletion src/google/protobuf/map_field.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <utility>

#include "absl/base/attributes.h"
#include "absl/base/config.h"
#include "absl/hash/hash.h"
#include "absl/log/absl_check.h"
#include "absl/log/absl_log.h"
Expand Down Expand Up @@ -437,7 +438,7 @@ class PROTOBUF_EXPORT MapFieldBase : public MapFieldBaseForParse {
// thread calls either ConstAccess() or MutableAccess(), on the same
// MapFieldBase-derived object, and there is no synchronization going
// on between them, tsan will alert.
#if defined(PROTOBUF_TSAN)
#if defined(ABSL_HAVE_THREAD_SANITIZER)
void ConstAccess() const { ABSL_CHECK_EQ(seq1_, seq2_); }
void MutableAccess() {
if (seq1_ & 1) {
Expand Down
12 changes: 6 additions & 6 deletions src/google/protobuf/message.h
Original file line number Diff line number Diff line change
Expand Up @@ -1604,12 +1604,12 @@ bool SplitFieldHasExtraIndirectionStatic(const FieldDescriptor* field) {

inline void MaybePoisonAfterClear(Message* root) {
if (root == nullptr) return;
#ifndef PROTOBUF_ASAN
root->Clear();
#else
const Reflection* reflection = root->GetReflection();
reflection->MaybePoisonAfterClear(*root);
#endif
if constexpr (HasMemoryPoisoning()) {
const Reflection* reflection = root->GetReflection();
reflection->MaybePoisonAfterClear(*root);
} else {
root->Clear();
}
}

} // namespace internal
Expand Down
12 changes: 6 additions & 6 deletions src/google/protobuf/parse_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,10 @@ class PROTOBUF_EXPORT EpsCopyInputStream {
// __asan_address_is_poisoned is allowed to have false negatives.
class LimitToken {
public:
LimitToken() { PROTOBUF_POISON_MEMORY_REGION(&token_, sizeof(token_)); }
LimitToken() { internal::PoisonMemoryRegion(&token_, sizeof(token_)); }

explicit LimitToken(int token) : token_(token) {
PROTOBUF_UNPOISON_MEMORY_REGION(&token_, sizeof(token_));
internal::UnpoisonMemoryRegion(&token_, sizeof(token_));
}

LimitToken(const LimitToken&) = delete;
Expand All @@ -135,17 +135,17 @@ class PROTOBUF_EXPORT EpsCopyInputStream {
LimitToken(LimitToken&& other) { *this = std::move(other); }

LimitToken& operator=(LimitToken&& other) {
PROTOBUF_UNPOISON_MEMORY_REGION(&token_, sizeof(token_));
internal::UnpoisonMemoryRegion(&token_, sizeof(token_));
token_ = other.token_;
PROTOBUF_POISON_MEMORY_REGION(&other.token_, sizeof(token_));
internal::PoisonMemoryRegion(&other.token_, sizeof(token_));
return *this;
}

~LimitToken() { PROTOBUF_UNPOISON_MEMORY_REGION(&token_, sizeof(token_)); }
~LimitToken() { internal::UnpoisonMemoryRegion(&token_, sizeof(token_)); }

int token() && {
int t = token_;
PROTOBUF_POISON_MEMORY_REGION(&token_, sizeof(token_));
internal::PoisonMemoryRegion(&token_, sizeof(token_));
return t;
}

Expand Down
57 changes: 51 additions & 6 deletions src/google/protobuf/port.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@
#include "absl/strings/string_view.h"
#include "absl/types/optional.h"

#if defined(ABSL_HAVE_ADDRESS_SANITIZER)
#include <sanitizer/asan_interface.h>
#endif

// must be last
#include "google/protobuf/port_def.inc"

Expand Down Expand Up @@ -258,9 +262,18 @@ inline constexpr bool DebugHardenClearOneofMessageOnArena() {
#endif
}

constexpr bool HasAnySanitizer() {
#if defined(ABSL_HAVE_ADDRESS_SANITIZER) || \
defined(ABSL_HAVE_MEMORY_SANITIZER) || defined(ABSL_HAVE_THREAD_SANITIZER)
return true;
#else
return false;
#endif
}

constexpr bool PerformDebugChecks() {
#if defined(NDEBUG) && !defined(PROTOBUF_ASAN) && !defined(PROTOBUF_MSAN) && \
!defined(PROTOBUF_TSAN)
if (HasAnySanitizer()) return true;
#if defined(NDEBUG)
return false;
#else
return true;
Expand Down Expand Up @@ -355,13 +368,45 @@ PROTOBUF_ALWAYS_INLINE void Prefetch5LinesFrom1Line(const void* ptr) {
}
#endif

#ifdef PROTOBUF_TSAN
constexpr bool HasMemoryPoisoning() {
#if defined(ABSL_HAVE_ADDRESS_SANITIZER)
return true;
#else
return false;
#endif
}

// Poison memory region when supported by sanitizer config.
inline void PoisonMemoryRegion(const void* p, size_t n) {
#if defined(ABSL_HAVE_ADDRESS_SANITIZER)
ASAN_POISON_MEMORY_REGION(p, n);
#else
// Nothing
#endif
}

inline void UnpoisonMemoryRegion(const void* p, size_t n) {
#if defined(ABSL_HAVE_ADDRESS_SANITIZER)
ASAN_UNPOISON_MEMORY_REGION(p, n);
#else
// Nothing
#endif
}

inline bool IsMemoryPoisoned(const void* p) {
#if defined(ABSL_HAVE_ADDRESS_SANITIZER)
return __asan_address_is_poisoned(p);
#else
return false;
#endif
}

#if defined(ABSL_HAVE_THREAD_SANITIZER)
// TODO: it would be preferable to use __tsan_external_read/
// __tsan_external_write, but they can cause dlopen issues.
template <typename T>
PROTOBUF_ALWAYS_INLINE void TSanRead(const T* impl) {
char protobuf_tsan_dummy =
*reinterpret_cast<const char*>(&impl->_tsan_detect_race);
char protobuf_tsan_dummy = impl->_tsan_detect_race;
asm volatile("" : "+r"(protobuf_tsan_dummy));
}

Expand All @@ -370,7 +415,7 @@ PROTOBUF_ALWAYS_INLINE void TSanRead(const T* impl) {
// correctness of the rest of the class.
template <typename T>
PROTOBUF_ALWAYS_INLINE void TSanWrite(T* impl) {
*reinterpret_cast<char*>(&impl->_tsan_detect_race) = 0;
impl->_tsan_detect_race = 0;
}
#else
PROTOBUF_ALWAYS_INLINE void TSanRead(const void*) {}
Expand Down
Loading

0 comments on commit 1d55c2b

Please sign in to comment.