From 045dfe7879c6e3913865073bce099f0d86fecfd2 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Sun, 16 Feb 2025 23:04:39 +0100 Subject: [PATCH 1/3] kernel: format & refactor AddList3, export AddPlist3 Besides running `clang-format` on the code, we replace one IS_PLIST check by a direct tnum comparison, split 2 and 3 argument variant implementation. --- src/listfunc.c | 111 +++++++++++++++++++++++++------------------------ src/listfunc.h | 1 + 2 files changed, 57 insertions(+), 55 deletions(-) diff --git a/src/listfunc.c b/src/listfunc.c index c4b1818911..5da324d25a 100644 --- a/src/listfunc.c +++ b/src/listfunc.c @@ -44,86 +44,72 @@ */ static void AddList3(Obj list, Obj obj, Int pos) { - Int len; - Int i; + Int len; + Int i; len = LEN_LIST(list); - if (pos == (Int) -1) - pos = len + 1; - for (i = len +1; i > pos; i--) - ASS_LIST(list, i, ELM_LIST(list, i-1)); - ASS_LIST( list, pos, obj ); + if (pos == (Int)-1) + pos = len + 1; + for (i = len + 1; i > pos; i--) + ASS_LIST(list, i, ELM_LIST(list, i - 1)); + ASS_LIST(list, pos, obj); } -void AddList ( - Obj list, - Obj obj) +void AddList(Obj list, Obj obj) { - AddList3(list, obj, -1); + AddList3(list, obj, -1); } -static void AddPlist3(Obj list, Obj obj, Int pos) +void AddPlist3(Obj list, Obj obj, Int pos) { - UInt len; + UInt len; - if ( ! IS_PLIST_MUTABLE(list) ) { + if (!IS_PLIST_MUTABLE(list)) { ErrorMayQuit("List Assignment: must be a mutable list", 0, 0); } // in order to be optimistic when building list call assignment - len = LEN_PLIST( list ); + len = LEN_PLIST(list); if (pos == (Int)-1) - pos = len + 1; - if ( len == 0) { - AssPlistEmpty( list, pos, obj ); + pos = len + 1; + if (len == 0) { + AssPlistEmpty(list, pos, obj); return; } if (pos <= len) { - GROW_PLIST(list, len+1); - SET_LEN_PLIST(list, len+1); - Obj * ptr = ADDR_OBJ(list) + pos; - SyMemmove(ptr + 1, ptr, sizeof(Obj) * (len - pos + 1)); + GROW_PLIST(list, len + 1); + SET_LEN_PLIST(list, len + 1); + Obj * ptr = ADDR_OBJ(list) + pos; + SyMemmove(ptr + 1, ptr, sizeof(Obj) * (len - pos + 1)); } ASS_LIST(list, pos, obj); } -void AddPlist ( - Obj list, - Obj obj) +void AddPlist(Obj list, Obj obj) { - - AddPlist3(list, obj, -1); + AddPlist3(list, obj, -1); } static Obj AddListOper; static Obj FuncADD_LIST3(Obj self, Obj list, Obj obj, Obj pos) { - // dispatch - Int ipos; - if (pos == (Obj)0) - ipos = -1; - else if (IS_POS_INTOBJ(pos)) - ipos = INT_INTOBJ(pos); - else { - DoOperation3Args( self, list, obj, pos); - return (Obj) 0; - } - UInt tnum = TNUM_OBJ(list); - if ( IS_PLIST( list ) ) { - AddPlist3( list, obj, ipos ); - } else if ( FIRST_LIST_TNUM <= tnum && tnum <= LAST_LIST_TNUM ) { - AddList3( list, obj, ipos ); -#ifdef HPCGAP - // Only support adding to end of atomic lists - } else if ( tnum == T_ALIST && pos == (Obj)0 ) { - AddAList( list, obj ); -#endif - } else { - if (pos == 0) - DoOperation2Args( self, list, obj ); - else - DoOperation3Args( self, list, obj, pos); - } + if (!IS_POS_INTOBJ(pos)) { + DoOperation3Args(self, list, obj, pos); + return 0; + } + + UInt tnum = TNUM_OBJ(list); + if (FIRST_PLIST_TNUM <= tnum && tnum <= LAST_PLIST_TNUM) { + AddPlist3(list, obj, INT_INTOBJ(pos)); + return 0; + } + else if (FIRST_LIST_TNUM <= tnum && tnum <= LAST_LIST_TNUM) { + AddList3(list, obj, INT_INTOBJ(pos)); + return 0; + } + else { + DoOperation3Args(self, list, obj, pos); + } return 0; } @@ -131,8 +117,23 @@ static Obj FuncADD_LIST3(Obj self, Obj list, Obj obj, Obj pos) static Obj FuncADD_LIST(Obj self, Obj list, Obj obj) { - FuncADD_LIST3(self, list, obj, (Obj)0); - return (Obj) 0; + UInt tnum = TNUM_OBJ(list); + if (FIRST_PLIST_TNUM <= tnum && tnum <= LAST_PLIST_TNUM) { + AddPlist3(list, obj, -1); + } + else if (FIRST_LIST_TNUM <= tnum && tnum <= LAST_LIST_TNUM) { + AddList3(list, obj, -1); + } +#ifdef HPCGAP + else if (tnum == T_ALIST) { + AddAList(list, obj); + } +#endif + else { + DoOperation2Args(self, list, obj); + } + + return 0; } diff --git a/src/listfunc.h b/src/listfunc.h index 9bc7a598bc..b1609b4724 100644 --- a/src/listfunc.h +++ b/src/listfunc.h @@ -28,6 +28,7 @@ void AddList(Obj list, Obj obj); void AddPlist(Obj list, Obj obj); +void AddPlist3(Obj list, Obj obj, Int pos); /**************************************************************************** ** From 3fb6b3df69aed7d92f6779bdd0a3d6a70563f334 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Mon, 17 Feb 2025 23:07:57 +0100 Subject: [PATCH 2/3] kernel: pass stackBottom explicitly to init functions --- src/gap.c | 13 +++++-------- src/gap.h | 4 ++-- src/libgap-api.c | 2 +- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/gap.c b/src/gap.c index d40660e89d..129bb9f2a9 100644 --- a/src/gap.c +++ b/src/gap.c @@ -405,7 +405,7 @@ int realmain(int argc, const char * argv[]) Int4 crc; // crc of file to compile // initialize everything and read init.g which runs the GAP session - InitializeGap(&argc, argv, 1); + InitializeGap(&argc, argc, argv, 1); if (!STATE(UserHasQUIT)) { /* maybe the user QUIT from the initial read of init.g somehow*/ // maybe compile in which case init.g got skipped @@ -1452,21 +1452,18 @@ StructInitInfo * InitInfoGap ( void ) ** function. We use the resulting pointer as a hint to the garbage collector ** as to where the execution stack (might) start. */ -void InitializeGap(int * pargc, const char * argv[], BOOL handleSignals) +void InitializeGap(void * stackBottom, int argc, const char * argv[], BOOL handleSignals) { - const int argc = *pargc; - // initialize the basic system and gasman InitSystem(argc, argv, handleSignals); // Initialise memory -- have to do this here to make sure we are at top of C stack - InitBags( + Bag * alignedStackBottom = (Bag *)(((UInt)stackBottom / C_STACK_ALIGN) * C_STACK_ALIGN); #if defined(USE_GASMAN) - SyStorMin, + InitBags(SyStorMin, alignedStackBottom); #else - 0, + InitBags(0, alignedStackBottom); #endif - (Bag *)(((UInt)pargc / C_STACK_ALIGN) * C_STACK_ALIGN)); STATE(UserHasQUIT) = FALSE; STATE(UserHasQuit) = FALSE; diff --git a/src/gap.h b/src/gap.h index b3bab8df92..ff01637abc 100644 --- a/src/gap.h +++ b/src/gap.h @@ -83,9 +83,9 @@ BOOL IsUsingLibGap(void); /**************************************************************************** ** -*F InitializeGap( , , ) . . . . . . . init GAP +*F InitializeGap() . . . . . . . . . . . . . . . . . . . . . initialize GAP */ -void InitializeGap(int * pargc, const char * argv[], BOOL handleSignals); +void InitializeGap(void * stackBottom, int argc, const char * argv[], BOOL handleSignals); /**************************************************************************** diff --git a/src/libgap-api.c b/src/libgap-api.c index becc8858f0..55b5764099 100644 --- a/src/libgap-api.c +++ b/src/libgap-api.c @@ -59,7 +59,7 @@ void GAP_Initialize(int argc, { UsingLibGap = TRUE; - InitializeGap(&argc, (const char **)argv, handleSignals); + InitializeGap(&argc, argc, (const char **)argv, handleSignals); SetExtraMarkFuncBags(markBagsCallback); STATE(JumpToCatchCallback) = errorCallback; From 68a89688c0ad7ad4b8858cd7a2900e54daaf1a61 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Sun, 5 Jan 2025 22:34:02 +0100 Subject: [PATCH 3/3] kernel: split CLI arg parsing into two stages Most arguments are handled in the first stage. In particular all that are relevant for the GC. The second stage is only for the arguments which add GAP roots paths: those involve storing strings, which can be of arbitrary size. We'd like to switch their handling from C strings to GAP strings, but that requires the memory manager to be initialized. With this patch, it *is* initialized before we parse the root paths. --- src/gap.c | 2 ++ src/system.c | 84 +++++++++++++++++++++++++++++----------------------- src/system.h | 4 +++ 3 files changed, 53 insertions(+), 37 deletions(-) diff --git a/src/gap.c b/src/gap.c index 129bb9f2a9..b98438b841 100644 --- a/src/gap.c +++ b/src/gap.c @@ -1475,6 +1475,8 @@ void InitializeGap(void * stackBottom, int argc, const char * argv[], BOOL handl // call kernel initialisation ModulesInitKernel(); + InitRootPaths(argc, argv); + #ifdef HPCGAP InitMainThread(); #endif diff --git a/src/system.c b/src/system.c index df3d1ef1e3..eb28e2d09d 100644 --- a/src/system.c +++ b/src/system.c @@ -360,11 +360,12 @@ static void usage(void) } struct optInfo { - Char shortkey; - Char longkey[50]; - int (*handler)(const char * argv[], void *); - void *otherArg; - UInt minargs; + int phase; + char shortkey; + char longkey[50]; + int (*handler)(const char * argv[], void *); + void *otherArg; + UInt minargs; }; @@ -486,48 +487,48 @@ static int printVersion(const char * argv[], void * dummy) // These options must be kept in sync with those in system.g, so the help // output is correct static const struct optInfo options[] = { - { 'C', "", processCompilerArgs, 0, 4}, // must handle in kernel - { 'D', "debug-loading", toggle, &SyDebugLoading, 0}, // must handle in kernel + { 0, 'C', "", processCompilerArgs, 0, 4}, // must handle in kernel + { 0, 'D', "debug-loading", toggle, &SyDebugLoading, 0}, // must handle in kernel #if defined(USE_GASMAN) || defined(USE_BOEHM_GC) - { 'K', "maximal-workspace", storeMemory2, &SyStorKill, 1}, // could handle from library with new interface + { 0, 'K', "maximal-workspace", storeMemory2, &SyStorKill, 1}, // could handle from library with new interface #endif #ifdef GAP_ENABLE_SAVELOAD - { 'L', "", storeString, &SyRestoring, 1}, // must be handled in kernel - { 'R', "", unsetString, &SyRestoring, 0}, // kernel + { 0, 'L', "", storeString, &SyRestoring, 1}, // must be handled in kernel + { 0, 'R', "", unsetString, &SyRestoring, 0}, // kernel #endif - { 'M', "", toggle, &SyUseModule, 0}, // must be handled in kernel - { 'e', "", toggle, &SyCTRD, 0 }, // kernel - { 'f', "", forceLineEditing, (void *)2, 0 }, // probably library now - { 'E', "", toggle, &SyUseReadline, 0 }, // kernel - { 'l', "roots", setGapRootPath, 0, 1}, // kernel + { 0, 'M', "", toggle, &SyUseModule, 0}, // must be handled in kernel + { 0, 'e', "", toggle, &SyCTRD, 0 }, // kernel + { 0, 'f', "", forceLineEditing, (void *)2, 0 }, // probably library now + { 0, 'E', "", toggle, &SyUseReadline, 0 }, // kernel + { 1, 'l', "roots", setGapRootPath, 0, 1}, // kernel #ifdef USE_GASMAN - { 'm', "", storeMemory2, &SyStorMin, 1 }, // kernel + { 0, 'm', "", storeMemory2, &SyStorMin, 1 }, // kernel #endif - { 'r', "", toggle, &IgnoreGapRC, 0 }, // kernel + { 0, 'r', "", toggle, &IgnoreGapRC, 0 }, // kernel #ifdef USE_GASMAN - { 's', "", storeMemory, &SyAllocPool, 1 }, // kernel + { 0, 's', "", storeMemory, &SyAllocPool, 1 }, // kernel #endif - { 'n', "", forceLineEditing, 0, 0}, // prob library + { 0, 'n', "", forceLineEditing, 0, 0}, // prob library #ifdef USE_GASMAN - { 'o', "", storeMemory2, &SyStorMax, 1 }, // library with new interface + { 0, 'o', "", storeMemory2, &SyStorMax, 1 }, // library with new interface #endif - { 'p', "", toggle, &SyWindow, 0 }, // ?? - { 'q', "quiet", toggle, &SyQuiet, 0 }, // ?? + { 0, 'p', "", toggle, &SyWindow, 0 }, // ?? + { 0, 'q', "quiet", toggle, &SyQuiet, 0 }, // ?? #ifdef HPCGAP - { 'S', "", toggle, &ThreadUI, 0 }, // Thread UI - { 'Z', "", toggle, &DeadlockCheck, 0 }, // Deadlock prevention - { 'P', "", storePosInteger, &SyNumProcessors, 1 }, // number of CPUs - { 'G', "", storePosInteger, &SyNumGCThreads, 1 }, // number of GC threads - { 0 , "single-thread", toggle, &SingleThreadStartup, 0 }, // startup with one thread only + { 0, 'S', "", toggle, &ThreadUI, 0 }, // Thread UI + { 0, 'Z', "", toggle, &DeadlockCheck, 0 }, // Deadlock prevention + { 0, 'P', "", storePosInteger, &SyNumProcessors, 1 }, // number of CPUs + { 0, 'G', "", storePosInteger, &SyNumGCThreads, 1 }, // number of GC threads + { 0, 0 , "single-thread", toggle, &SingleThreadStartup, 0 }, // startup with one thread only #endif // The following options must be handled in the kernel so they are set up before loading the library - { 0 , "prof", enableProfilingAtStartup, 0, 1}, // enable profiling at startup - { 0 , "memprof", enableMemoryProfilingAtStartup, 0, 1 }, // enable memory profiling at startup - { 0 , "cover", enableCodeCoverageAtStartup, 0, 1}, // enable code coverage at startup - { 0 , "quitonbreak", toggle, &SyQuitOnBreak, 0}, // Quit GAP if we enter the break loop - { 0 , "enableMemCheck", enableMemCheck, 0, 0 }, - { 0 , "version", printVersion, 0, 0 }, - { 0, "", 0, 0, 0}}; + { 0, 0 , "prof", enableProfilingAtStartup, 0, 1}, // enable profiling at startup + { 0, 0 , "memprof", enableMemoryProfilingAtStartup, 0, 1 }, // enable memory profiling at startup + { 0, 0 , "cover", enableCodeCoverageAtStartup, 0, 1}, // enable code coverage at startup + { 0, 0 , "quitonbreak", toggle, &SyQuitOnBreak, 0}, // Quit GAP if we enter the break loop + { 0, 0 , "enableMemCheck", enableMemCheck, 0, 0 }, + { 0, 0 , "version", printVersion, 0, 0 }, + { 0, 0, "", 0, 0, 0}}; static void InitSysOpts(void) @@ -579,7 +580,7 @@ static void InitSysOpts(void) SyWindow = 0; } -static void ParseCommandLineOptions(int argc, const char * argv[]) +static void ParseCommandLineOptions(int argc, const char * argv[], int phase) { // scan the command line for options that we have to process in the kernel // we just scan the whole command line looking for the keys for the @@ -637,7 +638,8 @@ static void ParseCommandLineOptions(int argc, const char * argv[]) usage(); } - (*options[i].handler)(argv + 1, options[i].otherArg); + if (options[i].phase == phase) + (*options[i].handler)(argv + 1, options[i].otherArg); argv += options[i].minargs; argc -= options[i].minargs; @@ -687,7 +689,9 @@ void InitSystem(int argc, const char * argv[], BOOL handleSignals) InitSysFiles(); - ParseCommandLineOptions(argc, argv); + // first stage of command line options parsing: handle everything except + // for root paths + ParseCommandLineOptions(argc, argv, 0); #ifdef HAVE_LIBREADLINE InitReadline(); @@ -702,6 +706,12 @@ void InitSystem(int argc, const char * argv[], BOOL handleSignals) SyRedirectStderrToStdOut(); syWinPut(0, "@p", "1."); } +} + +void InitRootPaths(int argc, const char * argv[]) +{ + // second stage of command line options parsing: handle root paths + ParseCommandLineOptions(argc, argv, 1); InitDotGapPath(); } diff --git a/src/system.h b/src/system.h index c530d7c4ea..cbdab82dbb 100644 --- a/src/system.h +++ b/src/system.h @@ -136,4 +136,8 @@ void Panic_(const char * file, int line, const char * fmt, ...) NORETURN */ void InitSystem(int argc, const char * argv[], BOOL handleSignals); + +void InitRootPaths(int argc, const char * argv[]); + + #endif // GAP_SYSTEM_H