From 411fb0835cde54fa39356599c9cbf6d7b43c2228 Mon Sep 17 00:00:00 2001 From: jodavies Date: Tue, 9 Jun 2026 22:42:41 +0100 Subject: [PATCH] fix: give each SORTING struct its own OldPosOut, OldPosIn During a ground-level stage sort, if a sub-buffer par=1 MergePatches creates a sort file, it resets OldPosOut with `PUTZERO(AN.OldPosOut);`. The value of OldPosOut needed by the ground-level sort is lost, leading to terms being lost due to not finishing the Merge by jumping to ConMer, at the end of MergePatches. Fix by moving OldPosOut, OldPosIn to the SORTING struct. Now every sort has its own private values. This is a nasty one: users who have run scripts which they know caused both ground-level stage sorts and very nasty sub-buffer sorts (which created sort files) should be suspicious of the results! --- check/fixes.frm | 125 ++++++++++++++++++++++++++++++++++++++++++++++ sources/sort.c | 19 +++---- sources/structs.h | 4 +- 3 files changed, 134 insertions(+), 14 deletions(-) diff --git a/check/fixes.frm b/check/fixes.frm index 14676cac..17ff962f 100644 --- a/check/fixes.frm +++ b/check/fixes.frm @@ -4656,6 +4656,131 @@ Symbol x; .end assert succeeded? *--#] PullReq691 : +*--#[ PullReq843_1 : +#- + +#: TermsInSmall 16 +#: LargePatches 10 +#: FilePatches 16 + +#: SubTermsInSmall 16 +#: SubLargePatches 10 +#: SubFilePatches 10 + +Off statistics; +Off threadstats; + +CFunction f,sum; +Symbol i,j,x; + +#define N "{2*1776+1}" +#define BLOWUP "{160+1}" + +* Generate enough terms to cause a stage sort: +Local test = {`N'*(`N'+1)/2} + #do i = 1,`N' + - f(x^`i') + #enddo + ; +.sort + +* Generate enough terms in a sub-buffer sort to create a sort file: +Argument f; + Identify x^i?pos_ = sum_(j,1,`BLOWUP',x^i*i/`BLOWUP'); +EndArgument; + +* Cancel everything: +Identify f(x?) = x; +Identify x^i? = 1; +Print; +.end +assert succeeded? +assert result("test") =~ expr("0") +*--#] PullReq843_1 : +*--#[ PullReq843_2 : +#- + +#: TermsInSmall 16 +#: LargePatches 10 +#: FilePatches 16 + +#: SubTermsInSmall 16 +#: SubLargePatches 10 +#: SubFilePatches 10 + +Off statistics; +Off threadstats; + +CFunction f,sum; +Symbol i,j,x; + +#define N "{2*1776+1}" +#define BLOWUP "{160+1}" + +* Generate enough terms to cause a stage sort: +Local test = {`N'*(`N'+1)/2} + #do i = 1,`N' + - x^`i' + #enddo + ; +.sort + +* Generate enough terms in a sub-buffer sort to create a sort file: +Term; + Identify x^i?pos_ = sum_(j,1,`BLOWUP',x^i*i/`BLOWUP'); +EndTerm; + +* Cancel everything: +Identify x^i? = 1; +Print; +.end +assert succeeded? +assert result("test") =~ expr("0") +*--#] PullReq843_2 : +*--#[ PullReq843_3 : +#- + +#: TermsInSmall 16 +#: LargePatches 10 +#: FilePatches 16 + +#: SubTermsInSmall 16 +#: SubLargePatches 10 +#: SubFilePatches 10 + +Off statistics; +Off threadstats; + +CFunction f,sum; +Symbol i,j,x; + +#define N "{2*1776+1}" +#define BLOWUP "{160+1}" + +* Generate enough terms to cause a stage sort: +Local test = {`N'*(`N'+1)/2} + #do i = 1,`N' + - f(x^`i') + #enddo + ; +.sort + +#$dol = 1; +* Generate enough terms in a sub-buffer sort to create a sort file: +Identify f(x?$dol) = 1; +Inside $dol; + Identify x^i?pos_ = sum_(j,1,`BLOWUP',x^i*i/`BLOWUP'); +EndInside; +Multiply $dol; + +* Cancel everything: +Identify x^i? = 1; +Print; +ModuleOption local $dol; +.end +assert succeeded? +assert result("test") =~ expr("0") +*--#] PullReq843_3 : *--#[ Issue808 : #do i=0,9 Global E`i' = `i'; diff --git a/sources/sort.c b/sources/sort.c index 13c5d20c..10ae37d0 100644 --- a/sources/sort.c +++ b/sources/sort.c @@ -407,12 +407,7 @@ int NewSort(PHEAD0) PUTZERO(S->file.POposition); S->stage4 = 0; if ( AR.sLevel > AN.MaxFunSorts ) AN.MaxFunSorts = AR.sLevel; -/* - The next variable is for the staged sort only. - It should be treated differently - PUTZERO(AN.OldPosOut); -*/ // Zero the SortVerbose counters: S->verbComparisons = 0; S->verbSBsortTerms = 0; @@ -3493,7 +3488,7 @@ int MergePatches(WORD par) fout = &(S->file); if ( fout->handle < 0 ) { FileMake: - PUTZERO(AN.OldPosOut); + PUTZERO(S->OldPosOut); if ( ( fhandle = CreateFile(fout->name) ) < 0 ) { MLOCK(ErrorMessageLock); MesPrint("Cannot create file %s",fout->name); @@ -3552,7 +3547,7 @@ int MergePatches(WORD par) S->fPatches = S->inPatches; S->inPatches = S->iPatches; (S->inNum) = S->fPatchN; - AN.OldPosIn = AN.OldPosOut; + S->OldPosIn = S->OldPosOut; #ifdef WITHZLIB m1 = S->fpincompressed; S->fpincompressed = S->fpcompressed; @@ -4121,9 +4116,9 @@ int MergePatches(WORD par) */ (S->fPatchN)++; S->fPatches[S->fPatchN] = position; - if ( ISNOTZEROPOS(AN.OldPosIn) ) { /* We are not done */ + if ( ISNOTZEROPOS(S->OldPosIn) ) { /* We are not done */ - SeekFile(fin->handle,&(AN.OldPosIn),SEEK_SET); + SeekFile(fin->handle,&(S->OldPosIn),SEEK_SET); /* We don't need extra provisions for the zlib compression here. If part of an expression has been sorted, the whole has been so. @@ -4131,7 +4126,7 @@ int MergePatches(WORD par) */ if ( (ULONG)ReadFile(fin->handle,(UBYTE *)(&(S->inNum)),(LONG)sizeof(WORD)) != sizeof(WORD) - || (ULONG)ReadFile(fin->handle,(UBYTE *)(&AN.OldPosIn),(LONG)sizeof(POSITION)) != + || (ULONG)ReadFile(fin->handle,(UBYTE *)(&(S->OldPosIn)),(LONG)sizeof(POSITION)) != sizeof(POSITION) || (ULONG)ReadFile(fin->handle,(UBYTE *)S->iPatches,(LONG)((S->inNum)+1) *sizeof(POSITION)) != ((S->inNum)+1)*sizeof(POSITION) ) { @@ -4408,7 +4403,7 @@ void StageSort(FILEHANDLE *fout) */ if ( (ULONG)WriteFile(fout->handle,(UBYTE *)(&(S->fPatchN)),(LONG)sizeof(WORD)) != sizeof(WORD) - || (ULONG)WriteFile(fout->handle,(UBYTE *)(&(AN.OldPosOut)),(LONG)sizeof(POSITION)) != + || (ULONG)WriteFile(fout->handle,(UBYTE *)(&(S->OldPosOut)),(LONG)sizeof(POSITION)) != sizeof(POSITION) || (ULONG)WriteFile(fout->handle,(UBYTE *)(S->fPatches),(LONG)(S->fPatchN+1) *sizeof(POSITION)) != (S->fPatchN+1)*sizeof(POSITION) ) { @@ -4417,7 +4412,7 @@ void StageSort(FILEHANDLE *fout) MUNLOCK(ErrorMessageLock); Terminate(-1); } - AN.OldPosOut = position; + S->OldPosOut = position; fout->filesize = position; ADDPOS(fout->filesize,(S->fPatchN+2)*sizeof(POSITION) + sizeof(WORD)); fout->POposition = fout->filesize; diff --git a/sources/structs.h b/sources/structs.h index 36787109..a891bdca 100644 --- a/sources/structs.h +++ b/sources/structs.h @@ -1114,6 +1114,8 @@ typedef struct PaRtI { typedef struct sOrT { FILEHANDLE file; /* The own sort file */ POSITION SizeInFile[3]; /* Sizes in the various files */ + POSITION OldPosIn; /* Sort file fill positions */ + POSITION OldPosOut; WORD *lBuffer; /* The large buffer */ WORD *lTop; /* End of the large buffer */ WORD *lFill; /* The filling point of the large buffer */ @@ -2198,8 +2200,6 @@ struct T_const { */ struct N_const { - POSITION OldPosIn; /* (R) Used in sort. */ - POSITION OldPosOut; /* (R) Used in sort */ POSITION theposition; /* () Used in index.c */ WORD *EndNest; /* (R) Nesting of function levels etc. */ WORD *Frozen; /* (R) Bracket info */