Skip to content

Commit

Permalink
Remove OSReadLittle* due to alignment requirements (#6678)
Browse files Browse the repository at this point in the history
The OSReadLittleInt64 function as defined by Apple reduces down to:

`return *(volatile uint64_t *)((uintptr_t)base + byteOffset);`

which means we are type-punning using a cast. On ARMv7 and other aligned architectures this can cause crashes. 
Minimal example: https://gist.github.com/dmaclach/b10b0a71ae614d304c067cb9bd264336

Fixes #6679
  • Loading branch information
dmaclach authored and thomasvl committed Sep 20, 2019
1 parent 33da471 commit 397e017
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions objectivec/GPBCodedInputStream.m
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,22 @@ static int8_t ReadRawByte(GPBCodedInputStreamState *state) {

static int32_t ReadRawLittleEndian32(GPBCodedInputStreamState *state) {
CheckSize(state, sizeof(int32_t));
int32_t value = OSReadLittleInt32(state->bytes, state->bufferPos);
// Not using OSReadLittleInt32 because it has undocumented dependency
// on reads being aligned.
int32_t value;
memcpy(&value, state->bytes + state->bufferPos, sizeof(int32_t));
value = OSSwapLittleToHostInt32(value);
state->bufferPos += sizeof(int32_t);
return value;
}

static int64_t ReadRawLittleEndian64(GPBCodedInputStreamState *state) {
CheckSize(state, sizeof(int64_t));
int64_t value = OSReadLittleInt64(state->bytes, state->bufferPos);
// Not using OSReadLittleInt64 because it has undocumented dependency
// on reads being aligned.
int64_t value;
memcpy(&value, state->bytes + state->bufferPos, sizeof(int64_t));
value = OSSwapLittleToHostInt64(value);
state->bufferPos += sizeof(int64_t);
return value;
}
Expand Down

0 comments on commit 397e017

Please sign in to comment.