Fix === and objectid of object with potentially undef inline immutable field #37557
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yuyichao
added
compiler:codegen
|
This is what led to #37511 and depends on #37511 . It came out of an attempt to reuse |
yuyichao
force-pushed
the
yyc/codegen/cmp
branch
from
3894699 to
9707be4
Compare
vtjnash
reviewed
Sep 15, 2020
src/codegen.cpp
Outdated
|
|
||
| // TODO: handle the case where arg1.typ != arg2.typ, or when one of these isn't union, | ||
| // or when the union can be pointer | ||
| if (arg1.TIndex && arg2.TIndex && arg1.typ == arg2.typ && |
There was a problem hiding this comment.
Suggested change
| if (arg1.TIndex && arg2.TIndex && arg1.typ == arg2.typ && | |
| if (arg1.TIndex && arg2.TIndex && jl_egal(arg1.typ, arg2.typ) && |
There was a problem hiding this comment.
I assume so is the assert in the callee.
vtjnash
approved these changes
Sep 15, 2020
An undef field should always be treated equal to another undef field of the same type
since there's no other way for the user to tell the difference between these.
These could previously cause inconsistent comparison results or crashes.
* Mark these types as `haspadding` so that they'll not hit the `memcmp` fast path.
* Make sure `jl_egal` and `jl_object_id_` doesn't read bits fields in undef inline immutable field
* Use `emit_getfield_knownidx` in `emit_bits_compare` so that the check can be done more easily
Handle union fields of the same type in `emit_f_isa` to avoid regression.
* Allow input to `emit_f_isa` to be NULL and lazily emit NULL check if necessary
yuyichao
force-pushed
the
yyc/codegen/cmp
branch
from
9707be4 to
ba2f681
Compare
vchuravy
pushed a commit
that referenced
this pull request
Jan 22, 2021
) An undef field should always be treated equal to another undef field of the same type since there's no other way for the user to tell the difference between these. These could previously cause inconsistent comparison results or crashes. * Mark these types as `haspadding` so that they'll not hit the `memcmp` fast path. * Make sure `jl_egal` and `jl_object_id_` doesn't read bits fields in undef inline immutable field * Use `emit_getfield_knownidx` in `emit_bits_compare` so that the check can be done more easily Handle union fields of the same type in `emit_f_isa` to avoid regression. * Allow input to `emit_f_isa` to be NULL and lazily emit NULL check if necessary (cherry picked from commit e84fec4)
vchuravy
pushed a commit
that referenced
this pull request
Jan 22, 2021
) An undef field should always be treated equal to another undef field of the same type since there's no other way for the user to tell the difference between these. These could previously cause inconsistent comparison results or crashes. * Mark these types as `haspadding` so that they'll not hit the `memcmp` fast path. * Make sure `jl_egal` and `jl_object_id_` doesn't read bits fields in undef inline immutable field * Use `emit_getfield_knownidx` in `emit_bits_compare` so that the check can be done more easily Handle union fields of the same type in `emit_f_isa` to avoid regression. * Allow input to `emit_f_isa` to be NULL and lazily emit NULL check if necessary (cherry picked from commit e84fec4)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An undef field should always be treated equal to another undef field of the same type
since there's no other way for the user to tell the difference between these.
These could previously cause inconsistent comparison results or crashes.
Mark these types as
haspaddingso that they'll not hit thememcmpfast path.Make sure
jl_egalandjl_object_id_doesn't read bits fields in undef inline immutable fieldUse
emit_getfield_knownidxinemit_bits_compareso that the check can be done more easilyHandle union fields of the same type in
emit_f_isato avoid regression.Allow input to
emit_f_isato be NULL and lazily emit NULL check if necessary