att
Should we remove the dependency on the libast aso subsystem?
aso module in libast provides functions for atomic scalar operations. Currently only couple of functions from this module are being used by cdt code. This issue tracks if we can remove dependency over this module and switch to some other solution.
Note that one problem with the AST aso subsystem is that if we ever need to add support for a new CPU architecture the aso subsystem will likely need to be modified. And we really don't want to have to deal with that just to be able to build ksh.
The simplest solution is to change from C99 to C11 as the minimum acceptable C standard since C11 provides equivalent functionality via the <stdatomic.h> include.
Another option is to use the atomic extensions provided by gcc and clang. It's unclear what the oldest version of gcc is that provides these builtins but it was present at least as far back as gcc 3.3 which was released in 2003.
FWIW, my recommendation is to make one more release with the AST aso code. Then bump the minimum C standard to C11 so we can use the <stdatomic.h> include in place of the AST aso code.
On the other hand note that the only reason this functionality is required is to efficiently support threaded code in hot paths. Ksh itself is not threaded and thus does not require this functionality. If we don't support threaded plugins there is no need for the AST aso code.
standard to C11 so we can use the <stdatomic.h>
Yes, I think that is a good and more portable solution. But better would be to eliminate the need for anything other than a variable declared 'volatile sig_atomic_t'.
I looked at some use of the AST ASO ops last year (I think). These ASO ops are within the job-control code of KSH (src/cmd/ksh93/sh/jobs.c - subroutine |job_post()|). There appears to be only two uses of ASO subroutines (that I now know about). Both of these cases appear to provide communication between a signal-handing (interrupt) execution context and the base-line main code. I think that both of these cases can be eliminated if the subroutine |job_waitsafe()| is refactored to not execute in signal-handling context (as per a comment in the code already, by this project I think). In truth, the use of signal-handling (interrupt) context should be avoided, but if retained, communication between the signal-handler and the main-line code should be done using the portable (C89/C90) variable type 'volatile sig_atomic_t'. But I think that the main obstacle is in the refactoring of the subroutine |job_waitsafe()|. If this code (whole area) is refactored properly, I think that even the C11 atomic types should not be needed, and at most only 'volatile sig_atomic_t' should be used.
Note that it isn't just the ksh code that uses the ASO subsystem. It's also used by the AST CDT and SFIO subsystems. Replacing those uses with anything other than obviously identically behaving functions from the C11 <stdatomic.h> header is probably more risk than can be justified. But I do agree that we should eliminate those uses within the ksh signal handling code.
The ASO subsystem has the worst test coverage of any major subsystem in this project at just 12% line coverage, 15% function coverage, and 9% branch coverage. These stats would be greatly improved simply by removing ASO functions, such as asoget8() that the current code does not use. Which might also be helpful by putting more focus on the handful of ASO functions that are used.
Too, what in the world are preprocessor statements like #ifndef asoget8 meant to do? Those symbols are not defined by any standard or platform. So how would they be defined other than in src/lib/libast/aso/aso.c? It looks like the idea was that you could define them as no-ops by doing the equivalent of #define asoget8(p) do { } while (0). But why would you do that? How would it be useful other than as a debugging aid; e.g., similar to enabling race detection? But even that doesn't make any sense.
LOL! The current code uses build time feature tests to figure out if functions like asoget8() can be implemented as macros that call the platform function appropriate for the situation or use the fallback implementation. Such as protected by #ifndef asoget8.