RFR: 8332400: isspace argument should be a valid unsigned char

[Robert Toyonaga]

On Wed, 5 Jun 2024 20:08:10 GMT, Robert Toyonaga <duke at openjdk.org> wrote:

> ### Summary
> This change ensures we don't get undefined behavior when calling[`isspace`](https://pubs.opengroup.org/onlinepubs/007904975/functions/isspace.html).  `isspace` accepts an `int` argument that "the application shall ensure is a character representable as an unsigned char or equal to the value of the macro EOF.". 
> 
> Previously, there was no checking of the values passed to `isspace`. I've replaced direct calls with a new wrapper `os::is_space` that performs a range check and prevents the possibility of undefined behavior from happening. For instances outside of Hotspot, I've added casts to `unsigned char`. 
> 
> **Testing**
> - Added a new test in `test/hotspot/gtest/runtime/test_os.cpp` to check `os::is_space` is working correctly.
> - tier1

Thanks for the feedback everyone! 

> But I did a bit of searching and it seems it comes down to potential arithmetic operations on the "char" the might behave differently depending on the signed-ness. :(

Ok, so it seems like we ought to cast to unsigned char whenever char is passed, even if the representable range is the same. 

One more argument in favor of the current wrapper approach is that it makes it easy to at-a-glance confirm `isspace` is being used correctly everywhere.

However, I am fine with just using casts wherever there is char and removing the wrapper. This is simpler and also avoids the cast to int. But what about when an int is passed to `isspace`? Maybe having the range check here would be better than casting to unsigned char or leaving as is?

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

PR Comment: https://git.openjdk.org/jdk/pull/19567#issuecomment-2158401790