Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Don't use signed int type for anchor segment #16008

Merged
merged 3 commits into from
Dec 9, 2019

Conversation

alexshalamov
Copy link
Contributor

@alexshalamov alexshalamov commented Dec 4, 2019

Erroneous signed to unsigned implicit conversion was used when PlacedSymbol(s) were created in SymbolLayout and signed values were used as indexes in a few other places.

@alexshalamov alexshalamov force-pushed the alexshalamov_signed_to_unsigned_conversion branch from 53e3052 to bed453e Compare December 5, 2019 10:39
@alexshalamov alexshalamov marked this pull request as ready for review December 5, 2019 13:13
src/mbgl/geometry/anchor.hpp Outdated Show resolved Hide resolved
src/mbgl/layout/symbol_layout.cpp Outdated Show resolved Hide resolved
src/mbgl/layout/symbol_layout.cpp Show resolved Hide resolved
src/mbgl/layout/symbol_layout.cpp Show resolved Hide resolved
src/mbgl/text/collision_feature.cpp Show resolved Hide resolved
@alexshalamov alexshalamov force-pushed the alexshalamov_signed_to_unsigned_conversion branch from bed453e to c2899f7 Compare December 5, 2019 17:04
Copy link
Contributor

@springmeyer springmeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that this resolved the UBSAN "runtime error" in downstream testing against c2899f7. Thanks @alexshalamov!

@alexshalamov alexshalamov force-pushed the alexshalamov_signed_to_unsigned_conversion branch from d715cf2 to 6e79d71 Compare December 9, 2019 14:45
@alexshalamov alexshalamov merged commit 8966532 into master Dec 9, 2019
@alexshalamov alexshalamov deleted the alexshalamov_signed_to_unsigned_conversion branch December 9, 2019 15:26
@alexshalamov alexshalamov self-assigned this Dec 10, 2019
@chloekraw chloekraw added this to the release-unicorn milestone Dec 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants