Skip to content

Conversation

@jiulongw
Copy link
Contributor

@jiulongw jiulongw commented Jul 9, 2020

ASAN strictly forbids read over allocated boundary, so length checking
should go before \0 testing.

Fixes #11598.

ASAN strictly forbids read over allocated boundary, so length checking
should go before `\0` testing.

Fixes #11598.
@welcome
Copy link

welcome bot commented Jul 9, 2020

Thank you for submitting a pull request! If this is your first PR, make sure to add yourself to AUTHORS.

@sbc100
Copy link
Collaborator

sbc100 commented Jul 9, 2020

Looks great to me.. @kripken what do you think?

Copy link
Contributor

@jgravelle-google jgravelle-google left a comment

Choose a reason for hiding this comment

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

🤦

Also that's amazing that ASAN catches this. Maybe not that amazing? No it's definitely amazing.

LGTM

@jgravelle-google
Copy link
Contributor

Oh, should probably re-enable the asan.test_embind_val test #11158 ; I suspect it will pass now

@kripken
Copy link
Member

kripken commented Jul 10, 2020

Great PR, thank you @jiulongw ! 😄

And yes, this might allow the asan test to work now for embind, I'll open a PR to check.

@kripken kripken merged commit 0bfe9f5 into emscripten-core:master Jul 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ASAN violation in embind fromWireType for std::string

4 participants