RFR: 8313302: Fix formatting errors on Windows [v3]

[Julian Waters]

On Fri, 28 Jul 2023 09:24:33 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Julian Waters has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   zPhysicalMemoryBacking_windows.cpp
>
> src/hotspot/os/windows/osThread_windows.hpp line 30:
> 
>> 28:   typedef void* HANDLE;
>> 29:  public:
>> 30:   typedef unsigned int thread_id_t;
> 
> Since Windows is LLP64, this has no effect, even though beginthreadex returns an unsigned int.

It's also another formatting change, as this is printed as %d in shared code. This also matches the definition of int on other platforms. This should functionally be harmless, because ints are promoted to longs (DWORD) whenever thread_id_t is passed to a callee that expects a DWORD, and since longs are the same size as ints this is effectively an implicit cast with no effect in those cases

> src/hotspot/os/windows/perfMemory_windows.cpp line 259:
> 
>> 257:     if (PrintMiscellaneous && Verbose) {
>> 258:       warning("%s is not a directory, file attributes = "
>> 259:               "0x%08lx\n", path, fa);
> 
> UINT32_FORMAT

Noted, if this is accepted

> src/hotspot/os_cpu/windows_x86/os_windows_x86.cpp line 422:
> 
>> 420:   st->print(", RBX=0x%016llx", uc->Rbx);
>> 421:   st->print(", RCX=0x%016llx", uc->Rcx);
>> 422:   st->print(", RDX=0x%016llx", uc->Rdx);
> 
> Why?

All of these are either DWORD64 or DWORD, none are actually intptr_t's or uintptr_t's

> src/hotspot/share/code/codeHeapState.cpp line 1334:
> 
>> 1332:   if (SizeDistributionArray != nullptr) {
>> 1333:     unsigned long total_count = 0;
>> 1334:     uint64_t total_size  = 0;
> 
> This is a functional change (actually the only one I could spot), and it increases the size of this type to 64-bit on Windows. 
> 
> It may also be incorrect, since on 32-bit platforms you probably don't want 64-bit here. The correct type to use would be size_t.
> 
> Since this change is hidden in a deluge of unnecessary cosmetic changes, if we feel this is needed, this should be addressed in a separate RFE. 
> 
> But then again, probably not: code cache size is limited to 32-bit, and I bet we have a lot more places to change if we wanted to change that.
> 
> Also way out of scope for the RFE description.

For this one (and the one below) I couldn't figure out whether the intention was for a 32 or 64 bit number in this case. This would be 64 bit on every platform but on Windows, which is 32 bit, and the code directly below this also faces the same issue too, is this intentional?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15063#discussion_r1277362853
PR Review Comment: https://git.openjdk.org/jdk/pull/15063#discussion_r1277363449
PR Review Comment: https://git.openjdk.org/jdk/pull/15063#discussion_r1277364035
PR Review Comment: https://git.openjdk.org/jdk/pull/15063#discussion_r1277365355