RFR : 8227869: fix wrong format specifiers in os_aix.cpp

David Holmes david.holmes at oracle.com
Thu Jul 18 10:03:58 UTC 2019 

On 18/07/2019 6:25 pm, Baesken, Matthias wrote:
> Hi David, do you see an issue  using  p2i   with char* pointers ,  should I add a cast or some other conversion ?
> (afaik it is usually used without other casts/conversions in the codebase)
> 
> jdk/src/hotspot/share/utilities/globalDefinitions.hpp  :
> 
> 1055 // Convert pointer to intptr_t, for use in printing pointers.
> 1056 inline intptr_t p2i(const void * p) {
> 1057  return (intptr_t) p;
> 1058 }

p2i is what you should always use when printing a pointer to convert it 
to an integral type. But it should really be used with INTPTR_FORMAT. It 
will work with PTR_FORMAT due to other integral conversions.

>> If this fixes things on AIX then that's fine.
> 
> Yes it does .
> But I have to agree with you it feels a bit shaky  ...

Changing PTR_FORMAT to INTPTR_FORMAT would remove that shakiness IMHO. :)

Cheers,
David

> 
> Regards, Matthias
> 
> 
> 
>> -----Original Message-----
>> From: David Holmes <david.holmes at oracle.com>
>> Sent: Donnerstag, 18. Juli 2019 10:05
>> To: Baesken, Matthias <matthias.baesken at sap.com>; Langer, Christoph
>> <christoph.langer at sap.com>; 'hotspot-dev at openjdk.java.net' <hotspot-
>> dev at openjdk.java.net>; 'ppc-aix-port-dev at openjdk.java.net' <ppc-aix-
>> port-dev at openjdk.java.net>
>> Subject: Re: RFR : 8227869: fix wrong format specifiers in os_aix.cpp
>>
>> On 18/07/2019 5:40 pm, Baesken, Matthias wrote:
>>>> pointers should be used with PTR_FORMAT. p2i(p) should be used with
>>>> INTPTR_FORMAT. So the above looks like it was already correct and now
>> is
>>>> not correct.
>>>
>>> Hi David,    I noticed  p2i   is used  together  with  PTR_FORMAT  at  dozens
>> locations in the HS code ,  did I miss something ?
>>
>> Okay our usage is a bit of a historical mess. :(
>>
>>> In os_aix.cpp   we currently get these warnings ,     seems  PTR_FORMAT  is
>> unsigned  long     ,   that’s why  we see these warnings  :
>>
>> Defining PTR_FORMAT as an integral format it just broken - but dates
>> back forever because %p wasn't portable.
>>
>> If this fixes things on AIX then that's fine. For new code I'd recommend
>> use of INTPTR_FORMAT and p2i to print pointers.
>>
>> Thanks,
>> David
>>
>>>
>>> /nightly/jdk/src/hotspot/os/aix/os_aix.cpp:1894:15: warning: format
>> specifies type 'unsigned long' but the argument has type 'char *' [-Wformat]
>>>                 p, p + s, addr, addr + size);
>>> ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> /nightly/jdk/src/hotspot/os/aix/misc_aix.hpp:40:28: note: expanded from
>> macro 'trcVerbose'
>>>       fprintf(stderr, fmt, ##__VA_ARGS__); \
>>>                              ^~~~~~~~~~~
>>> /nightly/jdk/src/hotspot/os/aix/os_aix.cpp:1894:18: warning: format
>> specifies type 'unsigned long' but the argument has type 'char *' [-Wformat]
>>>                 p, p + s, addr, addr + size);
>>> ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
>>> /nightly/jdk/src/hotspot/os/aix/misc_aix.hpp:40:28: note: expanded from
>> macro 'trcVerbose'
>>>       fprintf(stderr, fmt, ##__VA_ARGS__); \
>>>                              ^~~~~~~~~~~
>>> /nightly/jdk/src/hotspot/os/aix/os_aix.cpp:1894:25: warning: format
>> specifies type 'unsigned long' but the argument has type 'char *' [-Wformat]
>>>                 p, p + s, addr, addr + size);
>>> ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
>>> /nightly/jdk/src/hotspot/os/aix/misc_aix.hpp:40:28: note: expanded from
>> macro 'trcVerbose'
>>>       fprintf(stderr, fmt, ##__VA_ARGS__); \
>>>                              ^~~~~~~~~~~
>>> /nightly/jdk/src/hotspot/os/aix/os_aix.cpp:1894:31: warning: format
>> specifies type 'unsigned long' but the argument has type 'char *' [-Wformat]
>>>                 p, p + s, addr, addr + size);
>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
>>> /nightly/jdk/src/hotspot/os/aix/misc_aix.hpp:40:28: note: expanded from
>> macro 'trcVerbose'
>>>       fprintf(stderr, fmt, ##__VA_ARGS__); \
>>>                              ^~~~~~~~~~~
>>> /nightly/jdk/src/hotspot/os/aix/os_aix.cpp:1899:45: warning: format
>> specifies type 'unsigned long' but the argument has type 'char *' [-Wformat]
>>>                 " aligned to pagesize (%lu)", p, p + s, (unsigned long) pagesize);
>>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
>> ~~~~~~~~~~~~~~~~~~~~
>>> /nightly/jdk/src/hotspot/os/aix/misc_aix.hpp:40:28: note: expanded from
>> macro 'trcVerbose'
>>>       fprintf(stderr, fmt, ##__VA_ARGS__); \
>>>                              ^~~~~~~~~~~
>>> /nightly/jdk/src/hotspot/os/aix/os_aix.cpp:1899:48: warning: format
>> specifies type 'unsigned long' but the argument has type 'char *' [-Wformat]
>>>                 " aligned to pagesize (%lu)", p, p + s, (unsigned long) pagesize);
>>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
>> ~~~~~~~~~~~~~~~~~~~~
>>>
>>> Best regards, Matthias
>>>
>>>
>>>> -----Original Message-----
>>>> From: David Holmes <david.holmes at oracle.com>
>>>> Sent: Donnerstag, 18. Juli 2019 09:08
>>>> To: Baesken, Matthias <matthias.baesken at sap.com>; Langer, Christoph
>>>> <christoph.langer at sap.com>; 'hotspot-dev at openjdk.java.net' <hotspot-
>>>> dev at openjdk.java.net>; 'ppc-aix-port-dev at openjdk.java.net' <ppc-aix-
>>>> port-dev at openjdk.java.net>
>>>> Subject: Re: RFR : 8227869: fix wrong format specifiers in os_aix.cpp
>>>>
>>>> Hi Matthias,
>>>>
>>>> On 18/07/2019 5:00 pm, Baesken, Matthias wrote:
>>>>> Thanks !  May I get a second review please ?
>>>>
>>>> @@ -1888,12 +1887,12 @@
>>>>         if (!contains_range(p, s)) {
>>>>           trcVerbose("[" PTR_FORMAT " - " PTR_FORMAT "] is not a sub "
>>>>                   "range of [" PTR_FORMAT " - " PTR_FORMAT "].",
>>>> -              p, p + s, addr, addr + size);
>>>> +              p2i(p), p2i(p + s), p2i(addr), p2i(addr + size));
>>>>
>>>> pointers should be used with PTR_FORMAT. p2i(p) should be used with
>>>> INTPTR_FORMAT. So the above looks like it was already correct and now
>> is
>>>> not correct. Using p2i with UINTX_FORMAT also looks dubious to me.
>>>>
>>>> Cheers,
>>>> David
>>>> -----
>>>>
>>>>> Best regards, Matthias
>>>>>
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Langer, Christoph
>>>>>> Sent: Mittwoch, 17. Juli 2019 18:45
>>>>>> To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
>>>>>> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>; 'ppc-aix-
>> port-
>>>>>> dev at openjdk.java.net' <ppc-aix-port-dev at openjdk.java.net>
>>>>>> Subject: RE: RFR : 8227869: fix wrong format specifiers in os_aix.cpp
>>>>>>
>>>>>> Hi Matthias,
>>>>>>
>>>>>> thanks for this tedious cleanup. Looks good to me.
>>>>>>
>>>>>> Best regards
>>>>>> Christoph
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On
>>>> Behalf
>>>>>> Of
>>>>>>> Baesken, Matthias
>>>>>>> Sent: Mittwoch, 17. Juli 2019 17:07
>>>>>>> To: 'hotspot-dev at openjdk.java.net' <hotspot-
>> dev at openjdk.java.net>;
>>>>>>> 'ppc-aix-port-dev at openjdk.java.net' <ppc-aix-port-
>>>>>> dev at openjdk.java.net>
>>>>>>> Subject: RFR : 8227869: fix wrong format specifiers in os_aix.cpp
>>>>>>>
>>>>>>> Hello, there are a couple of non matching  format specifiers in
>> os_aix.cpp
>>>> .
>>>>>>> I adjust them with my change .
>>>>>>>
>>>>>>> Please review !
>>>>>>>
>>>>>>> Bug/webrev :
>>>>>>>
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8227869
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8227869.0/
>>>>>>>
>>>>>>> Thanks, Matthias

More information about the hotspot-dev mailing list