On Wed, 25 May 2022 01:50:57 GMT, Kim Barrett <kbarr...@openjdk.org> 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<T*>(reinterpret_cast<uintptr_t>(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<T*>( > reinterpret_cast<char*>(this) + base_offset_in_bytes()); > } > > and optionally add this: > > const T* data() const { > return reinterpret_cast<const T*>( > reinterpret_cast<const char*>(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