On Wed, 8 Nov 2023 22:10:20 GMT, Mikael Vidstedt <mik...@openjdk.org> wrote:

>> Oracle is updating the version of GCC for building the JDK on Linux to 
>> 13.2.0.
>> 
>> Apart from the "obvious" changes, I'll add some color to the CompileJvm.gmk 
>> changes. In particular, I ran into two different types of new warnings with 
>> GCC 13.2.0:
>> 
>> 1. linux-aarch64-debug + stringop-overflow
>> 
>> `src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: 
>> 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 
>> bytes into a region of size 0 overflows the destination 
>> [-Werror=stringop-overflow=]`
>> 
>> Only reproduces with fastdebug on linux-aarch64. I tried to understand why 
>> the warning is generated and how the code could be fixed but eventually had 
>> to give up.. I ended up disabling the warning for linux-aarch64-debug 
>> specifically but open to feedback and other alternatives.
>> 
>> 2. linux + zero + dangling-pointer
>> 
>> 
>> `src/hotspot/share/runtime/thread.hpp:579:77: error: storing the address of 
>> local variable 'rm' in '*_thr_current.Thread::_current_resource_mark' 
>> [-Werror=dangling-pointer=]`
>> 
>> The linux/zero build generates lots and lots of dangling pointer warnings. 
>> As with the first warning I tried to understand why but also gave up in the 
>> end. Like the first warning I disabled it instead, for zero builds. Again 
>> appreciating feedback/suggestions.
>
> Mikael Vidstedt has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Move ASSERT ResourceMark constructor to resourceArea.cpp to avoid dangling 
> pointer warning with zero

> For `linux + zero + dangling-pointer`, your current solution is somehow 
> coarse-grained. Perhaps, we may use `PRAGMA_DANGLING_POINTER_IGNORED`. See 
> #13751 and #13789.

@shqking Good feedback, thank you! I tried to use 
`PRAGMA_DANGLING_POINTER_IGNORED` to silence the warning but ran into issues 
using it in the header files - seems like there may be limitations to where 
pragma can be used and in particular it doesn't seem to have any effect in 
header files and/or inside of `#if`/#ifdef` blocks. That basically means that 
in order to fix this particular warning we would have to put the PRAGMAs in all 
the various places in the .cpp files instead, which turns out to be _lots_ of 
places. Not particularly appetizing.

However, @stefank suggested another, better solution: move the relevant 
`ASSERT` specific `ResourceMark` constructor code to the `resourceArea.cpp` 
file instead. Since it's "just" used for `ASSERT` builds it's not really 
performance sensitive, so can't be that important to inline. I did exactly that 
and - lo and behold - I didn't even have to silence the warning in the .cpp 
file, it just works.

I just pushed a change that moves the constructor implantation to 
`resourceArea.cpp` and removes the dangling pointer warning logic from 
`CompileJvm.gmk` again. Have a look and let me know what you think.

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

PR Comment: https://git.openjdk.org/jdk/pull/16550#issuecomment-1802763593

Reply via email to