RFR: 8314114: Fix -Wconversion warnings in os code, primarily linux [v2]

[David Holmes]

On Fri, 11 Aug 2023 11:58:01 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> src/hotspot/os/aix/attachListener_aix.cpp line 279:
>> 
>>> 277:   // name ("load", "datadump", ...), and <arg> is an argument
>>> 278:   int expected_str_count = 2 + AttachOperation::arg_count_max;
>>> 279:   const unsigned max_len = (sizeof(ver_str) + 1) + (AttachOperation::name_length_max + 1) +
>> 
>> size_t instead?
>
> The reason I changed it was to assign into size_t below without sign conversion.

Sure, but can't it be size_t itself in the first place?

>> src/hotspot/os/linux/os_linux.cpp line 404:
>> 
>>> 402: // thread id is used to access /proc.
>>> 403: pid_t os::Linux::gettid() {
>>> 404:   int64_t rslt = syscall(SYS_gettid);
>> 
>> syscall returns an int
>
> It doesn't.  I got a Wconversion warning converting to int here.  I wouldn't have changed it otherwise.
> 
> src/hotspot/os/linux/os_linux.cpp:404:21: warning: conversion from 'long int' to 'int' may change value [-Wconversion]
>   404 |   int rslt = syscall(SYS_gettid);
>       |              ~~~~~~~^~~~~~~~~~~~

I have to prove I'm not an idiot here :). This is from the Linux manpage:

NAME
       syscall - indirect system call

SYNOPSIS
       #define _GNU_SOURCE         /* See feature_test_macros(7) */
       #include <unistd.h>
       #include <sys/syscall.h>   /* For SYS_xxx definitions */

       int syscall(int number, ...);

But yes the definition in unistd.h is `long int'.

In platform specific code such as this, when a function returns a long then perhaps we should type the result variable as a long as well, rather than forcing to a 64-bit type?

>> src/hotspot/os/linux/os_linux.cpp line 745:
>> 
>>> 743:   Monitor* sync = osthread->startThread_lock();
>>> 744: 
>>> 745:   osthread->set_thread_id(checked_cast<int>(os::current_thread_id()));
>> 
>> This doesn't look right: `current_thread_id()` returns `intx` and `set_thread_id` takes `thread_id_t`
>
> src/hotspot/os/linux/os_linux.cpp:745:48: warning: conversion from 'intx' {aka 'long int'} to 'OSThread::thread_id_t' {aka 'int'} may change value [-Wconversion]
>   745 |   osthread->set_thread_id(os::current_thread_id());
>       |                           ~~~~~~~~~~~~~~~~~~~~~^~

I understand there is a conversion warning, but it seems wrong to `check_cast<int>` when the expected type is `thread_id_t` (which just happens to be an `int`).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15229#discussion_r1292910754
PR Review Comment: https://git.openjdk.org/jdk/pull/15229#discussion_r1292914563
PR Review Comment: https://git.openjdk.org/jdk/pull/15229#discussion_r1292916137