While running acid3.acidtests.org on a ARMv5 processor a ldr r8,[r1, #7] will be generated This instruction should be ldrb r8,[r1, #7] The problem code is the set8 and setTest8 implementations call load32 where they should be calling load8.
Created attachment 68102 [details] Fix alignment failures
Good fix. Could you add a common function for the common code of setTest8 and setTest32 to avoid code duplication?
Created attachment 68235 [details] Add new setTest32 with RegisterID parameter
Comment on attachment 68235 [details] Add new setTest32 with RegisterID parameter Have you tested perf impact of this on non-armv5?
(In reply to comment #4) > (From update of attachment 68235 [details]) > Have you tested perf impact of this on non-armv5? No I didn't measure a pref impact. The old code was just wrong; as the fill on the register would be wrong because it would be using data around the address instead of letting the core fill in zeros into the register in top 3 bytes. So one could expect an incorrect branch occasionally and possible data exposure.
This seems like an easy patch for gbarra to review.
Comment on attachment 68235 [details] Add new setTest32 with RegisterID parameter Hi David, These methods had been rather poorly named, and it was rather unclear what they were doing. I've hopefully just cleared this up a little: https://bugs.webkit.org/show_bug.cgi?id=50509 Looking at your patch, I think your changes to (the method previously known as) setTest8 are correct, but the change to set8 is incorrect, and should be reverted. Please update & check your change. cheers, G.
Created attachment 80657 [details] Re-apply patch to set32Test8 as per comments Applied patch as per comments.
Comment on attachment 80657 [details] Re-apply patch to set32Test8 as per comments Looks great David.
Comment on attachment 80657 [details] Re-apply patch to set32Test8 as per comments Clearing flags on attachment: 80657 Committed r77248: <http://trac.webkit.org/changeset/77248>
All reviewed patches have been landed. Closing bug.