RFR: 8345159: RISCV: Fix -Wzero-as-null-pointer-constant warning in emit_static_call_stub

Robbin Ehn rehn at openjdk.org
Fri Nov 29 08:51:37 UTC 2024


On Fri, 29 Nov 2024 07:12:40 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

> > Change is okay and I sanity tested, all ok.
> 
> Thanks.
> 
> > But I must object to keep the underlying type of address. For all our purposes it is an integer value. I.e. `address dest_end = dest->_total_start + dest->_total_size;` Furthermore address 0 is a valid _address_, changing that to nullptr make little sense.
> 
> 0 is only barely a valid address, and only if very careful.

In this case we use address 0 for uninitiated immediates value, this is a very valid instructions encoding at least.

> 
> All supported platforms use 0 to represent all null pointers (including null pointer constants). The standards don't guarantee that (neither C nor C++.) (There exist or have existed platforms that used a different representation, but we don't support any of those.) There are certainly places in our code that assume that representation. Trying to avoid that is effectively impossible. It tends to uglify code. And we can't expect testing to uncover any of those places that exist now, or might accidentally be introduced in the future.
> 
> And tools like ubsan and asan won't help, since on the platforms we support, those tools make the same assumption about the representation of null pointers being 0, and indeed complain about arithmetic involving pointers with a value of 0.
> 

It sound like you or on my page, we should use 0 when we mean 0 :)

In this particular case we really want to generate code which moves the address **0** into a register.
Hence your fix is good as you avoid nullptr by using movptr2.
But my concerns was this may not always be possible.

> > So I believe the correct fix is to change the type of address.
> 
> I'm not sure what you mean by this.
> 
> Do you mean something like "change the type of the argument involved here"?
> 
> Or do you mean something like "change the alias type `address` to be an integral type (say, `intptr_t`) instead of `char*`"?
> 
> Neither of those seem like a good idea to me. But maybe you mean something else that I haven't thought of?

I haven't thought it throught. I just like seeing 0 when I mean the numeric value 0.
And seeing nullptr when I mean this pointer doesn't point to anything.

Maybe we should not be using the type address at all in some places, I'm not sure.

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

PR Comment: https://git.openjdk.org/jdk/pull/22435#issuecomment-2507343023


More information about the hotspot-compiler-dev mailing list