Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ObjC: clang for 32bit arm (iOS) can generate an unaligned read that traps #6679

Closed
thomasvl opened this issue Sep 20, 2019 · 0 comments
Closed

ObjC: clang for 32bit arm (iOS) can generate an unaligned read that traps #6679

thomasvl opened this issue Sep 20, 2019 · 0 comments
Assignees
@thomasvl
Labels
ios objective-c

Comments

@thomasvl
Copy link
Contributor

Also reported in firebase/firebase-ios-sdk#3851.
@thomasvl thomasvl self-assigned this Sep 20, 2019
thomasvl added a commit that referenced this issue Sep 20, 2019
@thomasvl
[ObjC] Don't use unions and instead use memcpy for the type swaps. (#…
5846439 
…6672)

The code in question hasn't change in a long time so the cause of
firebase/firebase-ios-sdk#3851 still appears to be
an Xcode 11 clang change/bug; but this does appear to be slightly better
code for the work being done.

Cleanup along the way for #6679
thomasvl added a commit to thomasvl/protobuf that referenced this issue Sep 20, 2019
@thomasvl
[ObjC] Don't use unions and instead use memcpy for the type swaps. (p…
42e0ff5 
…rotocolbuffers#6672)

The code in question hasn't change in a long time so the cause of
firebase/firebase-ios-sdk#3851 still appears to be
an Xcode 11 clang change/bug; but this does appear to be slightly better
code for the work being done.

Cleanup along the way for protocolbuffers#6679
thomasvl pushed a commit to thomasvl/protobuf that referenced this issue Sep 20, 2019
@dmaclach @thomasvl
Remove OSReadLittle* due to alignment requirements (protocolbuffers#6678
a0eb83f 
)

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 protocolbuffers#6679
thomasvl added a commit to thomasvl/protobuf that referenced this issue Sep 20, 2019
@thomasvl
[ObjC] Don't use unions and instead use memcpy for the type swaps. (p…
33da471 
…rotocolbuffers#6672)

The code in question hasn't change in a long time so the cause of
firebase/firebase-ios-sdk#3851 still appears to be
an Xcode 11 clang change/bug; but this does appear to be slightly better
code for the work being done.

Cleanup along the way for protocolbuffers#6679
thomasvl pushed a commit to thomasvl/protobuf that referenced this issue Sep 20, 2019
@dmaclach @thomasvl
Remove OSReadLittle* due to alignment requirements (protocolbuffers#6678
397e017 
)

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 protocolbuffers#6679
TeBoring added a commit that referenced this issue Sep 20, 2019
@TeBoring
Merge pull request #6680 from thomasvl/objc_fix
cfee7d8 
Pull the ObjC fixes from #6679 to the 3.9.x branch
TeBoring added a commit that referenced this issue Sep 20, 2019
@TeBoring
Merge pull request #6681 from thomasvl/objc_fix2
3a0be88 
Pull the ObjC fixes from #6679 to the 3.10.x branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@thomasvl thomasvl

Labels
ios objective-c
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@thomasvl