On Wed, 2 Aug 2023 01:31:43 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
> Replace SIZE_FORMAT with %zu and SIZE_FORMAT_X with 0x%zx using a 4 line sed
> script:
>
> sed -e 's/" SIZE_FORMAT "/%zu/g' -e 's/" SIZE_FORMAT,/%zu",/' \
> -e 's/print(SIZE_FORMAT "/print("%zu/g' \
> -e 's/" SIZE_FORMAT_X "/0x%zx/g' -e 's/" SIZE_FORMAT_X,/0x%zx",/g' \
> -e 's/" SIZE_FORMAT$/%zu"/' \
> $f >$f.new
>
> Minor fixups afterwards. I didn't change SIZE_FORMAT_W(number) because it
> would have required a better sed script.
>
> The definitions in globalDefinitions.hpp can be removed with a later PR, once
> all instances are cleaned out. This is partial so that people start using
> %zu instead.
>
> Tested with tier1 on Oracle supported platforms.
I actually agree with using `%z` for this, but this seems an unusual way to
approach it. The `SIZE_FORMAT` is an abstraction which allows the actual format
specifier to be anything defined in `globalDefinitions.hpp`. Removing it to
hardcode a format thus seems like a bit of a backward step to me.
I've actually had a patch hanging around for the best part of a decade that
uses `%z` as the format specifier, but only on s390 (31-bit). As you may know,
`size_t` and `int` aren't interchangeable on s390, which used to throw up all
sorts of errors with the casts in HotSpot. On that platform, it really is
necessary to use `%z` for the size format.
The patch pretty much got dropped on the floor as we didn't build on s390 on
newer JDKs and obviously we'd need to start there to get it upstream and then
backport. The actual patch was pretty simple though:
~~~
diff --git openjdk.orig/hotspot/src/share/vm/utilities/globalDefinitions.hpp
openjdk/hotspot/src/share/vm/utilities/globalDefinitions.hpp
--- openjdk.orig/hotspot/src/share/vm/utilities/globalDefinitions.hpp
+++ openjdk/hotspot/src/share/vm/utilities/globalDefinitions.hpp
@@ -1389,12 +1389,21 @@
#define INTPTR_FORMAT_W(width) "%" #width PRIxPTR
+#if defined(S390) && !defined(_LP64)
+#define SSIZE_FORMAT "%z" PRIdPTR
+#define SIZE_FORMAT "%z" PRIuPTR
+#define SIZE_FORMAT_HEX "0x%z" PRIxPTR
+#define SSIZE_FORMAT_W(width) "%" #width "z" PRIdPTR
+#define SIZE_FORMAT_W(width) "%" #width "z" PRIuPTR
+#define SIZE_FORMAT_HEX_W(width) "0x%" #width "z" PRIxPTR
+#else // !S390
#define SSIZE_FORMAT "%" PRIdPTR
#define SIZE_FORMAT "%" PRIuPTR
#define SIZE_FORMAT_HEX "0x%" PRIxPTR
#define SSIZE_FORMAT_W(width) "%" #width PRIdPTR
#define SIZE_FORMAT_W(width) "%" #width PRIuPTR
#define SIZE_FORMAT_HEX_W(width) "0x%" #width PRIxPTR
+#endif // S390
#define INTX_FORMAT "%" PRIdPTR
#define UINTX_FORMAT "%" PRIuPTR
~~~
plus a bunch of cases where the wrong format was being used in the `printf`
statements (likely resolved now anyway; this was against 8u)
I think you could do something similar and include backwards compatibility by
detecting whether `%z` is usable during `configure` and then adding a similar
`#ifdef` but based on something defined by `configure`.
I'd be happy to propose something if you don't already have an alternate
solution in the works.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/15115#issuecomment-1664765034
