kernel: catch infinite recursion comparing plists and precords

Fixes #1150

src/funcs.c

@@ -766,9 +766,9 @@ Obj EvalFunccallXargs (
 */
 
 /* TL: Int RecursionDepth; */
-static UInt RecursionTrapInterval;
+UInt RecursionTrapInterval;
 
-static void RecursionDepthTrap( void )
+void RecursionDepthTrap( void )
 {
  Int recursionDepth;
  /* in interactive work the RecursionDepth could become slightly negative
@@ -785,15 +785,6 @@ static void RecursionDepthTrap( void )
  }
 }
 
-static inline void CheckRecursionBefore( void )
-{
- TLS(RecursionDepth)++; 
- if ( RecursionTrapInterval && 
- 0 == (TLS(RecursionDepth) % RecursionTrapInterval) )
- RecursionDepthTrap();
-}
-
-
 Obj STEVES_TRACING;
 
 #define CHECK_RECURSION_BEFORE \

src/funcs.h

@@ -43,9 +43,20 @@ extern void ExecEnd (
 
 
 /* TL: extern Int RecursionDepth; */
+extern UInt RecursionTrapInterval;
+extern void RecursionDepthTrap( void );
+
+static inline void CheckRecursionBefore( void )
+{
+ TLS(RecursionDepth)++;
+ if ( RecursionTrapInterval &&
+ 0 == (TLS(RecursionDepth) % RecursionTrapInterval) )
+ RecursionDepthTrap();
+}
+
+
 /****************************************************************************
 **
-
 *F * * * * * * * * * * * * * initialize package * * * * * * * * * * * * * * *
 */
 

src/plist.c

@@ -43,6 +43,8 @@
 
 #include "gap.h" /* error handling, initialisation */
 
+#include "funcs.h"
+
 #include "gvars.h" /* global variables */
 
 #include "calls.h" /* generic call mechanism */
@@ -1012,22 +1014,20 @@ Int EqPlist (
  return 0L;
  }
 
+ CheckRecursionBefore();
+
  /* loop over the elements and compare them */
  for ( i = 1; i <= lenL; i++ ) {
  elmL = ELM_PLIST( left, i );
  elmR = ELM_PLIST( right, i );
- if ( elmL == 0 && elmR != 0 ) {
- return 0L;
- }
- else if ( elmR == 0 && elmL != 0 ) {
- return 0L;
- }
- else if ( ! EQ( elmL, elmR ) ) {
+ if ( ( (elmL == 0 ) != (elmR == 0) ) || ! EQ( elmL, elmR ) ) {
+ TLS(RecursionDepth)--;
  return 0L;
  }
  }
 
  /* no differences found, the lists are equal */
+ TLS(RecursionDepth)--;
  return 1L;
 }
 
@@ -1050,28 +1050,36 @@ Int LtPlist (
  Obj elmL; /* element of the left operand */
  Obj elmR; /* element of the right operand */
  Int i; /* loop variable */
+ Int res; /* result of comparison */
 
  /* get the lengths of the lists and compare them */
  lenL = LEN_PLIST( left );
  lenR = LEN_PLIST( right );
+ res = (lenL < lenR);
+
+ CheckRecursionBefore();
 
  /* loop over the elements and compare them */
  for ( i = 1; i <= lenL && i <= lenR; i++ ) {
  elmL = ELM_PLIST( left, i );
  elmR = ELM_PLIST( right, i );
  if ( elmL == 0 && elmR != 0 ) {
- return 1L;
+ res = 1L;
+ break;
  }
  else if ( elmR == 0 && elmL != 0 ) {
- return 0L;
+ res = 0L;
+ break;
  }
  else if ( ! EQ( elmL, elmR ) ) {
- return LT( elmL, elmR );
+ res = LT( elmL, elmR );
+ break;
  }
  }
 
  /* reached the end of at least one list */
- return (lenL < lenR);
+ TLS(RecursionDepth)--;
+ return res;
 }
 
 

src/precord.c

@@ -36,6 +36,8 @@
 
 #include "gap.h" /* error handling, initialisation */
 
+#include "funcs.h"
+
 #include "gvars.h" /* global variables */
 #include "calls.h" /* generic call mechanism */
 #include "opers.h" /* generic operations */
@@ -774,21 +776,26 @@ Obj FuncEQ_PREC (
  SortPRecRNam(left,0);
  SortPRecRNam(right,0);
 
+ CheckRecursionBefore();
+
  /* compare componentwise */
  for ( i = 1; i <= LEN_PREC(right); i++ ) {
 
  /* compare the names */
  if ( GET_RNAM_PREC(left,i) != GET_RNAM_PREC(right,i) ) {
+ TLS(RecursionDepth)--;
  return False;
  }
 
  /* compare the values */
  if ( ! EQ(GET_ELM_PREC(left,i),GET_ELM_PREC(right,i)) ) {
+ TLS(RecursionDepth)--;
  return False;
  }
  }
 
  /* the records are equal */
+ TLS(RecursionDepth)--;
  return True;
 }
 
@@ -807,6 +814,7 @@ Obj FuncLT_PREC (
  Obj right )
 {
  UInt i; /* loop variable */
+ Int res; /* result of comparison */
 
  /* quick first checks */
  if ( ! IS_PREC_REP(left) || ! IS_PREC_REP(right) ) {
@@ -818,35 +826,38 @@ Obj FuncLT_PREC (
  SortPRecRNam(left,0);
  SortPRecRNam(right,0);
 
+ CheckRecursionBefore();
+ res = 0;
+
  /* compare componentwise */
  for ( i = 1; i <= LEN_PREC(right); i++ ) {
 
  /* if the left is a proper prefix of the right one */
- if ( LEN_PREC(left) < i ) return True;
+ if ( LEN_PREC(left) < i ) {
+ res = 1;
+ break;
+ }
 
  /* compare the names */
  /* The sense of this comparison is determined by the rule that
  unbound entries compare less than bound ones */
  if ( GET_RNAM_PREC(left,i) != GET_RNAM_PREC(right,i) ) {
- if ( strcmp( NAME_RNAM( labs((Int)(GET_RNAM_PREC(left,i))) ),
- NAME_RNAM( labs((Int)(GET_RNAM_PREC(right,i))) ) ) > 0 ) {
- return True;
- }
- else {
- return False;
- }
+ res = ( strcmp( NAME_RNAM( labs((Int)(GET_RNAM_PREC(left,i))) ),
+ NAME_RNAM( labs((Int)(GET_RNAM_PREC(right,i))) ) ) > 0 );
+ break;
  }
 
  /* compare the values */
  if ( ! EQ(GET_ELM_PREC(left,i),GET_ELM_PREC(right,i)) ) {
- return LT( GET_ELM_PREC(left,i), GET_ELM_PREC(right,i) ) ?
-  True : False;
+ res = LT( GET_ELM_PREC(left,i), GET_ELM_PREC(right,i) );
+ break;
  }
 
  }
 
  /* the records are equal or the right is a prefix of the left */
- return False;
+ TLS(RecursionDepth)--;
+ return res ? True : False;
 }
 
 

tst/testbugfix/00351.tst

@@ -0,0 +1,10 @@
+# 2017-02-18 (MH): Comparing recursive data structures should not
+# crash. See issue #1150
+gap> [~] < [~];
+Error, recursion depth trap (5000)
+gap> [~] = [~];
+Error, recursion depth trap (5000)
+gap> rec(a:=~) = rec(a:=~);
+Error, recursion depth trap (5000)
+gap> rec(a:=~) < rec(a:=~);
+Error, recursion depth trap (5000)