8150894: Unused -Xlog tag sequences are silently ignored.
authormlarsson
Tue, 05 Apr 2016 16:51:58 +0200
changeset 40884 0e28c526f5d3
parent 40883 765d366c28e1
child 40885 b3958b331451
8150894: Unused -Xlog tag sequences are silently ignored. Reviewed-by: rehn, sla
hotspot/src/share/vm/logging/logConfiguration.cpp
hotspot/src/share/vm/logging/logTagLevelExpression.cpp
hotspot/src/share/vm/logging/logTagLevelExpression.hpp
hotspot/src/share/vm/logging/logTagSet.hpp
hotspot/test/native/logging/test_logConfiguration.cpp
--- a/hotspot/src/share/vm/logging/logConfiguration.cpp	Fri Aug 26 14:47:52 2016 -0700
+++ b/hotspot/src/share/vm/logging/logConfiguration.cpp	Tue Apr 05 16:51:58 2016 +0200
@@ -289,6 +289,8 @@
   }
   expr.set_level(level);
   expr.new_combination();
+  assert(expr.verify_tagsets(),
+         "configure_stdout() called with invalid/non-existing tag set");
 
   // Apply configuration to stdout (output #0), with the same decorators as before.
   ConfigurationLock cl;
@@ -334,9 +336,16 @@
   char errbuf[512];
   stringStream ss(errbuf, sizeof(errbuf));
   bool success = parse_log_arguments(output, what, decorators, output_options, &ss);
-  if (!success) {
-    errbuf[strlen(errbuf) - 1] = '\0'; // Strip trailing newline.
-    log_error(logging)("%s", errbuf);
+
+  if (ss.size() > 0) {
+    errbuf[strlen(errbuf) - 1] = '\0'; // Strip trailing newline
+    // If it failed, log the error. If it didn't fail, but something was written
+    // to the stream, log it as a warning.
+    if (!success) {
+      log_error(logging)("%s", ss.base());
+    } else {
+      log_warning(logging)("%s", ss.base());
+    }
   }
 
   os::free(copy);
@@ -386,6 +395,7 @@
   }
   configure_output(idx, expr, decorators);
   notify_update_listeners();
+  expr.verify_tagsets(errstream);
   return true;
 }
 
--- a/hotspot/src/share/vm/logging/logTagLevelExpression.cpp	Fri Aug 26 14:47:52 2016 -0700
+++ b/hotspot/src/share/vm/logging/logTagLevelExpression.cpp	Tue Apr 05 16:51:58 2016 +0200
@@ -29,6 +29,65 @@
 
 const char* LogTagLevelExpression::DefaultExpressionString = "all";
 
+static bool matches_tagset(const LogTagType tags[],
+                           bool allow_other_tags,
+                           const LogTagSet& ts) {
+  bool contains_all = true;
+  size_t tag_idx;
+  for (tag_idx = 0; tag_idx < LogTag::MaxTags && tags[tag_idx] != LogTag::__NO_TAG; tag_idx++) {
+    if (!ts.contains(tags[tag_idx])) {
+      contains_all = false;
+      break;
+    }
+  }
+  // All tags in the expression must be part of the tagset,
+  // and either the expression allows other tags (has a wildcard),
+  // or the number of tags in the expression and tagset must match.
+  return contains_all && (allow_other_tags || tag_idx == ts.ntags());
+}
+
+bool LogTagLevelExpression::verify_tagsets(outputStream* out) const {
+  bool valid = true;
+
+  for (size_t i = 0; i < _ncombinations; i++) {
+    bool matched = false;
+    for (LogTagSet* ts = LogTagSet::first(); ts != NULL; ts = ts->next()) {
+      if (matches_tagset(_tags[i], _allow_other_tags[i], *ts)) {
+        matched = true;
+        break;
+      }
+    }
+
+    if (!matched) {
+      // If this was the first invalid combination, write the message header
+      if (valid && out != NULL) {
+        out->print("No tag set matches selection(s): ");
+      }
+      valid = false;
+
+      // Break as soon as possible unless listing all invalid combinations
+      if (out == NULL) {
+        break;
+      }
+
+      // List the combination on the outputStream
+      for (size_t t = 0; t < LogTag::MaxTags && _tags[i][t] != LogTag::__NO_TAG; t++) {
+        out->print("%s%s", (t == 0 ? "" : "+"), LogTag::name(_tags[i][t]));
+      }
+      if (_allow_other_tags[i]) {
+        out->print("*");
+      }
+      out->print(" ");
+    }
+  }
+
+  if (!valid && out != NULL) {
+    out->cr();
+  }
+
+  return valid;
+}
+
 bool LogTagLevelExpression::parse(const char* str, outputStream* errstream) {
   bool success = true;
   if (str == NULL || strcmp(str, "") == 0) {
@@ -120,20 +179,10 @@
   // Return NotMentioned if the given tagset isn't covered by this expression.
   LogLevelType level = LogLevel::NotMentioned;
   for (size_t combination = 0; combination < _ncombinations; combination++) {
-    bool contains_all = true;
-    size_t tag_idx;
-    for (tag_idx = 0; tag_idx < LogTag::MaxTags && _tags[combination][tag_idx] != LogTag::__NO_TAG; tag_idx++) {
-      if (!ts.contains(_tags[combination][tag_idx])) {
-        contains_all = false;
-        break;
-      }
-    }
-    // All tags in the expression must be part of the tagset,
-    // and either the expression allows other tags (has a wildcard),
-    // or the number of tags in the expression and tagset must match.
-    if (contains_all && (_allow_other_tags[combination] || tag_idx == ts.ntags())) {
+    if (matches_tagset(_tags[combination], _allow_other_tags[combination], ts)) {
       level = _level[combination];
     }
   }
   return level;
 }
+
--- a/hotspot/src/share/vm/logging/logTagLevelExpression.hpp	Fri Aug 26 14:47:52 2016 -0700
+++ b/hotspot/src/share/vm/logging/logTagLevelExpression.hpp	Tue Apr 05 16:51:58 2016 +0200
@@ -83,6 +83,11 @@
 
   bool parse(const char* str, outputStream* errstream = NULL);
   LogLevelType level_for(const LogTagSet& ts) const;
+
+  // Verify the tagsets/selections mentioned in this expression.
+  // Returns false if some invalid tagset was found. If given an outputstream,
+  // this function will list all the invalid selections on the stream.
+  bool verify_tagsets(outputStream* out = NULL) const;
 };
 
 #endif // SHARE_VM_LOGGING_LOGTAGLEVELEXPRESSION_HPP
--- a/hotspot/src/share/vm/logging/logTagSet.hpp	Fri Aug 26 14:47:52 2016 -0700
+++ b/hotspot/src/share/vm/logging/logTagSet.hpp	Tue Apr 05 16:51:58 2016 +0200
@@ -86,7 +86,7 @@
   }
 
   bool contains(LogTagType tag) const {
-    for (size_t i = 0; _tag[i] != LogTag::__NO_TAG; i++) {
+    for (size_t i = 0; i < LogTag::MaxTags && _tag[i] != LogTag::__NO_TAG; i++) {
       if (tag == _tag[i]) {
         return true;
       }
--- a/hotspot/test/native/logging/test_logConfiguration.cpp	Fri Aug 26 14:47:52 2016 -0700
+++ b/hotspot/test/native/logging/test_logConfiguration.cpp	Tue Apr 05 16:51:58 2016 +0200
@@ -287,5 +287,17 @@
     const LogDecorators::Decorator decorator = static_cast<LogDecorators::Decorator>(d);
     EXPECT_TRUE(LogConfiguration::parse_log_arguments("#0", "", LogDecorators::name(decorator), "", &ss));
   }
-  EXPECT_STREQ("", ss.as_string()) << "Error reported while parsing: " << ss.as_string();
 }
+
+TEST_F(LogConfigurationTest, parse_invalid_tagset) {
+  static const char* invalid_tagset = "logging+start+exit+safepoint+gc"; // Must not exist for test to function.
+
+  // Make sure warning is produced if one or more configured tagsets are invalid
+  ResourceMark rm;
+  stringStream ss;
+  bool success = LogConfiguration::parse_log_arguments("stdout", invalid_tagset, NULL, NULL, &ss);
+  const char* msg = ss.as_string();
+  EXPECT_TRUE(success) << "Should only cause a warning, not an error";
+  EXPECT_TRUE(string_contains_substring(msg, "No tag set matches selection(s):"));
+  EXPECT_TRUE(string_contains_substring(msg, invalid_tagset));
+}