Improved error handling

git-svn-id: https://smali.googlecode.com/svn/trunk@102 55b6fa8a-2a1e-11de-a435-ffa8d773f76a

src/main/antlr3/org/JesusFreke/smali/smaliLexer.g

@@ -185,9 +185,15 @@ import java.util.ArrayDeque;
     		tokens.add(token);
     	}
 
+	private int lexerErrors = 0;
 	public String getErrorHeader(RecognitionException e) {
+		lexerErrors++;
 		return getSourceName()+"["+ e.line+","+e.charPositionInLine+"]";
 	}
+	
+	public int getNumberOfLexerErrors() {
+		return lexerErrors;
+	}
 }
 
 

src/main/antlr3/org/JesusFreke/smali/smaliParser.g

@@ -207,6 +207,7 @@ access_list
 
 
 field	:	FIELD_DIRECTIVE access_list MEMBER_NAME nonvoid_type_descriptor literal? 
+		//TODO: get rid of this predicate
 		(	(annotation+ END_FIELD_DIRECTIVE)=> annotation+ END_FIELD_DIRECTIVE
 		| 	END_FIELD_DIRECTIVE?
 		)

src/main/antlr3/org/JesusFreke/smali/smaliTreeWalker.g

@@ -50,48 +50,60 @@ import org.JesusFreke.dexlib.code.Format.*;
 	public ClassDefItem classDefItem;
 	public ClassDataItem classDataItem;
 
-	private static byte parseRegister_nibble(String register, int totalMethodRegisters, int methodParameterRegisters) {
+	private byte parseRegister_nibble(String register, int totalMethodRegisters, int methodParameterRegisters)
+		throws SemanticException {
 		//register should be in the format "v12"		
 		int val = Byte.parseByte(register.substring(1));
 		if (register.charAt(0) == 'p') {
 			val = totalMethodRegisters - methodParameterRegisters + val;
 		}		
 		if (val >= 2<<4) {
-			//TODO: throw correct exception type
+			throw new SemanticException(input, "The maximum allowed register in this context is list of registers is v15");
-			throw new RuntimeException("The maximum allowed register in this context is list of registers is v15");
 		}
-		//the parser wouldn't accept a negative register, i.e. v-1, so we don't have to check for val<0;
+		//the parser wouldn't have accepted a negative register, i.e. v-1, so we don't have to check for val<0;
 		return (byte)val;
 	}
 	
 	//return a short, because java's byte is signed
-	private static short parseRegister_byte(String register, int totalMethodRegisters, int methodParameterRegisters) {
+	private short parseRegister_byte(String register, int totalMethodRegisters, int methodParameterRegisters)
+		throws SemanticException {
 		//register should be in the format "v123"
 		int val = Short.parseShort(register.substring(1));
 		if (register.charAt(0) == 'p') {
 			val = totalMethodRegisters - methodParameterRegisters + val;
 		}
 		if (val >= 2<<8) {
-			//TODO: throw correct exception type
+			throw new SemanticException(input, "The maximum allowed register in this context is v255");
-			throw new RuntimeException("The maximum allowed register in this context is v255");
 		}
 		return (short)val;
 	}
 	
 	//return an int because java's short is signed
-	private static int parseRegister_short(String register, int totalMethodRegisters, int methodParameterRegisters) {
+	private int parseRegister_short(String register, int totalMethodRegisters, int methodParameterRegisters)
+		throws SemanticException {
 		//register should be in the format "v12345"		
 		int val = Integer.parseInt(register.substring(1));
 		if (register.charAt(0) == 'p') {
 			val = totalMethodRegisters - methodParameterRegisters + val;
 		}
 		if (val >= 2<<16) {
-			//TODO: throw correct exception type
+			throw new SemanticException(input, "The maximum allowed register in this context is v65535");
-			throw new RuntimeException("The maximum allowed register in this context is v65535");
 		}
 		//the parser wouldn't accept a negative register, i.e. v-1, so we don't have to check for val<0;
 		return val;
 	}
+	
+	public String getErrorMessage(RecognitionException e, String[] tokenNames) {
+		if ( e instanceof SemanticException ) {
+			return e.getMessage();
+		} else {
+			return super.getErrorMessage(e, tokenNames);
+		}
+	}
+	
+	public String getErrorHeader(RecognitionException e) {
+		return getSourceName()+"["+ e.line+","+e.charPositionInLine+"]";
+	}
 }
 
 
@@ -217,8 +229,7 @@ field returns[ClassDataItem.EncodedField encodedField, EncodedValue encodedValue
 		
 		if ($field_initial_value.encodedValue != null) {
 			if (($access_list.value & AccessFlags.STATIC) == 0) {
-				//TODO: change to an appropriate exception type?
+				throw new SemanticException(input, "Initial field values can only be specified for static fields.");
-				throw new RuntimeException("Initial field values can only be specified for static fields.");
 			}
 			
 			$encodedValue = $field_initial_value.encodedValue;
@@ -408,16 +419,14 @@ method returns[	ClassDataItem.EncodedMethod encodedMethod,
 			
 		} else {
 			if (totalMethodRegisters < methodParameterRegisters) {
-				//TODO: throw the correct exception type
+				throw new SemanticException(input, "This method requires at least " +
-				throw new RuntimeException(	"This method requires at least " +
 								Integer.toString(methodParameterRegisters) +
 								" registers, for the method parameters");
 			}
 			
 			int methodParameterCount = methodIdItem.getParameterCount();
 			if ($method::debugInfo.getParameterNameCount() > methodParameterCount) {
-				//TODO: throw the correct exception type
+				throw new SemanticException(input, "Too many parameter names specified. This method only has " +
-				throw new RuntimeException(	"Too many parameter names specified. This method only has " +
 								Integer.toString(methodParameterCount) +
 								" parameters.");
 			}
@@ -505,8 +514,7 @@ label_def
 		{
 			String labelName = $label.labelName;
 			if ($method::labels.containsKey(labelName)) {
-				//TODO: use appropriate exception type
+				throw new SemanticException(input, "Label " + labelName + " has multiple defintions.");
-				throw new RuntimeException("Label " + labelName + " has multiple defintions.");
 			}
 				
 			
@@ -635,8 +643,10 @@ statements[int totalMethodRegisters, int methodParameterRegisters] returns[Array
 	}
 	:	^(I_STATEMENTS	(instruction[$totalMethodRegisters, $methodParameterRegisters]
 				{
-					$instructions.add($instruction.instruction);
+					if ($instruction.instruction != null) {
-					$method::currentAddress += $instruction.instruction.getBytes().length/2;
+						$instructions.add($instruction.instruction);
+						$method::currentAddress += $instruction.instruction.getBytes().length/2;
+					}
 				})*);
 			
 label_ref returns[int labelAddress]
@@ -647,8 +657,7 @@ label_ref returns[int labelAddress]
 			Integer labelAdd = $method::labels.get(labelName);
 			
 			if (labelAdd == null) {
-				//TODO: throw correct exception type
+				throw new SemanticException(input, "Label \"" + labelName + "\" is not defined.");
-				throw new RuntimeException("Label \"" + labelName + "\" is not defined.");
 			}
 			
 			$labelAddress = labelAdd;
@@ -686,8 +695,7 @@ instruction[int totalMethodRegisters, int methodParameterRegisters]  returns[Ins
 			int addressOffset = $offset_or_label.offsetValue;
 
 			if (addressOffset < Byte.MIN_VALUE || addressOffset > Byte.MAX_VALUE) {
-				//TODO: throw correct exception type
+				throw new SemanticException(input, "The offset/label is out of range. The offset is " + Integer.toString(addressOffset) + " and the range for this opcode is [-128, 127].");
-				throw new RuntimeException("The offset/label is out of range. The offset is " + Integer.toString(addressOffset) + " and the range for this opcode is [-128, 127].");
 			}
 			
 			$instruction = Format10t.Format.make(dexFile, opcode.value, (byte)addressOffset);
@@ -734,8 +742,7 @@ instruction[int totalMethodRegisters, int methodParameterRegisters]  returns[Ins
 			int addressOffset = $offset_or_label.offsetValue;
 
 			if (addressOffset < Short.MIN_VALUE || addressOffset > Short.MAX_VALUE) {
-				//TODO: throw correct exception type
+				throw new SemanticException(input, "The offset/label is out of range. The offset is " + Integer.toString(addressOffset) + " and the range for this opcode is [-32768, 32767].");
-				throw new RuntimeException("The offset/label is out of range. The offset is " + Integer.toString(addressOffset) + " and the range for this opcode is [-32768, 32767].");
 			}
 			
 			$instruction = Format20t.Format.make(dexFile, opcode.value, (short)addressOffset);
@@ -799,8 +806,7 @@ instruction[int totalMethodRegisters, int methodParameterRegisters]  returns[Ins
 			int addressOffset = $offset_or_label.offsetValue;
 
 			if (addressOffset < Short.MIN_VALUE || addressOffset > Short.MAX_VALUE) {
-				//TODO: throw correct exception type
+				throw new SemanticException(input, "The offset/label is out of range. The offset is " + Integer.toString(addressOffset) + " and the range for this opcode is [-32768, 32767].");
-				throw new RuntimeException("The offset/label is out of range. The offset is " + Integer.toString(addressOffset) + " and the range for this opcode is [-32768, 32767].");
 			}
 			
 			$instruction = Format21t.Format.make(dexFile, opcode.value, regA, (short)addressOffset);
@@ -860,8 +866,7 @@ instruction[int totalMethodRegisters, int methodParameterRegisters]  returns[Ins
 			int addressOffset = $offset_or_label.offsetValue;
 
 			if (addressOffset < Short.MIN_VALUE || addressOffset > Short.MAX_VALUE) {
-				//TODO: throw correct exception type
+				throw new SemanticException(input, "The offset/label is out of range. The offset is " + Integer.toString(addressOffset) + " and the range for this opcode is [-32768, 32767].");
-				throw new RuntimeException("The offset/label is out of range. The offset is " + Integer.toString(addressOffset) + " and the range for this opcode is [-32768, 32767].");
 			}
 			
 			$instruction = Format22t.Format.make(dexFile, opcode.value, regA, regB, (short)addressOffset);
@@ -959,12 +964,10 @@ instruction[int totalMethodRegisters, int methodParameterRegisters]  returns[Ins
 			
 			int registerCount = endRegister-startRegister+1;
 			if (registerCount > 256) {
-				//TODO: throw appropriate exception type
+				throw new SemanticException(input, "A register range can span a maximum of 256 registers");
-				throw new RuntimeException("A register range can span a maximum of 256 registers");
 			}
 			if (registerCount < 1) {
-				//TODO: throw appropriate exception type
+				throw new SemanticException(input, "A register range must have the lower register listed first");
-				throw new RuntimeException("A register range must have the lower register listed first");
 			}
 			
 			MethodIdItem methodIdItem = $fully_qualified_method.methodIdItem;
@@ -1019,9 +1022,9 @@ register_list[int totalMethodRegisters, int methodParameterRegisters] returns[by
 	:	^(I_REGISTER_LIST 
 			(REGISTER
 			{
+				//TODO: shouldn't this be == 6?
 				if ($registerCount == 5) {
-					//TODO: throw the correct type of exception
+					throw new SemanticException(input, "A list of registers can only have a maximum of 5 registers. Use the <op>/range alternate opcode instead.");
-					throw new RuntimeException("A list of registers can only have a maximum of 5 registers. Use the <op>/range alternate opcode instead.");
 				}
 				$registers[$registerCount++] = parseRegister_nibble($REGISTER.text, $totalMethodRegisters, $methodParameterRegisters);
 			})*);

src/main/java/org/JesusFreke/smali/SemanticException.java

@@ -0,0 +1,46 @@
+/*
+ * [The "BSD licence"]
+ * Copyright (c) 2009 Ben Gruver
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. The name of the author may not be used to endorse or promote products
+ *    derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.JesusFreke.smali;
+
+import org.antlr.runtime.RecognitionException;
+import org.antlr.runtime.IntStream;
+
+public class SemanticException extends RecognitionException {
+    private String errorMessage;
+
+
+    SemanticException(IntStream input, String errorMessage) {
+        super(input);
+        this.errorMessage = errorMessage;
+    }
+
+    public String getMessage() {
+        return errorMessage;
+    }
+}

src/main/java/org/JesusFreke/smali/smali.java

@@ -117,8 +117,16 @@ public class smali
 
         DexFile dexFile = DexFile.makeBlankDexFile();
 
+        boolean errors = false;
+
         for (File file: filesToProcess) {
-            assembleSmaliFile(file, dexFile);
+            if (!assembleSmaliFile(file, dexFile)) {
+                errors = true;
+            }
+        }
+
+        if (errors) {
+            System.exit(1);
         }
 
         dexFile.place();
@@ -152,6 +160,7 @@ public class smali
         } catch (Exception ex)
         {
             System.out.println(ex.toString());
+            System.exit(1);
         }
     }
 
@@ -165,17 +174,23 @@ public class smali
         }
     }
 
-    private static void assembleSmaliFile(File smaliFile, DexFile dexFile)
+    private static boolean assembleSmaliFile(File smaliFile, DexFile dexFile)
             throws Exception {
         ANTLRInputStream input = new ANTLRInputStream(new FileInputStream(smaliFile));
         input.name = smaliFile.getAbsolutePath();
 
         smaliLexer lexer = new smaliLexer(input);
-
+       
         CommonTokenStream tokens = new CommonTokenStream(lexer);
         smaliParser parser = new smaliParser(tokens);
 
         smaliParser.smali_file_return result = parser.smali_file();
+
+        if (parser.getNumberOfSyntaxErrors() > 0 || lexer.getNumberOfLexerErrors() > 0) {
+            return false;
+        }
+
+
         CommonTree t = (CommonTree) result.getTree();
 
         CommonTreeNodeStream treeStream = new CommonTreeNodeStream(t);
@@ -186,6 +201,11 @@ public class smali
         dexGen.dexFile = dexFile;
         dexGen.smali_file();
 
+        if (dexGen.getNumberOfSyntaxErrors() > 0) {
+            return false;
+        }
+
         dexFile.ClassDefsSection.intern(dexFile, dexGen.classDefItem);
+        return true;
     }
 }