Skip to content

Commit

Permalink
Let ArenaStringPtr debug-fail if it ever attempts to clear a default …
Browse files Browse the repository at this point in the history
…string.

As an optimization, we maintain a default instance of a string in memory, and
messages with uninitialized fields will be constructed with a pointer to this
default string object. The destructor should clear the field only when it is
"set" to a nondefault object.

If `ClearNonDefaultToEmpty()` is ever called on a default string...
- It will result in undefined behaviour. Most likely, it results in overwriting
  an object in memory (which is already full of zeros) with another bunch of
  zeros.
- It's quite bad for a number of reasons:
  1. It can confuse TSAN.
  2. It blocks us from moving the default instance of the string into the
     `.data` section. Having the default instance of the string live in
     read-only memory would be beneficial for memory safety. It would play well
     with hugepages. It would also eliminate some runtime cost of instantiating
     the default instance of the string.

This change adds a debug-fail so that users don't call this function "unsafely".

PiperOrigin-RevId: 684569674
  • Loading branch information
tonyliaoss authored and copybara-github committed Oct 10, 2024
1 parent 0d7fd35 commit 6c9c12c
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions src/google/protobuf/arenastring.h
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,7 @@ inline PROTOBUF_NDEBUG_INLINE void ArenaStringPtr::InternalSwap(

inline void ArenaStringPtr::ClearNonDefaultToEmpty() {
// Unconditionally mask away the tag.
ABSL_DCHECK(!tagged_ptr_.IsDefault());
tagged_ptr_.Get()->clear();
}

Expand Down

0 comments on commit 6c9c12c

Please sign in to comment.