RFR : 8227869: fix wrong format specifiers in os_aix.cpp
Doerr, Martin
martin.doerr at sap.com
Thu Jul 18 10:15:25 UTC 2019
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 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