Skip to content

Commit

Permalink
Fix issue where a tmp buffer could have been too small when handling …
Browse files Browse the repository at this point in the history
…a serialized FeatureSet.

Rename upb_Log2CeilingSize() to upb_RoundUpToPowerOfTwo() to minimize the chance of this confusion happening in the future.

In practice this condition could never be hit by descriptors generated by protoc since FeatureSet is small and 1-byte-on-wire fields.

PiperOrigin-RevId: 702381123
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Dec 3, 2024
1 parent 9db4d1c commit eb8a34d
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 6 deletions.
4 changes: 3 additions & 1 deletion upb/base/internal/log2.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ UPB_INLINE int upb_Log2Ceiling(int x) {
#endif
}

UPB_INLINE int upb_Log2CeilingSize(int x) { return 1 << upb_Log2Ceiling(x); }
UPB_INLINE int upb_RoundUpToPowerOfTwo(int x) {
return 1 << upb_Log2Ceiling(x);
}

#ifdef __cplusplus
} /* extern "C" */
Expand Down
4 changes: 2 additions & 2 deletions upb/message/internal/message.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ bool UPB_PRIVATE(_upb_Message_EnsureAvailable)(struct upb_Message* msg,
upb_Message_Internal* in = UPB_PRIVATE(_upb_Message_GetInternal)(msg);
if (!in) {
// No internal data, allocate from scratch.
size_t size = UPB_MAX(128, upb_Log2CeilingSize(need + overhead));
size_t size = UPB_MAX(128, upb_RoundUpToPowerOfTwo(need + overhead));
in = upb_Arena_Malloc(a, size);
if (!in) return false;

Expand All @@ -65,7 +65,7 @@ bool UPB_PRIVATE(_upb_Message_EnsureAvailable)(struct upb_Message* msg,
UPB_PRIVATE(_upb_Message_SetInternal)(msg, in);
} else if (in->ext_begin - in->unknown_end < need) {
// Internal data is too small, reallocate.
size_t new_size = upb_Log2CeilingSize(in->size + need);
size_t new_size = upb_RoundUpToPowerOfTwo(in->size + need);
size_t ext_bytes = in->size - in->ext_begin;
size_t new_ext_begin = new_size - ext_bytes;
in = upb_Arena_Realloc(a, in, in->size, new_size);
Expand Down
2 changes: 1 addition & 1 deletion upb/message/map_sorter.c
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ static bool _upb_mapsorter_resize(_upb_mapsorter* s, _upb_sortedmap* sorted,

if (sorted->end > s->cap) {
const int oldsize = s->cap * sizeof(*s->entries);
s->cap = upb_Log2CeilingSize(sorted->end);
s->cap = upb_RoundUpToPowerOfTwo(sorted->end);
const int newsize = s->cap * sizeof(*s->entries);
s->entries = upb_grealloc(s->entries, oldsize, newsize);
if (!s->entries) return false;
Expand Down
2 changes: 1 addition & 1 deletion upb/mini_descriptor/internal/encode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class MtDataEncoder {
if (!end) return false;
// C++ does not guarantee that string has doubling growth behavior, but
// we need it to avoid O(n^2).
str_.reserve(upb_Log2CeilingSize(str_.size() + (end - buf_)));
str_.reserve(upb_RoundUpToPowerOfTwo(str_.size() + (end - buf_)));
str_.append(buf_, end - buf_);
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion upb/reflection/internal/def_builder.c
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ upb_StringView _upb_DefBuilder_MakeKey(upb_DefBuilder* ctx,
upb_StringView key) {
size_t need = key.size + sizeof(void*);
if (ctx->tmp_buf_size < need) {
ctx->tmp_buf_size = UPB_MAX(64, upb_Log2Ceiling(need));
ctx->tmp_buf_size = UPB_MAX(64, upb_RoundUpToPowerOfTwo(need));
ctx->tmp_buf = upb_Arena_Malloc(ctx->tmp_arena, ctx->tmp_buf_size);
if (!ctx->tmp_buf) _upb_DefBuilder_OomErr(ctx);
}
Expand Down

0 comments on commit eb8a34d

Please sign in to comment.