On the 0x684 day of Apache Harmony Vijay Menon wrote:
> Egor,
>
> Thanks for the link! I think I see the original poster's issue & I retract
> my statement. The original example is basically identical to:
>
> tmp1 = this.hashCode
> if (tmp1 == 0) {
> // calculate hash
> this.hashCode = hash
> }
> tmp2 = this.hashCode
> return tmp2
>
>
> (You can think of tmp1 and tmp2 as internal registers introduced by the
> compiler.)
>
> The compiler is allowed to reorder the above to:
>
> tmp2 = this.hashCode
> tmp1 = this.hashCode
> if (tmp1 == 0) {
> // calculate hash without accessing this.hashCode
> this.hashCode = hash
> tmp2 = this.hashCode
> }
> return tmp2
>
>
> This is just fine in a single-threaded setting. If tmp1 is 0, tmp2 is
> always assigned the new hash. If tmp1 is non-zero, then - in the
> single-threaded code - the earlier read of tmp2 would have been non-zero as
> well.
>
> In a multithreaded setting, you can get the following:
>
> tmp2 = this.hashCode // reads zero
> // <--- another thread writes 42 to this.hashCode
> tmp1 = this.hashCode // reads 42, if clause not executed
> if (tmp1 == 0) {
> // calculate hash without accessing this.hashCode
> this.hashCode = hash
> tmp2 = this.hashCode
> }
> return tmp2 // returns zero!
>
>
> Now, you get zero. Basically, a compiler is allowed to reorder reads if it
> sees no (potentially aliasing) intervening writes.
>
> Egor: are you sure that DRLVM doesn't reorder reads to the heap? StarJIT
> did do CSE of reads to the heap (do_memopt) - which is effectively the same
> thing. Did DRLVM remove that from Jitrino?
I was not 100% correct ;) .. and should recheck this. The memopt
limitation comes in presence of virtual calls that we don't have
here. The String.hashCode() is officially dangerous again.
--
Egor Pasko
