diff --git a/make/hotspot/lib/CompileJvm.gmk b/make/hotspot/lib/CompileJvm.gmk index e73d8702c28..80dfc821735 100644 --- a/make/hotspot/lib/CompileJvm.gmk +++ b/make/hotspot/lib/CompileJvm.gmk @@ -100,7 +100,7 @@ endif DISABLED_WARNINGS_xlc := tautological-compare shift-negative-value -DISABLED_WARNINGS_microsoft := 4624 4244 4291 4146 4127 4722 +DISABLED_WARNINGS_microsoft := 4624 4244 4291 4146 4127 ################################################################################ # Platform specific setup diff --git a/src/hotspot/cpu/x86/macroAssembler_x86.cpp b/src/hotspot/cpu/x86/macroAssembler_x86.cpp index b0a732b6ce8..ca322b36a28 100644 --- a/src/hotspot/cpu/x86/macroAssembler_x86.cpp +++ b/src/hotspot/cpu/x86/macroAssembler_x86.cpp @@ -42,6 +42,7 @@ #include "oops/klass.inline.hpp" #include "prims/methodHandles.hpp" #include "runtime/continuation.hpp" +#include "runtime/flags/flagSetting.hpp" #include "runtime/interfaceSupport.inline.hpp" #include "runtime/javaThread.hpp" #include "runtime/jniHandles.hpp" @@ -402,7 +403,7 @@ void MacroAssembler::debug32(int rdi, int rsi, int rbp, int rsp, int rbx, int rd void MacroAssembler::print_state32(int rdi, int rsi, int rbp, int rsp, int rbx, int rdx, int rcx, int rax, int eip) { ttyLocker ttyl; - DebuggingContext debugging{}; + FlagSetting fs(Debugging, true); tty->print_cr("eip = 0x%08x", eip); #ifndef PRODUCT if ((WizardMode || Verbose) && PrintMiscellaneous) { @@ -831,7 +832,7 @@ void MacroAssembler::debug64(char* msg, int64_t pc, int64_t regs[]) { void MacroAssembler::print_state64(int64_t pc, int64_t regs[]) { ttyLocker ttyl; - DebuggingContext debugging{}; + FlagSetting fs(Debugging, true); tty->print_cr("rip = 0x%016lx", (intptr_t)pc); #ifndef PRODUCT tty->cr(); diff --git a/src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp b/src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp index 8d665d6bc60..c5064be46b7 100644 --- a/src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp +++ b/src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp @@ -628,9 +628,7 @@ HeapWord* ParallelScavengeHeap::block_start(const void* addr) const { assert(young_gen()->is_in(addr), "addr should be in allocated part of young gen"); // called from os::print_location by find or VMError - if (DebuggingContext::is_enabled() || VMError::is_error_reported()) { - return nullptr; - } + if (Debugging || VMError::is_error_reported()) return nullptr; Unimplemented(); } else if (old_gen()->is_in_reserved(addr)) { assert(old_gen()->is_in(addr), diff --git a/src/hotspot/share/oops/accessBackend.cpp b/src/hotspot/share/oops/accessBackend.cpp index 61f58f1d335..441335e67b8 100644 --- a/src/hotspot/share/oops/accessBackend.cpp +++ b/src/hotspot/share/oops/accessBackend.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -209,7 +209,7 @@ namespace AccessInternal { #ifdef ASSERT void check_access_thread_state() { - if (VMError::is_error_reported() || DebuggingContext::is_enabled()) { + if (VMError::is_error_reported() || Debugging) { return; } diff --git a/src/hotspot/share/runtime/javaThread.cpp b/src/hotspot/share/runtime/javaThread.cpp index ebb1a41f068..dbca723d698 100644 --- a/src/hotspot/share/runtime/javaThread.cpp +++ b/src/hotspot/share/runtime/javaThread.cpp @@ -376,9 +376,6 @@ void JavaThread::check_possible_safepoint() { } void JavaThread::check_for_valid_safepoint_state() { - // Don't complain if running a debugging command. - if (DebuggingContext::is_enabled()) return; - // Check NoSafepointVerifier, which is implied by locks taken that can be // shared with the VM thread. This makes sure that no locks with allow_vm_block // are held. diff --git a/src/hotspot/share/runtime/mutexLocker.cpp b/src/hotspot/share/runtime/mutexLocker.cpp index 649b7a3fe41..3940ba443ed 100644 --- a/src/hotspot/share/runtime/mutexLocker.cpp +++ b/src/hotspot/share/runtime/mutexLocker.cpp @@ -171,7 +171,6 @@ static int _num_mutex; #ifdef ASSERT void assert_locked_or_safepoint(const Mutex* lock) { - if (DebuggingContext::is_enabled()) return; // check if this thread owns the lock (common case) assert(lock != nullptr, "Need non-null lock"); if (lock->owned_by_self()) return; @@ -182,7 +181,6 @@ void assert_locked_or_safepoint(const Mutex* lock) { // a weaker assertion than the above void assert_locked_or_safepoint_weak(const Mutex* lock) { - if (DebuggingContext::is_enabled()) return; assert(lock != nullptr, "Need non-null lock"); if (lock->is_locked()) return; if (SafepointSynchronize::is_at_safepoint()) return; @@ -192,7 +190,6 @@ void assert_locked_or_safepoint_weak(const Mutex* lock) { // a stronger assertion than the above void assert_lock_strong(const Mutex* lock) { - if (DebuggingContext::is_enabled()) return; assert(lock != nullptr, "Need non-null lock"); if (lock->owned_by_self()) return; fatal("must own lock %s", lock->name()); diff --git a/src/hotspot/share/utilities/debug.cpp b/src/hotspot/share/utilities/debug.cpp index ec67c18734f..afa472cf0de 100644 --- a/src/hotspot/share/utilities/debug.cpp +++ b/src/hotspot/share/utilities/debug.cpp @@ -76,16 +76,8 @@ static intx g_asserting_thread = 0; static void* g_assertion_context = nullptr; #endif // CAN_SHOW_REGISTERS_ON_ASSERT -int DebuggingContext::_enabled = 0; // Initially disabled. - -DebuggingContext::DebuggingContext() { - _enabled += 1; // Increase nesting count. -} - -DebuggingContext::~DebuggingContext() { - assert(is_enabled(), "Debugging nesting confusion"); - _enabled -= 1; // Decrease nesting count. -} +// Set to suppress secondary error reporting. +bool Debugging = false; #ifndef ASSERT # ifdef _DEBUG @@ -174,6 +166,7 @@ static void print_error_for_unit_test(const char* message, const char* detail_fm void report_vm_error(const char* file, int line, const char* error_msg, const char* detail_fmt, ...) { + if (Debugging) return; va_list detail_args; va_start(detail_args, detail_fmt); void* context = nullptr; @@ -195,6 +188,7 @@ void report_vm_status_error(const char* file, int line, const char* error_msg, } void report_fatal(VMErrorType error_type, const char* file, int line, const char* detail_fmt, ...) { + if (Debugging) return; va_list detail_args; va_start(detail_args, detail_fmt); void* context = nullptr; @@ -214,6 +208,7 @@ void report_fatal(VMErrorType error_type, const char* file, int line, const char void report_vm_out_of_memory(const char* file, int line, size_t size, VMErrorType vm_err_type, const char* detail_fmt, ...) { + if (Debugging) return; va_list detail_args; va_start(detail_args, detail_fmt); @@ -283,11 +278,13 @@ void report_java_out_of_memory(const char* message) { class Command : public StackObj { private: - ResourceMark _rm; - DebuggingContext _debugging; + ResourceMark rm; + bool debug_save; public: static int level; Command(const char* str) { + debug_save = Debugging; + Debugging = true; if (level++ > 0) return; tty->cr(); tty->print_cr("\"Executing %s\"", str); @@ -295,6 +292,7 @@ class Command : public StackObj { ~Command() { tty->flush(); + Debugging = debug_save; level--; } }; diff --git a/src/hotspot/share/utilities/debug.hpp b/src/hotspot/share/utilities/debug.hpp index 91b3e656415..773bae206bd 100644 --- a/src/hotspot/share/utilities/debug.hpp +++ b/src/hotspot/share/utilities/debug.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2022, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -46,98 +46,6 @@ bool handle_assert_poison_fault(const void* ucVoid, const void* faulting_address #define TOUCH_ASSERT_POISON #endif // CAN_SHOW_REGISTERS_ON_ASSERT -// The DebuggingContext class provides a mechanism for temporarily disabling -// asserts and various consistency checks. Ordinarily that would be a really -// bad idea, but it's essential for some of the debugging commands provided by -// HotSpot. (See the Command class in debug.cpp.) These commands are intended -// to be invoked from the debugger while the program is otherwise stopped. -// The commands may invoke operations while the program is in a state where -// those operations are not normally permitted, with the state checked by an -// assert. We want the debugging commands to bypass those checks. -class DebuggingContext { - static int _enabled; // Nesting counter. - -public: - DebuggingContext(); - ~DebuggingContext(); - // Asserts and other code use this to determine whether to bypass checks - // that would otherwise lead to program termination. - static bool is_enabled() { return _enabled > 0; } -}; - -// VMASSERT_CHECK_PASSED(P) provides the mechanism by which DebuggingContext -// disables asserts. It returns true if P is true or DebuggingContext is -// enabled. Assertion failure is reported if it returns false, terminating -// the program. -// -// The DebuggingContext check being enabled isn't placed inside the report -// function, as that would prevent the report function from being noreturn. -// The report function should be noreturn so there isn't a control path to the -// assertion's continuation that has P being false. Otherwise, the compiler -// might logically split the continuation to include that path explicitly, -// possibly leading to discovering (and warning about) invalid code. For -// example, if P := x != nullptr, and the continuation contains a dereference -// of x, the compiler might warn because there is a control path (!P -> report -// -> continuation) where that dereference is known to be invalid. (Of -// course, if execution actually took that path things would go wrong, but -// that's the risk the DebuggingContext mechanism takes.) -// -// Similarly, the check for enabled DebuggingContext shouldn't follow P. -// Having this macro expand to `P || DebuggingContext::is_enabled()` has the -// same problem of a control path through !P to the assertion's continuation. -// -// But it can't be just `DebuggingContext::is_enabled() || P` either. That -// prevents the compiler from inferring based on P that it is true in the -// continuation. But it also prevents the use of assertions in constexpr -// contexts, since that expression is not constexpr. -// -// We could accomodate constexpr usage with std::is_constant_evaluated() (from -// C++20). Unfortunately, we don't currently support C++20. However, most -// supported compilers have implemented it, and that implementation uses a -// compiler intrinsic that we can use directly without otherwise using C++20. -// -// Note that if we could use std::is_constant_evaluated() then we could just -// use this definition for DebuggingContext::is_enabled: -// static constexpr bool is_enabled() { -// return !std::is_constant_evaluated() && _enabled; -// } -// The idea being that we are definitely not executing for debugging if doing -// constant evaluation in the compiler. We don't do something like that now, -// because we need a fallback when we don't have any mechanism for detecting -// constant evaluation. -#if defined(TARGET_COMPILER_gcc) || defined(TARGET_COMPILER_xlc) - -// gcc10 added both __has_builtin and __builtin_is_constant_evaluated. -// clang has had __has_builtin for a long time, so likely also in xlclang++. -// Similarly, clang has had __builtin_is_constant_evaluated for a long time. - -#ifdef __has_builtin -#if __has_builtin(__builtin_is_constant_evaluated) -#define VMASSERT_CHECK_PASSED(p) \ - ((! __builtin_is_constant_evaluated() && DebuggingContext::is_enabled()) || (p)) -#endif -#endif - -#elif defined(TARGET_COMPILER_visCPP) - -// std::is_constant_evaluated() and it's associated intrinsic are available in -// VS 2019 16.5. The minimum supported version of VS 2019 is already past -// that, so we can rely on the intrinsic being available. -#define VMASSERT_CHECK_PASSED(p) \ - ((! __builtin_is_constant_evaluated() && DebuggingContext::is_enabled()) || (p)) - -#endif // End dispatch on TARGET_COMPILER_xxx - -// If we don't have a way to detect constant evaluation, then fall back to the -// less than ideal form of the check, and hope it works. This succeeds at -// least for gcc. The support needed to use the above definition was added in -// gcc10. The problems arising from analyzing the failed P case don't seem to -// appear until gcc12. An alternative is to not provide DebuggingContext -// support for such a configuration. -#ifndef VMASSERT_CHECK_PASSED -#define VMASSERT_CHECK_PASSED(p) ((p) || DebuggingContext::is_enabled()) -#endif - // assertions #ifndef ASSERT #define vmassert(p, ...) @@ -148,9 +56,10 @@ public: // compiler can't handle an empty ellipsis in a macro without a warning. #define vmassert(p, ...) \ do { \ - if (! VMASSERT_CHECK_PASSED(p)) { \ + if (!(p)) { \ TOUCH_ASSERT_POISON; \ report_vm_error(__FILE__, __LINE__, "assert(" #p ") failed", __VA_ARGS__); \ + BREAKPOINT; \ } \ } while (0) #endif @@ -173,13 +82,14 @@ do { \ // like "Invalid argument", "out of memory" etc #define vmassert_status(p, status, msg) \ do { \ - if (! VMASSERT_CHECK_PASSED(p)) { \ + if (!(p)) { \ TOUCH_ASSERT_POISON; \ report_vm_status_error(__FILE__, __LINE__, "assert(" #p ") failed", \ status, msg); \ + BREAKPOINT; \ } \ } while (0) -#endif // ASSERT +#endif // For backward compatibility. #define assert_status(p, status, msg) vmassert_status(p, status, msg) @@ -187,12 +97,12 @@ do { \ // guarantee is like vmassert except it's always executed -- use it for // cheap tests that catch errors that would otherwise be hard to find. // guarantee is also used for Verify options. -// guarantee is not subject to DebuggingContext bypass. #define guarantee(p, ...) \ do { \ if (!(p)) { \ TOUCH_ASSERT_POISON; \ report_vm_error(__FILE__, __LINE__, "guarantee(" #p ") failed", __VA_ARGS__); \ + BREAKPOINT; \ } \ } while (0) @@ -200,30 +110,35 @@ do { do { \ TOUCH_ASSERT_POISON; \ report_fatal(INTERNAL_ERROR, __FILE__, __LINE__, __VA_ARGS__); \ + BREAKPOINT; \ } while (0) // out of memory #define vm_exit_out_of_memory(size, vm_err_type, ...) \ do { \ report_vm_out_of_memory(__FILE__, __LINE__, size, vm_err_type, __VA_ARGS__); \ + BREAKPOINT; \ } while (0) #define ShouldNotCallThis() \ do { \ TOUCH_ASSERT_POISON; \ report_should_not_call(__FILE__, __LINE__); \ + BREAKPOINT; \ } while (0) #define ShouldNotReachHere() \ do { \ TOUCH_ASSERT_POISON; \ report_should_not_reach_here(__FILE__, __LINE__); \ + BREAKPOINT; \ } while (0) #define Unimplemented() \ do { \ TOUCH_ASSERT_POISON; \ report_unimplemented(__FILE__, __LINE__); \ + BREAKPOINT; \ } while (0) #define Untested(msg) \ @@ -242,37 +157,25 @@ enum VMErrorType { OOM_JAVA_HEAP_FATAL = 0xe0000004 }; +// Set to suppress secondary error reporting. +// Really should have a qualified name or something. +extern bool Debugging; + // error reporting helper functions -[[noreturn]] void report_vm_error(const char* file, int line, const char* error_msg); - -[[noreturn]] -ATTRIBUTE_PRINTF(4, 5) void report_vm_error(const char* file, int line, const char* error_msg, - const char* detail_fmt, ...); - -[[noreturn]] + const char* detail_fmt, ...) ATTRIBUTE_PRINTF(4, 5); void report_vm_status_error(const char* file, int line, const char* error_msg, int status, const char* detail); - -[[noreturn]] -ATTRIBUTE_PRINTF(4, 5) -void report_fatal(VMErrorType error_type, const char* file, int line, const char* detail_fmt, ...); - -[[noreturn]] -ATTRIBUTE_PRINTF(5, 6) +void report_fatal(VMErrorType error_type, const char* file, int line, const char* detail_fmt, ...) ATTRIBUTE_PRINTF(4, 5); void report_vm_out_of_memory(const char* file, int line, size_t size, VMErrorType vm_err_type, - const char* detail_fmt, ...); - -[[noreturn]] void report_should_not_call(const char* file, int line); -[[noreturn]] void report_should_not_reach_here(const char* file, int line); -[[noreturn]] void report_unimplemented(const char* file, int line); - -// NOT [[noreturn]] + const char* detail_fmt, ...) ATTRIBUTE_PRINTF(5, 6); +void report_should_not_call(const char* file, int line); +void report_should_not_reach_here(const char* file, int line); +void report_unimplemented(const char* file, int line); void report_untested(const char* file, int line, const char* message); -ATTRIBUTE_PRINTF(1, 2) -void warning(const char* format, ...); +void warning(const char* format, ...) ATTRIBUTE_PRINTF(1, 2); #define STATIC_ASSERT(Cond) static_assert((Cond), #Cond)