From f1989d3ae4bbec6860ff73e88adfb1dad6c61bea Mon Sep 17 00:00:00 2001 From: Arne Goedeke <el@laramies.com> Date: Fri, 5 Jun 2015 08:59:37 +0200 Subject: [PATCH] enumerate(): fixed integer overflow handling This change fixes two bugs 1) the overflow checks were incorrect, because signed integers do not overflow 2) when overflow was actually detected, the code fell back to the slow path (using f_add) with the last array element as the new start value, which led to wrong results --- lib/modules/testsuite.in | 1 + src/builtin_functions.c | 31 ++++++++++++++++++------------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/lib/modules/testsuite.in b/lib/modules/testsuite.in index 92f86dafae..5263f770a9 100644 --- a/lib/modules/testsuite.in +++ b/lib/modules/testsuite.in @@ -591,6 +591,7 @@ test_equal(Array.enumerate(5), ({ 0,1,2,3,4 })) test_equal(Array.enumerate(5,2), ({ 0,2,4,6,8 })) test_equal(Array.enumerate(5,2,7), ({ 7,9,11,13,15 })) test_equal(Array.enumerate(5,2,7,`-), ({ 7,5,3,1,-1 })) +test_equal(Array.enumerate(5,1,Int.NATIVE_MAX-1), ({ Int.NATIVE_MAX-1, Int.NATIVE_MAX, Int.NATIVE_MAX+1, Int.NATIVE_MAX+2, Int.NATIVE_MAX+3 })) test_eq([[ Array.reduce(`>>, ({})) ]], 0) test_eq([[ Array.reduce(`==, ({}), 1) ]], 1) diff --git a/src/builtin_functions.c b/src/builtin_functions.c index dc39bd43c7..f535c96346 100644 --- a/src/builtin_functions.c +++ b/src/builtin_functions.c @@ -9282,23 +9282,26 @@ void f_enumerate(INT32 args) INT_TYPE step,start; get_all_args("enumerate", args, "%+%i%i", &n, &step, &start); + + { + INT_TYPE tmp; + + /* this checks if + * (n - 1) * step + start + * will overflow. if it does, we continue with the slow path. If it does not, + * adding step to start repeatedly will not overflow below. This check has + * false positives, but is much simpler to check than e.g. doing one check + * for every iteration + */ + if (DO_INT_TYPE_MUL_OVERFLOW(n-1, step, &tmb) || INT_TYPE_ADD_OVERFLOW(tmp, start)) + goto slow_path; + } + pop_n_elems(args); push_array(d=allocate_array(n)); for (i=0; i<n; i++) { ITEM(d)[i].u.integer=start; - if ((step>0 && start+step<start) || - (step<0 && start+step>start)) /* overflow */ - { - pop_stack(); - push_int(n); - push_int(step); - convert_stack_top_to_bignum(); - push_int(start); - convert_stack_top_to_bignum(); - f_enumerate(3); - return; - } start+=step; } d->type_field = BIT_INT; @@ -9323,7 +9326,9 @@ void f_enumerate(INT32 args) } else { - TYPE_FIELD types = 0; + TYPE_FIELD types; +slow_path: + types = 0; get_all_args("enumerate", args, "%+", &n); if (args>4) pop_n_elems(args-4); push_array(d=allocate_array(n)); -- GitLab