Hi hackers,
Earlier today Peter came across an issue on the chicken-5 branch where
the program `(number-string)` would produce bad C and fail to compile
due to a C procedure being called with too few arguments.
I traced this back to the c-platform rewrite from `number-string` to
`C_number_to_string`, which happily produces such a call when no
arguments are provided. Attached is a patch that adds argument count
checking for this class of compiler rewrites, in the style of some of
the others.
Why did Peter hit this issue on chicken-5 but not on master? Well, on
master `number-string` is subject to constant folding, which in the
0-ary case the optimizer attempts and detects as erroneous, thereby
marking the call site broken and exempt from further optimizations
(including compiler rewrites). This broken-node tracking aspect of the
compiler effectively masks the fact that this class of rewrites doesn't
work for calls with a bad argument count. (Mostly; you can still trigger
the issue on master if you really try, for example with the program
`(apply number-string '())`. Note that a couple of other common
procedures have this problem, too, including `-` and `/`, which is why
I've included a patch against master as well as chicken-5.) On
chicken-5, where `number-string` is no longer foldable (per d701161),
the argument count mismatch isn't detected during optimization and thus
c-platform.scm is free to rewrite it (to an invalid C call).
Let me know if any of that was unclear. The patch against 5 is obviously
more important, but I'd like them both to go in, assuming they look OK.
Cheers,
Evan
From 2ae62c5f09f1eb37609a9316a03d52b20d6d3ffd Mon Sep 17 00:00:00 2001
From: Evan Hanson ev...@foldling.org
Date: Sun, 22 Mar 2015 15:25:49 +1300
Subject: [PATCH] Add arity checks for ##core#proc-class platform rewrites
This prevents the c-backend from producing code containing invalid C
procedure calls when a Scheme procedure with a ##core#proc (class 13)
rewrite is invoked with the wrong number of arguments.
---
c-platform.scm | 53 +++--
optimizer.scm | 18 ++
2 files changed, 41 insertions(+), 30 deletions(-)
diff --git a/c-platform.scm b/c-platform.scm
index 1d60dcd..98eb99c 100644
--- a/c-platform.scm
+++ b/c-platform.scm
@@ -538,14 +538,14 @@
(rewrite 'call-with-values 8 rewrite-c-w-v)
(rewrite '##sys#call-with-values 8 rewrite-c-w-v) )
-(rewrite 'values 13 C_values #t)
-(rewrite '##sys#values 13 C_values #t)
-(rewrite 'call-with-values 13 C_u_call_with_values #f)
-(rewrite 'call-with-values 13 C_call_with_values #t)
-(rewrite '##sys#call-with-values 13 C_u_call_with_values #f)
-(rewrite '##sys#call-with-values 13 C_call_with_values #t)
-(rewrite 'locative-ref 13 C_locative_ref #t)
-(rewrite '##sys#continuation-graft 13 C_continuation_graft #t)
+(rewrite 'values 13 #f C_values #t)
+(rewrite '##sys#values 13 #f C_values #t)
+(rewrite 'call-with-values 13 2 C_u_call_with_values #f)
+(rewrite 'call-with-values 13 2 C_call_with_values #t)
+(rewrite '##sys#call-with-values 13 2 C_u_call_with_values #f)
+(rewrite '##sys#call-with-values 13 2 C_call_with_values #t)
+(rewrite 'locative-ref 13 1 C_locative_ref #t)
+(rewrite '##sys#continuation-graft 13 2 C_continuation_graft #t)
(rewrite 'caar 2 1 C_u_i_caar #f)
(rewrite 'cdar 2 1 C_u_i_cdar #f)
@@ -800,24 +800,25 @@
(rewrite '= 17 2 C_i_greater_or_equalp)
(rewrite '= 17 2 C_i_less_or_equalp)
-(rewrite '* 13 C_times #t)
-(rewrite '- 13 C_minus #t)
-(rewrite '+ 13 C_plus #t)
-(rewrite '/ 13 C_divide #t)
-(rewrite '= 13 C_nequalp #t)
-(rewrite ' 13 C_greaterp #t)
-(rewrite ' 13 C_lessp #t)
-(rewrite '= 13 C_greater_or_equal_p #t)
-(rewrite '= 13 C_less_or_equal_p #t)
-
-(rewrite 'number-string 13 C_number_to_string #t)
-(rewrite '##sys#call-with-current-continuation 13 C_call_cc #t)
-(rewrite '##sys#allocate-vector 13 C_allocate_vector #t)
-(rewrite '##sys#ensure-heap-reserve 13 C_ensure_heap_reserve #t)
-(rewrite 'return-to-host 13 C_return_to_host #t)
-(rewrite '##sys#context-switch 13 C_context_switch #t)
-(rewrite '##sys#intern-symbol 13 C_string_to_symbol #t)
-(rewrite '##sys#make-symbol 13 C_make_symbol #t)
+(rewrite '= 13 #f C_nequalp #t)
+(rewrite ' 13 #f C_greaterp #t)
+(rewrite ' 13 #f C_lessp #t)
+(rewrite '= 13 #f C_greater_or_equal_p #t)
+(rewrite '= 13 #f C_less_or_equal_p #t)
+
+(rewrite '* 13 #f C_times #t)
+(rewrite '+ 13 #f C_plus #t)
+(rewrite '/ 13 '(1 . #f) C_divide #t)
+(rewrite '- 13 '(1 . #f) C_minus #t)
+
+(rewrite 'number-string 13 '(1 . 2) C_number_to_string #t)
+(rewrite '##sys#call-with-current-continuation 13 1 C_call_cc #t)
+(rewrite '##sys#allocate-vector 13 4 C_allocate_vector #t)
+(rewrite '##sys#ensure-heap-reserve 13 1 C_ensure_heap_reserve #t)
+(rewrite 'return-to-host 13 0 C_return_to_host #t)
+(rewrite '##sys#context-switch 13 1 C_context_switch #t)
+(rewrite '##sys#intern-symbol 13 1 C_string_to_symbol #t)
+(rewrite '##sys#make-symbol 13 1
