From 85b3dab92d7e8d24034a34bb946894f9c90f3cdd Mon Sep 17 00:00:00 2001 From: Josh Davies Date: Thu, 5 Feb 2026 17:50:35 +0000 Subject: [PATCH] fix: various problems in TakeContent and related functions TakeContent did not correctly remove function content from the terms, it got stuck in infinite loops in several cases. MergeDotproductLists got stuck in infinite loops, missed tail cases, and produced a mal-formed result. In some of these cases, the poly class proceeded to compute a gcd anyway (since the non-symbol objects are converted to extrasymbols). The flint interface Terminated, since it anticipates content-free input and checks this. --- check/fixes.frm | 36 +++++++++ sources/ratio.c | 195 ++++++++++++++++++++++++++++++------------------ 2 files changed, 159 insertions(+), 72 deletions(-) diff --git a/check/fixes.frm b/check/fixes.frm index feaade11..34725434 100644 --- a/check/fixes.frm +++ b/check/fixes.frm @@ -4478,6 +4478,42 @@ assert compile_error?("Illegally formed function/tensor name") assert compile_error?("Illegal name for set") assert compile_error?("Illegal name for expression") *--#] Issue782 : +*--#[ Issue790 : +#- +CFunction cfx,cfy; +Function fx,fy; +Index ix,iy; +Index jx,jy; +Vector vx,vy; +Symbol sx,sy; +Local Fcf = gcd_(cfx+cfx*cfy, cfx-cfx*cfy); +Local Ff = gcd_(fx+fx*fy, fx-fx*fy); +Local Fi = gcd_(ix+ix*iy, ix-ix*iy); +Local Fv = gcd_(vx+vx*vy, vx-vx*vy); +Local Fvi = gcd_(vx(ix)+vx(ix)*vy(iy), vx(ix)-vx(ix)*vy(iy)); +Local Fdp = gcd_(vx.vx+vx.vx*vy.vy, vx.vx-vx.vx*vy.vy); +Local Fs = gcd_(sx+sx*sy, sx-sx*sy); +* For now, this fails in PutExtraSymbols, though TakeContent works +*Local Fdel = gcd_(d_(ix,jx)+d_(ix,jx)*d_(iy,jy), d_(ix,jx)-d_(ix,jx)*d_(iy,jy)); +Print; +.end +assert succeeded? +assert result("Fcf") =~ expr("cfx") +assert result("Ff") =~ expr("fx") +assert result("Fi") =~ expr("ix") +assert result("Fv") =~ expr("vx") +assert result("Fvi") =~ expr("vx(ix)") +assert result("Fdp") =~ expr("vx.vx") +assert result("Fs") =~ expr("sx") +*--#] Issue790 : +*--#[ Issue790b : +#- +Function fx,fy,fz; +Local Ff = gcd_(fx*fz+fx*fy, fx*fz-fx*fy); +Print; +.end +assert runtime_error?("GCD or factorization of more than one noncommuting object not allowed") +*--#] Issue790b : *--#[ Issue796 : * Regression: the fix for 796 deadlocks here: Symbol x; diff --git a/sources/ratio.c b/sources/ratio.c index 6c8a70bf..63a202a5 100644 --- a/sources/ratio.c +++ b/sources/ratio.c @@ -1167,7 +1167,6 @@ WORD *GCDfunction3(PHEAD WORD *in1, WORD *in2) AR.SortType = SORTHIGHFIRST; term1 = TermMalloc("GCDfunction3-a"); term2 = TermMalloc("GCDfunction3-b"); - confree1 = TakeContent(BHEAD in1,term1); tryterm1 = AN.tryterm; AN.tryterm = 0; confree2 = TakeContent(BHEAD in2,term2); @@ -1333,7 +1332,8 @@ WORD *MultiplyWithTerm(PHEAD WORD *in, WORD *term, WORD par) length = INCLENG(length); tt += ABS(length); tt[-1] = length; *termout = tt - termout; - SymbolNormalize(termout); + // in or term can contain non-symbols: we call this in TakeContent + Normalize(BHEAD termout); StoreTerm(BHEAD termout); in += *in; } @@ -1409,18 +1409,20 @@ WORD *TakeContent(PHEAD WORD *in, WORD *term) while ( m < tout ) { for ( i = 2; i < r[1]; i++ ) { if ( *m == r[i] ) break; - if ( *m > r[i] ) continue; + } + if ( i == r[1] ) { // index at m was not found, scratch from list mm = m+1; while ( mm < tout ) { mm[-1] = mm[0]; mm++; } tout--; tstore[1]--; m--; - break; } m++; } + r += r[1]; } - if ( r >= rstop || tout <= tstore+2 ) { + if ( tout <= tstore+2 ) { tout = tstore; break; } + t = tnext; } if ( tout > tstore+2 ) { /* Now we have to take out what is in tstore */ t = in; w = in; @@ -1452,66 +1454,68 @@ WORD *TakeContent(PHEAD WORD *in, WORD *term) */ code[0] = VECTOR; code[1] = DELTA; for ( k = 0; k < 2; k++ ) { - t = in; - tnext = t + *t; - tstop = tnext-ABS(tnext[-1]); - t++; - while ( t < tstop ) { - if ( *t == code[k] ) { - i = t[1]; NCOPY(tout,t,i); break; - } - else t += t[1]; - } - if ( tout > tstore ) { /* There are vectors in the first term */ - t = tnext; - while ( *t ) { - tnext = t + *t; - rstop = tnext - ABS(tnext[-1]); - r = t+1; - if ( r == rstop ) { tstore = tout; goto novectors; } - while ( r < rstop ) { - if ( *r != code[k] ) { r += r[1]; continue; } - m = tstore+2; - while ( m < tout ) { - for ( i = 2; i < r[1]; i += 2 ) { - if ( *m == r[i] && m[1] == r[i+1] ) break; - if ( *m > r[i] || ( *m == r[i] && m[1] > r[i+1] ) ) continue; - mm = m+2; - while ( mm < tout ) { mm[-2] = mm[0]; mm[-1] = mm[1]; mm += 2; } - tout -= 2; tstore[1] -= 2; m -= 2; - break; - } - m += 2; - } - } - if ( r >= rstop || tout <= tstore+2 ) { - tout = tstore; break; + t = in; + tnext = t + *t; + tstop = tnext-ABS(tnext[-1]); + t++; + while ( t < tstop ) { + if ( *t == code[k] ) { + i = t[1]; NCOPY(tout,t,i); break; } + else t += t[1]; } - if ( tout > tstore+2 ) { /* Now we have to take out what is in tstore */ - t = in; w = in; + if ( tout > tstore ) { /* There are vectors in the first term */ + t = tnext; while ( *t ) { - wterm = w; - tnext = t + *t; t++; w++; - while ( *t != code[k] ) { i = t[1]; NCOPY(w,t,i); } - tt = t + t[1]; t += 2; r = tstore+2; ww = w; *w++ = code[k]; w++; - while ( r < tout && t < tt ) { - if ( ( *r > *t ) || ( *r == *t && r[1] > t[1] ) ) - { *w++ = *t++; *w++ = *t++; } - else if ( *r == *t && r[1] == t[1] ) { r += 2; t += 2; } - else goto CalledFrom; + tnext = t + *t; + rstop = tnext - ABS(tnext[-1]); + r = t+1; + if ( r == rstop ) { tstore = tout; goto novectors; } + while ( r < rstop ) { + if ( *r != code[k] ) { r += r[1]; continue; } + m = tstore+2; + while ( m < tout ) { + for ( i = 2; i < r[1]; i += 2 ) { + if ( *m == r[i] && m[1] == r[i+1] ) break; + } + if ( i == r[1] ) { // object was not found, scratch from list + mm = m+2; + while ( mm < tout ) { mm[-2] = mm[0]; mm[-1] = mm[1]; mm += 2; } + tout -= 2; tstore[1] -= 2; m -= 2; + } + m += 2; + } + r += r[1]; + } + if ( tout <= tstore+2 ) { + tout = tstore; break; + } + t = tnext; + } + if ( tout > tstore+2 ) { /* Now we have to take out what is in tstore */ + t = in; w = in; + while ( *t ) { + wterm = w; + tnext = t + *t; t++; w++; + while ( *t != code[k] ) { i = t[1]; NCOPY(w,t,i); } + tt = t + t[1]; t += 2; r = tstore+2; ww = w; *w++ = code[k]; w++; + while ( r < tout && t < tt ) { + if ( ( *r > *t ) || ( *r == *t && r[1] > t[1] ) ) + { *w++ = *t++; *w++ = *t++; } + else if ( *r == *t && r[1] == t[1] ) { r += 2; t += 2; } + else goto CalledFrom; + } + if ( r < tout ) goto CalledFrom; + while ( t < tt ) *w++ = *t++; + ww[1] = w - ww; + if ( ww[1] == 2 ) w = ww; + while ( t < tnext ) *w++ = *t++; + *wterm = w - wterm; } - if ( r < tout ) goto CalledFrom; - while ( t < tt ) *w++ = *t++; - ww[1] = w - ww; - if ( ww[1] == 2 ) w = ww; - while ( t < tnext ) *w++ = *t++; - *wterm = w - wterm; + *w = 0; } - *w = 0; + tstore = tout; } - tstore = tout; - } } novectors:; /* @@ -1526,7 +1530,7 @@ novectors:; while ( t < tstop ) { if ( *t >= FUNCTION ) { if ( functions[*t-FUNCTION].commute ) { - if ( tcom == 0 ) { tcom = tstore; } + if ( tcom == 0 ) { tcom = tout; } else { for ( i = 0; i < t[1]; i++ ) { if ( t[i] != tcom[i] ) { @@ -1538,11 +1542,11 @@ novectors:; } } } - i = t[1]; NCOPY(tstore,t,i); + i = t[1]; NCOPY(tout,t,i); } else t += t[1]; } - if ( tout > tstore ) { + if ( tout > tstore ) { /* There are functions in the first term */ t = tnext; while ( *t ) { tnext = t + *t; tstop = tnext - ABS(tnext[-1]); t++; @@ -1555,6 +1559,7 @@ novectors:; if ( r[i] != tt[i] ) break; } if ( i == r[1] ) { r += r[1]; goto nextr1; } + tt += tt[1]; } /* Not encountered in this term. Scratch from list @@ -1565,7 +1570,7 @@ novectors:; nextr1:; } if ( tout <= tstore ) break; - t += *t; + t = tnext; } } if ( tout > tstore ) { @@ -1575,21 +1580,23 @@ nextr1:; */ r = tstore; while ( r < tout ) { - t = in; ww = in; w = ww+1; + t = in; ww = w = in; while ( *t ) { + ww = w; // store the location of the term size + w += 1; // term data copied here, following the size tnext = t + *t; t++; for(;;) { - for ( i = 0; i < r[1]; i++ ) { + for ( i = 0; i < r[1]; i++ ) { // search for the current tstore object if ( t[i] != r[i] ) { j = t[1]; NCOPY(w,t,j); break; } } - if ( i == r[1] ) { - t += t[1]; - while ( t < tnext ) *w++ = *t++; - *ww = w - ww; + if ( i == r[1] ) { // we found the current tstore object at t + t += t[1]; // skip over it + while ( t < tnext ) *w++ = *t++; // copy the rest of the term + *ww = w - ww; // update the size break; } } @@ -1659,6 +1666,15 @@ nextr1:; */ tout[0] = DOTPRODUCT; tout[1] = 2; t = in; + tnext = t + *t; tstop = tnext - ABS(tnext[-1]); t++; + while ( t < tstop ) { + if ( *t == DOTPRODUCT ) { + for ( i = 0; i < t[1]; i++ ) tout[i] = t[i]; + break; + } + t += t[1]; + } + t = tnext; while ( *t ) { tnext = t + *t; tstop = tnext - ABS(tnext[-1]); t++; if ( t == tstop ) { @@ -1811,7 +1827,7 @@ int MergeSymbolLists(PHEAD WORD *old, WORD *extra, int par) WORD *new = TermMalloc("MergeSymbolLists"); WORD *t1, *t2, *fill; int i1,i2; - fill = new + 2; *new = SYMBOL; + fill = new + 2; i1 = old[1] - 2; i2 = extra[1] - 2; t1 = old + 2; t2 = extra + 2; switch ( par ) { @@ -1906,6 +1922,7 @@ int MergeSymbolLists(PHEAD WORD *old, WORD *extra, int par) for ( ; i2 > 0; i2-- ) *fill++ = *t2++; break; } + new[0] = SYMBOL; i1 = new[1] = fill - new; t2 = new; t1 = old; NCOPY(t1,t2,i1); TermFree(new,"MergeSymbolLists"); @@ -1939,66 +1956,100 @@ int MergeDotproductLists(PHEAD WORD *old, WORD *extra, int par) if ( ( *t1 > *t2 ) || ( *t1 == *t2 && t1[1] > t2[1] ) ) { if ( t2[2] < 0 ) { *fill++ = *t2++; *fill++ = *t2++; *fill++ = *t2++; } else t2 += 3; + i2 -= 3; } else if ( ( *t1 < *t2 ) || ( *t1 == *t2 && t1[1] < t2[1] ) ) { if ( t1[2] < 0 ) { *fill++ = *t1++; *fill++ = *t1++; *fill++ = *t1++; } else t1 += 3; + i1 -= 3; } else if ( t1[2] < t2[2] ) { *fill++ = *t1++; *fill++ = *t1++; *fill++ = *t1++; t2 += 3; + i1 -= 3; i2 -= 3; } else { *fill++ = *t2++; *fill++ = *t2++; *fill++ = *t2++; t1 += 3; + i2 -= 3; i1 -= 3; } } + for ( ; i1 > 0; i1 -= 3 ) { + if ( t1[2] < 0 ) { *fill++ = *t1++; *fill++ = *t1++; *fill++ = *t1++; } + else t1 += 3; + } + for ( ; i2 > 0; i2 -= 3 ) { + if ( t2[2] < 0 ) { *fill++ = *t2++; *fill++ = *t2++; *fill++ = *t2++; } + else t2 += 3; + } break; case 1: while ( i1 > 0 && i2 > 0 ) { if ( ( *t1 > *t2 ) || ( *t1 == *t2 && t1[1] > t2[1] ) ) { if ( t2[2] > 0 ) { *fill++ = *t2++; *fill++ = *t2++; *fill++ = *t2++; } else t2 += 3; + i2 -= 3; } else if ( ( *t1 < *t2 ) || ( *t1 == *t2 && t1[1] < t2[1] ) ) { if ( t1[2] > 0 ) { *fill++ = *t1++; *fill++ = *t1++; *fill++ = *t1++; } else t1 += 3; + i1 -= 3; } else if ( t1[2] > t2[2] ) { *fill++ = *t1++; *fill++ = *t1++; *fill++ = *t1++; t2 += 3; + i1 -= 3; i2 -= 3; } else { *fill++ = *t2++; *fill++ = *t2++; *fill++ = *t2++; t1 += 3; + i2 -= 3; i1 -= 3; } } + for ( ; i1 > 0; i1 -= 3 ) { + if ( t1[2] > 0 ) { *fill++ = *t1++; *fill++ = *t1++; *fill++ = *t1++; } + else t1 += 3; + } + for ( ; i2 > 0; i2 -= 3 ) { + if ( t2[2] > 0 ) { *fill++ = *t2++; *fill++ = *t2++; *fill++ = *t2++; } + else t2 += 3; + } break; case 0: while ( i1 > 0 && i2 > 0 ) { if ( ( *t1 > *t2 ) || ( *t1 == *t2 && t1[1] > t2[1] ) ) { t2 += 3; + i2 -= 3; } else if ( ( *t1 < *t2 ) || ( *t1 == *t2 && t1[1] < t2[1] ) ) { t1 += 3; + i1 -= 3; } - else if ( ( t1[2] > 0 ) && ( t2[2] < 0 ) ) { t1 += 3; t2 += 3; } - else if ( ( t1[2] < 0 ) && ( t2[2] > 0 ) ) { t1 += 3; t2 += 3; } + else if ( ( t1[2] > 0 ) && ( t2[2] < 0 ) ) { t1 += 3; t2 += 3; i1 -= 3; i2 -= 3; } + else if ( ( t1[2] < 0 ) && ( t2[2] > 0 ) ) { t1 += 3; t2 += 3; i1 -= 3; i2 -= 3; } else if ( t1[2] > 0 ) { if ( t1[2] < t2[2] ) { *fill++ = *t1++; *fill++ = *t1++; *fill++ = *t1++; t2 += 3; + i1 -= 3; i2 -= 3; } else { *fill++ = *t2++; *fill++ = *t2++; *fill++ = *t2++; t1 += 3; + i2 -= 3; i1 -= 3; + } } else { if ( t2[2] < t1[2] ) { *fill++ = *t2++; *fill++ = *t2++; *fill++ = *t2++; t1 += 3; + i2 -= 3; i1 -= 3; } else { *fill++ = *t1++; *fill++ = *t1++; *fill++ = *t1++; t2 += 3; + i1 -= 3; i2 -= 3; } } } + for ( ; i1 > 0; i1-- ) *fill++ = *t1++; + for ( ; i2 > 0; i2-- ) *fill++ = *t2++; break; } + new[0] = DOTPRODUCT; i1 = new[1] = fill - new; t2 = new; t1 = old; NCOPY(t1,t2,i1); TermFree(new,"MergeDotproductLists");