Simplify error reporting.
This CL proposes that we ensure that origins are the defining place for
locations of resources. For example, API methods on paths are changed to encode
the context of the inputs in the origin. We assume the origin to have this
information and thus, IOExceptionDiagnostics should not attempt to retrive it
again. We should, in time, avoid throwing IOException in the compiler and
instead associate proper origins at the point of exception. This is not
completed in the present CL.
Bug: 78893028
Change-Id: I750e3837a70799a8ee44b51a486a70bf20791319
diff --git a/src/main/java/com/android/tools/r8/BaseCommand.java b/src/main/java/com/android/tools/r8/BaseCommand.java
index eb2d0fe..77284e5 100644
--- a/src/main/java/com/android/tools/r8/BaseCommand.java
+++ b/src/main/java/com/android/tools/r8/BaseCommand.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.utils.AbortException;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.DefaultDiagnosticsHandler;
+import com.android.tools.r8.utils.ExceptionDiagnostic;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.Reporter;
import com.android.tools.r8.utils.StringDiagnostic;
@@ -66,6 +67,34 @@
// Internal access to the internal options.
abstract InternalOptions getInternalOptions();
+ abstract static class InputFileOrigin extends PathOrigin {
+ private final String inputType;
+
+ public InputFileOrigin(String inputType, Path file) {
+ super(file);
+ this.inputType = inputType;
+ }
+
+ @Override
+ public String part() {
+ return inputType + " '" + super.part() + "'";
+ }
+ }
+
+ private static class ProgramInputOrigin extends InputFileOrigin {
+
+ public ProgramInputOrigin(Path file) {
+ super("program input", file);
+ }
+ }
+
+ private static class LibraryInputOrigin extends InputFileOrigin {
+
+ public LibraryInputOrigin(Path file) {
+ super("library input", file);
+ }
+ }
+
/**
* Base builder for commands.
*
@@ -147,7 +176,7 @@
app.addProgramFile(path);
programFiles.add(path);
} catch (IOException | CompilationError e) {
- error("Error with input file: ", path, e);
+ error(new ProgramInputOrigin(path), e);
}
});
});
@@ -181,7 +210,7 @@
try {
app.addLibraryFile(path);
} catch (IOException | CompilationError e) {
- error("Error with library file: ", path, e);
+ error(new LibraryInputOrigin(path), e);
}
});
});
@@ -289,9 +318,8 @@
void validate() {}
// Helper to signify an error.
- void error(String baseMessage, Path path, Throwable throwable) {
- reporter.error(new StringDiagnostic(
- baseMessage + throwable.getMessage(), new PathOrigin(path)), throwable);
+ void error(Origin origin, Throwable throwable) {
+ reporter.error(new ExceptionDiagnostic(throwable, origin));
}
// Helper to guard and handle exceptions.
diff --git a/src/main/java/com/android/tools/r8/ClassFileConsumer.java b/src/main/java/com/android/tools/r8/ClassFileConsumer.java
index 0394441..6d7ad87 100644
--- a/src/main/java/com/android/tools/r8/ClassFileConsumer.java
+++ b/src/main/java/com/android/tools/r8/ClassFileConsumer.java
@@ -8,7 +8,7 @@
import com.android.tools.r8.utils.ArchiveBuilder;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.DirectoryBuilder;
-import com.android.tools.r8.utils.IOExceptionDiagnostic;
+import com.android.tools.r8.utils.ExceptionDiagnostic;
import com.android.tools.r8.utils.OutputBuilder;
import com.android.tools.r8.utils.ZipUtils;
import com.google.common.io.ByteStreams;
@@ -135,7 +135,7 @@
try {
outputBuilder.close();
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, outputBuilder.getOrigin()));
+ handler.error(new ExceptionDiagnostic(e, outputBuilder.getOrigin()));
}
}
diff --git a/src/main/java/com/android/tools/r8/D8Command.java b/src/main/java/com/android/tools/r8/D8Command.java
index c300bbf..fc58a4a 100644
--- a/src/main/java/com/android/tools/r8/D8Command.java
+++ b/src/main/java/com/android/tools/r8/D8Command.java
@@ -32,6 +32,13 @@
*/
public class D8Command extends BaseCompilerCommand {
+ private static class ClasspathInputOrigin extends InputFileOrigin {
+
+ public ClasspathInputOrigin(Path file) {
+ super("classpath input", file);
+ }
+ }
+
/**
* Builder for constructing a D8Command.
*
@@ -78,7 +85,7 @@
try {
getAppBuilder().addClasspathFile(file);
} catch (IOException e) {
- error("Error with classpath entry: ", file, e);
+ error(new ClasspathInputOrigin(file), e);
}
});
}
diff --git a/src/main/java/com/android/tools/r8/DexFilePerClassFileConsumer.java b/src/main/java/com/android/tools/r8/DexFilePerClassFileConsumer.java
index f93c338..ca86c70 100644
--- a/src/main/java/com/android/tools/r8/DexFilePerClassFileConsumer.java
+++ b/src/main/java/com/android/tools/r8/DexFilePerClassFileConsumer.java
@@ -8,8 +8,8 @@
import com.android.tools.r8.utils.ArchiveBuilder;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.DirectoryBuilder;
+import com.android.tools.r8.utils.ExceptionDiagnostic;
import com.android.tools.r8.utils.FileUtils;
-import com.android.tools.r8.utils.IOExceptionDiagnostic;
import com.android.tools.r8.utils.OutputBuilder;
import com.android.tools.r8.utils.ZipUtils;
import com.google.common.io.ByteStreams;
@@ -156,7 +156,7 @@
try {
outputBuilder.close();
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, outputBuilder.getOrigin()));
+ handler.error(new ExceptionDiagnostic(e, outputBuilder.getOrigin()));
}
}
diff --git a/src/main/java/com/android/tools/r8/DexIndexedConsumer.java b/src/main/java/com/android/tools/r8/DexIndexedConsumer.java
index 0f932d5..f26ab73 100644
--- a/src/main/java/com/android/tools/r8/DexIndexedConsumer.java
+++ b/src/main/java/com/android/tools/r8/DexIndexedConsumer.java
@@ -3,11 +3,12 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8;
+import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.origin.PathOrigin;
import com.android.tools.r8.utils.ArchiveBuilder;
import com.android.tools.r8.utils.DirectoryBuilder;
+import com.android.tools.r8.utils.ExceptionDiagnostic;
import com.android.tools.r8.utils.FileUtils;
-import com.android.tools.r8.utils.IOExceptionDiagnostic;
import com.android.tools.r8.utils.OutputBuilder;
import com.android.tools.r8.utils.ZipUtils;
import com.google.common.io.ByteStreams;
@@ -126,6 +127,10 @@
}
}
+ public Origin getOrigin() {
+ return outputBuilder.getOrigin();
+ }
+
@Override
public DataResourceConsumer getDataResourceConsumer() {
return consumeDataResources ? this : null;
@@ -154,7 +159,7 @@
try {
outputBuilder.close();
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, outputBuilder.getOrigin()));
+ handler.error(new ExceptionDiagnostic(e, outputBuilder.getOrigin()));
}
}
@@ -219,7 +224,7 @@
try {
prepareDirectory();
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, new PathOrigin(directory)));
+ handler.error(new ExceptionDiagnostic(e, new PathOrigin(directory)));
}
outputBuilder.addFile(getDexFileName(fileIndex), data, handler);
}
@@ -240,7 +245,7 @@
try {
outputBuilder.close();
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, outputBuilder.getOrigin()));
+ handler.error(new ExceptionDiagnostic(e, outputBuilder.getOrigin()));
}
}
diff --git a/src/main/java/com/android/tools/r8/R8Command.java b/src/main/java/com/android/tools/r8/R8Command.java
index 83f9dc2..10e5828 100644
--- a/src/main/java/com/android/tools/r8/R8Command.java
+++ b/src/main/java/com/android/tools/r8/R8Command.java
@@ -15,8 +15,9 @@
import com.android.tools.r8.shaking.ProguardConfigurationSourceFile;
import com.android.tools.r8.shaking.ProguardConfigurationSourceStrings;
import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.ExceptionDiagnostic;
+import com.android.tools.r8.utils.ExceptionUtils;
import com.android.tools.r8.utils.FileUtils;
-import com.android.tools.r8.utils.IOExceptionDiagnostic;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.InternalOptions.LineNumberOptimization;
import com.android.tools.r8.utils.Reporter;
@@ -286,14 +287,12 @@
return makeR8Command();
} catch (IOException e) {
- throw getReporter().fatalError(new IOExceptionDiagnostic(e), e);
- } catch (CompilationException e) {
- throw getReporter().fatalError(new StringDiagnostic(e.getMessage()), e);
+ throw getReporter()
+ .fatalError(new ExceptionDiagnostic(e, ExceptionUtils.extractIOExceptionOrigin(e)));
}
}
- private R8Command makeR8Command()
- throws IOException, CompilationException {
+ private R8Command makeR8Command() throws IOException {
Reporter reporter = getReporter();
DexItemFactory factory = new DexItemFactory();
ImmutableList<ProguardConfigurationRule> mainDexKeepRules;
diff --git a/src/main/java/com/android/tools/r8/StringConsumer.java b/src/main/java/com/android/tools/r8/StringConsumer.java
index 23883e9..cccb72e 100644
--- a/src/main/java/com/android/tools/r8/StringConsumer.java
+++ b/src/main/java/com/android/tools/r8/StringConsumer.java
@@ -5,7 +5,7 @@
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.origin.PathOrigin;
-import com.android.tools.r8.utils.IOExceptionDiagnostic;
+import com.android.tools.r8.utils.ExceptionDiagnostic;
import java.io.BufferedWriter;
import java.io.IOException;
import java.io.OutputStream;
@@ -102,7 +102,8 @@
try {
Files.write(outputPath, string.getBytes(encoding));
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, new PathOrigin(outputPath)));
+ Origin origin = new PathOrigin(outputPath);
+ handler.error(new ExceptionDiagnostic(e, origin));
}
}
}
@@ -149,7 +150,7 @@
writer.write(string);
writer.flush();
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, origin));
+ handler.error(new ExceptionDiagnostic(e, origin));
}
}
}
diff --git a/src/main/java/com/android/tools/r8/UsageInformationConsumer.java b/src/main/java/com/android/tools/r8/UsageInformationConsumer.java
index e949276..a572908 100644
--- a/src/main/java/com/android/tools/r8/UsageInformationConsumer.java
+++ b/src/main/java/com/android/tools/r8/UsageInformationConsumer.java
@@ -5,8 +5,8 @@
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.origin.PathOrigin;
+import com.android.tools.r8.utils.ExceptionDiagnostic;
import com.android.tools.r8.utils.FileUtils;
-import com.android.tools.r8.utils.IOExceptionDiagnostic;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.file.Path;
@@ -89,7 +89,8 @@
try {
FileUtils.writeToFile(outputPath, null, data);
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, new PathOrigin(outputPath)));
+ Origin origin = new PathOrigin(outputPath);
+ handler.error(new ExceptionDiagnostic(e, origin));
}
}
}
@@ -125,7 +126,7 @@
try {
outputStream.write(data);
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, origin));
+ handler.error(new ExceptionDiagnostic(e, origin));
}
}
}
diff --git a/src/main/java/com/android/tools/r8/compatdx/CompatDx.java b/src/main/java/com/android/tools/r8/compatdx/CompatDx.java
index 1de9606..b9131e5 100644
--- a/src/main/java/com/android/tools/r8/compatdx/CompatDx.java
+++ b/src/main/java/com/android/tools/r8/compatdx/CompatDx.java
@@ -24,9 +24,10 @@
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.Unimplemented;
import com.android.tools.r8.logging.Log;
+import com.android.tools.r8.origin.PathOrigin;
import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.ExceptionDiagnostic;
import com.android.tools.r8.utils.FileUtils;
-import com.android.tools.r8.utils.IOExceptionDiagnostic;
import com.android.tools.r8.utils.ThreadUtils;
import com.google.common.collect.ImmutableList;
import com.google.common.io.ByteStreams;
@@ -547,7 +548,7 @@
StandardOpenOption.TRUNCATE_EXISTING,
StandardOpenOption.WRITE);
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e));
+ handler.error(new ExceptionDiagnostic(e, new PathOrigin(output)));
}
}
}
@@ -566,7 +567,7 @@
try {
writeZipWithClasses(handler);
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e));
+ handler.error(new ExceptionDiagnostic(e, getOrigin()));
}
super.finished(handler);
}
diff --git a/src/main/java/com/android/tools/r8/dex/DexReader.java b/src/main/java/com/android/tools/r8/dex/DexReader.java
index 44bacc6..d8997f7 100644
--- a/src/main/java/com/android/tools/r8/dex/DexReader.java
+++ b/src/main/java/com/android/tools/r8/dex/DexReader.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.DexVersion;
import java.io.IOException;
+import java.nio.BufferUnderflowException;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
@@ -38,6 +39,12 @@
// Parse the magic header and determine the dex file version.
private int parseMagic(ByteBuffer buffer) {
+ try {
+ buffer.get();
+ buffer.rewind();
+ } catch (BufferUnderflowException e) {
+ throw new CompilationError("Dex file is empty", origin);
+ }
int index = 0;
for (byte prefixByte : DEX_FILE_MAGIC_PREFIX) {
if (buffer.get(index++) != prefixByte) {
diff --git a/src/main/java/com/android/tools/r8/dexfilemerger/DexFileMerger.java b/src/main/java/com/android/tools/r8/dexfilemerger/DexFileMerger.java
index 59aecfe..ba576e7 100644
--- a/src/main/java/com/android/tools/r8/dexfilemerger/DexFileMerger.java
+++ b/src/main/java/com/android/tools/r8/dexfilemerger/DexFileMerger.java
@@ -12,8 +12,8 @@
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.origin.PathOrigin;
+import com.android.tools.r8.utils.ExceptionDiagnostic;
import com.android.tools.r8.utils.FileUtils;
-import com.android.tools.r8.utils.IOExceptionDiagnostic;
import com.android.tools.r8.utils.OptionsParsing;
import com.android.tools.r8.utils.OptionsParsing.ParseContext;
import com.android.tools.r8.utils.StringDiagnostic;
@@ -233,7 +233,7 @@
Files.newOutputStream(
path, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING));
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, origin));
+ handler.error(new ExceptionDiagnostic(e, origin));
}
}
return stream;
@@ -246,7 +246,7 @@
getStream(handler), getDexFileName(fileIndex), data, ZipEntry.DEFLATED, true);
hasWrittenSomething = true;
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, origin));
+ handler.error(new ExceptionDiagnostic(e, origin));
}
}
@@ -264,7 +264,7 @@
stream = null;
}
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, origin));
+ handler.error(new ExceptionDiagnostic(e, origin));
}
}
}
diff --git a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
index bef2f57..47f476d 100644
--- a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
+++ b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
@@ -75,9 +75,12 @@
import com.android.tools.r8.utils.InternalOptions;
import it.unimi.dsi.fastutil.ints.Int2ReferenceAVLTreeMap;
import it.unimi.dsi.fastutil.ints.Int2ReferenceSortedMap;
+import java.io.BufferedInputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.nio.ByteBuffer;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.List;
@@ -102,6 +105,8 @@
*/
public class JarClassFileReader {
+ private static final byte[] CLASSFILE_HEADER = ByteBuffer.allocate(4).putInt(0xCAFEBABE).array();
+
// Hidden ASM "synthetic attribute" bit we need to clear.
private static final int ACC_SYNTHETIC_ATTRIBUTE = 0x40000;
// Descriptor used by ASM for missing annotations.
@@ -117,6 +122,24 @@
}
public void read(Origin origin, ClassKind classKind, InputStream input) throws IOException {
+ if (!input.markSupported()) {
+ input = new BufferedInputStream(input);
+ }
+ byte[] header = new byte[CLASSFILE_HEADER.length];
+ input.mark(header.length);
+ int size = 0;
+ while (size < header.length) {
+ int read = input.read(header, size, header.length - size);
+ if (read < 0) {
+ throw new CompilationError("Invalid empty classfile", origin);
+ }
+ size += read;
+ }
+ if (!Arrays.equals(CLASSFILE_HEADER, header)) {
+ throw new CompilationError("Invalid classfile header", origin);
+ }
+ input.reset();
+
ClassReader reader = new ClassReader(input);
int flags = SKIP_FRAMES;
if (application.options.enableCfFrontend) {
diff --git a/src/main/java/com/android/tools/r8/utils/AndroidApp.java b/src/main/java/com/android/tools/r8/utils/AndroidApp.java
index d6ddf5d..6252c40 100644
--- a/src/main/java/com/android/tools/r8/utils/AndroidApp.java
+++ b/src/main/java/com/android/tools/r8/utils/AndroidApp.java
@@ -301,8 +301,7 @@
}
/** Add filtered archives of program resources. */
- public Builder addFilteredProgramArchives(Collection<FilteredClassPath> filteredArchives)
- throws NoSuchFileException {
+ public Builder addFilteredProgramArchives(Collection<FilteredClassPath> filteredArchives) {
for (FilteredClassPath archive : filteredArchives) {
assert isArchive(archive.getPath());
ArchiveResourceProvider archiveResourceProvider =
diff --git a/src/main/java/com/android/tools/r8/utils/ArchiveBuilder.java b/src/main/java/com/android/tools/r8/utils/ArchiveBuilder.java
index 4a7abc3..cd21d17 100644
--- a/src/main/java/com/android/tools/r8/utils/ArchiveBuilder.java
+++ b/src/main/java/com/android/tools/r8/utils/ArchiveBuilder.java
@@ -61,7 +61,7 @@
Files.newOutputStream(
archive, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING));
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, origin));
+ handler.error(new ExceptionDiagnostic(e, origin));
}
}
return stream;
@@ -71,9 +71,9 @@
if (e instanceof ZipException && e.getMessage().startsWith("duplicate entry")) {
// For now we stick to the Proguard behaviour, see section "Warning: can't write resource ...
// Duplicate zip entry" on https://www.guardsquare.com/en/proguard/manual/troubleshooting.
- handler.warning(new IOExceptionDiagnostic(e, origin));
+ handler.warning(new ExceptionDiagnostic(e, origin));
} else {
- handler.error(new IOExceptionDiagnostic(e, origin));
+ handler.error(new ExceptionDiagnostic(e, origin));
}
}
diff --git a/src/main/java/com/android/tools/r8/utils/DiagnosticWithThrowable.java b/src/main/java/com/android/tools/r8/utils/DiagnosticWithThrowable.java
index da439fe..b169297 100644
--- a/src/main/java/com/android/tools/r8/utils/DiagnosticWithThrowable.java
+++ b/src/main/java/com/android/tools/r8/utils/DiagnosticWithThrowable.java
@@ -14,4 +14,8 @@
assert throwable != null;
this.throwable = throwable;
}
+
+ public Throwable getThrowable() {
+ return throwable;
+ }
}
diff --git a/src/main/java/com/android/tools/r8/utils/DirectoryBuilder.java b/src/main/java/com/android/tools/r8/utils/DirectoryBuilder.java
index b793f19..2dc4a2d 100644
--- a/src/main/java/com/android/tools/r8/utils/DirectoryBuilder.java
+++ b/src/main/java/com/android/tools/r8/utils/DirectoryBuilder.java
@@ -39,7 +39,7 @@
try {
Files.createDirectories(target.getParent());
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, new PathOrigin(target)));
+ handler.error(new ExceptionDiagnostic(e, new PathOrigin(target)));
}
}
@@ -48,7 +48,7 @@
try (InputStream in = content.getByteStream()) {
addFile(name, ByteStreams.toByteArray(in), handler);
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, content.getOrigin()));
+ handler.error(new ExceptionDiagnostic(e, content.getOrigin()));
} catch (ResourceException e) {
handler.error(new StringDiagnostic("Failed to open input: " + e.getMessage(),
content.getOrigin()));
@@ -62,7 +62,7 @@
Files.createDirectories(target.getParent());
FileUtils.writeToFile(target, null, content);
} catch (IOException e) {
- handler.error(new IOExceptionDiagnostic(e, new PathOrigin(target)));
+ handler.error(new ExceptionDiagnostic(e, new PathOrigin(target)));
}
}
diff --git a/src/main/java/com/android/tools/r8/utils/ExceptionDiagnostic.java b/src/main/java/com/android/tools/r8/utils/ExceptionDiagnostic.java
new file mode 100644
index 0000000..9fd91d9
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/utils/ExceptionDiagnostic.java
@@ -0,0 +1,43 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.utils;
+
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.position.Position;
+import java.io.FileNotFoundException;
+import java.nio.file.FileAlreadyExistsException;
+import java.nio.file.NoSuchFileException;
+
+public class ExceptionDiagnostic extends DiagnosticWithThrowable {
+
+ private final Origin origin;
+
+ public ExceptionDiagnostic(Throwable e, Origin origin) {
+ super(e);
+ this.origin = origin;
+ }
+
+ @Override
+ public Origin getOrigin() {
+ return origin;
+ }
+
+ @Override
+ public Position getPosition() {
+ return Position.UNKNOWN;
+ }
+
+ @Override
+ public String getDiagnosticMessage() {
+ Throwable e = getThrowable();
+ if (e instanceof NoSuchFileException || e instanceof FileNotFoundException) {
+ return "File not found: " + e.getMessage();
+ }
+ if (e instanceof FileAlreadyExistsException) {
+ return "File already exists: " + e.getMessage();
+ }
+ return e.getMessage();
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/utils/ExceptionUtils.java b/src/main/java/com/android/tools/r8/utils/ExceptionUtils.java
index f4d3ae7..9c7f525 100644
--- a/src/main/java/com/android/tools/r8/utils/ExceptionUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/ExceptionUtils.java
@@ -9,7 +9,11 @@
import com.android.tools.r8.ResourceException;
import com.android.tools.r8.StringConsumer;
import com.android.tools.r8.errors.CompilationError;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.origin.PathOrigin;
import java.io.IOException;
+import java.nio.file.FileSystemException;
+import java.nio.file.Paths;
import java.util.function.Consumer;
import java.util.function.Function;
@@ -55,16 +59,13 @@
try {
action.run();
} catch (IOException e) {
- throw reporter.fatalError(new IOExceptionDiagnostic(e));
+ throw reporter.fatalError(new ExceptionDiagnostic(e, extractIOExceptionOrigin(e)));
} catch (CompilationException e) {
throw reporter.fatalError(new StringDiagnostic(compilerMessage.apply(e)), e);
} catch (CompilationError e) {
throw reporter.fatalError(e);
} catch (ResourceException e) {
- throw reporter.fatalError(
- e.getCause() instanceof IOException
- ? new IOExceptionDiagnostic((IOException) e.getCause(), e.getOrigin())
- : new StringDiagnostic(e.getMessage(), e.getOrigin()));
+ throw reporter.fatalError(new ExceptionDiagnostic(e, e.getOrigin()));
}
reporter.failIfPendingErrors();
} catch (AbortException e) {
@@ -89,6 +90,19 @@
cause.printStackTrace();
System.exit(STATUS_ERROR);
}
-
}
+
+ // We should try to avoid the use of this extraction as it signifies a point where we don't have
+ // enough context to associate a specific origin with an IOException. Concretely, we should move
+ // towards always catching IOException and rethrowing CompilationError with proper origins.
+ public static Origin extractIOExceptionOrigin(IOException e) {
+ if (e instanceof FileSystemException) {
+ FileSystemException fse = (FileSystemException) e;
+ if (fse.getFile() != null && !fse.getFile().isEmpty()) {
+ return new PathOrigin(Paths.get(fse.getFile()));
+ }
+ }
+ return Origin.unknown();
+ }
+
}
diff --git a/src/main/java/com/android/tools/r8/utils/IOExceptionDiagnostic.java b/src/main/java/com/android/tools/r8/utils/IOExceptionDiagnostic.java
deleted file mode 100644
index 642f2d7..0000000
--- a/src/main/java/com/android/tools/r8/utils/IOExceptionDiagnostic.java
+++ /dev/null
@@ -1,73 +0,0 @@
-// Copyright (c) 2017, the R8 project authors. Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-
-package com.android.tools.r8.utils;
-
-import com.android.tools.r8.origin.Origin;
-import com.android.tools.r8.origin.PathOrigin;
-import com.android.tools.r8.position.Position;
-import java.io.FileNotFoundException;
-import java.io.IOException;
-import java.nio.file.FileAlreadyExistsException;
-import java.nio.file.FileSystemException;
-import java.nio.file.NoSuchFileException;
-import java.nio.file.Paths;
-
-public class IOExceptionDiagnostic extends DiagnosticWithThrowable {
-
- private final Origin origin;
- private final String message;
-
- public IOExceptionDiagnostic(IOException e) {
- super(e);
- origin = extractOrigin(e);
- message = extractMessage(e);
- }
-
- public IOExceptionDiagnostic(IOException e, Origin origin) {
- super(e);
- this.origin = origin;
- message = extractMessage(e);
- }
-
- private static String extractMessage(IOException e) {
- String message = e.getMessage();
- if (message == null || message.isEmpty()) {
- if (e instanceof NoSuchFileException || e instanceof FileNotFoundException) {
- message = "File not found";
- } else if (e instanceof FileAlreadyExistsException) {
- message = "File already exists";
- }
- }
- return message;
- }
-
- private static Origin extractOrigin(IOException e) {
- Origin origin = Origin.unknown();
-
- if (e instanceof FileSystemException) {
- FileSystemException fse = (FileSystemException) e;
- if (fse.getFile() != null && !fse.getFile().isEmpty()) {
- origin = new PathOrigin(Paths.get(fse.getFile()));
- }
- }
- return origin;
- }
-
- @Override
- public Origin getOrigin() {
- return origin;
- }
-
- @Override
- public Position getPosition() {
- return Position.UNKNOWN;
- }
-
- @Override
- public String getDiagnosticMessage() {
- return message;
- }
-
-}
diff --git a/src/test/java/com/android/tools/r8/D8CommandTest.java b/src/test/java/com/android/tools/r8/D8CommandTest.java
index ed7bbf8..dc16cac 100644
--- a/src/test/java/com/android/tools/r8/D8CommandTest.java
+++ b/src/test/java/com/android/tools/r8/D8CommandTest.java
@@ -15,8 +15,10 @@
import com.android.tools.r8.origin.EmbeddedOrigin;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.FileUtils;
import com.android.tools.r8.utils.ZipUtils;
import com.google.common.collect.ImmutableList;
+import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
@@ -139,7 +141,7 @@
Path input = Paths.get(EXAMPLES_BUILD_DIR, "arithmetic.jar");
ProcessResult result =
ToolHelper.forkD8(Paths.get("."), input.toString(), "--output", existingDir.toString());
- assertEquals(0, result.exitCode);
+ assertEquals(result.toString(), 0, result.exitCode);
assertTrue(Files.exists(classesFiles.get(0)));
for (int i = 1; i < classesFiles.size(); i++) {
Path file = classesFiles.get(i);
@@ -360,6 +362,62 @@
.build());
}
+ @Test(expected = CompilationFailedException.class)
+ public void errorOnEmptyClassfile() throws IOException, CompilationFailedException {
+ Path emptyFile = temp.getRoot().toPath().resolve("empty-file.class");
+ FileUtils.writeToFile(emptyFile, null, new byte[0]);
+ DiagnosticsChecker.checkErrorsContains(
+ "empty",
+ handler ->
+ D8.run(
+ D8Command.builder(handler)
+ .addProgramFiles(emptyFile)
+ .setProgramConsumer(DexIndexedConsumer.emptyConsumer())
+ .build()));
+ }
+
+ @Test(expected = CompilationFailedException.class)
+ public void errorOnInvalidClassfileHeader() throws IOException, CompilationFailedException {
+ Path emptyFile = temp.getRoot().toPath().resolve("empty-file.class");
+ FileUtils.writeToFile(emptyFile, null, new byte[] {'C', 'A', 'F', 'E', 'B', 'A', 'B', 'F'});
+ DiagnosticsChecker.checkErrorsContains(
+ "header",
+ handler ->
+ D8.run(
+ D8Command.builder(handler)
+ .addProgramFiles(emptyFile)
+ .setProgramConsumer(DexIndexedConsumer.emptyConsumer())
+ .build()));
+ }
+
+ @Test(expected = CompilationFailedException.class)
+ public void errorOnEmptyDex() throws IOException, CompilationFailedException {
+ Path emptyFile = temp.getRoot().toPath().resolve("empty-file.dex");
+ FileUtils.writeToFile(emptyFile, null, new byte[0]);
+ DiagnosticsChecker.checkErrorsContains(
+ "empty",
+ handler ->
+ D8.run(
+ D8Command.builder(handler)
+ .addProgramFiles(emptyFile)
+ .setProgramConsumer(DexIndexedConsumer.emptyConsumer())
+ .build()));
+ }
+
+ @Test(expected = CompilationFailedException.class)
+ public void errorOnInvalidDexHeader() throws IOException, CompilationFailedException {
+ Path emptyFile = temp.getRoot().toPath().resolve("empty-file.dex");
+ FileUtils.writeToFile(emptyFile, null, new byte[] {'C', 'A', 'F', 'E', 'B', 'A', 'B', 'E'});
+ DiagnosticsChecker.checkErrorsContains(
+ "header",
+ handler ->
+ D8.run(
+ D8Command.builder(handler)
+ .addProgramFiles(emptyFile)
+ .setProgramConsumer(DexIndexedConsumer.emptyConsumer())
+ .build()));
+ }
+
private D8Command parse(String... args) throws CompilationFailedException {
return D8Command.parse(args, EmbeddedOrigin.INSTANCE).build();
}
diff --git a/src/test/java/com/android/tools/r8/DiagnosticsChecker.java b/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
index 7cd32c7..fc22895 100644
--- a/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
+++ b/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
@@ -40,6 +40,8 @@
try {
runner.run(handler);
} catch (CompilationFailedException e) {
+ System.out.println("Expecting match for '" + snippet + "'");
+ System.out.println("StdErr:\n" + handler.errors);
assertTrue(
"Expected to find snippet '"
+ snippet
