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
Hi Matthias, thanks for this tedious cleanup. Looks good to me. Best regards Christoph
-----Original Message----- From: hotspot-dev <hotspot-dev-bounces@openjdk.java.net> On Behalf Of Baesken, Matthias Sent: Mittwoch, 17. Juli 2019 17:07 To: 'hotspot-dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix-port-dev@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
Thanks ! May I get a second review please ? Best regards, Matthias
-----Original Message----- From: Langer, Christoph Sent: Mittwoch, 17. Juli 2019 18:45 To: Baesken, Matthias <matthias.baesken@sap.com>; 'hotspot- dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc-aix-port- dev@openjdk.java.net' <ppc-aix-port-dev@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@openjdk.java.net> On Behalf Of Baesken, Matthias Sent: Mittwoch, 17. Juli 2019 17:07 To: 'hotspot-dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix-port- dev@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
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@sap.com>; 'hotspot- dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc-aix-port- dev@openjdk.java.net' <ppc-aix-port-dev@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@openjdk.java.net> On Behalf Of Baesken, Matthias Sent: Mittwoch, 17. Juli 2019 17:07 To: 'hotspot-dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix-port- dev@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
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 ? In os_aix.cpp we currently get these warnings , seems PTR_FORMAT is unsigned long , that’s why we see these warnings : /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@oracle.com> Sent: Donnerstag, 18. Juli 2019 09:08 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@sap.com>; 'hotspot- dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc-aix-port- dev@openjdk.java.net' <ppc-aix-port-dev@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@openjdk.java.net> On Behalf Of Baesken, Matthias Sent: Mittwoch, 17. Juli 2019 17:07 To: 'hotspot-dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix-port- dev@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
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@oracle.com> Sent: Donnerstag, 18. Juli 2019 09:08 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@sap.com>; 'hotspot- dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc-aix-port- dev@openjdk.java.net' <ppc-aix-port-dev@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@openjdk.java.net> On Behalf Of Baesken, Matthias Sent: Mittwoch, 17. Juli 2019 17:07 To: 'hotspot-dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix-port- dev@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
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 }
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 ... Regards, Matthias
-----Original Message----- From: David Holmes <david.holmes@oracle.com> Sent: Donnerstag, 18. Juli 2019 10:05 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@oracle.com> Sent: Donnerstag, 18. Juli 2019 09:08 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@sap.com>; 'hotspot- dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc-aix- port- dev@openjdk.java.net' <ppc-aix-port-dev@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@openjdk.java.net> On Behalf Of Baesken, Matthias Sent: Mittwoch, 17. Juli 2019 17:07 To: 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix-port- dev@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
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@oracle.com> Sent: Donnerstag, 18. Juli 2019 10:05 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@oracle.com> Sent: Donnerstag, 18. Juli 2019 09:08 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@sap.com>; 'hotspot- dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc-aix- port- dev@openjdk.java.net' <ppc-aix-port-dev@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@openjdk.java.net> On Behalf Of > Baesken, Matthias > Sent: Mittwoch, 17. Juli 2019 17:07 > To: 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; > 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix-port- dev@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
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. Best regards, Martin
-----Original Message----- From: ppc-aix-port-dev <ppc-aix-port-dev-bounces@openjdk.java.net> On Behalf Of David Holmes Sent: Donnerstag, 18. Juli 2019 12:04 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@oracle.com> Sent: Donnerstag, 18. Juli 2019 10:05 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@oracle.com> >>>> Sent: Donnerstag, 18. Juli 2019 09:08 >>>> To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph >>>> <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- >>>> dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc- aix- >>>> port-dev@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@sap.com>; 'hotspot- >>>>>> dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc-aix- >> port- >>>>>> dev@openjdk.java.net' <ppc-aix-port-dev@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@openjdk.java.net> On >>>> Behalf >>>>>> Of >>>>>>> Baesken, Matthias >>>>>>> Sent: Mittwoch, 17. Juli 2019 17:07 >>>>>>> To: 'hotspot-dev@openjdk.java.net' <hotspot- >> dev@openjdk.java.net>; >>>>>>> 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix-port- >>>>>> dev@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
Hi Martin, thanks for your input ! So I think PTR_FORMAT and p2i is okay . Do you have other concerns about 8227869 ? may I ad you as a reviewer ? Best regards, Matthias
-----Original Message----- From: Doerr, Martin Sent: Donnerstag, 18. Juli 2019 12:15 To: David Holmes <david.holmes@oracle.com>; Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@openjdk.java.net> Subject: RE: RFR : 8227869: fix wrong format specifiers in os_aix.cpp
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.
Best regards, Martin
-----Original Message----- From: ppc-aix-port-dev <ppc-aix-port-dev-bounces@openjdk.java.net> On Behalf Of David Holmes Sent: Donnerstag, 18. Juli 2019 12:04 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@oracle.com> Sent: Donnerstag, 18. Juli 2019 10:05 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer,
Christoph
<christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc- aix- port-dev@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@oracle.com> Sent: Donnerstag, 18. Juli 2019 09:08 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer,
Christoph
<christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc- aix- port-dev@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@sap.com>; 'hotspot- >> dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc- aix- port- >> dev@openjdk.java.net' <ppc-aix-port-dev@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@openjdk.java.net> On Behalf >> Of >>> Baesken, Matthias >>> Sent: Mittwoch, 17. Juli 2019 17:07 >>> To: 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; >>> 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix-port- >> dev@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
Hi Matthias, You can add me as reviewer. Looks good to me. Only indentation could be improved. But I don't need to see another webrev for that. Best regards, Martin
-----Original Message----- From: Baesken, Matthias Sent: Donnerstag, 18. Juli 2019 12:32 To: Doerr, Martin <martin.doerr@sap.com>; David Holmes <david.holmes@oracle.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@openjdk.java.net> Subject: RE: RFR : 8227869: fix wrong format specifiers in os_aix.cpp
Hi Martin, thanks for your input !
So I think PTR_FORMAT and p2i is okay .
Do you have other concerns about 8227869 ? may I ad you as a reviewer ?
Best regards, Matthias
-----Original Message----- From: Doerr, Martin Sent: Donnerstag, 18. Juli 2019 12:15 To: David Holmes <david.holmes@oracle.com>; Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@openjdk.java.net> Subject: RE: RFR : 8227869: fix wrong format specifiers in os_aix.cpp
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.
Best regards, Martin
-----Original Message----- From: ppc-aix-port-dev <ppc-aix-port-dev-bounces@openjdk.java.net> On Behalf Of David Holmes Sent: Donnerstag, 18. Juli 2019 12:04 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@oracle.com> Sent: Donnerstag, 18. Juli 2019 10:05 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer,
Christoph
<christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc- aix- port-dev@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@oracle.com> > Sent: Donnerstag, 18. Juli 2019 09:08 > To: Baesken, Matthias <matthias.baesken@sap.com>; Langer,
Christoph
> <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- > dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc- aix- > port-dev@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@sap.com>; 'hotspot- >>> dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc- aix- port- >>> dev@openjdk.java.net' <ppc-aix-port-dev@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@openjdk.java.net> On > Behalf >>> Of >>>> Baesken, Matthias >>>> Sent: Mittwoch, 17. Juli 2019 17:07 >>>> To: 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; >>>> 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix-port- >>> dev@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
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@openjdk.java.net> On Behalf Of David Holmes Sent: Donnerstag, 18. Juli 2019 12:04 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@oracle.com> Sent: Donnerstag, 18. Juli 2019 10:05 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@oracle.com> >>>> Sent: Donnerstag, 18. Juli 2019 09:08 >>>> To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph >>>> <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- >>>> dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc- aix- >>>> port-dev@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@sap.com>; 'hotspot- >>>>>> dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc-aix- >> port- >>>>>> dev@openjdk.java.net' <ppc-aix-port-dev@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@openjdk.java.net> On >>>> Behalf >>>>>> Of >>>>>>> Baesken, Matthias >>>>>>> Sent: Mittwoch, 17. Juli 2019 17:07 >>>>>>> To: 'hotspot-dev@openjdk.java.net' <hotspot- >> dev@openjdk.java.net>; >>>>>>> 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix-port- >>>>>> dev@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
Hi David may I add you as a reviewer too ? Best regards, Matthias
-----Original Message----- From: David Holmes <david.holmes@oracle.com> Sent: Donnerstag, 18. Juli 2019 12:52 To: Doerr, Martin <martin.doerr@sap.com>; Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@openjdk.java.net> On Behalf Of David Holmes Sent: Donnerstag, 18. Juli 2019 12:04 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@oracle.com> Sent: Donnerstag, 18. Juli 2019 10:05 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer,
Christoph
<christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc- aix- port-dev@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@oracle.com> >>>>>> Sent: Donnerstag, 18. Juli 2019 09:08 >>>>>> To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, >> Christoph >>>>>> <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' >> <hotspot- >>>>>> dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc- >> aix- >>>>>> port-dev@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@sap.com>; 'hotspot- >>>>>>>> dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc- aix- >>>> port- >>>>>>>> dev@openjdk.java.net' <ppc-aix-port-dev@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@openjdk.java.net> On >>>>>> Behalf >>>>>>>> Of >>>>>>>>> Baesken, Matthias >>>>>>>>> Sent: Mittwoch, 17. Juli 2019 17:07 >>>>>>>>> To: 'hotspot-dev@openjdk.java.net' <hotspot- >>>> dev@openjdk.java.net>; >>>>>>>>> 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix-port- >>>>>>>> dev@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
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@oracle.com> Sent: Donnerstag, 18. Juli 2019 12:52 To: Doerr, Martin <martin.doerr@sap.com>; Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@openjdk.java.net> On Behalf Of David Holmes Sent: Donnerstag, 18. Juli 2019 12:04 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, Christoph <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix- port-dev@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@oracle.com> Sent: Donnerstag, 18. Juli 2019 10:05 To: Baesken, Matthias <matthias.baesken@sap.com>; Langer,
Christoph
<christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' <hotspot- dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc- aix- port-dev@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@oracle.com> >>>>>> Sent: Donnerstag, 18. Juli 2019 09:08 >>>>>> To: Baesken, Matthias <matthias.baesken@sap.com>; Langer, >> Christoph >>>>>> <christoph.langer@sap.com>; 'hotspot-dev@openjdk.java.net' >> <hotspot- >>>>>> dev@openjdk.java.net>; 'ppc-aix-port-dev@openjdk.java.net' <ppc- >> aix- >>>>>> port-dev@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@sap.com>; 'hotspot- >>>>>>>> dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; 'ppc- aix- >>>> port- >>>>>>>> dev@openjdk.java.net' <ppc-aix-port-dev@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@openjdk.java.net> On >>>>>> Behalf >>>>>>>> Of >>>>>>>>> Baesken, Matthias >>>>>>>>> Sent: Mittwoch, 17. Juli 2019 17:07 >>>>>>>>> To: 'hotspot-dev@openjdk.java.net' <hotspot- >>>> dev@openjdk.java.net>; >>>>>>>>> 'ppc-aix-port-dev@openjdk.java.net' <ppc-aix-port- >>>>>>>> dev@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
participants (4)
-
Baesken, Matthias
-
David Holmes
-
Doerr, Martin
-
Langer, Christoph