RFR : 8227869: fix wrong format specifiers in os_aix.cpp
Baesken, Matthias
matthias.baesken at sap.com
Thu Jul 18 10:31:41 UTC 2019
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 at oracle.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
>
> 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 ppc-aix-port-dev
mailing list