Skip to content

Commit f602f7b

Browse files
committed
SCP: Fix scp_asynch_check cross-thread interference
Address a Clang/LLVM sanitizer warning that scp_asynch_check is written by both the AIO and main threads, one thread potentially clobbering the other's value. The "scp_asynch_check = 0" at scp.c:239 is where the thread sanitizer detects the issue. This check is supposed to cause the main thread to process I/O updates on the next AIO_CHECK_EVENT code's execution. To preserve that behavior, AIO_CHECK_EVENT now executes AIO_UPDATE_QUEUE when either sim_asynch_check decrements below 0 or there is work to be done on the AIO queue (sim_asynch_queue != QUEUE_HEAD.) Code refactoring: - Eliminate the asymmetry between the lock-based (mutex) and lock-free implementations. - Lock-free: AIO_ILOCK/AIO_IUNLOCK do not reacquire sim_asynch_lock when compiler intrinsics are present (GCC, Clang, MS C and DEC C on Itanium.) - Lock-based: If DONT_USE_AIO_INTRINSICS is defined, the AIO implementation becomes lock-based via mutexes and AIO_ILOCK/- AIO_IUNLOCK recursively acquire/release sim_asynch_lock. - AIO defaults to the lock-based implementation if compiler intrinsics are not available. - GCC, Clang: Prefer the __atomic intrinsics over the deprecated __sync intrinsics. The __sync intrinics still exist for older GCC compilers. - sim_asynch_lock is a recursive mutex for both lock-based and lock-free implementations. Eliminates implementation asymmetry. - AIO_CHECK_EVENT invokes AIO_ILOCK and AIO_IUNLOCK so that the lock-based code cannot alter sim_asynch_queue when checking for pending I/O work. - sim_debug_io_lock: Debug output serialization lock. Previously, sim_asynch_lock was semantically overloaded to serialize output from _sim_debug_write_flush. This lock provides better semantic clarity. - New builder script flag to disable AIO lock-free, force AIO lock-based code: - cmake-builder.ps1 -noaiointrinsics ... - cmake-builder.sh -no-aio-intrinics ...
1 parent 29d3900 commit f602f7b

7 files changed

Lines changed: 18829 additions & 18720 deletions

File tree

CMakeLists.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,9 @@ option(SIMH_PACKAGE_SUFFIX
226226
option(MAC_UNIVERSAL
227227
"macOS universal binary flag: TRUE -> build universal binaries, FALSE -> don't."
228228
${MAC_UNIVERSAL_OPTVAL})
229+
option(DONT_USE_AIO_INTRINSICS
230+
"Don't use compiler/platform intrinsics for AIO, revert to lock-based AIO"
231+
FALSE)
229232

230233
# Places where CMake should look for dependent package configuration fragments and artifacts:
231234
set(SIMH_PREFIX_PATH_LIST)

README-CMake.md

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -517,25 +517,22 @@ or video support.
517517
518518
# List the supported command line flags:
519519
$ cmake/cmake-builder.sh --help
520-
Configure and build simh simulators on Linux and *nix-like platforms.
520+
** cmake version 3.18.4
521521
522-
Subdirectories:
523-
cmake/build-unix: Makefile-based build simulators
524-
cmake/build-ninja: Ninja build-based simulators
522+
CMake suite maintained and supported by Kitware (kitware.com/cmake).
523+
Configure and build simh simulators on Linux and *nix-like platforms.
525524
526-
Options:
527-
--------
525+
Compile/Build options:
526+
----------------------
528527
--clean (-x) Remove the build subdirectory
529528
--generate (-g) Generate the build environment, don't compile/build
530529
--parallel (-p) Enable build parallelism (parallel builds)
531-
--nonetwork Build simulators without network support
532-
--novideo Build simulators without video support
533530
--notest Do not execute 'ctest' test cases
534531
--noinstall Do not install SIMH simulators.
535532
--testonly Do not build, execute the 'ctest' test cases
536533
--installonly Do not build, install the SIMH simulators
537534

538-
--flavor (-f) Specifies the build flavor. Valid flavors are:
535+
--flavor (-f) [Required] Specifies the build flavor. Valid flavors are:
539536
unix
540537
ninja
541538
xcode
@@ -547,8 +544,7 @@ or video support.
547544
--config (-c) Specifies the build configuration: 'Release' or 'Debug'
548545

549546
--target Build a specific simulator or simulators. Separate multiple
550-
targets by separating with a comma,
551-
e.g. "--target pdp8,pdp11,vax750,altairz80,3b2"
547+
targets with a comma, e.g. "--target pdp8,pdp11,vax750,altairz80,3b2"
552548
--lto Enable Link Time Optimization (LTO) in Release builds
553549
--debugWall Enable maximal warnings in Debug builds
554550
--cppcheck Enable cppcheck static code analysis rules
@@ -559,6 +555,17 @@ or video support.
559555
560556
--verbose Turn on verbose build output
561557
558+
SIMH feature control options:
559+
-----------------------------
560+
--nonetwork Build simulators without network support
561+
--novideo Build simulators without video support
562+
--no-aio-intrinsics
563+
Do not use compiler/platform intrinsics to implement AIO
564+
functions (aka "lock-free" AIO), reverts to lock-based AIO
565+
if threading libraries are detected.
566+
567+
Other options:
568+
--------------
562569
--help (-h) Print this help.
563570
```
564571
@@ -575,17 +582,17 @@ or video support.
575582
PS C:\...\open-simh> Get-Help -deatailed cmake\cmake-builder.ps1
576583
577584
NAME
578-
C:\Users\bsm21317\play\open-simh\cmake\cmake-builder.ps1
585+
C:\...\play\open-simh\cmake\cmake-builder.ps1
579586
580587
SYNOPSIS
581588
Configure and build SIMH's dependencies and simulators using the Microsoft Visual
582589
Studio C compiler or MinGW-W64-based gcc compiler.
583590

584591

585592
SYNTAX
586-
C:\Users\bsm21317\play\open-simh\cmake\cmake-builder.ps1 [[-flavor] <String>] [[-config] <String>] [[-cpack_suffix] <String>] [[-target] <String>]
587-
[-clean] [-help] [-nonetwork] [-novideo] [-notest] [-noinstall] [-parallel] [-generate] [-regenerate] [-testonly] [-installOnly] [-windeprecation]
588-
[-package] [-lto] [-debugWall] [-cppcheck] [<CommonParameters>]
593+
C:\...\play\open-simh\cmake\cmake-builder.ps1 [[-flavor] <String>] [[-config] <String>] [[-cpack_suffix] <String>] [[-target]
594+
<String[]>] [-clean] [-help] [-nonetwork] [-novideo] [-noaioinstrinsics] [-notest] [-noinstall] [-parallel] [-generate] [-testonly]
595+
[-installOnly] [-windeprecation] [-lto] [-debugWall] [-cppcheck] [<CommonParameters>]
589596

590597

591598
DESCRIPTION
@@ -594,9 +601,9 @@ or video support.
594601

595602
1. Configure and generate the build environment selected by '-flavor' option.
596603
2. Build missing runtime dependencies and the simulator suite with the compiler
597-
configuration selected by the '-config' option. The "Release" configuration
598-
generates optimized executables; the "Debug" configuration generates
599-
development executables with debugger information.
604+
configuration selected by the '-config' option. The "Release" configuration
605+
generates optimized executables; the "Debug" configuration generates
606+
development executables with debugger information.
600607
3. Test the simulators
601608

602609
There is an install phase that can be invoked separately as part of the SIMH
@@ -630,6 +637,9 @@ or video support.
630637
mingw-make MinGW GCC/mingw32-make
631638
mingw-ninja MinGW GCC/ninja
632639
640+
-config <String>
641+
The target build configuration. Valid values: "Release" and "Debug"
642+
633643
[...truncated for brevity...]
634644
```
635645

0 commit comments

Comments
 (0)