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.

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

hipe: bs_match_string bug fix #1234

Merged
merged 2 commits into from Nov 8, 2016
Merged

Conversation

kostis
Copy link
Contributor

@kostis kostis commented Nov 3, 2016

This fixes a HiPE bug reported on erlang-questions on 2/11/2016.

The BEAM to ICode translation of the bs_match_string instruction,
written long ago for binaries (i.e., with byte-sized strings), tried
to do a clever translation of even bit-sized strings using a HiPE
primop that took a Size argument expressed in bytes.
ICode is not really the place to do such a thing, and moreover there
is really no reason for the HiPE primop not to take a Size argument
expressed in bits instead. This commit changes the Size argument
to be in bits, postpones the translation of the bs_match_string primop
until RTL and does a proper translation using bit-sized quantities there.

Fixed in a pair-programming/debugging session with @margnus1.

This fixes a HiPE bug reported on erlang-questions on 2/11/2016.

The BEAM to ICode tranaslation of the bs_match_string instruction,
written long ago for binaries (i.e., with byte-sized strings), tried
to do a `clever' translation of even bit-sized strings using a HiPE
primop that took a `Size' argument expressed in *bytes*.
ICode is not really the place to do such a thing, and moreover there
is really no reason for the HiPE primop not to take a Size argument
expressed in *bits* instead.  This commit changes the `Size' argument
to be in bits, postpones the translation of the bs_match_string primop

Fixed in a pair-programming/debugging session with @margnus1.
until RTL and does a proper translation using bit-sized quantities there.
@sverker sverker self-assigned this Nov 3, 2016
@sverker sverker added team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI labels Nov 3, 2016
@sverker
Copy link
Contributor

sverker commented Nov 3, 2016

Put in testing for 19.2.

@kostis Would you like to say something informative but not too long
about the bug preconditions for the release note?

@kostis
Copy link
Contributor Author

kostis commented Nov 3, 2016

Something like the following perhaps or is it too long?

Fix native code compilation of binary pattern matching involving bitstring segments with explicit constants. The observed behavior was erroneous pattern matches. The bug only affects code containing constant-valued segments whose size is expressed in bits; it is triggered when the pattern matching against these segments fails (i.e., when the next clause needs to be tried). Such situations are quite rare, which probably explains why this bug managed to survive for many OTP releases; most likely from the first one that introduced bitstring (as opposed to binary) pattern matching.

@OTP-Maintainer
Copy link

Patch has passed first testings and has been assigned to be reviewed


I am a script, I am not human


@kostis kostis added kanban and removed kanban labels Nov 7, 2016
@dgud dgud merged commit 359caee into erlang:maint Nov 8, 2016
@kostis kostis deleted the hipe-bs_match_string branch November 8, 2016 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants