RFR: 8150426: Wrong cast in metadata_at_put
Coleen Phillimore
coleen.phillimore at oracle.com
Tue Feb 23 15:40:57 UTC 2016
yes, this looks good. As the bug states, there are no uses of this
function anymore but I may need to repurpose it for a change and would
hate to run into this bug again.
Thank you Timo for the contribution.
Coleen
On 2/22/16 10:49 PM, Kim Barrett wrote:
>> On Feb 22, 2016, at 9:36 PM, David Holmes <david.holmes at oracle.com> wrote:
>> On 23/02/2016 11:57 AM, Kim Barrett wrote:
>>> Please review this fix of an incorrect cast.
>>>
>>> This fix was contributed by Timo Kinnunen.
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8150426
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~kbarrett/8150426/webrev/
>> Fix seems fine, but have to wonder given:
>>
>> 128 #ifdef _LP64
>> 129 Metadata* metadata_at(int which) const {
>> 130 return (Metadata*)*long_at_addr(which); }
>> 131 void metadata_at_put(int which, Metadata* contents) {
>> 132 *long_at_addr(which) = (jlong)contents;
>> 133 }
>> 134 #else
>>
>> is Windows setting _LP64 even though it is LLP64? Or are we (incorrectly) forcing _LP64 for all 64-bit builds?
> Apparently yes.
>
> Coleen wasted a bunch of time on this very same bug a couple of months
> ago, when she used metadata_at and metadata_at_put as a starting point
> for something she was working on. I helped her track that down, so I
> was already familiar with this problem when Timo reported it.
>
> egrep -r -H --exclude-dir=.hg LP64 | egrep -v "\.(h|c|hpp|cpp|xml):" | grep windows
>
> produces around 15 hits in hotspot/make/windows and common/autoconf,
> though I think the autoconf hits don't affect windows, so it's only
> those in hotspot that are interesting.
>
> There is only one occurrence of LLP64 in the JDK forest, in a comment here:
> ./jdk/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/wrapper/PKCS11Constants.java: // This means it is 0xffffffff in ILP32/LLP64 but 0xffffffffffffffff in LP64.
>
> I have a *very* strong memory (from soon after I started working here)
> of seeing a comment or document somewhere that described why we're
> building Windows with _LP64, but haven't been able to re-find it. I
> know I was surprised at the time.
>
> I also wonder whether this still a problem?
> https://bugs.openjdk.java.net/browse/JDK-7000871
> _LP64 is not always set up correctly
>
> It looks like, in the current jdk9 forest, _LP64 is only set by the
> build system, never by headers. I think there might be a couple more
> open bugs related to LP64 that could be closed.
>
>
>
More information about the hotspot-runtime-dev
mailing list