Skip to content

Commit

Permalink
ensure catch_files are kept in sync with code emitting the messages t…
Browse files Browse the repository at this point in the history
…hey catch
  • Loading branch information
dougxc committed Oct 23, 2021
1 parent 9790a2f commit 7742b6c
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 16 deletions.
4 changes: 1 addition & 3 deletions common.hocon
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ labsjdk-ce-17Debug : { downloads : { JAVA_HOME : ${jdks.labsjdk-ce-17Debug} }}
labsjdk-ee-17Debug : { downloads : { JAVA_HOME : ${jdks.labsjdk-ee-17Debug} }}

common : ${mx} ${deps.common} {
catch_files : [
"Graal diagnostic output saved in (?P<filename>.+\.zip)"
]
catch_files : ${catch_files}
}

linux : ${common} ${deps.linux}
Expand Down
8 changes: 7 additions & 1 deletion common.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,19 @@
"labsjdk-ee-17Debug": {"name": "labsjdk", "version": "ee-17.0.1+9-jvmci-21.3-b03-debug", "platformspecific": true }
},

"COMMENT" : "The devkits versions reflect those used to build the JVMCI JDKs (e.g., see devkit_platform_revisions in <jdk>/make/conf/jib-profiles.js)",
"COMMENT.devkits" : "The devkits versions reflect those used to build the JVMCI JDKs (e.g., see devkit_platform_revisions in <jdk>/make/conf/jib-profiles.js)",
"devkits": {
"windows-oraclejdk8": { "packages" : { "devkit:VS2017-15.9.16+1" : "==0" }},
"windows-openjdk8": { "packages" : { "devkit:VS2017-15.9.16+1" : "==0" }},
"windows-jdk11": { "packages" : { "devkit:VS2017-15.9.24+1" : "==0" }},
"windows-jdk17": { "packages" : { "devkit:VS2019-16.9.3+1" : "==0" }}
},

"catch_files" : [
"Graal diagnostic output saved in '(?P<filename>[^']+)'",
"Dumping debug output to '(?P<filename>[^']+)'"
],

"deps": {
"COMMENT.common": [
"pip:isort is a dependency of pip:pylint. The explicit dependency on the pip package works around",
Expand Down
4 changes: 1 addition & 3 deletions common.jsonnet
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,7 @@
# enforce self.arch (useful for generating job names)
arch:: error "self.arch not set",
capabilities +: [],
catch_files +: [
"Graal diagnostic output saved in (?P<filename>.+\\.zip)"
]
catch_files +: common_json.catch_files
},

linux:: deps.linux + self.common + {
Expand Down
49 changes: 48 additions & 1 deletion compiler/mx.compiler/mx_compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@

import mx
import mx_gate
from mx_gate import Task
from mx_gate import Task, Tags
from mx import SafeDirectoryUpdater

import mx_unittest
Expand Down Expand Up @@ -395,7 +395,54 @@ def _gate_scala_dacapo(name, iterations, extraVMarguments=None):
args = ['-n', str(iterations), '--preserve']
return mx_benchmark.gate_mx_benchmark(["scala-dacapo:{}".format(name), "--tracker=none", "--"] + vmargs + ["--"] + args)

def _check_catch_files():
"""
Verifies that there is a "catch_files" array in common.json at the root of
the repository containing this suite and that the array contains elements
matching DebugContext.DUMP_FILE_MESSAGE_REGEXP and
StandardPathUtilitiesProvider.DIAGNOSTIC_OUTPUT_DIRECTORY_MESSAGE_REGEXP.
"""
catch_files_fields = (
('DebugContext', 'DUMP_FILE_MESSAGE_REGEXP'),
('StandardPathUtilitiesProvider', 'DIAGNOSTIC_OUTPUT_DIRECTORY_MESSAGE_REGEXP')
)

def get_regexp(class_name, field_name):
source_path = join(_suite.dir, 'src', 'org.graalvm.compiler.debug', 'src', 'org', 'graalvm', 'compiler', 'debug', class_name + '.java')
regexp = None
with open(source_path) as fp:
for line in fp.readlines():
decl = field_name + ' = "'
index = line.find(decl)
if index != -1:
start_index = index + len(decl)
end_index = line.find('"', start_index)
regexp = line[start_index:end_index]

# Convert from Java style regexp to Python style
return regexp.replace('(?<', '(?P<')

if not regexp:
mx.abort('Could not find value of ' + field_name + ' in ' + source_path)
return regexp

common_path = join(dirname(_suite.dir), 'common.json')
if not exists(common_path):
mx.abort('Required file does not exist: {}'.format(common_path))
with open(common_path) as common_file:
common_cfg = json.load(common_file)
catch_files = common_cfg.get('catch_files')
if catch_files is None:
mx.abort('Could not find catch_files attribute in {}'.format(common_path))
for class_name, field_name in catch_files_fields:
regexp = get_regexp(class_name, field_name)
if regexp not in catch_files:
mx.abort('Could not find catch_files entry in {} matching "{}"'.format(common_path, regexp))

def compiler_gate_runner(suites, unit_test_runs, bootstrap_tests, tasks, extraVMarguments=None, extraUnitTestArguments=None):
with Task('CheckCatchFiles', tasks, tags=[Tags.style]) as t:
if t: _check_catch_files()

if jdk.javaCompliance >= '9':
with Task('JDK_java_base_test', tasks, tags=['javabasetest']) as t:
if t: java_base_unittest(_remove_empty_entries(extraVMarguments) + [])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,21 @@
*/
public final class DebugContext implements AutoCloseable {

/**
* The format of the message printed on the console by {@link #getDumpPath} when
* {@link DebugOptions#ShowDumpFiles} is true. The {@code %s} placeholder is replaced with the
* absolute path of the dump file (i.e. the value returned by the method).
*/
public static final String DUMP_FILE_MESSAGE_FORMAT = "Dumping debug output to '%s'";

/**
* The regular expression for matching the message derived from
* {@link #DUMP_FILE_MESSAGE_FORMAT}.
*
* Keep in sync with the {@code catch_files} array in {@code common.json}.
*/
public static final String DUMP_FILE_MESSAGE_REGEXP = "Dumping debug output to '(?<filename>[^']+)'";

public static final Description NO_DESCRIPTION = new Description(null, "NO_DESCRIPTION");
public static final GlobalMetrics NO_GLOBAL_METRIC_VALUES = null;
public static final Iterable<DebugHandlersFactory> NO_CONFIG_CUSTOMIZERS = Collections.emptyList();
Expand Down Expand Up @@ -613,7 +628,7 @@ public String getDumpPath(String extension, boolean createMissingDirectory) {
String label = description == null ? null : description.getLabel();
String result = PathUtilities.createUnique(immutable.options, DumpPath, id, label, extension, createMissingDirectory);
if (ShowDumpFiles.getValue(immutable.options)) {
TTY.println("Dumping debug output to %s", result);
TTY.println(DUMP_FILE_MESSAGE_FORMAT, result);
}
return result;
} catch (IOException ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public enum PrintGraphTarget {
public static final OptionKey<Boolean> DebugStubsAndSnippets = new OptionKey<>(false);
@Option(help = "Send compiler IR to dump handlers on error.", type = OptionType.Debug)
public static final OptionKey<Boolean> DumpOnError = new OptionKey<>(false);
@Option(help = "Specify the DumpLevel if CompilationFailureAction#Diagnose is used." +
@Option(help = "Specify the dump level if CompilationFailureAction#Diagnose is used." +
"See CompilationFailureAction for details. file:doc-files/CompilationFailureActionHelp.txt", type = OptionType.Debug)
public static final OptionKey<Integer> DiagnoseDumpLevel = new OptionKey<>(DebugContext.VERBOSE_LEVEL);
@Option(help = "Disable intercepting exceptions in debug scopes.", type = OptionType.Debug)
Expand Down Expand Up @@ -196,6 +196,13 @@ protected void onValueUpdate(EconomicMap<OptionKey<?>, Object> values, Boolean o

// @formatter:on

/**
* The format of the message printed on the console by {@link #getDumpDirectory} when
* {@link DebugOptions#ShowDumpFiles} is true. The {@code %s} placeholder is replaced with the
* value returned by {@link #getDumpDirectory}.
*/
private static final String DUMP_DIRECTORY_MESSAGE_FORMAT = "Dumping debug output in '%s'";

/**
* Gets the directory in which {@link DebugDumpHandler}s can generate output. This will be the
* directory specified by {@link #DumpPath} if it has been set otherwise it will be derived from
Expand All @@ -214,7 +221,7 @@ public static String getDumpDirectory(OptionValues options) throws IOException {
if (!exists(dumpDir)) {
createDirectories(dumpDir);
if (ShowDumpFiles.getValue(options)) {
TTY.println("Dumping debug output in %s", dumpDir.toString());
TTY.println(DUMP_DIRECTORY_MESSAGE_FORMAT, dumpDir);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,22 @@
* A {@link PathUtilitiesProvider} implemented in terms of {@link File} and {@link Path} APIs.
*/
public class StandardPathUtilitiesProvider implements PathUtilitiesProvider {

/**
* The format of the message printed on the console by {@link #archiveAndDelete} showing the
* absolute path of the zip file created. The {@code %s} placeholder is replaced with the
* absolute path of the zip file.
*/
public static final String DIAGNOSTIC_OUTPUT_DIRECTORY_MESSAGE_FORMAT = "Graal diagnostic output saved in '%s'";

/**
* The regular expression for matching the message derived from
* {@link #DIAGNOSTIC_OUTPUT_DIRECTORY_MESSAGE_FORMAT}.
*
* Keep in sync with the {@code catch_files} array in {@code common.json}.
*/
public static final String DIAGNOSTIC_OUTPUT_DIRECTORY_MESSAGE_REGEXP = "Graal diagnostic output saved in '(?<filename>[^']+)'";

@Override
public String createDirectories(String path) throws IOException {
Files.createDirectories(Paths.get(path));
Expand Down Expand Up @@ -175,7 +191,7 @@ public FileVisitResult postVisitDirectory(Path d, IOException exc) throws IOExce
}
});
// Keep this in sync with the catch_files in common.hocon
TTY.println("Graal diagnostic output saved in %s", zipFile);
TTY.println(DIAGNOSTIC_OUTPUT_DIRECTORY_MESSAGE_FORMAT, zipFile);
return zipFile.getAbsolutePath();
} catch (IOException e) {
toDelete.clear();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
*/
package org.graalvm.compiler.hotspot.test;

import static org.graalvm.compiler.debug.StandardPathUtilitiesProvider.DIAGNOSTIC_OUTPUT_DIRECTORY_MESSAGE_FORMAT;
import static org.graalvm.compiler.debug.StandardPathUtilitiesProvider.DIAGNOSTIC_OUTPUT_DIRECTORY_MESSAGE_REGEXP;
import static org.graalvm.compiler.test.SubprocessUtil.getVMCommandLine;
import static org.graalvm.compiler.test.SubprocessUtil.withoutDebuggerArguments;

Expand All @@ -35,6 +37,8 @@
import java.util.Collections;
import java.util.Enumeration;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;

Expand Down Expand Up @@ -224,7 +228,10 @@ private static void testHelper(List<Probe> initialProbes, List<String> extraVmAr

try {
List<Probe> probes = new ArrayList<>(initialProbes);
Probe diagnosticProbe = new Probe("Graal diagnostic output saved in ", 1);
String format = DIAGNOSTIC_OUTPUT_DIRECTORY_MESSAGE_FORMAT;
assert format.endsWith("'%s'") : format;
String prefix = format.substring(0, format.length() - "'%s'".length());
Probe diagnosticProbe = new Probe(prefix, 1);
probes.add(diagnosticProbe);
probes.add(new Probe("Forced crash after compiling", Integer.MAX_VALUE) {
@Override
Expand All @@ -246,10 +253,12 @@ String test() {
Assert.fail(String.format("Did not find expected occurrences of '%s' in output of command: %s%n%s", probe.substring, error, proc));
}
}

Pattern diagnosticOutputFileRE = Pattern.compile(DIAGNOSTIC_OUTPUT_DIRECTORY_MESSAGE_REGEXP);
String line = diagnosticProbe.lastMatchingLine;
int substringStart = line.indexOf(diagnosticProbe.substring);
int substringLength = diagnosticProbe.substring.length();
String diagnosticOutputZip = line.substring(substringStart + substringLength).trim();
Matcher m = diagnosticOutputFileRE.matcher(line);
Assert.assertTrue(line, m.find());
String diagnosticOutputZip = m.group(1);

List<String> dumpPathEntries = Arrays.asList(dumpPath.list());

Expand Down

0 comments on commit 7742b6c

Please sign in to comment.