Skip to content

8320353: Reenable stringop-overflow warnings #26067

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

8320353: Reenable stringop-overflow warnings #26067

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6739a18
8320353: Reenabled stringop-overflow warning for linux zero/fastdebug
toxaart Jul 1, 2025
169a773
8320353: More work
toxaart Jul 1, 2025
db39eb5
8320353: More work
toxaart Jul 1, 2025
231480b
8320353: Added debug only case
toxaart Jul 1, 2025
06b3412
8320353: Fiixed build problem
toxaart Jul 1, 2025
452d598
8320353: Fixed whitespace error
toxaart Jul 1, 2025
3289ff1
8320353: Fixed comment
toxaart Jul 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions make/hotspot/lib/CompileJvm.gmk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,20 +188,16 @@ $(eval $(call SetupJdkLibrary, BUILD_LIBJVM, \
DISABLED_WARNINGS_gcc_cgroupV1Subsystem_linux.cpp := address, \
DISABLED_WARNINGS_gcc_cgroupV2Subsystem_linux.cpp := address, \
DISABLED_WARNINGS_gcc_g1FreeIdSet.cpp := unused-const-variable, \
DISABLED_WARNINGS_gcc_handshake.cpp := stringop-overflow, \
DISABLED_WARNINGS_gcc_interp_masm_x86.cpp := uninitialized, \
DISABLED_WARNINGS_gcc_javaClasses.cpp := unused-const-variable, \
DISABLED_WARNINGS_gcc_jfrChunkWriter.cpp := unused-const-variable, \
DISABLED_WARNINGS_gcc_jfrMemorySizer.cpp := unused-const-variable, \
DISABLED_WARNINGS_gcc_jfrTraceIdKlassQueue.cpp := unused-const-variable, \
DISABLED_WARNINGS_gcc_jvmciCodeInstaller.cpp := stringop-overflow, \
DISABLED_WARNINGS_gcc_jvmFlag.cpp := unused-const-variable, \
DISABLED_WARNINGS_gcc_jvmtiTagMap.cpp := stringop-overflow, \
DISABLED_WARNINGS_gcc_macroAssembler_ppc_sha.cpp := unused-const-variable, \
DISABLED_WARNINGS_gcc_postaloc.cpp := address, \
DISABLED_WARNINGS_gcc_shenandoahLock.cpp := stringop-overflow, \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about this one in shenandoahLock.cpp?

DISABLED_WARNINGS_gcc_stubGenerator_s390.cpp := unused-const-variable, \
DISABLED_WARNINGS_gcc_synchronizer.cpp := stringop-overflow, \
DISABLED_WARNINGS_gcc_templateInterpreterGenerator_x86.cpp := unused-const-variable, \
DISABLED_WARNINGS_gcc_xGlobals_ppc.cpp := unused-const-variable, \
DISABLED_WARNINGS_clang := $(DISABLED_WARNINGS_clang), \
Expand Down
11 changes: 11 additions & 0 deletions src/hotspot/share/jvmci/jvmciCodeInstaller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,17 @@ int CodeInstaller::estimate_stubs_size(HotSpotCompiledCodeStream* stream, JVMCI_
// perform data and call relocation on the CodeBuffer
JVMCI::CodeInstallResult CodeInstaller::initialize_buffer(JVMCIObject compiled_code, CodeBuffer& buffer, HotSpotCompiledCodeStream* stream, u1 code_flags, JVMCI_TRAPS) {
JavaThread* thread = stream->thread();
#if defined(__GNUC__) && !defined(__clang__) && !defined(PRODUCT)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that !PRODUCT does not imply ASSERT. There is also the "optimized" build type, which
defines neither PRODUCT nor ASSERT. I have no idea whether the false-positive gcc warnings
occur in that build configuration. If so, then my suggestion of using an assertion to give the compiler
the clue it needs won't work. But I'm guessing that isn't a problem, in which case the macro check
should be defined(ASSERT).

if (thread == nullptr) {
// This is to prevent --stringop-overflow warning from GCC on linux/fastdebug.
// GCC does believe that JavaThread::current() can return nullptr,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does believe -> believes

// though it cannot.
// GCC seems to not remember that this hint has been given to it when initializing
// HotSpotCompiledCodeStream object with a thread returned by JavaThread::current(),
// therefore 2nd hint is needed.
__builtin_unreachable();
}
#endif
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be replaced with asserting non-null. A failed assert calls a noreturn reporting
function.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I wonder why the assert in Thread::current() isn't sufficient? Maybe non-null info isn't
propagating through the cast to JavaThread?

HandleMark hm(thread);
int locs_buffer_size = _sites_count * (relocInfo::length_limit + sizeof(relocInfo));

Expand Down
11 changes: 10 additions & 1 deletion src/hotspot/share/runtime/javaThread.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1067,7 +1067,16 @@ class JavaThread: public Thread {
public:
// Returns the running thread as a JavaThread
static JavaThread* current() {
return JavaThread::cast(Thread::current());
auto result = JavaThread::cast(Thread::current());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why auto when the type is known?

#if defined(__GNUC__) && !defined(__clang__) && !defined(PRODUCT)
if (result == nullptr) {
// This is to prevent --stringop-overflow warning from GCC on linux/fastdebug.
// GCC does believe that JavaThread::current() can return nullptr,
// though it cannot.
__builtin_unreachable();
}
#endif
return result;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly here, I think this could be replaced with asserting non-null.

}

// Returns the current thread as a JavaThread, or nullptr if not attached
Expand Down