Skip to content

MemSet incorrect copy for native float array with -1 #1361

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

Merged
merged 2 commits into from
Aug 12, 2016

Conversation

Cellule
Copy link
Contributor

@Cellule Cellule commented Jul 30, 2016

We used to check if the memset value is -1 to determine if we can do a byte level memset.
This was valid ints but not for floats.
Changed the check to really see if the bytes are all the same.


This change is Reviewable

@Cellule
Copy link
Contributor Author

Cellule commented Jul 30, 2016

@LouisLaf @rajatd @pleath Please review

@Cellule Cellule force-pushed the memop_setsamebytes branch 2 times, most recently from 618b3fd to 4faa24a Compare August 5, 2016 01:21
@Cellule
Copy link
Contributor Author

Cellule commented Aug 5, 2016

I realized that there were other occurrences of this bug.
I made a function to gather gather them.
For the double type, I only check for 0 to do a memset as it is unlikely that all byte on a double be the same.
For Var, I don't even bother checking as it will most likely be a pointer or tagged value.
For Integer, I check if the byte are the same aka 0xabababab is valid to do a memset.

@rajatd
Copy link
Contributor

rajatd commented Aug 12, 2016

        }

remove this?


Refers to: lib/Runtime/Library/JavascriptArray.inl:1086 in 4faa24a. [](commit_id = 4faa24a, deletion_comment = False)

…a byte level memset.

This was valid ints but not for floats.
Changed the check to really see if the bytes are all the same
@Cellule Cellule force-pushed the memop_setsamebytes branch from 4faa24a to e961167 Compare August 12, 2016 01:47
@rajatd
Copy link
Contributor

rajatd commented Aug 12, 2016

:shipit:

@chakrabot chakrabot merged commit e961167 into chakra-core:master Aug 12, 2016
chakrabot pushed a commit that referenced this pull request Aug 12, 2016
…ith -1

Merge pull request #1361 from Cellule:memop_setsamebytes

We used to check if the memset value is -1 to determine if we can do a byte level memset.
This was valid ints but not for floats.
Changed the check to really see if the bytes are all the same.
@Cellule Cellule deleted the memop_setsamebytes branch August 12, 2016 02:06
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.

4 participants