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

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

On 18/07/2019 8:15 pm, Doerr, Martin wrote:
> Hi David,
> 
> there's no difference between INTPTR_FORMAT and PTR_FORMAT:
> 
> #ifdef  _LP64
> #define INTPTR_FORMAT "0x%016" PRIxPTR
> #define PTR_FORMAT    "0x%016" PRIxPTR
> #else   // !_LP64
> #define INTPTR_FORMAT "0x%08"  PRIxPTR
> #define PTR_FORMAT    "0x%08"  PRIxPTR
> #endif  // _LP64
> 
> I guess this was different in the past. I don't know why we still have both.

Sorry about that - was confused by the reported error message.

David

> Best regards,
> Martin
> 
> 
>> -----Original Message-----
>> From: ppc-aix-port-dev <ppc-aix-port-dev-bounces at openjdk.java.net> On
>> Behalf Of David Holmes
>> Sent: Donnerstag, 18. Juli 2019 12:04
>> 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 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 ppc-aix-port-dev mailing list