RFR : 8227869: fix wrong format specifiers in os_aix.cpp
David Holmes
david.holmes at oracle.com
Thu Jul 18 13:08:55 UTC 2019
On 18/07/2019 9:50 pm, Baesken, Matthias wrote:
> Hi David may I add you as a reviewer too ?
Yes.
Thanks,
David
> Best regards, Matthias
>
>> -----Original Message-----
>> From: David Holmes <david.holmes at oracle.com>
>> Sent: Donnerstag, 18. Juli 2019 12:52
>> To: Doerr, Martin <martin.doerr at sap.com>; 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 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 hotspot-dev
mailing list