diff --git a/Source/Assembler/secondPass.c b/Source/Assembler/secondPass.c index d20ea02..2b82b35 100644 --- a/Source/Assembler/secondPass.c +++ b/Source/Assembler/secondPass.c @@ -21,6 +21,11 @@ int debugSecondPass = 0; Label labelArray[MAX_LABELS]; int labelCount = 0; +// Beside the labels rather than beside the code that fills it, because the symbol file +// below reads both tables and this one would otherwise not be declared yet. +VectorEntry vectorArray[MAX_VECTORS]; +int vectorArrayCount = 0; + // ---- Where everything ended up ---- // // Every label with the memory and address it was given and the place it was written, one @@ -40,25 +45,57 @@ int labelCount = 0; // -F'\t', sort -k1,2 - and so that a name never has to be quoted. No header line, for the // same reason: nothing that reads it should have to know to skip one. // -// THE MEMORY IS THE FIRST FIELD BECAUSE THIS IS A HARVARD MACHINE. Program and Data are +// THE KIND IS THE FIRST FIELD BECAUSE THIS IS A HARVARD MACHINE. Program and Data are // separate address spaces, so 0x3000 is two different places and an address alone does not // say which. That is invisible in a loadable program, where the two segments are based // somewhere apart, and immediate in a boot image, where both start at zero and every // address in the file appears twice. // -// Sorted by memory and then address rather than by name, because the question asked of it -// is always "what is at this address"; sorting by address alone would interleave two -// address spaces and make the first column flicker between them. A label table is small -// enough that sorting it is free. +// VECTORS ARE IN HERE TOO, and they are the part nothing else can tell you. A pinned vector +// has its number written in the source that pins it, but an automatic one is handed a number +// by this assembler and that number appears nowhere at all - not in the source, not in the +// binary in any form a reader can find. A vector also costs two hops to follow by hand: the +// name in "SWI osFileRead" is not the name of the routine that implements it, so finding the +// code means grepping for the vector, reading the handler's name off the Vector Segment, and +// grepping again. A vector row gives its number, the handler's name, and the line the two +// were tied together on. +// +// A VECTOR CARRIES BOTH ITS NUMBERS, which is why there is a sixth field. Its number is what +// a program writes and what the machine dispatches on; its slot is where the handler's +// address is stored in Program Memory, base + number * 2, which is what the loader writes +// and what a memory dump shows. Neither can be worked out from the other without knowing +// which table it is in, so the file says both. +// +// Sorted by kind and then address rather than by name, because the question asked of it is +// always "what is at this address"; sorting by address alone would interleave separate +// address spaces and make the first column flicker between them. The table is small enough +// that sorting it is free. + +// The four kinds, in the order they are listed. Program and Data are the two memories, then +// the two vector tables. +#define ROW_PROGRAM 0 +#define ROW_DATA 1 +#define ROW_VECTOR 2 +#define ROW_DEVICE 3 + +typedef struct { + int kind; + uint16_t address; // Where it lives: a label's address, or a vector's slot. + const char *name; + const char *fileName; + int lineNumber; + int number; // The vector or port number, or -1 for a label, which has none. +} SymbolRow; + static int byPlace(const void *left, const void *right) { - const Label *a = left, *b = right; - if (a->type != b->type) { - return a->type < b->type ? -1 : 1; // PROGRAM is 1 and DATA is 2, so code first. + const SymbolRow *a = left, *b = right; + if (a->kind != b->kind) { + return a->kind < b->kind ? -1 : 1; } if (a->address != b->address) { return a->address < b->address ? -1 : 1; } - return strcmp(a->label, b->label); + return strcmp(a->name, b->name); } void writeSymbolFile(const char *path) { @@ -67,27 +104,61 @@ void writeSymbolFile(const char *path) { fprintf(stderr, RED "Error: Could not open file \"%s\" for writing.\n" RESET, path); exit(1); } - Label *sorted = malloc((size_t)labelCount * sizeof(Label)); - if (!sorted) { + int rowCount = labelCount + vectorArrayCount; + SymbolRow *rows = malloc((size_t)rowCount * sizeof(SymbolRow) + 1); + if (!rows) { fprintf(stderr, RED "Error: Out of memory writing the symbol file.\n" RESET); fclose(file); exit(1); } - memcpy(sorted, labelArray, (size_t)labelCount * sizeof(Label)); - qsort(sorted, (size_t)labelCount, sizeof(Label), byPlace); + int n = 0; for (int i = 0; i < labelCount; i++) { // A label is put in one of the two segments by populateLabelTable and there is no - // third, but the name is printed from the type rather than assumed, so a label that - // somehow arrived as neither says so instead of being filed under Data. - const char *memory = sorted[i].type == PROGRAM ? "Program" - : sorted[i].type == DATA ? "Data" - : "?"; - fprintf(file, "%s\t%04X\t%s\t%s\t%d\n", - memory, sorted[i].address, sorted[i].label, - sorted[i].fileName ? sorted[i].fileName : "?", - sorted[i].lineNumber); + // third, but the kind is taken from the type rather than assumed, so a label that + // somehow arrived as neither is not quietly filed under Data. + rows[n].kind = labelArray[i].type == PROGRAM ? ROW_PROGRAM : ROW_DATA; + rows[n].address = labelArray[i].address; + rows[n].name = labelArray[i].label; + rows[n].fileName = labelArray[i].fileName; + rows[n].lineNumber = labelArray[i].lineNumber; + rows[n].number = -1; + n++; } - free(sorted); + for (int i = 0; i < vectorArrayCount; i++) { + int hardware = vectorArray[i].base == HARDWARE_VECTOR_BASE; + rows[n].kind = hardware ? ROW_DEVICE : ROW_VECTOR; + // The same arithmetic the loader is given, so that what this says a vector's slot is + // and what actually gets written there cannot drift apart. + rows[n].address = vectorArray[i].base + + (uint16_t)vectorArray[i].index * VECTOR_ENTRY_BYTES; + // A device has no name of its own - it is named by the port it is plugged into - so + // it is listed under its handler, which is the only name it has. + rows[n].name = vectorArray[i].name ? vectorArray[i].name + : vectorArray[i].handlerName ? vectorArray[i].handlerName + : "?"; + rows[n].fileName = vectorArray[i].fileName; + rows[n].lineNumber = vectorArray[i].lineNumber; + rows[n].number = vectorArray[i].index; + n++; + } + qsort(rows, (size_t)n, sizeof(SymbolRow), byPlace); + static const char *kindName[4] = { "Program", "Data", "Vector", "Device" }; + for (int i = 0; i < n; i++) { + char number[12]; // Wide enough for any int, which is more than a vector needs. + if (rows[i].number < 0) { + // A LABEL HAS NO NUMBER, and the field says so rather than being left empty: + // a run of tabs with nothing between them is the one thing a reader of this + // file, human or otherwise, can miscount. + snprintf(number, sizeof(number), "-"); + } else { + snprintf(number, sizeof(number), "%d", rows[i].number); + } + fprintf(file, "%s\t%04X\t%s\t%s\t%d\t%s\n", + kindName[rows[i].kind], rows[i].address, rows[i].name, + rows[i].fileName ? rows[i].fileName : "?", + rows[i].lineNumber, number); + } + free(rows); fclose(file); } @@ -215,9 +286,6 @@ void populateLabelTable(intermediateElement *intermediateArray, int arraySize) { int findLabelAddress(const char *labelName); -VectorEntry vectorArray[MAX_VECTORS]; -int vectorArrayCount = 0; - int vectorCount() { return vectorArrayCount; } @@ -227,6 +295,9 @@ void freeVectorList() { if (vectorArray[i].name) { free(vectorArray[i].name); } + if (vectorArray[i].handlerName) { + free(vectorArray[i].handlerName); + } } vectorArrayCount = 0; } @@ -303,7 +374,8 @@ static int findVector(const char *name) { } static void addVector(char *name, uint8_t index, uint16_t base, uint16_t handler, - int declaredOnly, intermediateElement *element) { + int declaredOnly, intermediateElement *element, + const char *handlerName) { if (vectorArrayCount >= MAX_VECTORS) { vectorError("Too many vectors defined.", element); } @@ -325,6 +397,9 @@ static void addVector(char *name, uint8_t index, uint16_t base, uint16_t handler vectorArray[vectorArrayCount].base = base; vectorArray[vectorArrayCount].handler = handler; vectorArray[vectorArrayCount].declaredOnly = declaredOnly; + vectorArray[vectorArrayCount].handlerName = handlerName ? strdup(handlerName) : NULL; + vectorArray[vectorArrayCount].fileName = element->fileName; + vectorArray[vectorArrayCount].lineNumber = element->lineNumber; vectorArrayCount++; } @@ -375,7 +450,8 @@ void populateVectorTable(intermediateElement *intermediateArray, int arraySize) int handlerToken = nextVectorToken(intermediateArray, arraySize, portToken + 1); uint16_t handler = resolveHandler(intermediateArray, handlerToken, "Device"); addVector(NULL, intermediateArray[portToken].byteValue, HARDWARE_VECTOR_BASE, - handler, 0, &intermediateArray[i]); + handler, 0, &intermediateArray[i], + intermediateArray[handlerToken].token); i = nextVectorToken(intermediateArray, arraySize, handlerToken + 1); continue; } @@ -413,6 +489,12 @@ void populateVectorTable(intermediateElement *intermediateArray, int arraySize) } vectorArray[already].handler = resolveHandler(intermediateArray, handlerToken, token); vectorArray[already].declaredOnly = 0; + // The declaration said what it is called; this says where it was implemented, + // which is the more useful of the two places to be sent. + free(vectorArray[already].handlerName); + vectorArray[already].handlerName = strdup(intermediateArray[handlerToken].token); + vectorArray[already].fileName = intermediateArray[i].fileName; + vectorArray[already].lineNumber = intermediateArray[i].lineNumber; i = nextVectorToken(intermediateArray, arraySize, handlerToken + 1); continue; } @@ -459,12 +541,13 @@ void populateVectorTable(intermediateElement *intermediateArray, int arraySize) if (!hasHandler) { // Nothing follows it on the line, so this says what the vector is called and // what number it has, and leaves implementing it to somebody else. - addVector(token, index, SOFTWARE_VECTOR_BASE, 0, 1, &intermediateArray[i]); + addVector(token, index, SOFTWARE_VECTOR_BASE, 0, 1, &intermediateArray[i], NULL); i = handlerToken; continue; } uint16_t handler = resolveHandler(intermediateArray, handlerToken, token); - addVector(token, index, SOFTWARE_VECTOR_BASE, handler, 0, &intermediateArray[i]); + addVector(token, index, SOFTWARE_VECTOR_BASE, handler, 0, &intermediateArray[i], + intermediateArray[handlerToken].token); i = nextVectorToken(intermediateArray, arraySize, handlerToken + 1); } } diff --git a/Source/Assembler/secondPass.h b/Source/Assembler/secondPass.h index abb1ef7..bacd886 100644 --- a/Source/Assembler/secondPass.h +++ b/Source/Assembler/secondPass.h @@ -38,8 +38,8 @@ typedef struct { int lineNumber; } Label; -// Every label and the address it was given, in address order, so that a tally of -// addresses can be turned back into a list of routine names. +// Every label and every vector, with where it lives and where it was written, so that a +// tally of addresses can be turned back into a list of routine names. void writeSymbolFile(const char *path); // One line of the Vector Segment, once it has been worked out. @@ -49,6 +49,12 @@ typedef struct { uint16_t base; // Which table: software or hardware. uint16_t handler; // Where the handler ended up. int declaredOnly; // Named and numbered, with nobody implementing it here. + // For the symbol file, which is the only thing that reads these. A device has no name + // of its own, so the handler's is the only name it can be listed under. fileName is + // borrowed from the include list the way a Label's is; handlerName is owned, like name. + char* handlerName; + const char* fileName; + int lineNumber; } VectorEntry; void freeLabelList(); diff --git a/SplitBit Assembler Manual.md b/SplitBit Assembler Manual.md index ec23ea7..f23e178 100644 --- a/SplitBit Assembler Manual.md +++ b/SplitBit Assembler Manual.md @@ -420,32 +420,48 @@ Assembler [options] | -o, --output \ | Write the output to this path. Without it, the output is named after the source file, in the directory the assembler was run from, taking .bin if it is a boot image and .sbx if it is a loadable program. | | -I, --include \ | Look in this directory for included files. May be given more than once, and the directories are searched in the order given. | | -M, --depend \ | Write out which source files went into the output, as a make rule. | -| -S, --symbols \ | Write a symbol table to this path: every label, with the memory and address it was given and the file and line it was written on. | +| -S, --symbols \ | Write a symbol table to this path: every label and every vector, with where it lives and where it was written. | | -h, --help | Print the options and stop. | Every option above that takes a \ names a file the assembler WRITES, and the source is the bare argument at the end. So `Assembler -S program.asm` does not dump the symbols of program.asm. It asks for the symbol file to be called program.asm, and then finds it has no source left to assemble. --S is for looking at what the assembler decided, and for finding your way around a program that has grown past holding in your head. It writes one line for every label, with five fields separated by tabs: +-S is for looking at what the assembler decided, and for finding your way around a program that has grown past holding in your head. It writes one line for every label and every vector, with six fields separated by tabs: | Field | Meaning | | -- | -- | -| Memory | Program or Data. | -| Address | Four hexadecimal digits: the address the label was given. | -| Name | The label itself, without its colon. | -| File | The source file the label was written in, named the way the assembler was given it. | +| Kind | Program, Data, Vector or Device. | +| Address | Four hexadecimal digits: where the thing lives. For a label, the address it was given. For a vector, the slot in the vector table that holds its handler's address. | +| Name | The label, or the vector's name. A device has no name of its own, so it is listed under its handler. | +| File | The source file it was written in, named the way the assembler was given it. | | Line | Which line of that file, counting from one. | +| Number | Which vector it is: the vector number, or the port for a device. A label has no number and the field is a dash. | ``` -Program 5000 start game.asm 31 -Program 670D int16add Libraries/math.asm 5 -Data 3000 Score game.asm 118 +Program 5000 start game.asm 31 - +Program 670D int16add Libraries/math.asm 5 - +Vector FC20 osPrintString services.asm 24 16 +Vector FC80 gameFrame game.asm 402 64 +Device FE40 diskDone system.asm 812 32 +Data 3000 Score game.asm 118 - ``` Tabs rather than aligned columns, so that the file is a table every ordinary tool already reads without being taught anything, and no heading line, so that nothing has to know to skip one. The file field is what makes it worth having on a big program: a name defined once and called in forty places is hard to find by searching, and this says where it was written. -The memory comes first because Program and Data are separate memories on this machine, so an address on its own does not say where something is. In a loadable program the two segments are usually based far enough apart that the difference is easy to overlook. In a boot image both start at 0x0000, and every address in the table appears twice. +The kind comes first because Program and Data are separate memories on this machine, so an address on its own does not say where something is. In a loadable program the two segments are usually based far enough apart that the difference is easy to overlook. In a boot image both start at 0x0000, and every address in the table appears twice. -The table is sorted by memory and then by address, with Program first. +### Vectors In The Table + +A vector is worth having here for two reasons that labels do not have. + +Its number is the one thing about a program that nothing else can tell you. A vector that was pinned has its number written in the source that pinned it, but a vector the assembler numbered has that number nowhere at all: not in the source, and not in the binary in any form you can read. The table is where you find out that gameFrame became vector 64. + +And a vector takes two hops to follow by hand. The name in `SWI osPrintString` is not the name of the routine that implements it, so finding the code means searching for the vector, reading the handler's name off the Vector Segment, and searching again. A vector's row names the handler and gives the line the two were tied together on. + +Both of a vector's numbers are given, because neither can be worked out from the other without knowing which table the vector is in. The Number is what a program writes and what the machine dispatches on. The Address is where the handler's address is stored - 0xFC00 plus twice the number for a software vector, 0xFE00 plus twice the port for a device - which is what a loader writes and what a memory dump shows. + +A vector that this program only declares, without implementing it, is listed like any other. That is how a program says which vectors it calls, and it is the way to check that two programs agree about a number. + +The table is sorted by kind and then by address, in the order Program, Data, Vector, Device. The assembler stops at the first error, says which file and line it was in, and exits without writing anything. diff --git a/Tests/docs.sh b/Tests/docs.sh index d50fad2..99d479e 100755 --- a/Tests/docs.sh +++ b/Tests/docs.sh @@ -837,37 +837,50 @@ if "## Running the Assembler:" not in am: pass # Already reported above. else: section = am.split("## Running the Assembler:")[1].split("\n## ")[0] - said = re.findall(r'^\| (Memory|Address|Name|File|Line) \|', section, re.M) - if said != ["Memory", "Address", "Name", "File", "Line"]: - problems.append("the Assembler Manual does not describe the symbol file's five" + said = [row.split(" |")[0] for row in re.findall(r'^\| (\w+ \|.*)$', section, re.M) + if row.split(" |")[0] in ("Kind", "Address", "Name", "File", "Line", "Number")] + if said != ["Kind", "Address", "Name", "File", "Line", "Number"]: + problems.append("the Assembler Manual does not describe the symbol file's six" " fields in order; it lists %s" % (said or "none")) else: - with tempfile.TemporaryDirectory() as scratch: + # Two programs, because no one of them covers everything the format claims. + # + # Keys is a small LOADABLE program, where the two segments are based apart, and + # the smallest thing that produces all four kinds at once: labels in both + # memories, vectors it implements, vectors it only declares, and a device. + # + # cosmos is a BOOT IMAGE, where both segments start at zero, which is the case + # the kind field exists for - and it is the one that holds strings with newlines + # inside them, which is what the line numbers below go wrong on. + # + # Both assemble in a few thousandths of a second, so there is nothing to save by + # checking only one. + for program in ["Programs/CosmOS/Apps/Keys.asm", + "Programs/CosmOS/Source/cosmos.asm"]: + with tempfile.TemporaryDirectory() as scratch: dump = os.path.join(scratch, "symbols") - # A program with labels in BOTH memories, so that both names the first field can - # take are seen. replCalculator is a boot image, which is also the case where the - # memory field is load bearing: both segments start at zero. built = subprocess.run(["./Assembler", "-I", "Programs/Libraries", "-I", "Programs/CosmOS/Source", "-S", dump, - "-o", os.path.join(scratch, "out.bin"), - "Programs/Examples/replCalculator.asm"], + "-o", os.path.join(scratch, "out"), + program], capture_output=True, text=True) if built.returncode != 0: - problems.append("could not assemble a program to check the symbol file") + problems.append("could not assemble %s to check the symbol file" % program) else: - memories = set() + kinds = set() for number, line in enumerate(open(dump).read().splitlines(), 1): fields = line.split("\t") - if len(fields) != 5: + if len(fields) != 6: problems.append("line %d of a symbol file has %d fields and the" - " Assembler Manual describes five" + " Assembler Manual describes six" % (number, len(fields))) break - memory, address, name, source, where = fields - memories.add(memory) - if memory not in ("Program", "Data"): - problems.append("a symbol file says a label is in %r, and the" - " Assembler Manual allows Program and Data" % memory) + kind, address, name, source, where, index = fields + kinds.add(kind) + if kind not in ("Program", "Data", "Vector", "Device"): + problems.append("a symbol file calls something a %r, and the" + " Assembler Manual allows Program, Data, Vector" + " and Device" % kind) break if not re.fullmatch(r'[0-9A-F]{4}', address): problems.append("a symbol file gives %r as an address, and the" @@ -878,13 +891,70 @@ else: problems.append("a symbol file line is missing a name, a file or a" " line number: %r" % line) break + # A label has no number and says so with a dash; a vector has one, and + # the address it sits at has to be the slot that number works out to, + # which is the claim the manual makes about both fields at once. + if kind in ("Program", "Data"): + if index != "-": + problems.append("a %s label was given the number %r, and the" + " Assembler Manual says a label has none" + % (kind, index)) + break + elif not index.isdigit(): + problems.append("a %s row has %r where its number should be" + % (kind, index)) + break + else: + base = 0xFC00 if kind == "Vector" else 0xFE00 + if int(address, 16) != base + int(index) * 2: + problems.append("%s is number %s and the Assembler Manual puts" + " that at 0x%04X, but the symbol file says %s" + % (name, index, base + int(index) * 2, address)) + break else: # Only meaningful if every line was well formed, which is what the else # on the loop says. - if memories != {"Program", "Data"}: - problems.append("a program with labels in both memories produced a" - " symbol file naming only %s" - % ", ".join(sorted(memories))) + for wanted in ("Program", "Data", "Vector", "Device"): + if wanted not in kinds: + problems.append("a program with labels in both memories and a" + " Vector Segment produced a symbol file with no" + " %s row" % wanted) + # ---- And the line really is the line ---- + # + # "Which line of that file, counting from one" is a claim, and it was + # wrong for years without anything noticing: a string literal with a + # newline inside it was read as one token and the newline was never + # counted, so every line number after one was short by one. cosmos.asm + # has thirteen, and its last label was reported thirteen lines early - + # which had been sending every error message after that point at + # somebody else's code, not only the symbol file. + # + # A name has to actually appear on the line its row names. That is a + # weak test of a right answer and a very strong test of a wrong one: + # drift lands on a line that has nothing to do with the name. + sources = {} + for row in open(dump).read().splitlines(): + kind, address, name, source, where, index = row.split("\t") + if source not in sources: + try: + sources[source] = open(source).read().splitlines() + except OSError: + sources[source] = None + lines = sources[source] + if lines is None: + problems.append("a symbol file names %r, which cannot be read" + % source) + break + at = int(where) + if at < 1 or at > len(lines): + problems.append("%s is said to be on line %d of %s, which has" + " %d lines" % (name, at, source, len(lines))) + break + if not re.search(r'\b%s\b' % re.escape(name), lines[at - 1]): + problems.append("%s is said to be on line %d of %s, and that" + " line is %r" + % (name, at, source, lines[at - 1].strip())) + break if problems: print("The manuals and the code disagree:")