Skip to content

Commit

Permalink
Fixed a bug with maps on 32-bit machines.
Browse files Browse the repository at this point in the history
We were not calculating proper offsets for upb_MapEntryData in cases
where the compiler was run on a different word size than the runtime.

PiperOrigin-RevId: 492114576
  • Loading branch information
haberman authored and copybara-github committed Dec 1, 2022
1 parent b198dd0 commit ca82889
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
9 changes: 6 additions & 3 deletions upb/mini_table/decode.c
Original file line number Diff line number Diff line change
Expand Up @@ -696,9 +696,12 @@ static void upb_MtDecoder_ParseMap(upb_MtDecoder* d, const char* data,
upb_MtDecoder_AssignHasbits(d->table);

// Map entries have a pre-determined layout, regardless of types.
d->fields[0].offset = offsetof(upb_MapEntryData, k);
d->fields[1].offset = offsetof(upb_MapEntryData, v);
d->table->size = UPB_ALIGN_UP(sizeof(upb_MapEntryData), 8);
// NOTE: sync with mini_table/message_internal.h.
const size_t kv_size = d->platform == kUpb_MiniTablePlatform_32Bit ? 8 : 16;
const size_t hasbit_size = 8;
d->fields[0].offset = hasbit_size;
d->fields[1].offset = hasbit_size + kv_size;
d->table->size = UPB_ALIGN_UP(hasbit_size + kv_size + kv_size, 8);

// Map entries have a special bit set to signal it's a map entry, used in
// upb_MiniTable_SetSubMessage() below.
Expand Down
6 changes: 5 additions & 1 deletion upb/mini_table/message_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,12 @@ struct upb_MiniTable {
//
// Note that users can and do create map entries directly, which will also use
// this layout.
//
// NOTE: sync with mini_table/decode.c.
typedef struct {
uint32_t hasbits;
// We only need 2 hasbits max, but due to alignment we'll use 8 bytes here,
// and the uint64_t helps make this clear.
uint64_t hasbits;
union {
upb_StringView str; // For str/bytes.
upb_value val; // For all other types.
Expand Down

0 comments on commit ca82889

Please sign in to comment.