Skip to content
Permalink
Browse files
8264285: Clean the modification of ccstr JVM flags
Reviewed-by: dholmes, coleenp
  • Loading branch information
iklam committed Apr 1, 2021
1 parent 6e0da99 commit 5858399062543cc5300b18971d5c4c2639118fc8
Show file tree
Hide file tree
Showing 9 changed files with 181 additions and 45 deletions.
@@ -1372,18 +1372,16 @@ WB_ENTRY(void, WB_SetStringVMFlag(JNIEnv* env, jobject o, jstring name, jstring
ccstrValue = env->GetStringUTFChars(value, NULL);
CHECK_JNI_EXCEPTION(env);
}
ccstr ccstrResult = ccstrValue;
bool needFree;
{
ccstr param = ccstrValue;
ThreadInVMfromNative ttvfn(thread); // back to VM
needFree = SetVMFlag <JVM_FLAG_TYPE(ccstr)> (thread, env, name, &ccstrResult);
if (SetVMFlag <JVM_FLAG_TYPE(ccstr)> (thread, env, name, &param)) {
assert(param == NULL, "old value is freed automatically and not returned");
}
}
if (value != NULL) {
env->ReleaseStringUTFChars(value, ccstrValue);
}
if (needFree) {
FREE_C_HEAP_ARRAY(char, ccstrResult);
}
WB_END

WB_ENTRY(void, WB_LockCompilation(JNIEnv* env, jobject o, jlong timeout))
@@ -28,6 +28,7 @@
#include "compiler/compiler_globals.hpp"
#include "gc/shared/gc_globals.hpp"
#include "gc/shared/tlab_globals.hpp"
#include "runtime/flags/debug_globals.hpp"
#include "runtime/globals.hpp"

// Put LP64/ARCH/JVMCI/COMPILER1/COMPILER2 at the top,
@@ -112,6 +113,15 @@
range, \
constraint) \
\
DEBUG_RUNTIME_FLAGS( \
develop, \
develop_pd, \
product, \
product_pd, \
notproduct, \
range, \
constraint) \
\
GC_FLAGS( \
develop, \
develop_pd, \
@@ -0,0 +1,72 @@
/*
* Copyright (c) 2021, 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
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*
*/

#ifndef SHARE_RUNTIME_DEBUG_GLOBALS_HPP
#define SHARE_RUNTIME_DEBUG_GLOBALS_HPP

#include "runtime/globals_shared.hpp"
#include "utilities/macros.hpp"

//
// These flags are needed for testing the implementation of various flag access
// APIs.
//
// For example, DummyManageableStringFlag is needed because we don't
// have any MANAGEABLE flags of the ccstr type, but we really need to
// make sure the implementation is correct (in terms of memory allocation)
// just in case someone may add such a flag in the future.
//

#ifndef ASSERT

#define DEBUG_RUNTIME_FLAGS(develop, \
develop_pd, \
product, \
product_pd, \
notproduct, \
range, \
constraint) \
\

#else

#define DEBUG_RUNTIME_FLAGS(develop, \
develop_pd, \
product, \
product_pd, \
notproduct, \
range, \
constraint) \
\
product(ccstr, DummyManageableStringFlag, NULL, MANAGEABLE, \
"Dummy flag for testing string handling in WriteableFlags") \
\

// end of DEBUG_RUNTIME_FLAGS

#endif // ASSERT

DECLARE_FLAGS(DEBUG_RUNTIME_FLAGS)

#endif // SHARE_RUNTIME_DEBUG_GLOBALS_HPP
@@ -317,40 +317,29 @@ JVMFlag::Error JVMFlagAccess::ccstrAtPut(JVMFlag* flag, ccstr* value, JVMFlagOri
new_value = os::strdup_check_oom(*value);
}
flag->set_ccstr(new_value);
if (flag->is_default() && old_value != NULL) {
// Prior value is NOT heap allocated, but was a literal constant.
old_value = os::strdup_check_oom(old_value);
if (!flag->is_default() && old_value != NULL) {
// Old value is heap allocated so free it.
FREE_C_HEAP_ARRAY(char, old_value);
}
*value = old_value;
// Unlike the other APIs, the old vale is NOT returned, so the caller won't need to free it.
// The callers typically don't care what the old value is.
// If the caller really wants to know the old value, read it (and make a copy if necessary)
// before calling this API.
*value = NULL;
flag->set_origin(origin);
return JVMFlag::SUCCESS;
}

// This is called by the FLAG_SET_XXX macros.
JVMFlag::Error JVMFlagAccess::set_impl(JVMFlagsEnum flag_enum, int type_enum, void* value, JVMFlagOrigin origin) {
if (type_enum == JVMFlag::TYPE_ccstr || type_enum == JVMFlag::TYPE_ccstrlist) {
return ccstrAtPut((JVMFlagsEnum)flag_enum, *((ccstr*)value), origin);
}

JVMFlag* flag = JVMFlag::flag_from_enum(flag_enum);
assert(flag->type() == type_enum, "wrong flag type");
return set_impl(flag, type_enum, value, origin);
}

// This is called by the FLAG_SET_XXX macros.
JVMFlag::Error JVMFlagAccess::ccstrAtPut(JVMFlagsEnum flag, ccstr value, JVMFlagOrigin origin) {
JVMFlag* faddr = JVMFlag::flag_from_enum(flag);
assert(faddr->is_ccstr(), "wrong flag type");
ccstr old_value = faddr->get_ccstr();
trace_flag_changed<ccstr, EventStringFlagChanged>(faddr, old_value, value, origin);
char* new_value = os::strdup_check_oom(value);
faddr->set_ccstr(new_value);
if (!faddr->is_default() && old_value != NULL) {
// Prior value is heap allocated so free it.
FREE_C_HEAP_ARRAY(char, old_value);
if (type_enum == JVMFlag::TYPE_ccstr || type_enum == JVMFlag::TYPE_ccstrlist) {
assert(flag->is_ccstr(), "must be");
return ccstrAtPut(flag, (ccstr*)value, origin);
} else {
assert(flag->type() == type_enum, "wrong flag type");
return set_impl(flag, type_enum, value, origin);
}
faddr->set_origin(origin);
return JVMFlag::SUCCESS;
}

JVMFlag::Error JVMFlagAccess::check_range(const JVMFlag* flag, bool verbose) {
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020, 2021, 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
@@ -54,7 +54,6 @@ class JVMFlagAccess : AllStatic {
inline static const FlagAccessImpl* access_impl(const JVMFlag* flag);
static JVMFlag::Error set_impl(JVMFlagsEnum flag_enum, int type_enum, void* value, JVMFlagOrigin origin);
static JVMFlag::Error set_impl(JVMFlag* flag, int type_enum, void* value, JVMFlagOrigin origin);
static JVMFlag::Error ccstrAtPut(JVMFlagsEnum flag, ccstr value, JVMFlagOrigin origin);

public:
static JVMFlag::Error check_range(const JVMFlag* flag, bool verbose);
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2021, 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
@@ -25,6 +25,7 @@
#include "precompiled.hpp"
#include "classfile/javaClasses.hpp"
#include "memory/allocation.inline.hpp"
#include "memory/resourceArea.hpp"
#include "runtime/arguments.hpp"
#include "runtime/flags/jvmFlag.hpp"
#include "runtime/flags/jvmFlagAccess.hpp"
@@ -244,6 +245,9 @@ JVMFlag::Error WriteableFlags::set_double_flag(const char* name, double value, J
JVMFlag::Error WriteableFlags::set_ccstr_flag(const char* name, const char* value, JVMFlagOrigin origin, FormatBuffer<80>& err_msg) {
JVMFlag* flag = JVMFlag::find_flag(name);
JVMFlag::Error err = JVMFlagAccess::ccstrAtPut(flag, &value, origin);
if (err == JVMFlag::SUCCESS) {
assert(value == NULL, "old value is freed automatically and not returned");
}
print_flag_error_message_if_needed(err, flag, err_msg);
return err;
}
@@ -357,11 +361,9 @@ JVMFlag::Error WriteableFlags::set_flag_from_jvalue(JVMFlag* f, const void* valu
err_msg.print("flag value is missing");
return JVMFlag::MISSING_VALUE;
}
ResourceMark rm;
ccstr svalue = java_lang_String::as_utf8_string(str);
JVMFlag::Error ret = WriteableFlags::set_ccstr_flag(f->name(), svalue, origin, err_msg);
if (ret != JVMFlag::SUCCESS) {
FREE_C_HEAP_ARRAY(char, svalue);
}
return ret;
} else {
ShouldNotReachHere();
@@ -25,6 +25,7 @@
#include "compiler/compiler_globals.hpp"
#include "gc/shared/gc_globals.hpp"
#include "runtime/globals.hpp"
#include "runtime/globals_extension.hpp"
#include "runtime/flags/flagSetting.hpp"
#include "runtime/flags/jvmFlag.hpp"
#include "unittest.hpp"
@@ -71,3 +72,25 @@ TEST_VM(FlagGuard, double_flag) {
TEST_VM(FlagGuard, ccstr_flag) {
TEST_FLAG(PerfDataSaveFile, ccstr, "/a/random/path");
}


// SharedArchiveConfigFile is used only during "java -Xshare:dump", so
// it's safe to modify its value in gtest

TEST_VM(FlagAccess, ccstr_flag) {
FLAG_SET_CMDLINE(SharedArchiveConfigFile, "");
ASSERT_EQ(FLAG_IS_CMDLINE(SharedArchiveConfigFile), true);
ASSERT_EQ(strcmp(SharedArchiveConfigFile, ""), 0);

FLAG_SET_ERGO(SharedArchiveConfigFile, "foobar");
ASSERT_EQ(FLAG_IS_ERGO(SharedArchiveConfigFile), true);
ASSERT_EQ(strcmp(SharedArchiveConfigFile, "foobar") , 0);

FLAG_SET_ERGO(SharedArchiveConfigFile, nullptr);
ASSERT_EQ(FLAG_IS_ERGO(SharedArchiveConfigFile), true);
ASSERT_EQ(SharedArchiveConfigFile, nullptr);

FLAG_SET_ERGO(SharedArchiveConfigFile, "xyz");
ASSERT_EQ(FLAG_IS_ERGO(SharedArchiveConfigFile), true);
ASSERT_EQ(strcmp(SharedArchiveConfigFile, "xyz"), 0);
}
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2021, 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
@@ -21,6 +21,7 @@
* questions.
*/

import jdk.test.lib.Platform;
import jdk.test.lib.process.OutputAnalyzer;
import jdk.test.lib.dcmd.CommandExecutor;
import jdk.test.lib.dcmd.JMXExecutor;
@@ -47,6 +48,7 @@ public void run(CommandExecutor executor) {
setMutableFlagWithInvalidValue(executor);
setImmutableFlag(executor);
setNonExistingFlag(executor);
setStringFlag(executor);
}

@Test
@@ -147,6 +149,24 @@ private void setNonExistingFlag(CommandExecutor executor) {
out.stdoutShouldContain("flag " + unknownFlag + " does not exist");
}

private void setStringFlag(CommandExecutor executor) {
// Today we don't have any manageable flags of the string type in the product build,
// so we can only test DummyManageableStringFlag in the debug build.
if (!Platform.isDebugBuild()) {
return;
}

String flag = "DummyManageableStringFlag";
String toValue = "DummyManageableStringFlag_Is_Set_To_Hello";

System.out.println("### Setting a string flag '" + flag + "'");
OutputAnalyzer out = executor.execute("VM.set_flag " + flag + " " + toValue);
out.stderrShouldBeEmpty();

out = getAllFlags(executor);
out.stdoutShouldContain(toValue);
}

private OutputAnalyzer getAllFlags(CommandExecutor executor) {
return executor.execute("VM.flags -all", true);
}
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2015, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2021, 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
@@ -29,6 +29,7 @@
* @author Mandy Chung
* @author Jaroslav Bachorik
*
* @library /test/lib
* @run main/othervm -XX:+HeapDumpOnOutOfMemoryError SetVMOption
*/

@@ -37,6 +38,7 @@
import com.sun.management.HotSpotDiagnosticMXBean;
import com.sun.management.VMOption;
import com.sun.management.VMOption.Origin;
import jdk.test.lib.Platform;

public class SetVMOption {
private static final String HEAP_DUMP_ON_OOM = "HeapDumpOnOutOfMemoryError";
@@ -94,6 +96,23 @@ public static void main(String[] args) throws Exception {
option.isWriteable() + " expected: " + o.isWriteable());
}


// Today we don't have any manageable flags of the string type in the product build,
// so we can only test DummyManageableStringFlag in the debug build.
if (Platform.isDebugBuild()) {
String optionName = "DummyManageableStringFlag";
String toValue = "DummyManageableStringFlag_Is_Set_To_Hello";

mbean.setVMOption(optionName, toValue);

VMOption stringOption = findOption(optionName);
Object newValue = stringOption.getValue();
if (!toValue.equals(newValue)) {
throw new RuntimeException("Unmatched value: " +
newValue + " expected: " + toValue);
}
}

// check if ManagementServer is not writeable
List<VMOption> options = mbean.getDiagnosticOptions();
VMOption mgmtServerOption = null;
@@ -123,18 +142,22 @@ public static void main(String[] args) throws Exception {
}

public static VMOption findHeapDumpOnOomOption() {
return findOption(HEAP_DUMP_ON_OOM);
}

private static VMOption findOption(String optionName) {
List<VMOption> options = mbean.getDiagnosticOptions();
VMOption gcDetails = null;
VMOption found = null;
for (VMOption o : options) {
if (o.getName().equals(HEAP_DUMP_ON_OOM)) {
gcDetails = o;
if (o.getName().equals(optionName)) {
found = o;
break;
}
}
if (gcDetails == null) {
throw new RuntimeException("VM option " + HEAP_DUMP_ON_OOM +
if (found == null) {
throw new RuntimeException("VM option " + optionName +
" not found");
}
return gcDetails;
return found;
}
}

1 comment on commit 5858399

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.