Merge pull request #105 from kulp/misc-cleanup

Collect miscellaneous cleanups/fixes

Author: kulp

.github/workflows/c.yml

@@ -21,10 +21,7 @@ jobs:
       uses: Homebrew/actions/setup-homebrew@master
     - run: brew install bison expect icarus-verilog lcov
     - if: env.SDL != 0
-      run: |
-        brew install sdl2
-        # See below for notes about `--HEAD` here.
-        brew install --HEAD sdl2_image
+      run: brew install sdl2 sdl2_image
     - uses: actions/checkout@v2
       with:
         submodules: true
@@ -43,26 +40,19 @@ jobs:
       matrix:
         os:
         - ubuntu-latest
-        - macos-10.15
         - macos-latest
     runs-on: ${{ matrix.os }}
     steps:
     - name: Set up Homebrew
       id: set-up-homebrew
       uses: Homebrew/actions/setup-homebrew@master
+    - run: brew upgrade || true # Avoid failures due to incomplete links.
     - run: brew install bison expect icarus-verilog
     - if: runner.os == 'macOS'
       run: echo "/usr/local/opt/bison/bin" >> $GITHUB_PATH
     - if: env.SDL != 0
       run: |
-        brew install sdl2
-        # In SDL2_image 2.6.0, CMake support was added, but as of the current
-        # release 2.6.1, the Homebrew build does not yet work due to prefix
-        # computation that was fixed in
-        # https://github.com/libsdl-org/SDL_image/pull/295
-        # TODO stop installing from HEAD when a package with a fix (SDL2_image
-        # 2.6.2 perhaps?) is released.
-        brew install --HEAD sdl2_image
+        brew install sdl2 sdl2_image
     - uses: actions/checkout@v2
       with:
         submodules: true

COPYING

@@ -1,4 +1,4 @@
-Copyright (C) 2011-2022 Darren Kulp
+Copyright (C) 2011-2023 Darren Kulp
 Portions Copyright (C) 2012 Forest Belton
 
 Permission is hereby granted, free of charge, to any person obtaining a copy of

Makefile

@@ -18,7 +18,7 @@ all:
 check: all
 	cmake -S . -B build -DJIT=${JIT} -DSDL=${SDL} -DICARUS=${ICARUS} -DTESTING=1
 	cmake --build build
-	export PATH=$(abspath .):$$PATH && cd build && ctest --rerun-failed --output-on-failure
+	export PATH=$(abspath .):$$PATH && cd build && ctest --output-on-failure
 
 # Use CMAKE_BUILD_TYPE=Debug for coverage to avoid glitchy optimizations.
 coverage: CMAKE_BUILD_TYPE = Debug

hw/verilog/top.v

@@ -52,13 +52,13 @@ module Tenyr(
         .INIT(0),
         .PBITS(32), .ABITS(RAMABITS), .OFFSET(`RESETVECTOR)
     ) ram(
-        .clka  ( clk_core ), .clkb  ( '0 ),
-        .ena   ( r_stb    ), .enb   ( '0 ),
-        .acka  ( r_ack    ), .ackb  (    ),
-        .wea   ( r_wen    ), .web   ( '0 ),
-        .addra ( r_adr    ), .addrb ( '0 ),
-        .dina  ( r_ddn    ), .dinb  ( '0 ),
-        .douta ( r_dup    ), .doutb (    )
+        .clka  ( clk_core ), .clkb  (  1'b0 ),
+        .ena   ( r_stb    ), .enb   (  1'b0 ),
+        .acka  ( r_ack    ), .ackb  (       ),
+        .wea   ( r_wen    ), .web   (  1'b0 ),
+        .addra ( r_adr    ), .addrb ( 32'h0 ),
+        .dina  ( r_ddn    ), .dinb  ( 32'h0 ),
+        .douta ( r_dup    ), .doutb (       )
     );
 
 // -----------------------------------------------------------------------------
@@ -154,10 +154,10 @@ module Tenyr(
         .wbs_cyc_o ({ o_cyc, g_cyc, v_cyc, s_cyc, r_cyc, x_cyc }),
 
         // unused ports
-        .wbm_cti_i ( '0 ),
-        .wbm_bte_i ( '0 ),
-        .wbs_cti_o (    ),
-        .wbs_bte_o (    )
+        .wbm_cti_i ( 3'h0 ),
+        .wbm_bte_i ( 2'h0 ),
+        .wbs_cti_o (      ),
+        .wbs_bte_o (      )
     );
 
 endmodule

src/asm.c

@@ -471,7 +471,7 @@ static int text_in(STREAM *stream, struct element *i, void *ud)
     char next_char = 0;
     size_t len = stream->op.fread(&next_char, 1, 1, stream);
     if (len > 0 && !isspace(next_char))
-        return -1;
+        fatal(0, "Invalid character '%c' (ordinal %#hhx)", next_char, next_char);
     return result ? 1 : -1;
 }
 
@@ -491,8 +491,7 @@ static int text_out(STREAM *stream, struct element *i, void *ud)
  */
 
 struct memh_state {
-    int32_t written, marked, offset;
-    int emit_zeros;
+    int32_t seen, written, marked, offset;
     bool first_done;
 };
 
@@ -505,8 +504,6 @@ static int memh_init(STREAM *stream, struct param_state *p, void **ud)
     param_get_int(p, "format.memh.offset", &state->offset);
     state->marked = state->written = state->offset;
 
-    param_get_int(p, "format.memh.explicit", &state->emit_zeros);
-
     return 0;
 }
 
@@ -522,9 +519,12 @@ static int memh_in(STREAM *stream, struct element *i, void *ud)
         if (stream->op.fscanf(stream, " @%x", &state->marked) == 1)
             return 0; // let next call handle it
 
-        if (stream->op.fscanf(stream, " %x", &i->insn.u.word) != 1)
+        if (stream->op.feof(stream))
             return -1;
 
+        if (stream->op.fscanf(stream, " %x", &i->insn.u.word) != 1)
+            fatal(0, "Failed to read hex digits");
+
         i->insn.reladdr = state->written++;
         state->marked = state->written;
         return 1;
@@ -542,7 +542,13 @@ static int memh_out(STREAM *stream, struct element *i, void *ud)
     int32_t word = i->insn.u.word;
     int32_t diff = addr - state->written;
 
-    if (word == 0 && !state->emit_zeros)
+    // `seen` refers to the last index seen, inclusive -- therefore we need to
+    // subtract one after adding the size. In the normal case, size will be 1,
+    // so `seen` will point at `addr`. In the case of a zero-sized element,
+    // seen will become the element right *before* `addr`.
+    state->seen = addr + i->insn.size - 1;
+
+    if (word == 0)
         return 0; // 0 indicates success but nothing was output
 
     state->written = addr;
@@ -555,6 +561,25 @@ static int memh_out(STREAM *stream, struct element *i, void *ud)
     return (printed + stream->op.fprintf(stream, "%08x\n", word) > 3) ? 1 : -1;
 }
 
+static int memh_fini(STREAM *stream, void **ud)
+{
+    const struct memh_state *state = *ud;
+    const int32_t addr = state->seen;
+    const int32_t diff = addr - state->written;
+
+    // There may be unprinted zeros.
+    if (diff > 0)
+    {
+        if (stream->op.fprintf(stream, "@%x ", addr) < 3)
+            fatal(0, "Failed to emit address adjustment");
+
+        if (stream->op.fprintf(stream, "%08x\n", 0) < 8)
+            fatal(0, "Failed to emit hex value");
+    }
+
+    return gen_fini(stream, ud);
+}
+
 const struct format tenyr_asm_formats[] = {
     // first format is default
     { "obj",
@@ -574,10 +599,11 @@ const struct format tenyr_asm_formats[] = {
         .init  = memh_init,
         .in    = memh_in,
         .out   = memh_out,
-        .fini  = gen_fini },
+        .fini  = memh_fini },
 };
 
-const size_t tenyr_asm_formats_count = countof(tenyr_asm_formats);
+const size_t tenyr_asm_formats_count =
+    (sizeof tenyr_asm_formats) / (sizeof tenyr_asm_formats[0]);
 
 size_t make_format_list(int (*pred)(const struct format *), size_t flen,
         const struct format *fmts, size_t len, char *buf, const char *sep)

src/common.h

@@ -8,7 +8,6 @@
 
 #include "ops.h"
 
-#define countof(X) (sizeof (X) / sizeof (X)[0])
 #define MIN(X,Y) ((X) < (Y) ? (X) : (Y))
 
 static inline int32_t SEXTEND32(unsigned int bits, int32_t val)

src/os/default/findself.c

@@ -11,7 +11,7 @@
 typedef int path_walker(size_t len, const char *name, void *ud);
 // Returns walker's last result (stops walking when walker returns true), or
 // returns 0 if PATH is empty or missing.
-int os_walk_path_search_list(path_walker walker, void *ud)
+static int os_walk_path_search_list(path_walker walker, void *ud)
 {
     int found = 0;
 

src/plugin.c

@@ -26,13 +26,16 @@ int plugin_load(const char *basepath, const char **paths, const char *base,
 {
     int rc = 0;
 
+    const int max_impls = 16;
+    // We use a constant `16` here, instead of `max_impls`, to avoid making
+    // `impls` a VLA (at least under some compilers) and thus making it illegal
+    // to initialize it with an initializer list.
     const void *impls[16] = { 0 };
     int inst = 0;
     do {
         char buf[256] = { 0 }, parent[256] = { 0 };
         snprintf(parent, sizeof parent, "%s[%d]", base, inst);
-        int count = p->gops.param_get(p, parent, countof(impls), impls);
-        const int max_impls = (signed)countof(impls);
+        int count = p->gops.param_get(p, parent, max_impls, impls);
         if (count > max_impls)
             debug(0, "Saw %d plugins names, handling only the first %d", count, max_impls);
         else if (count <= 0)

src/tas.c

@@ -204,7 +204,7 @@ int main(int argc, char *argv[])
     if (disassemble)
         flags |= ASM_DISASSEMBLE;
 
-    for (int i = optind; i < argc; i++) {
+    for (int i = optind; rc == 0 && i < argc; i++) {
         rc = process_file(params, flags, fmt, argv[i], out);
         // We may have created an output file already, but we do not try to
         // remove it, because doing so by filename would be a race condition.

src/tsim.c

@@ -309,7 +309,7 @@ static int add_recipe(struct sim_state *s, const char *name)
         #define Entry(Name,Desc) { STR(Name), recipe_##Name },
         RECIPES(Entry)
     };
-    lfind_size_t sz = countof(entries);
+    lfind_size_t sz = (sizeof entries) / (sizeof entries[0]);
 
     struct recipe_entry *r = lfind(&(struct recipe_entry){ .name = name },
             entries, &sz, sizeof entries[0],

test/compare/tas/err/memh_explicit

@@ -0,0 +1 @@
+agda

test/compare/tas/in/memh_explicit

@@ -0,0 +1,4 @@
+.zero 2
+.zero 0
+.zero (2 + 3)
+.zero 0

test/compare/tas/out/memh_explicit

@@ -342,7 +342,6 @@ set_tests_properties(
 check_std_outputs(NAME "text disassembly"       COMMAND ${CMAKE_TENYR_COMPILER} ARGS -ftext -d)
 check_std_outputs(NAME "quiet disassembly"      COMMAND ${CMAKE_TENYR_COMPILER} ARGS -ftext -d -q)
 check_std_outputs(NAME "verbose disassembly"    COMMAND ${CMAKE_TENYR_COMPILER} ARGS -ftext -d -v)
-check_std_outputs(NAME "memh explicit"          COMMAND ${CMAKE_TENYR_COMPILER} ARGS -fmemh -pformat.memh.explicit=1)
 check_std_outputs(NAME "memh offset"            COMMAND ${CMAKE_TENYR_COMPILER} ARGS -fmemh -pformat.memh.offset=5)
 check_std_outputs(NAME "params overflow"        COMMAND ${CMAKE_TENYR_COMPILER} ARGS -fmemh -pA=1 -pB=1 -pC=1 -pD=1 -pE=1 -pF=1 -pG=1 -pH=1 -pI=1 -pJ=1 -pK=1 -pL=1 -pM=1 -pN=1 -pO=1 -pP=1 -pQ=1 -pR=1 -pS=1 -pT=1 -pU=1 -pV=1 -pW=1 -pX=1 -pY=1 -pZ=1 -pformat.memh.offset=5)
 
@@ -633,3 +632,16 @@ check_failure(
 )
 
 
+check_failure(
+    NAME    "invalid input for text"
+    COMMAND ${CMAKE_TENYR_COMPILER}
+    ARGS    --format=text -d ${CMAKE_SOURCE_DIR}/test/misc/memh/agda.memh
+    EXPECT  "Invalid character"
+)
+
+check_failure(
+    NAME    "invalid input for memh"
+    COMMAND ${CMAKE_TENYR_COMPILER}
+    ARGS    --format=memh -d ${CMAKE_SOURCE_DIR}/test/misc/memh/agda.memh
+    EXPECT  "Failed to read hex digits"
+)