From 2d2628df0001d2b643811dc69717c4afe70c4e31 Mon Sep 17 00:00:00 2001
From: David Phillips <david@sighup.nz>
Date: Sun, 11 Aug 2019 17:22:20 +1200
Subject: Fix and add tests for NULL deref cases found by scan-build

---
 assembler.c                                        |  3 +-
 lex.c                                              | 18 ++++---
 lex.h                                              |  3 +-
 parse.c                                            |  7 +--
 test/Makefile                                      |  1 +
 test/asm/run-asm.sh                                | 59 ++++++++++++++++++++++
 test/asm/should-fail/001-segfault-li-comma-end.asm |  2 +
 test/asm/should-fail/002-segfault-b-short.asm      |  2 +
 test/asm/should-fail/003-segfault-j-short.asm      |  2 +
 test/asm/should-fail/004-segfault-imm-short.asm    |  2 +
 test/full-pipeline/001-nop.asm                     |  1 -
 test/full-pipeline/002-nops.asm                    | 12 -----
 test/full-pipeline/003-rtype.asm                   |  6 ---
 test/full-pipeline/004-itype.asm                   |  5 --
 test/full-pipeline/005-small-loop.asm              |  7 ---
 test/full-pipeline/006-2-inst-2-words.asm          |  2 -
 test/full-pipeline/007-3-inst-3-words.asm          |  3 --
 test/full-pipeline/008-3-inst-5-words.asm          |  3 --
 test/full-pipeline/009-3-inst-5-words-rev.asm      |  3 --
 test/full-pipeline/010-empty.asm                   |  0
 test/full-pipeline/run-full-pipeline.sh            | 21 ++++----
 test/full-pipeline/should-pass/001-nop.asm         |  1 +
 test/full-pipeline/should-pass/002-nops.asm        | 12 +++++
 test/full-pipeline/should-pass/003-rtype.asm       |  6 +++
 test/full-pipeline/should-pass/004-itype.asm       |  5 ++
 test/full-pipeline/should-pass/005-small-loop.asm  |  7 +++
 .../should-pass/006-2-inst-2-words.asm             |  2 +
 .../should-pass/007-3-inst-3-words.asm             |  3 ++
 .../should-pass/008-3-inst-5-words.asm             |  3 ++
 .../should-pass/009-3-inst-5-words-rev.asm         |  3 ++
 test/full-pipeline/should-pass/010-empty.asm       |  0
 util.c                                             |  5 +-
 32 files changed, 143 insertions(+), 66 deletions(-)
 create mode 100755 test/asm/run-asm.sh
 create mode 100644 test/asm/should-fail/001-segfault-li-comma-end.asm
 create mode 100644 test/asm/should-fail/002-segfault-b-short.asm
 create mode 100644 test/asm/should-fail/003-segfault-j-short.asm
 create mode 100644 test/asm/should-fail/004-segfault-imm-short.asm
 delete mode 100644 test/full-pipeline/001-nop.asm
 delete mode 100644 test/full-pipeline/002-nops.asm
 delete mode 100644 test/full-pipeline/003-rtype.asm
 delete mode 100644 test/full-pipeline/004-itype.asm
 delete mode 100644 test/full-pipeline/005-small-loop.asm
 delete mode 100644 test/full-pipeline/006-2-inst-2-words.asm
 delete mode 100644 test/full-pipeline/007-3-inst-3-words.asm
 delete mode 100644 test/full-pipeline/008-3-inst-5-words.asm
 delete mode 100644 test/full-pipeline/009-3-inst-5-words-rev.asm
 delete mode 100644 test/full-pipeline/010-empty.asm
 create mode 100644 test/full-pipeline/should-pass/001-nop.asm
 create mode 100644 test/full-pipeline/should-pass/002-nops.asm
 create mode 100644 test/full-pipeline/should-pass/003-rtype.asm
 create mode 100644 test/full-pipeline/should-pass/004-itype.asm
 create mode 100644 test/full-pipeline/should-pass/005-small-loop.asm
 create mode 100644 test/full-pipeline/should-pass/006-2-inst-2-words.asm
 create mode 100644 test/full-pipeline/should-pass/007-3-inst-3-words.asm
 create mode 100644 test/full-pipeline/should-pass/008-3-inst-5-words.asm
 create mode 100644 test/full-pipeline/should-pass/009-3-inst-5-words-rev.asm
 create mode 100644 test/full-pipeline/should-pass/010-empty.asm

diff --git a/assembler.c b/assembler.c
index bd94c33..bc6d7dd 100644
--- a/assembler.c
+++ b/assembler.c
@@ -63,8 +63,6 @@ int main(int argc, char **argv)
 
 	if ((tokens = lex(path_in, fin, &tok_count)) == NULL)
 		return error_ret;
-
-	fclose(fin);
 	debug("Lexed.\n");
 
 	/* FIXME package these things into `tok_result`, `parse_result` etc */
@@ -86,6 +84,7 @@ int main(int argc, char **argv)
 
 	parse_free(insts, insts_count, labels, labels_count);
 	lex_free(tokens, tok_count);
+	fclose(fin);
 	fclose(fout);
 	debug("Output.\n");
 
diff --git a/lex.c b/lex.c
index 6ed8007..29cd8c2 100644
--- a/lex.c
+++ b/lex.c
@@ -296,6 +296,13 @@ static int lex_eol(struct token *t) {
 	return 0;
 }
 
+static int lex_eof(struct token *t) {
+	t->type = TOKEN_EOF;
+	t->span = 1;
+	store_location(t);
+	return 0;
+}
+
 int lex_line(void) {
 	int ret = 0;
 	size_t len = strlen(buffer);
@@ -424,12 +431,11 @@ struct token* lex(const char *filename_local, FILE *fin, size_t *len)
 		return NULL;
 	}
 
-	/* no tokens? just an EOL then */
-	if (tokens_count == 0) {
-		struct token eol = {0};
-		lex_eol(&eol);
-		add_token(eol);
-	}
+	/* tack on EOF */
+	line--; column--;
+	struct token eof = {0};
+	lex_eof(&eof);
+	add_token(eof);
 
 	*len = tokens_count;
 	return tokens;
diff --git a/lex.h b/lex.h
index 908d23e..389a5d7 100644
--- a/lex.h
+++ b/lex.h
@@ -12,7 +12,8 @@ enum TOKEN_TYPE {
 	TOKEN_STRING, /* string literal */
 	TOKEN_NUMERIC, /* numeric literal, incl literal chars */
 	TOKEN_REGISTER, /* $0, $H, $1 */
-	TOKEN_EOL /* end of line */
+	TOKEN_EOL, /* end of line */
+	TOKEN_EOF, /* end of file */
 };
 
 struct token {
diff --git a/parse.c b/parse.c
index 2bfc7da..a091939 100644
--- a/parse.c
+++ b/parse.c
@@ -22,6 +22,7 @@ static size_t labels_count;
 static struct instruction *insts;
 static size_t insts_count;
 static size_t byte_offset;
+static struct token token_eof = { .type = TOKEN_EOF };
 
 static void emit(const char *fmt, ...)
 {
@@ -59,7 +60,7 @@ static int expect(enum TOKEN_TYPE e)
 		if (cursor) {
 			observed_desc = get_token_description(cursor->type);
 		} else {
-			observed_desc = "end of file";
+			observed_desc = "(error: unknown)";
 		}
 		emit("Error: Expected %s, got %s\n", expected_desc, observed_desc);
 		return 1;
@@ -73,7 +74,7 @@ void kerchunk()
 	if (tokens_pos < tokens_count - 1) {
 		cursor = &tokens[++tokens_pos];
 	} else {
-		cursor = NULL;
+		cursor = &token_eof;
 	}
 }
 
@@ -500,7 +501,7 @@ int parse(const char *filename_local, FILE* fd_local, struct label **labels_loca
 	byte_offset = 0;
 
 	cursor = tokens;
-	while (cursor) {
+	while (cursor && cursor->type != TOKEN_EOF) {
 		switch(cursor->type) {
 			case TOKEN_EOL:
 				kerchunk();
diff --git a/test/Makefile b/test/Makefile
index cb0f3ee..083802c 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -1,3 +1,4 @@
 test:
+	./asm/run-asm.sh
 	./full-pipeline/run-full-pipeline.sh
 	./emul/run-emul.sh
diff --git a/test/asm/run-asm.sh b/test/asm/run-asm.sh
new file mode 100755
index 0000000..7574abb
--- /dev/null
+++ b/test/asm/run-asm.sh
@@ -0,0 +1,59 @@
+#!/bin/bash -e
+
+#
+# Script for running all of the automated which will go from source to binary.
+#
+
+fail() {
+	echo -e '[\e[1;31mFAIL\e[0m] '"$1:" "$2"
+	has_failure=1
+}
+
+pass() {
+	echo -e '[\e[1;32mPASS\e[0m] '"$1"
+}
+
+clean() {
+	echo "Removing work dir $WORK"
+	rm -r "$WORK"
+}
+
+if [ "$1" == "noclean" ]; then
+	NO_CLEAN=1
+else
+	NO_CLEAN=0
+fi
+WORK=$(mktemp -d)
+pushd $(dirname "$0") >/dev/null
+source ../valgrind.sh
+export ASM="$PWD/../../assembler"
+export DISASM="$PWD/../../disassembler"
+has_failure=0
+
+for first_stage_asm in should-fail/*.asm ; do
+	t=$(basename "$first_stage_asm")
+	first_stage_bin="$WORK/${t}-first_stage.bin"
+	log="$WORK/${t}.log"
+
+	# Assemble test code
+	set +e
+	$VALGRIND $VALGRIND_OPTS "$ASM" "$first_stage_asm" "$first_stage_bin" 2>"$log"
+	xc="$?"
+	set -e
+	if (( xc > 0 && xc < 128 )); then
+			pass "$t" "assembly xfailed"
+	elif (( xc == 0 )); then
+		fail "$t" "assembly didn't fail as expected"
+	else
+		fail "$t" "assembler was sent signal $(( xc - 128 ))"
+	fi
+done
+popd >/dev/null
+
+if [[ "$failure" != "0" && "$NO_CLEAN" == "1"  ]] ; then
+	echo "Warning: Leaving work dir $WORK in place. Please remove this yourself"
+else
+	clean
+fi
+
+exit "$has_failure"
diff --git a/test/asm/should-fail/001-segfault-li-comma-end.asm b/test/asm/should-fail/001-segfault-li-comma-end.asm
new file mode 100644
index 0000000..bde586c
--- /dev/null
+++ b/test/asm/should-fail/001-segfault-li-comma-end.asm
@@ -0,0 +1,2 @@
+; Test for bug previously found with scan-build. Important: no CR/LF at EOF
+ldi $1, 
\ No newline at end of file
diff --git a/test/asm/should-fail/002-segfault-b-short.asm b/test/asm/should-fail/002-segfault-b-short.asm
new file mode 100644
index 0000000..898a3b0
--- /dev/null
+++ b/test/asm/should-fail/002-segfault-b-short.asm
@@ -0,0 +1,2 @@
+; Test for bug previously found with scan-build. Important: no CR/LF at EOF
+bnz
\ No newline at end of file
diff --git a/test/asm/should-fail/003-segfault-j-short.asm b/test/asm/should-fail/003-segfault-j-short.asm
new file mode 100644
index 0000000..0b1486b
--- /dev/null
+++ b/test/asm/should-fail/003-segfault-j-short.asm
@@ -0,0 +1,2 @@
+; Test for bug previously found with scan-build. Important: no CR/LF at EOF
+jmp
\ No newline at end of file
diff --git a/test/asm/should-fail/004-segfault-imm-short.asm b/test/asm/should-fail/004-segfault-imm-short.asm
new file mode 100644
index 0000000..42e8c4b
--- /dev/null
+++ b/test/asm/should-fail/004-segfault-imm-short.asm
@@ -0,0 +1,2 @@
+; Test for bug previously found with scan-build. Important: no CR/LF at EOF
+addi $0, $0,
\ No newline at end of file
diff --git a/test/full-pipeline/001-nop.asm b/test/full-pipeline/001-nop.asm
deleted file mode 100644
index c27745a..0000000
--- a/test/full-pipeline/001-nop.asm
+++ /dev/null
@@ -1 +0,0 @@
-nop
diff --git a/test/full-pipeline/002-nops.asm b/test/full-pipeline/002-nops.asm
deleted file mode 100644
index 45a3cbf..0000000
--- a/test/full-pipeline/002-nops.asm
+++ /dev/null
@@ -1,12 +0,0 @@
-nop
-nop
-nop
-nop
-add $0, $0, $0
-nop
-bn 0
-nop
-nop
-jn $0
-nop
-nop
diff --git a/test/full-pipeline/003-rtype.asm b/test/full-pipeline/003-rtype.asm
deleted file mode 100644
index 7ed5f78..0000000
--- a/test/full-pipeline/003-rtype.asm
+++ /dev/null
@@ -1,6 +0,0 @@
-; Test for some sort of parity between pseudo instructions for rtypes
-mv $0, $1
-mv $H, $2
-mv $2, $1
-neg $1
-neg $0
diff --git a/test/full-pipeline/004-itype.asm b/test/full-pipeline/004-itype.asm
deleted file mode 100644
index 4fbc032..0000000
--- a/test/full-pipeline/004-itype.asm
+++ /dev/null
@@ -1,5 +0,0 @@
-; Test for some sort of parity between pseudo instructions for itypes
-ldi $0, 1234
-ldi $1, 0x1234
-ldi $1, 1
-ldi $1, 10
diff --git a/test/full-pipeline/005-small-loop.asm b/test/full-pipeline/005-small-loop.asm
deleted file mode 100644
index 5c47e51..0000000
--- a/test/full-pipeline/005-small-loop.asm
+++ /dev/null
@@ -1,7 +0,0 @@
-ldi $1, 2
-ldi $2, 20
-ldi $3, 0
-loop:
-	add $3, $3, $1
-	subi $2, $2, 1
-	bnz loop
diff --git a/test/full-pipeline/006-2-inst-2-words.asm b/test/full-pipeline/006-2-inst-2-words.asm
deleted file mode 100644
index c4e2dbc..0000000
--- a/test/full-pipeline/006-2-inst-2-words.asm
+++ /dev/null
@@ -1,2 +0,0 @@
-nop
-nop
diff --git a/test/full-pipeline/007-3-inst-3-words.asm b/test/full-pipeline/007-3-inst-3-words.asm
deleted file mode 100644
index 717a122..0000000
--- a/test/full-pipeline/007-3-inst-3-words.asm
+++ /dev/null
@@ -1,3 +0,0 @@
-nop
-nop
-nop
diff --git a/test/full-pipeline/008-3-inst-5-words.asm b/test/full-pipeline/008-3-inst-5-words.asm
deleted file mode 100644
index c0a33b5..0000000
--- a/test/full-pipeline/008-3-inst-5-words.asm
+++ /dev/null
@@ -1,3 +0,0 @@
-jmp 0
-jmp 0
-nop
diff --git a/test/full-pipeline/009-3-inst-5-words-rev.asm b/test/full-pipeline/009-3-inst-5-words-rev.asm
deleted file mode 100644
index 5592edd..0000000
--- a/test/full-pipeline/009-3-inst-5-words-rev.asm
+++ /dev/null
@@ -1,3 +0,0 @@
-nop
-jmp 0
-jmp 0
diff --git a/test/full-pipeline/010-empty.asm b/test/full-pipeline/010-empty.asm
deleted file mode 100644
index e69de29..0000000
diff --git a/test/full-pipeline/run-full-pipeline.sh b/test/full-pipeline/run-full-pipeline.sh
index 25f7bf7..c5a1700 100755
--- a/test/full-pipeline/run-full-pipeline.sh
+++ b/test/full-pipeline/run-full-pipeline.sh
@@ -11,6 +11,7 @@
 
 fail() {
 	echo -e '[\e[1;31mFAIL\e[0m] '"$1:" "$2"
+	has_failure=1
 }
 
 pass() {
@@ -34,33 +35,33 @@ export ASM="$PWD/../../assembler"
 export DISASM="$PWD/../../disassembler"
 has_failure=0
 
-for first_stage_asm in *.asm ; do
-	first_stage_bin="$WORK/${first_stage_asm}-first_stage.bin"
-	second_stage_asm="$WORK/${first_stage_asm}-second_stage.asm"
-	second_stage_bin="$WORK/${first_stage_asm}-second_stage.bin"
+for first_stage_asm in should-pass/*.asm ; do
+	t=$(basename "$first_stage_asm")
+	first_stage_bin="$WORK/${t}-first_stage.bin"
+	second_stage_asm="$WORK/${t}-second_stage.asm"
+	second_stage_bin="$WORK/${t}-second_stage.bin"
 
 	# Assemble test code
 	if ! $VALGRIND $VALGRIND_OPTS "$ASM" "$first_stage_asm" "$first_stage_bin" ; then
-		fail "$first_stage_asm" "first stage assembly failed"
+		fail "$t" "first stage assembly failed"
 		continue
 	fi
 
 	# Disassemble test code and re-assemble that disassembly
 	if ! $VALGRIND $VALGRIND_OPTS "$DISASM" "$first_stage_bin" "$second_stage_asm" ; then
-		fail "$first_stage_asm" "first stage disassembly failed"
+		fail "$t" "first stage disassembly failed"
 		continue
 	fi
 	if ! $VALGRIND $VALGRIND_OPTS "$ASM" "$second_stage_asm" "$second_stage_bin" ; then
-		fail "$first_stage_asm" "second stage assembly failed"
+		fail "$t" "second stage assembly failed"
 		continue
 	fi
 
 	# first stage bin and second stage identical for test pass
 	if diff "$first_stage_bin" "$second_stage_bin" >/dev/null; then
-		pass "$first_stage_asm"
+		pass "$t"
 	else
-		fail "$first_stage_asm" "binary mismatch"
-		has_failure=1
+		fail "$t" "binary mismatch"
 	fi
 
 done
diff --git a/test/full-pipeline/should-pass/001-nop.asm b/test/full-pipeline/should-pass/001-nop.asm
new file mode 100644
index 0000000..c27745a
--- /dev/null
+++ b/test/full-pipeline/should-pass/001-nop.asm
@@ -0,0 +1 @@
+nop
diff --git a/test/full-pipeline/should-pass/002-nops.asm b/test/full-pipeline/should-pass/002-nops.asm
new file mode 100644
index 0000000..45a3cbf
--- /dev/null
+++ b/test/full-pipeline/should-pass/002-nops.asm
@@ -0,0 +1,12 @@
+nop
+nop
+nop
+nop
+add $0, $0, $0
+nop
+bn 0
+nop
+nop
+jn $0
+nop
+nop
diff --git a/test/full-pipeline/should-pass/003-rtype.asm b/test/full-pipeline/should-pass/003-rtype.asm
new file mode 100644
index 0000000..7ed5f78
--- /dev/null
+++ b/test/full-pipeline/should-pass/003-rtype.asm
@@ -0,0 +1,6 @@
+; Test for some sort of parity between pseudo instructions for rtypes
+mv $0, $1
+mv $H, $2
+mv $2, $1
+neg $1
+neg $0
diff --git a/test/full-pipeline/should-pass/004-itype.asm b/test/full-pipeline/should-pass/004-itype.asm
new file mode 100644
index 0000000..4fbc032
--- /dev/null
+++ b/test/full-pipeline/should-pass/004-itype.asm
@@ -0,0 +1,5 @@
+; Test for some sort of parity between pseudo instructions for itypes
+ldi $0, 1234
+ldi $1, 0x1234
+ldi $1, 1
+ldi $1, 10
diff --git a/test/full-pipeline/should-pass/005-small-loop.asm b/test/full-pipeline/should-pass/005-small-loop.asm
new file mode 100644
index 0000000..5c47e51
--- /dev/null
+++ b/test/full-pipeline/should-pass/005-small-loop.asm
@@ -0,0 +1,7 @@
+ldi $1, 2
+ldi $2, 20
+ldi $3, 0
+loop:
+	add $3, $3, $1
+	subi $2, $2, 1
+	bnz loop
diff --git a/test/full-pipeline/should-pass/006-2-inst-2-words.asm b/test/full-pipeline/should-pass/006-2-inst-2-words.asm
new file mode 100644
index 0000000..c4e2dbc
--- /dev/null
+++ b/test/full-pipeline/should-pass/006-2-inst-2-words.asm
@@ -0,0 +1,2 @@
+nop
+nop
diff --git a/test/full-pipeline/should-pass/007-3-inst-3-words.asm b/test/full-pipeline/should-pass/007-3-inst-3-words.asm
new file mode 100644
index 0000000..717a122
--- /dev/null
+++ b/test/full-pipeline/should-pass/007-3-inst-3-words.asm
@@ -0,0 +1,3 @@
+nop
+nop
+nop
diff --git a/test/full-pipeline/should-pass/008-3-inst-5-words.asm b/test/full-pipeline/should-pass/008-3-inst-5-words.asm
new file mode 100644
index 0000000..c0a33b5
--- /dev/null
+++ b/test/full-pipeline/should-pass/008-3-inst-5-words.asm
@@ -0,0 +1,3 @@
+jmp 0
+jmp 0
+nop
diff --git a/test/full-pipeline/should-pass/009-3-inst-5-words-rev.asm b/test/full-pipeline/should-pass/009-3-inst-5-words-rev.asm
new file mode 100644
index 0000000..5592edd
--- /dev/null
+++ b/test/full-pipeline/should-pass/009-3-inst-5-words-rev.asm
@@ -0,0 +1,3 @@
+nop
+jmp 0
+jmp 0
diff --git a/test/full-pipeline/should-pass/010-empty.asm b/test/full-pipeline/should-pass/010-empty.asm
new file mode 100644
index 0000000..e69de29
diff --git a/util.c b/util.c
index a4b6949..e291f90 100644
--- a/util.c
+++ b/util.c
@@ -36,6 +36,7 @@ static struct {
 	{ .look = TOKEN_IDENT   , .str = "identifier"          },
 	{ .look = TOKEN_DOT     , .str = "assembler directive" },
 	{ .look = TOKEN_EOL     , .str = "end of line"         },
+	{ .look = TOKEN_EOF     , .str = "end of file"         },
 	{ .str = NULL },
 };
 
@@ -191,8 +192,8 @@ void indicate_file_area(FILE* fd, size_t line, size_t column, size_t span)
 	fputs(margin, stderr);
 	fputs(s, stderr);
 
-	/* corner case (still needed?) - buf was just return */
-	if (strlen(buf) == 1 && buf[0] == '\n') {
+	/* corner case: buf had no return */
+	if (s[strlen(s) - 1] != '\n') {
 		fputc('\n', stderr);
 	}
 
-- 
cgit v1.1