diff --git a/.gitignore b/.gitignore index d551c4e..a886581 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ /Assembler /SplitBit /SplitDisk +/SplitLint /CLAUDE.md /claudeResume.sh /codexResume.sh diff --git a/Programs/testPrograms/branchTest.asm b/Programs/testPrograms/branchTest.asm index 07b666b..634ec38 100644 --- a/Programs/testPrograms/branchTest.asm +++ b/Programs/testPrograms/branchTest.asm @@ -46,7 +46,7 @@ bTaken: OUTA 0x00 CCF - BNC cTaken + BNC cTaken ; splitlint: this test exists to check a branch whose carry is known BRI wrong cTaken: CALL blankSpace @@ -78,7 +78,7 @@ cTaken: INIB 0x01 CCF ADD - BNC wrong + BNC wrong ; splitlint: the point is that a set carry does NOT take this CALL blankSpace INIA 0d99 ; 'c' OUTA 0x00 diff --git a/Programs/testPrograms/interruptFlagTest.asm b/Programs/testPrograms/interruptFlagTest.asm index dc46193..8755445 100644 --- a/Programs/testPrograms/interruptFlagTest.asm +++ b/Programs/testPrograms/interruptFlagTest.asm @@ -28,7 +28,7 @@ start: ADD ; Q = 0, and the Carry Flag is set. SIF CIF - BRC carryHeld + BRC carryHeld ; splitlint: whether carry survived SIF and CIF is what is being tested ; Falling through here means one flag trampled the other. INIA 0d88 ; 'X' diff --git a/README.md b/README.md index d16091e..369331a 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ wrote Asm.sbx: program 7533, data 4099, labels 555 | [`Source/Emulator`](Source/Emulator) | The machine: CPU, memory controller, devices, console, disk | | [`Source/Assembler`](Source/Assembler) | The assembler that runs on a host | | [`Source/DiskTool`](Source/DiskTool) | SplitDisk, which reads and writes SplitBit's filesystem | +| [`Source/Linter`](Source/Linter) | SplitLint, which points out needlessly long assembly forms | | [`Programs/Examples`](Programs/Examples) | Programs to read: hello, a calculator, Fibonacci, a prime sieve, Life | | [`Programs/Libraries`](Programs/Libraries) | Code included by name rather than linked, since there is no linker | | [`Programs/Loader`](Programs/Loader) | The standalone loader CosmOS grew out of | @@ -49,7 +50,7 @@ wrote Asm.sbx: program 7533, data 4099, labels 555 ## Getting Started: -Clone it and build the three tools. You need gcc and make, or similar: +Clone it and build the four tools. You need gcc and make, or similar: ``` git clone https://github.com/RealBusinessAccount/SplitBit-Emulator.git @@ -156,6 +157,37 @@ this is what turns such a count into a list of routine names. Without `-o` the output takes the source file's name, in the directory you called the assembler from, with the extension the format asks for: `.bin` for a boot image and `.sbx` for a loadable program. Included files are looked for beside the file that includes them, and then along the directories given with `-I`. +## Checking Assembly: SplitLint + +``` +./SplitLint [--fatal-warnings] [sourcefile ...] +``` + +SplitLint reports valid assembly that has a shorter direct expression, beginning with +zero loads that can use `RSTA` or `RSTB`, Q-to-register transfers that need not pass +through the stack, self-push/pop pairs, register assignments overwritten by the next +instruction, instructions with no ordinary fallthrough path, and one-byte Data Pointer +changes that can use `INCD` or `DECD`, including direct branches whose target is already +the next labeled address. It also tracks symbolic Data Pointer bases and known offsets to +find a `SETD` that reloads an address the pointer already holds. Device input and stack +pops are excluded from dead-assignment checks because consuming their input is itself an effect. SplitLint's control-flow +knowledge is deliberately local: labels begin reachable regions, while an unparsed +directive or data item ends the current claim. It does not yet expand includes or build a +complete control-flow graph. Warnings normally leave a successful exit status, while +`--fatal-warnings` makes any warning fail the command for use in automated checks. + +**A line whose comment says `splitlint: ` is not reported on, and the reason is +required.** A suppression with no explanation is a way to make a tool quiet rather than a +way to say something, and a bare marker is refused rather than honoured. The corpus has +three of them, all in test programs: `branchTest.asm` exists to check that a branch whose +carry is known behaves correctly, so a diagnostic saying the outcome is known is exactly +right and exactly unwanted. Suppressions are counted and reported at the end of a run, so +that the claim they make is visible rather than silent. + +The same local model tracks whether carry is known set or clear. It reports a redundant +`CCF`, a `BRC` or `BNC` whose outcome is already determined, and computes carry through +increment, decrement, addition, and subtraction when their inputs are known. + ## Managing Disks: SplitDisk ``` diff --git a/Source/Linter/Linter.c b/Source/Linter/Linter.c new file mode 100644 index 0000000..2c05c0c --- /dev/null +++ b/Source/Linter/Linter.c @@ -0,0 +1,734 @@ +// Linter.c +// Small, deliberately conservative source linter for SplitBit assembly. +// +// This first version works one source file at a time and looks only at instructions +// which are adjacent in that file. It is not an assembler and does not expand +// includes or follow control flow. That modest boundary makes every diagnostic easy +// to explain while the useful rules, and the interface they need, are still being +// discovered. + +#include +#include +#include +#include +#include "../Assembler/assembly.h" +#include "../Emulator/cpu.h" + +#define LINE_CAPACITY 1024 +#define TOKEN_CAPACITY 64 + +typedef struct { + char mnemonic[TOKEN_CAPACITY]; + char spelling[TOKEN_CAPACITY]; + char operand[TOKEN_CAPACITY]; + long literal; + int hasLiteral; + int line; +} SourceInstruction; + +typedef enum { + SOURCE_OTHER, + SOURCE_INSTRUCTION, + SOURCE_LABEL, + SOURCE_BOUNDARY +} SourceLine; + +typedef struct { + int aKnown; + int bKnown; + uint8_t a; + uint8_t b; +} KnownRegisters; + +typedef struct { + int known; + char base[TOKEN_CAPACITY]; + uint16_t offset; +} KnownPointer; + +typedef enum { + CARRY_UNKNOWN = -1, + CARRY_CLEAR = 0, + CARRY_SET = 1 +} KnownCarry; + +static void usage(const char *name) { + printf("Usage: %s [--fatal-warnings] [sourcefile ...]\n", name); + printf("\n"); + printf("Checks SplitBit assembly source for correct but needlessly long forms.\n"); + printf("Warnings do not fail the command unless --fatal-warnings is given.\n"); + printf("\n"); + printf("A line whose comment says \"splitlint: \" is not reported on. The\n"); + printf("reason is required, so that a deliberate exception says what makes it one.\n"); +} + +static void uppercase(char *text) { + for (; *text; text++) { + *text = (char)toupper((unsigned char)*text); + } +} + +// A semicolon inside a string belongs to the string. Anything after one outside a +// string is a comment and must not accidentally look like an instruction. +static void removeComment(char *line) { + int inString = 0; + for (char *at = line; *at; at++) { + if (*at == '"') { + inString = !inString; + } else if (*at == ';' && !inString) { + *at = '\0'; + return; + } + } +} + +static int literalValue(const char *text, long *value) { + int base; + if (text[0] != '0' || (text[1] != 'x' && text[1] != 'X' + && text[1] != 'd' && text[1] != 'D')) { + return 0; + } + base = (text[1] == 'x' || text[1] == 'X') ? 16 : 10; + char *end; + long parsed = strtol(text + 2, &end, base); + if (end == text + 2 || *end != '\0' || parsed < 0 || parsed > 255) { + return 0; + } + *value = parsed; + return 1; +} + +// Classifies enough of a physical source line for local analysis. Directives, strings, +// literal data, and unknown tokens are boundaries: until the linter shares the full +// assembler frontend, it makes no control-flow claim across something it did not parse. +static SourceLine parseInstruction(char *line, int lineNumber, SourceInstruction *result) { + removeComment(line); + char *token = strtok(line, " \t\r\n"); + if (!token) { + return SOURCE_OTHER; + } + if (token[0] == '#' || token[0] == '"') { + return SOURCE_BOUNDARY; + } + size_t length = strlen(token); + if (length && token[length - 1] == ':') { + memset(result, 0, sizeof(*result)); + if (length - 1 < sizeof(result->spelling)) { + memcpy(result->spelling, token, length - 1); + result->spelling[length - 1] = '\0'; + } + result->line = lineNumber; + return SOURCE_LABEL; + } + + char mnemonic[TOKEN_CAPACITY]; + if (length >= sizeof(mnemonic)) { + return SOURCE_BOUNDARY; + } + strcpy(mnemonic, token); + uppercase(mnemonic); + char *selector = strchr(mnemonic, '.'); + if (selector) { + *selector = '\0'; + } + if (getOpcode(mnemonic) == NOT_AN_OPCODE) { + return SOURCE_BOUNDARY; + } + + memset(result, 0, sizeof(*result)); + strcpy(result->mnemonic, mnemonic); + strncpy(result->spelling, token, sizeof(result->spelling) - 1); + result->line = lineNumber; + char *operand = strtok(NULL, " \t\r\n"); + if (operand) { + strncpy(result->operand, operand, sizeof(result->operand) - 1); + result->hasLiteral = literalValue(operand, &result->literal); + } + return SOURCE_INSTRUCTION; +} + +// ---- Saying that something is deliberate ---- +// +// A line whose comment carries "splitlint: " is not reported on. The reason is +// REQUIRED, and that is the whole design: a suppression with no explanation is a way to +// make a tool quiet rather than a way to say something, and this file already has three +// real ones - branchTest exists to check that a branch whose outcome is known behaves +// correctly, so the diagnostic saying the outcome is known is exactly right and exactly +// unwanted. +// +// Warnings are reported against a line that has already been read, sometimes an earlier +// one than the line in hand, so what is kept is the set of lines seen so far. +static int *suppressedLines = NULL; +static int suppressedCount = 0; +static int suppressedRoom = 0; +static int suppressionsUsed = 0; + +static void forgetSuppressions(void) { + free(suppressedLines); + suppressedLines = NULL; + suppressedCount = 0; + suppressedRoom = 0; +} + +static int rememberSuppression(int line) { + if (suppressedCount == suppressedRoom) { + int room = suppressedRoom ? suppressedRoom * 2 : 16; + int *grown = realloc(suppressedLines, (size_t)room * sizeof(int)); + if (!grown) { + return 0; + } + suppressedLines = grown; + suppressedRoom = room; + } + suppressedLines[suppressedCount++] = line; + return 1; +} + +static int isSuppressed(int line) { + for (int i = 0; i < suppressedCount; i++) { + if (suppressedLines[i] == line) { + return 1; + } + } + return 0; +} + +// The reason has to be there and has to say something. A bare marker is refused rather +// than honoured, because a suppression nobody explained is the one that outlives whatever +// made it necessary. +// +// Returns 1 if the line is marked, 0 if it is not, and -1 if it is marked with nothing. +static int suppressionOn(const char *rawLine) { + const char *at = strstr(rawLine, "splitlint:"); + if (!at) { + return 0; + } + at += strlen("splitlint:"); + while (*at == ' ' || *at == '\t') { + at++; + } + return (*at == '\0' || *at == '\n' || *at == '\r') ? -1 : 1; +} + +// Returns 1 if the warning was reported and 0 if the line said it was deliberate, so that +// a suppressed warning is not counted and --fatal-warnings does not fail on one. +static int warning(const char *path, int line, const char *message, const char *help) { + if (isSuppressed(line)) { + suppressionsUsed++; + return 0; + } + printf("%s:%d: style: %s\n", path, line, message); + printf(" help: %s\n", help); + return 1; +} + +// Assignments whose only architectural effect is replacing one operand register. INA, +// INB and POP are intentionally absent: even when their result is overwritten, removing +// them would leave a byte unread from a device or an entry unconsumed from the stack. +static char assignedRegister(const SourceInstruction *instruction) { + static const char *aWriters[] = { "RSTA", "INIA", "MVQA", "LDA" }; + static const char *bWriters[] = { "RSTB", "INIB", "MVQB", "LDB" }; + for (size_t i = 0; i < sizeof(aWriters) / sizeof(aWriters[0]); i++) { + if (strcmp(instruction->mnemonic, aWriters[i]) == 0) { + return 'A'; + } + } + for (size_t i = 0; i < sizeof(bWriters) / sizeof(bWriters[0]); i++) { + if (strcmp(instruction->mnemonic, bWriters[i]) == 0) { + return 'B'; + } + } + return '\0'; +} + +static int stopsFallthrough(const SourceInstruction *instruction) { + return strcmp(instruction->mnemonic, "BRI") == 0 + || strcmp(instruction->mnemonic, "BRD") == 0 + || strcmp(instruction->mnemonic, "RET") == 0 + || strcmp(instruction->mnemonic, "RRET") == 0 + || strcmp(instruction->mnemonic, "RETI") == 0 + || strcmp(instruction->mnemonic, "HALT") == 0; +} + +static int isDirectBranch(const SourceInstruction *instruction) { + static const char *branches[] = { + "BRI", "BRQ", "BRA", "BRB", "BRC", "BNQ", "BNA", "BNB", "BNC" + }; + for (size_t i = 0; i < sizeof(branches) / sizeof(branches[0]); i++) { + if (strcmp(instruction->mnemonic, branches[i]) == 0) { + return 1; + } + } + return 0; +} + +static void forgetRegisters(KnownRegisters *known) { + memset(known, 0, sizeof(*known)); +} + +static const char *selectorSuffix(const SourceInstruction *instruction) { + const char *suffix = strchr(instruction->spelling, '.'); + return suffix ? suffix : ""; +} + +static int selectedPointer(const SourceInstruction *instruction) { + const char *selector = strchr(instruction->spelling, '.'); + if (!selector) { + return 0; + } + char *end; + long selected = strtol(selector + 1, &end, 10); + if (end == selector + 1 || selected < 0 || selected >= DATA_POINTERS) { + return 0; + } + return (int)selected; +} + +static void forgetPointers(KnownPointer pointers[DATA_POINTERS]) { + memset(pointers, 0, sizeof(*pointers) * DATA_POINTERS); +} + +static int lintKnownPointers(const char *path, const SourceInstruction *instruction, + const KnownPointer pointers[DATA_POINTERS]) { + if (strcmp(instruction->mnemonic, "SETD") != 0 || !instruction->operand[0]) { + return 0; + } + int selected = selectedPointer(instruction); + const KnownPointer *pointer = &pointers[selected]; + if (!pointer->known || pointer->offset != 0 + || strcmp(pointer->base, instruction->operand) != 0) { + return 0; + } + char message[180]; + snprintf(message, sizeof(message), "DP%d is already known to hold %s", + selected, instruction->operand); + return warning(path, instruction->line, message, "remove the redundant SETD"); +} + +static void moveKnownPointer(KnownPointer *pointer, uint16_t amount, int downward) { + if (!pointer->known) { + return; + } + pointer->offset = downward ? (uint16_t)(pointer->offset - amount) + : (uint16_t)(pointer->offset + amount); +} + +static void updateKnownPointers(const SourceInstruction *instruction, + const KnownRegisters *registers, + KnownPointer pointers[DATA_POINTERS]) { + int selected = selectedPointer(instruction); + KnownPointer *pointer = &pointers[selected]; + + if (strcmp(instruction->mnemonic, "SETD") == 0) { + if (instruction->operand[0]) { + pointer->known = 1; + strncpy(pointer->base, instruction->operand, sizeof(pointer->base) - 1); + pointer->base[sizeof(pointer->base) - 1] = '\0'; + pointer->offset = 0; + } else { + pointer->known = 0; + } + } else if (strcmp(instruction->mnemonic, "INCD") == 0) { + moveKnownPointer(pointer, 1, 0); + } else if (strcmp(instruction->mnemonic, "DECD") == 0) { + moveKnownPointer(pointer, 1, 1); + } else if (strcmp(instruction->mnemonic, "DPUP") == 0 && instruction->hasLiteral) { + moveKnownPointer(pointer, (uint16_t)instruction->literal, 0); + } else if (strcmp(instruction->mnemonic, "DPDN") == 0 && instruction->hasLiteral) { + moveKnownPointer(pointer, (uint16_t)instruction->literal, 1); + } else if (strcmp(instruction->mnemonic, "DPUA") == 0 && registers->aKnown) { + moveKnownPointer(pointer, registers->a, 0); + } else if (strcmp(instruction->mnemonic, "DPDA") == 0 && registers->aKnown) { + moveKnownPointer(pointer, registers->a, 1); + } else if (strcmp(instruction->mnemonic, "DPUW") == 0 + && registers->aKnown && registers->bKnown) { + moveKnownPointer(pointer, ((uint16_t)registers->a << 8) | registers->b, 0); + } else if (strcmp(instruction->mnemonic, "DPDW") == 0 + && registers->aKnown && registers->bKnown) { + moveKnownPointer(pointer, ((uint16_t)registers->a << 8) | registers->b, 1); + } else if (strcmp(instruction->mnemonic, "LDD") == 0 + || strcmp(instruction->mnemonic, "POPD") == 0 + || strcmp(instruction->mnemonic, "MVSD") == 0) { + pointer->known = 0; + } + + if (strcmp(instruction->mnemonic, "CALL") == 0) { + pointers[3].known = 0; + } else if (strcmp(instruction->mnemonic, "RCAL") == 0 + || strcmp(instruction->mnemonic, "SWI") == 0) { + forgetPointers(pointers); + } +} + +static int lintKnownRegisters(const char *path, const SourceInstruction *instruction, + const SourceInstruction *previous, + const KnownRegisters *known) { + int warnings = 0; + char message[180]; + char help[160]; + char previousAssignment = assignedRegister(previous); + + if (known->aKnown && previousAssignment != 'A' + && ((strcmp(instruction->mnemonic, "RSTA") == 0 && known->a == 0) + || (strcmp(instruction->mnemonic, "INIA") == 0 + && instruction->hasLiteral && known->a == instruction->literal))) { + snprintf(message, sizeof(message), "%s leaves A at its known value of %u", + instruction->spelling, known->a); + warnings += warning(path, instruction->line, message, "remove the redundant assignment"); + } + if (known->bKnown && previousAssignment != 'B' + && ((strcmp(instruction->mnemonic, "RSTB") == 0 && known->b == 0) + || (strcmp(instruction->mnemonic, "INIB") == 0 + && instruction->hasLiteral && known->b == instruction->literal))) { + snprintf(message, sizeof(message), "%s leaves B at its known value of %u", + instruction->spelling, known->b); + warnings += warning(path, instruction->line, message, "remove the redundant assignment"); + } + + if (known->aKnown && (strcmp(instruction->mnemonic, "DPUA") == 0 + || strcmp(instruction->mnemonic, "DPDA") == 0)) { + if (known->a == 0) { + warnings += warning(path, instruction->line, "pointer offset is known to be zero", + "remove the pointer instruction"); + } else if (known->a == 1) { + snprintf(help, sizeof(help), "use %s%s", + strcmp(instruction->mnemonic, "DPUA") == 0 ? "INCD" : "DECD", + selectorSuffix(instruction)); + warnings += warning(path, instruction->line, "pointer offset is known to be one", help); + } + } + + if (known->aKnown && known->bKnown + && (strcmp(instruction->mnemonic, "DPUW") == 0 + || strcmp(instruction->mnemonic, "DPDW") == 0)) { + uint16_t word = ((uint16_t)known->a << 8) | known->b; + if (word == 0) { + warnings += warning(path, instruction->line, "word-sized pointer offset is known to be zero", + "remove the pointer instruction"); + } else if (word == 1) { + snprintf(help, sizeof(help), "use %s%s", + strcmp(instruction->mnemonic, "DPUW") == 0 ? "INCD" : "DECD", + selectorSuffix(instruction)); + warnings += warning(path, instruction->line, "word-sized pointer offset is known to be one", help); + } + } + return warnings; +} + +static void updateKnownRegisters(const SourceInstruction *instruction, + KnownRegisters *known) { + if (strcmp(instruction->mnemonic, "SHL") == 0 + || strcmp(instruction->mnemonic, "SHR") == 0) { + if (known->aKnown && known->bKnown) { + uint16_t word = ((uint16_t)known->a << 8) | known->b; + if (strcmp(instruction->mnemonic, "SHL") == 0) { + word = (uint16_t)((word << 1) | (word >> 15)); + } else { + word = (uint16_t)((word >> 1) | (word << 15)); + } + known->a = (uint8_t)(word >> 8); + known->b = (uint8_t)word; + } else { + known->aKnown = 0; + known->bKnown = 0; + } + return; + } + + if (strcmp(instruction->mnemonic, "RSTA") == 0) { + known->aKnown = 1; + known->a = 0; + } else if (strcmp(instruction->mnemonic, "INIA") == 0 && instruction->hasLiteral) { + known->aKnown = 1; + known->a = (uint8_t)instruction->literal; + } else if (strcmp(instruction->mnemonic, "INCA") == 0) { + if (known->aKnown) known->a++; + } else if (strcmp(instruction->mnemonic, "DECA") == 0) { + if (known->aKnown) known->a--; + } else if (strcmp(instruction->mnemonic, "MVQA") == 0 + || strcmp(instruction->mnemonic, "LDA") == 0 + || strcmp(instruction->mnemonic, "POPA") == 0 + || strcmp(instruction->mnemonic, "INA") == 0) { + known->aKnown = 0; + } + + if (strcmp(instruction->mnemonic, "RSTB") == 0) { + known->bKnown = 1; + known->b = 0; + } else if (strcmp(instruction->mnemonic, "INIB") == 0 && instruction->hasLiteral) { + known->bKnown = 1; + known->b = (uint8_t)instruction->literal; + } else if (strcmp(instruction->mnemonic, "INCB") == 0) { + if (known->bKnown) known->b++; + } else if (strcmp(instruction->mnemonic, "DECB") == 0) { + if (known->bKnown) known->b--; + } else if (strcmp(instruction->mnemonic, "MVQB") == 0 + || strcmp(instruction->mnemonic, "LDB") == 0 + || strcmp(instruction->mnemonic, "POPB") == 0 + || strcmp(instruction->mnemonic, "INB") == 0) { + known->bKnown = 0; + } + + // A raw callee and a software interrupt may return with operand values the source + // immediately around the call cannot describe. CALL's documented convention saves + // A and B, so it deliberately does not appear here. + if (strcmp(instruction->mnemonic, "RCAL") == 0 + || strcmp(instruction->mnemonic, "SWI") == 0) { + forgetRegisters(known); + } +} + +static int lintKnownCarry(const char *path, const SourceInstruction *instruction, + KnownCarry carry) { + if (carry == CARRY_UNKNOWN) { + return 0; + } + if (strcmp(instruction->mnemonic, "CCF") == 0 && carry == CARRY_CLEAR) { + return warning(path, instruction->line, "carry is already known to be clear", + "remove the redundant CCF"); + } + if (strcmp(instruction->mnemonic, "BRC") == 0) { + if (carry == CARRY_SET) { + return warning(path, instruction->line, "BRC is always taken because carry is known set", + "use BRI"); + } + return warning(path, instruction->line, "BRC is never taken because carry is known clear", + "remove the branch"); + } + if (strcmp(instruction->mnemonic, "BNC") == 0) { + if (carry == CARRY_CLEAR) { + return warning(path, instruction->line, "BNC is always taken because carry is known clear", + "use BRI"); + } + return warning(path, instruction->line, "BNC is never taken because carry is known set", + "remove the branch"); + } + return 0; +} + +static void updateKnownCarry(const SourceInstruction *instruction, + const KnownRegisters *registers, + KnownCarry *carry) { + if (strcmp(instruction->mnemonic, "CCF") == 0) { + *carry = CARRY_CLEAR; + } else if (strcmp(instruction->mnemonic, "INCA") == 0) { + *carry = registers->aKnown ? (registers->a == 0xFF ? CARRY_SET : CARRY_CLEAR) + : CARRY_UNKNOWN; + } else if (strcmp(instruction->mnemonic, "INCB") == 0) { + *carry = registers->bKnown ? (registers->b == 0xFF ? CARRY_SET : CARRY_CLEAR) + : CARRY_UNKNOWN; + } else if (strcmp(instruction->mnemonic, "DECA") == 0) { + *carry = registers->aKnown ? (registers->a == 0 ? CARRY_SET : CARRY_CLEAR) + : CARRY_UNKNOWN; + } else if (strcmp(instruction->mnemonic, "DECB") == 0) { + *carry = registers->bKnown ? (registers->b == 0 ? CARRY_SET : CARRY_CLEAR) + : CARRY_UNKNOWN; + } else if (strcmp(instruction->mnemonic, "ADD") == 0) { + if (registers->aKnown && registers->bKnown && *carry != CARRY_UNKNOWN) { + unsigned result = registers->a + registers->b + (unsigned)*carry; + *carry = result > 255 ? CARRY_SET : CARRY_CLEAR; + } else { + *carry = CARRY_UNKNOWN; + } + } else if (strcmp(instruction->mnemonic, "SUB") == 0) { + if (registers->aKnown && registers->bKnown && *carry != CARRY_UNKNOWN) { + int result = (int)registers->a - (int)registers->b - (int)*carry; + *carry = result < 0 ? CARRY_SET : CARRY_CLEAR; + } else { + *carry = CARRY_UNKNOWN; + } + } else if (strcmp(instruction->mnemonic, "CALL") == 0 + || strcmp(instruction->mnemonic, "RCAL") == 0 + || strcmp(instruction->mnemonic, "SWI") == 0 + || strcmp(instruction->mnemonic, "RET") == 0 + || strcmp(instruction->mnemonic, "RETI") == 0) { + *carry = CARRY_UNKNOWN; + } +} + +static int lintInstruction(const char *path, const SourceInstruction *previous, + const SourceInstruction *current) { + int warnings = 0; + + if (current->hasLiteral && current->literal == 0 + && strcmp(current->mnemonic, "INIA") == 0) { + warnings += warning(path, current->line, "loading zero into A takes two bytes", "use RSTA"); + } else if (current->hasLiteral && current->literal == 0 + && strcmp(current->mnemonic, "INIB") == 0) { + warnings += warning(path, current->line, "loading zero into B takes two bytes", "use RSTB"); + } + + char previousAssignment = assignedRegister(previous); + if (previousAssignment && previousAssignment == assignedRegister(current)) { + char message[180]; + snprintf(message, sizeof(message), "%s assigns %c, but %s replaces it immediately", + previous->spelling, previousAssignment, current->spelling); + warnings += warning(path, previous->line, message, "remove the first assignment"); + } + + // Q already has direct moves to both operand registers. The older stack transfer + // idiom does the same job, but costs a write, a read, and an extra instruction. This + // is precisely the kind of sequence the linter exists to find in code written before + // the newer instruction was available. + if (strcmp(previous->mnemonic, "PSHQ") == 0 + && strcmp(current->mnemonic, "POPA") == 0) { + warnings += warning(path, previous->line, "moving Q to A through the stack takes two instructions", + "use MVQA"); + } else if (strcmp(previous->mnemonic, "PSHQ") == 0 + && strcmp(current->mnemonic, "POPB") == 0) { + warnings += warning(path, previous->line, "moving Q to B through the stack takes two instructions", + "use MVQB"); + } else if (strcmp(previous->mnemonic, "PSHA") == 0 + && strcmp(current->mnemonic, "POPA") == 0) { + warnings += warning(path, previous->line, "pushing A and immediately restoring it leaves A unchanged", + "remove both instructions if the stack write is not intentional"); + } else if (strcmp(previous->mnemonic, "PSHB") == 0 + && strcmp(current->mnemonic, "POPB") == 0) { + warnings += warning(path, previous->line, "pushing B and immediately restoring it leaves B unchanged", + "remove both instructions if the stack write is not intentional"); + } + + return warnings; +} + +static int lintFile(const char *path) { + FILE *file = fopen(path, "r"); + if (!file) { + fprintf(stderr, "%s: error: could not open source file\n", path); + return -1; + } + + char line[LINE_CAPACITY]; + int lineNumber = 0; + int warnings = 0; + int havePrevious = 0; + int fallthroughStopped = 0; + int havePendingBranch = 0; + SourceInstruction pendingBranch; + KnownRegisters known = {0}; + KnownPointer pointers[DATA_POINTERS] = {0}; + KnownCarry carry = CARRY_UNKNOWN; + SourceInstruction previous; + forgetSuppressions(); + while (fgets(line, sizeof(line), file)) { + lineNumber++; + // Refuse to silently analyze only the first part of an unusually long line. + if (!strchr(line, '\n') && !feof(file)) { + fprintf(stderr, "%s:%d: error: line is longer than %d characters\n", + path, lineNumber, LINE_CAPACITY - 2); + fclose(file); + return -1; + } + // BEFORE THE COMMENT IS STRIPPED, because the marker lives in one. Recorded for + // the line rather than acted on here: a warning is reported against a line that + // has already been read, and not always the one in hand. + int marked = suppressionOn(line); + if (marked < 0) { + fprintf(stderr, "%s:%d: error: splitlint: needs a reason after it\n", + path, lineNumber); + fclose(file); + forgetSuppressions(); + return -1; + } + if (marked && !rememberSuppression(lineNumber)) { + fprintf(stderr, "%s: error: out of memory recording a suppression\n", path); + fclose(file); + forgetSuppressions(); + return -1; + } + + SourceInstruction current; + SourceLine sourceLine = parseInstruction(line, lineNumber, ¤t); + if (sourceLine != SOURCE_INSTRUCTION) { + havePrevious = 0; + if (sourceLine == SOURCE_LABEL) { + if (havePendingBranch && current.spelling[0] + && strcmp(pendingBranch.operand, current.spelling) == 0) { + warnings += warning(path, pendingBranch.line, + "branch target is the next labeled address", + "remove the branch"); + havePendingBranch = 0; + } + fallthroughStopped = 0; + forgetRegisters(&known); + forgetPointers(pointers); + carry = CARRY_UNKNOWN; + } else if (sourceLine == SOURCE_BOUNDARY) { + fallthroughStopped = 0; + havePendingBranch = 0; + forgetRegisters(&known); + forgetPointers(pointers); + carry = CARRY_UNKNOWN; + } + continue; + } + // Reaching another instruction before the target label makes this something + // other than a branch to the next address. + havePendingBranch = 0; + if (fallthroughStopped) { + warnings += warning(path, current.line, + "no ordinary fallthrough reaches this instruction", + "remove it, or give an intentional indirect entry point a label"); + } + SourceInstruction empty = {0}; + warnings += lintKnownRegisters(path, ¤t, + havePrevious ? &previous : &empty, &known); + warnings += lintKnownPointers(path, ¤t, pointers); + warnings += lintKnownCarry(path, ¤t, carry); + warnings += lintInstruction(path, havePrevious ? &previous : &empty, ¤t); + if (stopsFallthrough(¤t)) { + fallthroughStopped = 1; + } + if (isDirectBranch(¤t) && current.operand[0]) { + pendingBranch = current; + havePendingBranch = 1; + } + updateKnownPointers(¤t, &known, pointers); + updateKnownCarry(¤t, &known, &carry); + updateKnownRegisters(¤t, &known); + previous = current; + havePrevious = 1; + } + if (ferror(file)) { + fprintf(stderr, "%s: error: could not read source file\n", path); + fclose(file); + forgetSuppressions(); + return -1; + } + fclose(file); + forgetSuppressions(); + return warnings; +} + +int main(int argc, char **argv) { + int fatalWarnings = 0; + int firstFile = 1; + if (argc > 1 && strcmp(argv[1], "--fatal-warnings") == 0) { + fatalWarnings = 1; + firstFile++; + } + if (argc <= firstFile || strcmp(argv[firstFile], "--help") == 0 + || strcmp(argv[firstFile], "-h") == 0) { + usage(argv[0]); + return argc <= firstFile ? 1 : 0; + } + + int warnings = 0; + for (int i = firstFile; i < argc; i++) { + int found = lintFile(argv[i]); + if (found < 0) { + return 2; + } + warnings += found; + } + if (warnings) { + printf("%d style warning%s found.\n", warnings, warnings == 1 ? "" : "s"); + } + // Said out loud rather than left implicit. A suppression is a claim that something is + // deliberate, and a count of them is how anybody notices the claim has spread. + if (suppressionsUsed) { + printf("%d warning%s suppressed by splitlint comments.\n", + suppressionsUsed, suppressionsUsed == 1 ? "" : "s"); + } + return fatalWarnings && warnings ? 1 : 0; +} diff --git a/Tests/lint.sh b/Tests/lint.sh new file mode 100755 index 0000000..65c26f2 --- /dev/null +++ b/Tests/lint.sh @@ -0,0 +1,212 @@ +#!/usr/bin/env bash +# Checks that SplitLint finds what it claims to find, one rule at a time. +# +# This used to compare a TOTAL. Twenty three warnings came out and twenty three were +# expected, which is a number that stays right while the thing behind it goes wrong: a +# change that stopped one rule firing and made another fire twice would pass without a +# murmur, and so would a rule that moved to the wrong line. What is checked here now is +# WHICH warning came out and AT WHICH LINE, so a rule that stops working says which one it +# was. +# +# The fixture is written here rather than kept as a file because every line of it exists +# to trip exactly one rule, and a reader wants the pattern and the expectation side by +# side rather than in two places. +# +# Written by Anachronaut + +set -u +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +cd "$ROOT" || exit 1 + +LINT="$ROOT/SplitLint" +[ -x "$LINT" ] || { echo "SplitLint is not built."; exit 1; } + +mkdir -p Tests/build +fixture=Tests/build/lint.asm +output=Tests/build/lint.out + +GREEN=$'\033[32m'; RED=$'\033[31m'; RESET=$'\033[0m' +[ -t 1 ] || { GREEN=""; RED=""; RESET=""; } + +PASS=0 +FAIL=0 +MISSING=() + +printf '%s\n' \ + '#Program' \ + ' INIA 0d0' \ + ' INIB 0x00' \ + ' INIA 0d1' \ + ' DPUA.2' \ + ' INIA 0x01' \ + ' DPDA.3' \ + ' PSHQ' \ + ' POPA' \ + ' PSHQ ; Comments do not break adjacent instructions.' \ + ' POPB' \ + ' PSHA' \ + ' POPA' \ + ' PSHB' \ + ' POPB' \ + ' INIA 0d5' \ + ' RSTA' \ + ' LDB.2' \ + ' MVQB' \ + ' INA ; Device reads are not removable assignments.' \ + ' INIA 0d8' \ + ' BRI reachable' \ + ' NOP' \ + ' ; A comment does not make fallthrough possible.' \ + ' NOP' \ + 'reachable:' \ + ' NOP' \ + ' BNA alreadyHere' \ + ' ; Blank space and comments do not move an address.' \ + 'alreadyHere:' \ + ' NOP' \ + 'knownValues:' \ + ' INIA 0d1' \ + ' NOP' \ + ' DPUA.1' \ + ' RSTA' \ + ' INIB 0d1' \ + ' NOP' \ + ' DPUW.1' \ + ' INIA 0d7' \ + ' NOP' \ + ' INIA 0d7' \ + 'pointerState:' \ + ' SETD.2 Thing' \ + ' NOP' \ + ' SETD.2 Thing' \ + ' SETD.1 Other' \ + ' DPUP.1 0d3' \ + ' DPDN.1 0d3' \ + ' SETD.1 Other' \ + 'carryState:' \ + ' CCF' \ + ' CCF' \ + ' INIA 0xFF' \ + ' INCA' \ + ' BRC carryTaken' \ + ' NOP' \ + 'carryTaken:' \ + ' CCF' \ + ' BRC carryClear' \ + ' BNC carryClear' \ + ' NOP' \ + 'carryClear:' \ + ' NOP' \ + ' "INIA 0d0; DPUA.0"' \ + ' ; INIB 0d0' \ + > "$fixture" + +"$LINT" "$fixture" > "$output" 2>&1 + +# ---- What every line of the fixture is for ---- +# +# One row per rule: the line it should be reported at, and enough of the message to name +# the rule without pinning its exact wording. +expect() { + local line="$1" want="$2" + local got + got=$(grep -c "^${fixture}:${line}: style: ${want}" "$output") + if [ "$got" = "1" ]; then + PASS=$((PASS + 1)) + printf " [%sok %s] line %-3s %s\n" "$GREEN" "$RESET" "$line" "$want" + else + FAIL=$((FAIL + 1)); MISSING+=("line $line: $want") + printf " [%sFAIL%s] line %-3s %s\n" "$RED" "$RESET" "$line" "$want" + fi +} + +echo "Checking that SplitLint still finds each thing it knows about." + +expect 2 "loading zero into A takes two bytes" +expect 3 "loading zero into B takes two bytes" +expect 5 "pointer offset is known to be one" +expect 6 "INIA leaves A at its known value of 1" +expect 7 "pointer offset is known to be one" +expect 8 "moving Q to A through the stack takes two instructions" +expect 10 "moving Q to B through the stack takes two instructions" +expect 12 "pushing A and immediately restoring it leaves A unchanged" +expect 14 "pushing B and immediately restoring it leaves B unchanged" +expect 16 "INIA assigns A, but RSTA replaces it immediately" +expect 18 "LDB.2 assigns B, but MVQB replaces it immediately" +expect 23 "no ordinary fallthrough reaches this instruction" +expect 25 "no ordinary fallthrough reaches this instruction" +expect 28 "branch target is the next labeled address" +expect 35 "pointer offset is known to be one" +expect 39 "word-sized pointer offset is known to be one" +expect 42 "INIA leaves A at its known value of 7" +expect 46 "DP2 is already known to hold Thing" +expect 50 "DP1 is already known to hold Other" +expect 53 "carry is already known to be clear" +expect 56 "BRC is always taken because carry is known set" +expect 60 "BRC is never taken because carry is known clear" +expect 61 "BNC is always taken because carry is known clear" + +# ---- And nothing it does not know about ---- +# +# The fixture carries four lines that must stay quiet: a device read whose result is +# overwritten, a self-restoring push pair that is NOT one, a string that happens to spell +# instructions, and a commented-out instruction. A rule that started firing on any of +# those would be a rule reading source that is not code. +total=$(grep -c "^${fixture}:[0-9]*: style:" "$output") +if [ "$total" = "$PASS" ] && [ "$FAIL" = "0" ]; then + printf " [%sok %s] and nothing else %s warnings, no more\n" "$GREEN" "$RESET" "$total" + PASS=$((PASS + 1)) +else + printf " [%sFAIL%s] and nothing else %s warnings for %s expectations\n" \ + "$RED" "$RESET" "$total" "$PASS" + FAIL=$((FAIL + 1)) + grep "^${fixture}:[0-9]*: style:" "$output" | while read -r line; do + echo " $line" + done +fi + +# ---- Saying a warning is deliberate ---- +# +# The reason is required. A marker with nothing after it is refused rather than honoured, +# because a suppression nobody explained outlives whatever made it necessary. +printf '%s\n' '#Program' ' INIA 0d0 ; splitlint: on purpose, for the test' > Tests/build/lintSup.asm +supOut=$("$LINT" Tests/build/lintSup.asm 2>&1) +if [ -z "$(echo "$supOut" | grep 'style:')" ] && echo "$supOut" | grep -q "1 warning suppressed"; then + PASS=$((PASS + 1)); printf " [%sok %s] a reasoned suppression is honoured and counted\n" "$GREEN" "$RESET" +else + FAIL=$((FAIL + 1)); MISSING+=("a reasoned suppression") + printf " [%sFAIL%s] a reasoned suppression is honoured and counted\n" "$RED" "$RESET" +fi + +printf '%s\n' '#Program' ' INIA 0d0 ; splitlint:' > Tests/build/lintBare.asm +if "$LINT" Tests/build/lintBare.asm >/dev/null 2>&1; then + FAIL=$((FAIL + 1)); MISSING+=("a bare suppression is refused") + printf " [%sFAIL%s] a suppression with no reason is refused\n" "$RED" "$RESET" +else + PASS=$((PASS + 1)); printf " [%sok %s] a suppression with no reason is refused\n" "$GREEN" "$RESET" +fi + +# ---- And warnings can be made to fail a build ---- +if "$LINT" --fatal-warnings "$fixture" >/dev/null 2>&1; then + FAIL=$((FAIL + 1)); MISSING+=("--fatal-warnings") + printf " [%sFAIL%s] --fatal-warnings fails on a warning\n" "$RED" "$RESET" +else + PASS=$((PASS + 1)); printf " [%sok %s] --fatal-warnings fails on a warning\n" "$GREEN" "$RESET" +fi + +# A suppressed warning is not a warning, so it must not fail one either. +if "$LINT" --fatal-warnings Tests/build/lintSup.asm >/dev/null 2>&1; then + PASS=$((PASS + 1)) + printf " [%sok %s] and does not fail on a suppressed one\n" "$GREEN" "$RESET" +else + FAIL=$((FAIL + 1)); MISSING+=("--fatal-warnings on a suppressed warning") + printf " [%sFAIL%s] and does not fail on a suppressed one\n" "$RED" "$RESET" +fi + +echo +if [ "$FAIL" -eq 0 ]; then + echo "All $PASS SplitLint checks passed." + exit 0 +fi +echo "$PASS passed, $FAIL failed: ${MISSING[*]}" +exit 1 diff --git a/makefile b/makefile index b9bde54..99584f0 100644 --- a/makefile +++ b/makefile @@ -29,24 +29,28 @@ POSIXFLAGS = -D_XOPEN_SOURCE=700 SRC_DIR_EMU = Source/Emulator SRC_DIR_ASM = Source/Assembler SRC_DIR_DSK = Source/DiskTool +SRC_DIR_LINT = Source/Linter OBJ_DIR = Object # Source files EMU_SRCS = emulator.c io.c controller.c utility.c cpu.c bootstrap.c assembly.c ASM_SRCS = Assembler.c assembly.c firstPass.c Assm-util.c secondPass.c DSK_SRCS = SplitDisk.c +LINT_SRCS = Linter.c EMU_OBJS = $(EMU_SRCS:%.c=$(OBJ_DIR)/%.o) ASM_OBJS = $(ASM_SRCS:%.c=$(OBJ_DIR)/%.o) DSK_OBJS = $(DSK_SRCS:%.c=$(OBJ_DIR)/%.o) +LINT_OBJS = $(LINT_SRCS:%.c=$(OBJ_DIR)/%.o) $(OBJ_DIR)/assembly.o # Output binary names EMU_TARGET = SplitBit ASM_TARGET = Assembler DSK_TARGET = SplitDisk +LINT_TARGET = SplitLint -# Default target: build both emulator and assembler -all: $(EMU_TARGET) $(ASM_TARGET) $(DSK_TARGET) +# Default target: build the emulator and its three host-side tools. +all: $(EMU_TARGET) $(ASM_TARGET) $(DSK_TARGET) $(LINT_TARGET) # Emulator binary $(EMU_TARGET): $(EMU_OBJS) @@ -65,6 +69,15 @@ $(OBJ_DIR)/%.o: $(SRC_DIR_EMU)/%.c $(DSK_TARGET): $(DSK_OBJS) $(CC) $(CFLAGS) -o $(DSK_TARGET) $(DSK_OBJS) +# Assembly source linter. The instruction table is shared with the assembler and +# emulator so that adding an opcode cannot leave the linter with a private copy. +$(LINT_TARGET): $(LINT_OBJS) + $(CC) $(CFLAGS) -o $(LINT_TARGET) $(LINT_OBJS) + +$(OBJ_DIR)/Linter.o: $(SRC_DIR_LINT)/Linter.c + @mkdir -p $(OBJ_DIR) + $(CC) $(CFLAGS) $(POSIXFLAGS) $(DEPFLAGS) -c $< -o $@ + # Compile disk tool source files to object files $(OBJ_DIR)/%.o: $(SRC_DIR_DSK)/%.c @mkdir -p $(OBJ_DIR) @@ -76,7 +89,7 @@ $(OBJ_DIR)/%.o: $(SRC_DIR_ASM)/%.c $(CC) $(CFLAGS) $(POSIXFLAGS) $(DEPFLAGS) -c $< -o $@ # Pull in the header dependencies written out by the compiler above. --include $(EMU_OBJS:.o=.d) $(ASM_OBJS:.o=.d) $(DSK_OBJS:.o=.d) +-include $(EMU_OBJS:.o=.d) $(ASM_OBJS:.o=.d) $(DSK_OBJS:.o=.d) $(LINT_OBJS:.o=.d) # ---- The strict build the README promises ---- # @@ -87,13 +100,15 @@ $(OBJ_DIR)/%.o: $(SRC_DIR_ASM)/%.c STRICT = -std=c11 -pedantic -Wall -Wextra -Werror $(POSIXFLAGS) strict: - @for source in $(SRC_DIR_EMU)/*.c $(SRC_DIR_ASM)/*.c $(SRC_DIR_DSK)/*.c; do \ + @for source in $(SRC_DIR_EMU)/*.c $(SRC_DIR_ASM)/*.c $(SRC_DIR_DSK)/*.c $(SRC_DIR_LINT)/*.c; do \ $(CC) $(STRICT) -c $$source -o /dev/null || exit 1; \ done @echo "The sources build clean under -std=c11 -pedantic." # Run the test suite against the programs in Programs/ -test: $(EMU_TARGET) $(ASM_TARGET) $(DSK_TARGET) strict +test: $(EMU_TARGET) $(ASM_TARGET) $(DSK_TARGET) $(LINT_TARGET) strict + @./Tests/lint.sh + @echo @echo @./Tests/run.sh @echo @@ -131,6 +146,8 @@ sanitize: @$(MAKE) --no-print-directory clean @$(MAKE) --no-print-directory CFLAGS="$(SANITIZE_FLAGS)" @echo "Running the test suite under AddressSanitizer and UndefinedBehaviorSanitizer." + @./Tests/lint.sh + @echo @./Tests/run.sh @echo @./Tests/disk.sh @@ -155,10 +172,10 @@ bless: $(EMU_TARGET) $(ASM_TARGET) clean: rm -rf $(OBJ_DIR) rm -rf Tests/build - rm -f $(EMU_TARGET) $(ASM_TARGET) $(DSK_TARGET) + rm -f $(EMU_TARGET) $(ASM_TARGET) $(DSK_TARGET) $(LINT_TARGET) # Install compiled binaries -install: $(EMU_TARGET) $(ASM_TARGET) $(DSK_TARGET) +install: $(EMU_TARGET) $(ASM_TARGET) $(DSK_TARGET) $(LINT_TARGET) mkdir -p "$(PREFIX)/bin" install -m 755 $^ "$(PREFIX)/bin/"