Re: [Chicken-hackers] [PATCH] Avoid segfault when expanding FFI macros

[felix . winkelmann]

> On Mon, Dec 04, 2017 at 10:51:31PM +0100, felix.winkelm...@bevuta.com wrote:
> > Also signed off and pushed.
> 
> Thanks!  Any idea why this doesn't fail on CHICKEN 4?
> 

No idea.


felix


___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] [PATCH] Avoid segfault when expanding FFI macros

[Peter Bex]

On Mon, Dec 04, 2017 at 10:51:31PM +0100, felix.winkelm...@bevuta.com wrote:
> Also signed off and pushed.

Thanks!  Any idea why this doesn't fail on CHICKEN 4?

Cheers,
Peter


signature.asc
Description: PGP signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] [PATCH] Avoid segfault when expanding FFI macros

[felix . winkelmann]

Also signed off and pushed.


felix


___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

[Chicken-hackers] [PATCH] Avoid segfault when expanding FFI macros

[Peter Bex]

Hi all,

I discovered that running code which contains FFI macro calls through
the interpreter will cause a segfault.  This happens because these
macros call procedures from the compiler directly.

I don't know why this is not an issue in CHICKEN 4; it seems that the
module itself can be imported, but the macros are not defined by it
in the interpreter?  I would really love a clarification of this, so
that we're sure master doesn't need a similar patch.  It calls some
 procedures which are prefixed ##compiler#, so I suppose the same
problem would exist there if one were able to expand these macros in
the interpreter.

I tried to move foreign-type->scrutiny-type from support.scm into
chicken-ffi-syntax.scm, but it turns out the comment is misleading:
foreign-type->scrutiny-type is used in the scrutinizer too, so we
can't simply move it.  Besides, foreign-type->scrutiny-type calls
final-foreign-type, which uses lookup-foreign-type, which consults
foreign-type-table and so on and so on, so if we wanted to move it
we'd have to do a pretty large restructuring (if possible at all).

So instead, I decided it would be better if the macros themselves
would simply raise a syntax error upon expansion.  Alternatively,
we could try to raise an error in chicken.foreign.import.scm when
an attempt is made to load it, but I think the approach of letting
the macros themselves bail out is somewhat cleaner.

A disadvantage of this is that you can't expand the macro in the
interpreter to see what kind of code it generates.  Another approach
would be to stub out the helper functions so they return a fake value,
but that would be even more confusing because the code wouldn't be
representative for the actual code it expands to.

Cheers,
Peter
From e02b684150b75e526b22d0857e0dbc0492958611 Mon Sep 17 00:00:00 2001
From: Peter Bex 
Date: Sun, 12 Nov 2017 14:03:33 +0100
Subject: [PATCH] Avoid expanding to unbound procedures in FFI macros.

These macros call procedures that are defined inside the compiler
only.  This will result in an "unbound variable" error in DEBUGBUILD
CHICKENs, and a segfault(!) in non-DEBUGBUILDs.

So, we check to see if the macro expander is running inside the
compiler and emit an error if that's not the case.
---
 chicken-ffi-syntax.scm | 46 --
 support.scm|  4 ++--
 2 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/chicken-ffi-syntax.scm b/chicken-ffi-syntax.scm
index 0b3008c0..255c9c93 100644
--- a/chicken-ffi-syntax.scm
+++ b/chicken-ffi-syntax.scm
@@ -31,9 +31,6 @@
   (disable-interrupts)
   (fixnum))
 
-;; IMPORTANT: These macros expand directly into fully qualified names
-;; from the "chicken.compiler.c-backend" and "chicken.compiler.support" modules.
-
 #+(not debugbuild)
 (declare
   (no-bound-checks)
@@ -42,6 +39,8 @@
 (import chicken.base
 	chicken.format
 	chicken.internal
+	chicken.platform
+	chicken.syntax
 	chicken.string)
 
 (include "common-declarations.scm")
@@ -50,10 +49,21 @@
 (define ##sys#chicken-ffi-macro-environment
   (let ((me0 (##sys#macro-environment)))
 
+;; IMPORTANT: These macros directly call fully qualified names from
+;; the "chicken.compiler.c-backend" and "chicken.compiler.support"
+;; modules.  These are unbound in the interpreter, so check first:
+(define (compiler-only-er-transformer transformer)
+  (##sys#er-transformer
+   (lambda (form r c)
+ (if (feature? 'compiling)
+	 (transformer form r c)
+	 (syntax-error
+	  (car form) "The FFI is not supported in interpreted mode")
+
 (##sys#extend-macro-environment
  'define-external
  '()
- (##sys#er-transformer
+ (compiler-only-er-transformer
   (lambda (form r c)
 (let* ((form (cdr form))
 	   (quals (and (pair? form) (string? (car form
@@ -90,7 +100,7 @@
 (##sys#extend-macro-environment
  'location
  '()
- (##sys#er-transformer
+ (compiler-only-er-transformer
   (lambda (x r c)
 (##sys#check-syntax 'location x '(location _))
 `(##core#location ,(cadr x)
@@ -98,7 +108,7 @@
 (##sys#extend-macro-environment
  'define-location
  '()
- (##sys#er-transformer
+ (compiler-only-er-transformer
   (lambda (form r c)
 (##sys#check-syntax 'define-location form '(_ variable _ . #(_ 0 1)))
 (let ((var (cadr form))
@@ -115,7 +125,7 @@
 (##sys#extend-macro-environment
  'let-location
  '()
- (##sys#er-transformer
+ (compiler-only-er-transformer
   (lambda (form r c)
 (##sys#check-syntax 'let-location form '(_ #((variable _ . #(_ 0 1)) 0) . _))
 (let* ((bindings (cadr form))
@@ -151,7 +161,7 @@
 (##sys#extend-macro-environment
  'foreign-code
  '()
- (##sys#er-transformer
+ (compiler-only-er-transformer
   (lambda (form r c)
 (##sys#check-syntax 'foreign-code form '(_ . #(string 0)))
 (let ([tmp (gensym 'code_)])
@@ -166,7 +176,7 @@
 (##sys#extend-macro-environment
  'foreign-value
  '()
- (##sys#er-transformer
+ (compiler-only-er-transformer
   (lambda (form r c)
 (##sys#check-syntax