Skip to content

Commit

Permalink
IBM 1130: GUI resource file, RegSanityCheck fix
Browse files Browse the repository at this point in the history
- Add the missing ibm1130.rc GUI resource file to the Windows build so
  that the GUI renders correctly.

- Add "TEST_ARGS" argument to CMake's add_simulator function so that the
  IBM 1130 simulator can pass to "-g" on the command line to disable the
  GUI when running RegisterSanityCheck, i.e.:

    ibm1130 RegisterSanityCheck -g

  This fixes an edge case Heisenbug encountered during Github CI/CD
  tests where ibm1130 appears to hang indefinitely on the Windows
  runners.

  The cause is the GUI's Pump() thread function being prematurely
  terminated before all GUI resources are acquired. The net result is an
  infinite loop in the MS C runtime trying to exit the process with
  unstable internal state. (Separate patch: synchronization across main
  and Pump() threads to ensure resource acquisition completes.)

  This issue never shows up on non-Windows platforms or the SIMH makefile.

- cmake/generator.py, cmake/simgen: Add a "test_args" keyword argument
  to the BasicSimulator constructor that holds the tests argument
  parameter emitted as the "TEST_ARGS" argument to a simulator's
  add_simulator(). Ensure that the IBM 1130 emits 'TEST_ARG "-g"' in its
  add_simulator().

- scp.c: reset_all_p() adds 'P' to the existing switches, versus saving
  sim_switches and ONLY setting the 'P' power-up reset switch. Net effect
  is that the IBM 1130 simulator actually sees the 'G' flag that inhibits
  the GUI during the console device reset.

- Ibm1130/ibm1130_cr.c

  - Fix printf() warnings (format should be long, not int)
  - Signed/unsigned mismatch, size_t for array indexing
  - Comment out the unused trim() function.

- Ibm1130/ibm1130_cpu.c, ibm1130_gui.c: Remove undefined static functions.
  • Loading branch information
bscottm committed Mar 13, 2024
1 parent e444c67 commit abc73de
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 37 deletions.
9 changes: 7 additions & 2 deletions Ibm1130/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,16 @@ add_simulator(ibm1130
ibm1130_t2741.c
INCLUDES
${CMAKE_CURRENT_SOURCE_DIR}
TEST_ARGS "-g"
LABEL Ibm1130
PKG_FAMILY ibm_family
TEST ibm1130)

if (WIN32)
## Add GUI support, compile in resources:
target_compile_definitions(ibm1130 PRIVATE GUI_SUPPORT)
## missing source in IBM1130? ## target_sources(ibm1130 PRIVATE ibm1130.c)
endif()
target_sources(ibm1130 PRIVATE ibm1130.rc)
endif()

# IBM 1130 utilities:
# add_subdirectory(utils)
2 changes: 0 additions & 2 deletions Ibm1130/ibm1130_cpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,6 @@ extern UNIT cr_unit, prt_unit[];
# define ARFSET(v) /* without GUI, no need for setting ARF */
#endif

static void init_console_window (void);
static void destroy_console_window (void);
static t_stat view_cmd (int32 flag, CONST char *cptr);
static t_stat cpu_attach (UNIT *uptr, CONST char *cptr);
static t_bool bsctest (int32 DSPLC, t_bool reset_V);
Expand Down
22 changes: 14 additions & 8 deletions Ibm1130/ibm1130_cr.c
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ extern int cgi;
static int16 ascii_to_card[256];

static CPCODE *cardcode;
static int ncardcode;
static size_t ncardcode;
static FILE *deckfile = NULL;
static char tempfile[128];
static int any_punched = 0;
Expand Down Expand Up @@ -760,7 +760,7 @@ t_bool program_is_loaded = FALSE;

/* lookup_codetable - use code flag setting to get code table pointer and length */

static t_bool lookup_codetable (int32 match, CPCODE **pcode, int *pncode)
static t_bool lookup_codetable (int32 match, CPCODE **pcode, size_t *pncode)
{
switch (match) {
case CODE_029:
Expand Down Expand Up @@ -793,14 +793,16 @@ static t_bool lookup_codetable (int32 match, CPCODE **pcode, int *pncode)
t_stat set_active_cr_code (int match)
{
CPCODE *code;
int i, ncode;
size_t ncode;

SET_ACTCODE(cr_unit, match);

if (! lookup_codetable(match, &code, &ncode))
return SCPE_ARG;

if (code != NULL) { /* if an ASCII mode was selected */
size_t i;

memset(ascii_to_card, 0, sizeof(ascii_to_card));

for (i = 0; i < ncode; i++) /* set ascii to card code table */
Expand Down Expand Up @@ -881,7 +883,7 @@ static int32 guess_cr_code (void)
static t_stat cp_set_code (UNIT *uptr, int32 match, CONST char *cptr, void *desc)
{
CPCODE *code;
int ncode;
size_t ncode;

if (! lookup_codetable(match, &code, &ncode))
return SCPE_ARG;
Expand Down Expand Up @@ -1020,7 +1022,7 @@ t_stat cr_boot (int32 unitno, DEVICE *dptr)

char card_to_ascii (uint16 hol)
{
int i;
size_t i;

for (i = 0; i < ncardcode; i++)
if (cardcode[i].hollerith == hol)
Expand All @@ -1033,7 +1035,7 @@ char card_to_ascii (uint16 hol)

char hollerith_to_ascii (uint16 hol)
{
int i;
size_t i;

for (i = 0; i < ncardcode; i++)
if (cardcode_029[i].hollerith == hol)
Expand Down Expand Up @@ -1222,6 +1224,9 @@ static char * skipbl (char *str)
return str;
}

#if THIS_IS_EVER_NEEDED
/* This function is never called. Leaving it in if the IBM-1130 author
* deems it significant. */
static char * trim (char *str)
{
char *s, *lastnb;
Expand All @@ -1234,6 +1239,7 @@ static char * trim (char *str)

return str;
}
#endif

/* alltrim - remove all leading and trailing whitespace from a string */

Expand Down Expand Up @@ -2382,7 +2388,7 @@ static DWORD CALLBACK pcr_thread (LPVOID arg)
if (! GetOverlappedResult(hpcr, &ovRd, &nrcvd, TRUE))
report_error("PCR_Read", GetLastError());
else if (cr_unit.flags & UNIT_DEBUG)
printf("PCR_Read: event, %d rcvd\n", nrcvd);
printf("PCR_Read: event, %ld rcvd\n", nrcvd);
break;

case WAIT_OBJECT_0+1: /* write complete */
Expand All @@ -2391,7 +2397,7 @@ static DWORD CALLBACK pcr_thread (LPVOID arg)
if (! GetOverlappedResult(hpcr, &ovWr, &nwritten, TRUE))
report_error("PCR_Write", GetLastError());
else if (cr_unit.flags & UNIT_DEBUG)
printf("PCR_Write: event, %d sent\n", nwritten);
printf("PCR_Write: event, %ld sent\n", nwritten);
continue;

case WAIT_OBJECT_0+2: /* reset request from simulator */
Expand Down
2 changes: 0 additions & 2 deletions Ibm1130/ibm1130_gui.c
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,6 @@ extern t_bool program_is_loaded;
void disk_ready (int ready) {}
void disk_unlocked (int unlocked) {}
void gui_run (int running) {}
static void init_console_window (void) {}
static void destroy_console_window (void) {}

t_stat console_reset (DEVICE *dptr) {return SCPE_OK;}
long stuff_cmd (char *cmd) {return 0;}
Expand Down
7 changes: 7 additions & 0 deletions README-CMake.md
Original file line number Diff line number Diff line change
Expand Up @@ -1288,6 +1288,13 @@ add_simulator(simulator_name
## in its sim_instr() instruction simulation loop:
USES_AIO
## Arguments to append after "RegisterSanityCheck". These arguments
## appear between "RegisterSanityCheck" and the test script, if
## given, e.g.:
##
## mysimulator RegisterSanityCheck -r -t path/to/mysim_test.ini
TEST_ARGS "-r"
## Packaging "family" (group) to which the simulator belongs,
## for packagers that support grouping (Windows: NSIS .exe,
## WIX .msi; macOS)
Expand Down
6 changes: 6 additions & 0 deletions cmake/add_simulator.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,13 @@ list(APPEND ADD_SIMULATOR_1ARG
## DEFINES: List of extra command line manifest constants ("-D" items)
## INCLUDES: List of extra include directories
## SOURCES: List of source files
## TEST_ARGS: Additional arguments to append to the command line after
## "RegisterSanityCheck"
list(APPEND ADD_SIMULATOR_NARG
"DEFINES"
"INCLUDES"
"SOURCES"
"TEST_ARGS"
)

function (simh_executable_template _targ)
Expand Down Expand Up @@ -281,6 +284,9 @@ function (add_simulator _targ)

## Simulator-specific tests:
list(APPEND test_cmd "${_targ}" "RegisterSanityCheck")
if (SIMH_TEST_ARGS)
list(APPEND test_cmd ${SIMH_TEST_ARGS})
endif ()

if (DEFINED SIMH_TEST)
string(APPEND test_fname ${CMAKE_CURRENT_SOURCE_DIR} "/tests/${SIMH_TEST}_test.ini")
Expand Down
28 changes: 8 additions & 20 deletions cmake/simgen/basic_simulator.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
class SIMHBasicSimulator:
"""
"""
def __init__(self, sim_name, dir_macro, test_name, buildrom):
def __init__(self, sim_name, dir_macro, test_name, buildrom, test_args=None):
self.sim_name = sim_name
## self.dir_macro -> Directory macro (e.g., "${PDP11D}" for source
self.dir_macro = dir_macro
self.test_name = test_name
self.int64 = False
Expand All @@ -22,6 +23,10 @@ def __init__(self, sim_name, dir_macro, test_name, buildrom):
self.besm6_sdl_hack = False
## self.uses_aio -> True if the simulator uses AIO
self.uses_aio = False
## self.test_args -> Simulator flags to pass to the test phase. Used by ibm1130 to
## pass "-g" to disable to the GUI.
self.test_args = test_args

self.sources = []
self.defines = []
self.includes = []
Expand Down Expand Up @@ -128,6 +133,8 @@ def write_section(self, stream, section, indent, test_label='default', additiona
stream.write('\n' + indent4 + "BESM6_SDL_HACK")
if self.uses_aio:
stream.write('\n' + indent4 + "USES_AIO")
if self.test_args:
stream.write('\n' + indent4 + 'TEST_ARGS "{}"'.format(self.test_args))
if self.buildrom:
stream.write('\n' + indent4 + "BUILDROMS")
stream.write('\n' + indent4 + "LABEL " + test_label)
Expand Down Expand Up @@ -293,25 +300,6 @@ def write_simulator(self, stream, indent, test_label='ibm650'):
'endif()'
]))

class IBM1130Simulator(SIMHBasicSimulator):
'''The IBM650 simulator creates relatively deep stacks, which will fail on Windows.
Adjust target simulator link flags to provide a 8M stack, similar to Linux.
'''
def __init__(self, sim_name, dir_macro, test_name, buildrom):
super().__init__(sim_name, dir_macro, test_name, buildrom)

def write_simulator(self, stream, indent, test_label='ibm650'):
super().write_simulator(stream, indent, test_label)
stream.write('\n'.join([
'',
'if (WIN32)',
' target_compile_definitions(ibm1130 PRIVATE GUI_SUPPORT)',
' ## missing source in IBM1130?'
' ## target_sources(ibm1130 PRIVATE ibm1130.c)',
'endif()'
]))


if '_dispatch' in pprint.PrettyPrinter.__dict__:
def sim_pprinter(pprinter, sim, stream, indent, allowance, context, level):
cls = sim.__class__
Expand Down
30 changes: 30 additions & 0 deletions cmake/simgen/ibm1130_simulator.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
## IBM 1130 simulator customizations:
##
## - Add the Win32 resource file for Windows builds
## - Add the "-g" test flag to bypass/disable the simulator GUI when
## running RegisterSanityCheck test.
import simgen.basic_simulator as SBS

class IBM1130Simulator(SBS.SIMHBasicSimulator):
'''The IBM650 simulator creates relatively deep stacks, which will fail on Windows.
Adjust target simulator link flags to provide a 8M stack, similar to Linux.
'''
def __init__(self, sim_name, dir_macro, test_name, buildrom):
super().__init__(sim_name, dir_macro, test_name, buildrom, test_args="-g")

def write_simulator(self, stream, indent, test_label='ibm650'):
super().write_simulator(stream, indent, test_label)
stream.write('\n'.join([
'',
'if (WIN32)',
' ## Add GUI support, compile in resources:',
' target_compile_definitions(ibm1130 PRIVATE GUI_SUPPORT)',
' target_sources(ibm1130 PRIVATE ibm1130.rc)',
'endif()',
'',
'# IBM 1130 utilities:',
'# add_subdirectory(utils)',
''
]))


3 changes: 2 additions & 1 deletion cmake/simgen/sim_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import simgen.parse_makefile as SPM
import simgen.basic_simulator as SBS
import simgen.ibm1130_simulator as IBM1130
import simgen.vax_simulators as VAXen
import simgen.utils as SU

Expand All @@ -16,7 +17,7 @@
_special_simulators = {
"besm6": SBS.BESM6Simulator,
"i650": SBS.IBM650Simulator,
"ibm1130": SBS.IBM1130Simulator,
"ibm1130": IBM1130.IBM1130Simulator,
"pdp10-ka": SBS.KA10Simulator,
"vax": VAXen.VAXSimulator,
"vax730": VAXen.BasicVAXSimulator
Expand Down
7 changes: 5 additions & 2 deletions scp.c
Original file line number Diff line number Diff line change
Expand Up @@ -2738,6 +2738,7 @@ char cbuf[4*CBUFSIZE], *cptr, *cptr2;
char nbuf[PATH_MAX + 7];
char **targv = NULL;
int32 i, sw;
int first_arg = -1;
t_bool lookswitch;
t_bool register_check = FALSE;
t_stat stat = SCPE_OK;
Expand Down Expand Up @@ -2783,6 +2784,7 @@ for (i = 1; i < argc; i++) { /* loop thru args */
if (*cbuf) /* concat args */
strlcat (cbuf, " ", sizeof (cbuf));
sprintf(&cbuf[strlen(cbuf)], "%s%s%s", strchr(argv[i], ' ') ? "\"" : "", argv[i], strchr(argv[i], ' ') ? "\"" : "");
first_arg = i;
lookswitch = FALSE; /* no more switches */
}
} /* end for */
Expand Down Expand Up @@ -2894,7 +2896,7 @@ if (register_check) {
goto cleanup_and_exit;
}
sim_printf ("*** Good Registers in %s simulator.\n", sim_name);
if (argc < 2) { /* No remaining command arguments? */
if (first_arg < 0) { /* No remaining command arguments? */
sim_exit_status = EXIT_SUCCESS; /* then we're done */
goto cleanup_and_exit;
}
Expand Down Expand Up @@ -8053,7 +8055,8 @@ t_stat reset_all_p (uint32 start)
t_stat r;
int32 old_sw = sim_switches;

sim_switches = SWMASK ('P');
/* Add power-up reset to the sim switches. */
sim_switches = sim_switches | SWMASK ('P');
r = reset_all (start);
sim_switches = old_sw;
return r;
Expand Down

0 comments on commit abc73de

Please sign in to comment.