[Webkit-unassigned] [Bug 214142] [WTF] Fix PackedAlignedPtr for X86_64 canonical addresses

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=214142

--- Comment #17 from Jim Mason <jmason at ibinx.com> ---
(In reply to Mark Lam from comment #16)
> (In reply to Jim Mason from comment #15)
> > (In reply to Yusuke Suzuki from comment #14)
> > > Comment on attachment 403897 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=403897&action=review
> > > 
> > > > Source/WTF/wtf/Packed.h:147
> > > > +#if CPU(X86_64)
> > > 
> > > When doing that, please do it only in Solaris.
> > 
> > I was planning to make the changes requested by Mark tomorrow, but the whole
> > point of this patch is to provide support for X86_64 canonical addressing in
> > webkit.  It is not just a Solaris thing.
> > 
> > If webkit does not want to do the right thing for X86_64, let me know and I
> > will  just close the bug.
> 
> Just to clarify.  I think Yusuke's point is not that this is strictly
> incorrect, but that it is unnecessary for any other OS than Solaris.  In
> WebKit, we do a lot of work to make sure the code is optimal.  While this
> extra work does not hurt in terms of correctness, it is guaranteed to be an
> effective no-op on any OS that is not Solaris because for other OSes, user
> space never has an address with a set bit 47.  So, it is not incorrect to
> omit this extra code for any OS that is not Solaris.  Yusuke's concern here
> is with regards to performance.

With -O2 or -O3 in gcc, the test becomes two instructions, a bit test and a jump.  Probably similar in other compilers.  In the PackedAlignedPtr getter, there is already a test -- if (isAlignmentShiftProfitable) -- that is probably a no op on most every platform.  This test is similarly trivial.

The reason I submit this patch is that an operating system can map memory however it likes.  If webkit does the right thing, it ensures every OS will work correctly, no matter what.  As it stands now, the operating system has to make 'the right choices' based on the current assumptions, or webkit will fail, which means someone has to chase bugs down.

As you pointed out, the current TEST(WTF_Packed, AssignAndGet) is testing an address that is *invalid* for the X86_64 processor architecture.  And in the web assembly code, a signed value is being used for a pointer test.  Cleaning up these sorts of things helps make webkit robust.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200709/1a3c8ca8/attachment.htm>