RFR : 8227869: fix wrong format specifiers in os_aix.cpp
Baesken, Matthias
matthias.baesken at sap.com
Thu Jul 18 11:50:15 UTC 2019
Hi David may I add you as a reviewer too ?
Best regards, Matthias
> -----Original Message-----
> From: David Holmes <david.holmes at oracle.com>
> Sent: Donnerstag, 18. Juli 2019 12:52
> To: Doerr, Martin <martin.doerr at sap.com>; Baesken, Matthias
> <matthias.baesken at sap.com>; Langer, Christoph
> <christoph.langer at sap.com>; 'hotspot-dev at openjdk.java.net' <hotspot-
> dev at openjdk.java.net>; 'ppc-aix-port-dev at openjdk.java.net' <ppc-aix-
> port-dev at openjdk.java.net>
> Subject: Re: RFR : 8227869: fix wrong format specifiers in os_aix.cpp
>
> On 18/07/2019 8:15 pm, Doerr, Martin wrote:
> > Hi David,
> >
> > there's no difference between INTPTR_FORMAT and PTR_FORMAT:
> >
> > #ifdef _LP64
> > #define INTPTR_FORMAT "0x%016" PRIxPTR
> > #define PTR_FORMAT "0x%016" PRIxPTR
> > #else // !_LP64
> > #define INTPTR_FORMAT "0x%08" PRIxPTR
> > #define PTR_FORMAT "0x%08" PRIxPTR
> > #endif // _LP64
> >
> > I guess this was different in the past. I don't know why we still have both.
>
> Sorry about that - was confused by the reported error message.
>
> David
>
> > Best regards,
> > Martin
> >
> >
> >> -----Original Message-----
> >> From: ppc-aix-port-dev <ppc-aix-port-dev-bounces at openjdk.java.net>
> On
> >> Behalf Of David Holmes
> >> Sent: Donnerstag, 18. Juli 2019 12:04
> >> To: Baesken, Matthias <matthias.baesken at sap.com>; Langer, Christoph
> >> <christoph.langer at sap.com>; 'hotspot-dev at openjdk.java.net' <hotspot-
> >> dev at openjdk.java.net>; 'ppc-aix-port-dev at openjdk.java.net' <ppc-aix-
> >> port-dev at openjdk.java.net>
> >> Subject: Re: RFR : 8227869: fix wrong format specifiers in os_aix.cpp
> >>
> >> On 18/07/2019 6:25 pm, Baesken, Matthias wrote:
> >>> Hi David, do you see an issue using p2i with char* pointers , should I
> add
> >> a cast or some other conversion ?
> >>> (afaik it is usually used without other casts/conversions in the codebase)
> >>>
> >>> jdk/src/hotspot/share/utilities/globalDefinitions.hpp :
> >>>
> >>> 1055 // Convert pointer to intptr_t, for use in printing pointers.
> >>> 1056 inline intptr_t p2i(const void * p) {
> >>> 1057 return (intptr_t) p;
> >>> 1058 }
> >>
> >> p2i is what you should always use when printing a pointer to convert it
> >> to an integral type. But it should really be used with INTPTR_FORMAT. It
> >> will work with PTR_FORMAT due to other integral conversions.
> >>
> >>>> If this fixes things on AIX then that's fine.
> >>>
> >>> Yes it does .
> >>> But I have to agree with you it feels a bit shaky ...
> >>
> >> Changing PTR_FORMAT to INTPTR_FORMAT would remove that shakiness
> >> IMHO. :)
> >>
> >> Cheers,
> >> David
> >>
> >>>
> >>> Regards, Matthias
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: David Holmes <david.holmes at oracle.com>
> >>>> Sent: Donnerstag, 18. Juli 2019 10:05
> >>>> To: Baesken, Matthias <matthias.baesken at sap.com>; Langer,
> Christoph
> >>>> <christoph.langer at sap.com>; 'hotspot-dev at openjdk.java.net'
> <hotspot-
> >>>> dev at openjdk.java.net>; 'ppc-aix-port-dev at openjdk.java.net' <ppc-
> aix-
> >>>> port-dev at openjdk.java.net>
> >>>> Subject: Re: RFR : 8227869: fix wrong format specifiers in os_aix.cpp
> >>>>
> >>>> On 18/07/2019 5:40 pm, Baesken, Matthias wrote:
> >>>>>> pointers should be used with PTR_FORMAT. p2i(p) should be used
> with
> >>>>>> INTPTR_FORMAT. So the above looks like it was already correct and
> >> now
> >>>> is
> >>>>>> not correct.
> >>>>>
> >>>>> Hi David, I noticed p2i is used together with PTR_FORMAT at
> >> dozens
> >>>> locations in the HS code , did I miss something ?
> >>>>
> >>>> Okay our usage is a bit of a historical mess. :(
> >>>>
> >>>>> In os_aix.cpp we currently get these warnings , seems
> PTR_FORMAT
> >> is
> >>>> unsigned long , that’s why we see these warnings :
> >>>>
> >>>> Defining PTR_FORMAT as an integral format it just broken - but dates
> >>>> back forever because %p wasn't portable.
> >>>>
> >>>> If this fixes things on AIX then that's fine. For new code I'd recommend
> >>>> use of INTPTR_FORMAT and p2i to print pointers.
> >>>>
> >>>> Thanks,
> >>>> David
> >>>>
> >>>>>
> >>>>> /nightly/jdk/src/hotspot/os/aix/os_aix.cpp:1894:15: warning: format
> >>>> specifies type 'unsigned long' but the argument has type 'char *' [-
> >> Wformat]
> >>>>> p, p + s, addr, addr + size);
> >>>>> ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>>> /nightly/jdk/src/hotspot/os/aix/misc_aix.hpp:40:28: note: expanded
> >> from
> >>>> macro 'trcVerbose'
> >>>>> fprintf(stderr, fmt, ##__VA_ARGS__); \
> >>>>> ^~~~~~~~~~~
> >>>>> /nightly/jdk/src/hotspot/os/aix/os_aix.cpp:1894:18: warning: format
> >>>> specifies type 'unsigned long' but the argument has type 'char *' [-
> >> Wformat]
> >>>>> p, p + s, addr, addr + size);
> >>>>> ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
> >>>>> /nightly/jdk/src/hotspot/os/aix/misc_aix.hpp:40:28: note: expanded
> >> from
> >>>> macro 'trcVerbose'
> >>>>> fprintf(stderr, fmt, ##__VA_ARGS__); \
> >>>>> ^~~~~~~~~~~
> >>>>> /nightly/jdk/src/hotspot/os/aix/os_aix.cpp:1894:25: warning: format
> >>>> specifies type 'unsigned long' but the argument has type 'char *' [-
> >> Wformat]
> >>>>> p, p + s, addr, addr + size);
> >>>>> ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
> >>>>> /nightly/jdk/src/hotspot/os/aix/misc_aix.hpp:40:28: note: expanded
> >> from
> >>>> macro 'trcVerbose'
> >>>>> fprintf(stderr, fmt, ##__VA_ARGS__); \
> >>>>> ^~~~~~~~~~~
> >>>>> /nightly/jdk/src/hotspot/os/aix/os_aix.cpp:1894:31: warning: format
> >>>> specifies type 'unsigned long' but the argument has type 'char *' [-
> >> Wformat]
> >>>>> p, p + s, addr, addr + size);
> >>>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> >>>>> /nightly/jdk/src/hotspot/os/aix/misc_aix.hpp:40:28: note: expanded
> >> from
> >>>> macro 'trcVerbose'
> >>>>> fprintf(stderr, fmt, ##__VA_ARGS__); \
> >>>>> ^~~~~~~~~~~
> >>>>> /nightly/jdk/src/hotspot/os/aix/os_aix.cpp:1899:45: warning: format
> >>>> specifies type 'unsigned long' but the argument has type 'char *' [-
> >> Wformat]
> >>>>> " aligned to pagesize (%lu)", p, p + s, (unsigned long)
> pagesize);
> >>>>>
> >>>>
> >>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
> >>>> ~~~~~~~~~~~~~~~~~~~~
> >>>>> /nightly/jdk/src/hotspot/os/aix/misc_aix.hpp:40:28: note: expanded
> >> from
> >>>> macro 'trcVerbose'
> >>>>> fprintf(stderr, fmt, ##__VA_ARGS__); \
> >>>>> ^~~~~~~~~~~
> >>>>> /nightly/jdk/src/hotspot/os/aix/os_aix.cpp:1899:48: warning: format
> >>>> specifies type 'unsigned long' but the argument has type 'char *' [-
> >> Wformat]
> >>>>> " aligned to pagesize (%lu)", p, p + s, (unsigned long)
> pagesize);
> >>>>>
> >>>>
> >>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> >>>> ~~~~~~~~~~~~~~~~~~~~
> >>>>>
> >>>>> Best regards, Matthias
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: David Holmes <david.holmes at oracle.com>
> >>>>>> Sent: Donnerstag, 18. Juli 2019 09:08
> >>>>>> To: Baesken, Matthias <matthias.baesken at sap.com>; Langer,
> >> Christoph
> >>>>>> <christoph.langer at sap.com>; 'hotspot-dev at openjdk.java.net'
> >> <hotspot-
> >>>>>> dev at openjdk.java.net>; 'ppc-aix-port-dev at openjdk.java.net'
> <ppc-
> >> aix-
> >>>>>> port-dev at openjdk.java.net>
> >>>>>> Subject: Re: RFR : 8227869: fix wrong format specifiers in os_aix.cpp
> >>>>>>
> >>>>>> Hi Matthias,
> >>>>>>
> >>>>>> On 18/07/2019 5:00 pm, Baesken, Matthias wrote:
> >>>>>>> Thanks ! May I get a second review please ?
> >>>>>>
> >>>>>> @@ -1888,12 +1887,12 @@
> >>>>>> if (!contains_range(p, s)) {
> >>>>>> trcVerbose("[" PTR_FORMAT " - " PTR_FORMAT "] is not a sub "
> >>>>>> "range of [" PTR_FORMAT " - " PTR_FORMAT "].",
> >>>>>> - p, p + s, addr, addr + size);
> >>>>>> + p2i(p), p2i(p + s), p2i(addr), p2i(addr + size));
> >>>>>>
> >>>>>> pointers should be used with PTR_FORMAT. p2i(p) should be used
> with
> >>>>>> INTPTR_FORMAT. So the above looks like it was already correct and
> >> now
> >>>> is
> >>>>>> not correct. Using p2i with UINTX_FORMAT also looks dubious to
> me.
> >>>>>>
> >>>>>> Cheers,
> >>>>>> David
> >>>>>> -----
> >>>>>>
> >>>>>>> Best regards, Matthias
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>> -----Original Message-----
> >>>>>>>> From: Langer, Christoph
> >>>>>>>> Sent: Mittwoch, 17. Juli 2019 18:45
> >>>>>>>> To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
> >>>>>>>> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>; 'ppc-
> aix-
> >>>> port-
> >>>>>>>> dev at openjdk.java.net' <ppc-aix-port-dev at openjdk.java.net>
> >>>>>>>> Subject: RE: RFR : 8227869: fix wrong format specifiers in
> os_aix.cpp
> >>>>>>>>
> >>>>>>>> Hi Matthias,
> >>>>>>>>
> >>>>>>>> thanks for this tedious cleanup. Looks good to me.
> >>>>>>>>
> >>>>>>>> Best regards
> >>>>>>>> Christoph
> >>>>>>>>
> >>>>>>>>> -----Original Message-----
> >>>>>>>>> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net>
> On
> >>>>>> Behalf
> >>>>>>>> Of
> >>>>>>>>> Baesken, Matthias
> >>>>>>>>> Sent: Mittwoch, 17. Juli 2019 17:07
> >>>>>>>>> To: 'hotspot-dev at openjdk.java.net' <hotspot-
> >>>> dev at openjdk.java.net>;
> >>>>>>>>> 'ppc-aix-port-dev at openjdk.java.net' <ppc-aix-port-
> >>>>>>>> dev at openjdk.java.net>
> >>>>>>>>> Subject: RFR : 8227869: fix wrong format specifiers in os_aix.cpp
> >>>>>>>>>
> >>>>>>>>> Hello, there are a couple of non matching format specifiers in
> >>>> os_aix.cpp
> >>>>>> .
> >>>>>>>>> I adjust them with my change .
> >>>>>>>>>
> >>>>>>>>> Please review !
> >>>>>>>>>
> >>>>>>>>> Bug/webrev :
> >>>>>>>>>
> >>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8227869
> >>>>>>>>>
> >>>>>>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8227869.0/
> >>>>>>>>>
> >>>>>>>>> Thanks, Matthias
More information about the hotspot-dev
mailing list