Re: RFR: 8286562: GCC 12 reports some compiler warnings [v8]
On Wed, 25 May 2022 01:50:57 GMT, Kim Barrett wrote:
>> Yasumasa Suenaga has updated the pull request with a new target base due to
>> a merge or a rebase. The pull request now contains 11 commits:
>>
>> - Merge remote-tracking branch 'upstream/master' into gcc12-warnings
>> - Use getter function to access "_data"
>> - Revert changes for bytecodeAssembler.cpp, classFileParser.cpp,
>> symbolTable.cpp
>> - revert changes for memnode.cpp and type.cpp
>> - Add assert to check the range of BasicType
>> - Merge remote-tracking branch 'upstream/master' into HEAD
>> - Revert change for java.c , parse_manifest.c , LinuxPackage.c
>> - Calculate char offset before realloc()
>> - Use return value from JLI_Snprintf
>> - Avoid pragma error in before GCC 12
>> - ... and 1 more:
>> https://git.openjdk.java.net/jdk/compare/c156bcc5...042c1c70
>
> src/hotspot/share/oops/array.hpp line 102:
>
>> 100: // standard operations
>> 101: int length() const { return _length; }
>> 102: T* data() const { return
>> reinterpret_cast(reinterpret_cast(this) +
>> base_offset_in_bytes()); }
>
> Adding the const-qualifier to the `data()` function and then implicitly
> casting it away (by casting through intptr_t) is wrong. Either don't
> const-qualify (and leave it to some future use that needs such to address
> appropriately), or have two functions. Also, the line length is excessive.
> So this:
>
>
> T* data() {
> return reinterpret_cast(
> reinterpret_cast(this) + base_offset_in_bytes());
> }
>
> and optionally add this:
>
> const T* data() const {
> return reinterpret_cast(
> reinterpret_cast(this) + base_offset_in_bytes());
> }
Thanks a lot @kimbarrett !
I updated around stringop-overflow warning in jfrTraceIdBits.inline.hpp , and
added two `data()` in `Array` class. They works fine on my GCC 12 on Fedora 36.
Could you review again?
-
PR: https://git.openjdk.java.net/jdk/pull/8646
Re: RFR: 8286562: GCC 12 reports some compiler warnings [v8]
On Sun, 22 May 2022 08:40:28 GMT, Yasumasa Suenaga wrote:
>> I saw some compiler warnings when I tried to build OpenJDK with GCC 12.0.1
>> on Fedora 36.
>> As you can see, the warnings spreads several areas. Let me know if I should
>> separate them by area.
>>
>> * -Wstringop-overflow
>> * src/hotspot/share/oops/array.hpp
>> *
>> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp
>>
>> In member function 'void Array::at_put(int, const T&) [with T = unsigned
>> char]',
>> inlined from 'void ConstantPool::tag_at_put(int, jbyte)' at
>> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:126:64,
>> inlined from 'void ConstantPool::method_at_put(int, int, int)' at
>> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:380:15,
>> inlined from 'ConstantPool*
>> BytecodeConstantPool::create_constant_pool(JavaThread*) const' at
>> /home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/bytecodeAssembler.cpp:85:26:
>
> Yasumasa Suenaga has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 11 commits:
>
> - Merge remote-tracking branch 'upstream/master' into gcc12-warnings
> - Use getter function to access "_data"
> - Revert changes for bytecodeAssembler.cpp, classFileParser.cpp,
> symbolTable.cpp
> - revert changes for memnode.cpp and type.cpp
> - Add assert to check the range of BasicType
> - Merge remote-tracking branch 'upstream/master' into HEAD
> - Revert change for java.c , parse_manifest.c , LinuxPackage.c
> - Calculate char offset before realloc()
> - Use return value from JLI_Snprintf
> - Avoid pragma error in before GCC 12
> - ... and 1 more:
> https://git.openjdk.java.net/jdk/compare/c156bcc5...042c1c70
Changes requested by kbarrett (Reviewer).
src/hotspot/share/oops/array.hpp line 102:
> 100: // standard operations
> 101: int length() const { return _length; }
> 102: T* data() const { return
> reinterpret_cast(reinterpret_cast(this) +
> base_offset_in_bytes()); }
Adding the const-qualifier to the `data()` function and then implicitly
casting it away (by casting through intptr_t) is wrong. Either don't
const-qualify (and leave it to some future use that needs such to address
appropriately), or have two functions. Also, the line length is excessive.
So this:
T* data() {
return reinterpret_cast(
reinterpret_cast(this) + base_offset_in_bytes());
}
and optionally add this:
const T* data() const {
return reinterpret_cast(
reinterpret_cast(this) + base_offset_in_bytes());
}
-
PR: https://git.openjdk.java.net/jdk/pull/8646
Re: RFR: 8286562: GCC 12 reports some compiler warnings [v8]
> I saw some compiler warnings when I tried to build OpenJDK with GCC 12.0.1 on > Fedora 36. > As you can see, the warnings spreads several areas. Let me know if I should > separate them by area. > > * -Wstringop-overflow > * src/hotspot/share/oops/array.hpp > * > src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp > > In member function 'void Array::at_put(int, const T&) [with T = unsigned > char]', > inlined from 'void ConstantPool::tag_at_put(int, jbyte)' at > /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:126:64, > inlined from 'void ConstantPool::method_at_put(int, int, int)' at > /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:380:15, > inlined from 'ConstantPool* > BytecodeConstantPool::create_constant_pool(JavaThread*) const' at > /home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/bytecodeAssembler.cpp:85:26: Yasumasa Suenaga has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 11 commits: - Merge remote-tracking branch 'upstream/master' into gcc12-warnings - Use getter function to access "_data" - Revert changes for bytecodeAssembler.cpp, classFileParser.cpp, symbolTable.cpp - revert changes for memnode.cpp and type.cpp - Add assert to check the range of BasicType - Merge remote-tracking branch 'upstream/master' into HEAD - Revert change for java.c , parse_manifest.c , LinuxPackage.c - Calculate char offset before realloc() - Use return value from JLI_Snprintf - Avoid pragma error in before GCC 12 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/c156bcc5...042c1c70 - Changes: https://git.openjdk.java.net/jdk/pull/8646/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8646=07 Stats: 42 lines in 7 files changed: 26 ins; 1 del; 15 mod Patch: https://git.openjdk.java.net/jdk/pull/8646.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8646/head:pull/8646 PR: https://git.openjdk.java.net/jdk/pull/8646
