Skip to content

Commit

Permalink
Forbid returning and passing tuples to FFI functions
Browse files Browse the repository at this point in the history 
Passing complex objects by value in C is hard: there are many different
calling conventions depending on the platform. For example, a C
compiler is allowed to split the members of an object into different
parameters, or to change the return value into an out parameter.

The Pony compiler currently uses a naive implementation to pass objects
to FFI functions and isn't able to use the correct calling convention
for every call. Ideally, we'd want to rely on a C compiler API (like
LibClang) to generate the correct signatures and calls.

This change is a temporary measure to avoid spurious FFI bugs until
we have the interoperability detailed above. LLVM intrinsics are still
allowed to return and be passed tuples since their signatures are
defined by LLVM.
  • Loading branch information
Benoit Vey committed Jul 15, 2017
1 parent a087e64 commit c658d1a
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 70 deletions.
114 changes: 48 additions & 66 deletions src/libponyc/codegen/gencall.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -892,33 +892,62 @@ LLVMValueRef gen_pattern_eq(compile_t* c, ast_t* pattern, LLVMValueRef r_value)
return result;
}

static LLVMValueRef declare_ffi_vararg(compile_t* c, const char* f_name,
reach_type_t* t, bool err)
static LLVMTypeRef ffi_return_type(compile_t* c, reach_type_t* t,
bool intrinsic)
{
LLVMTypeRef f_type = LLVMFunctionType(t->use_type, NULL, 0, true);
LLVMValueRef func = LLVMAddFunction(c->module, f_name, f_type);

if(!err)
if(t->underlying == TK_TUPLETYPE)
{
#if PONY_LLVM >= 309
LLVM_DECLARE_ATTRIBUTEREF(nounwind_attr, nounwind, 0);
pony_assert(intrinsic);

// Can't use the named type. Build an unnamed type with the same elements.
unsigned int count = LLVMCountStructElementTypes(t->use_type);
size_t buf_size = count * sizeof(LLVMTypeRef);
LLVMTypeRef* e_types = (LLVMTypeRef*)ponyint_pool_alloc_size(buf_size);
LLVMGetStructElementTypes(t->use_type, e_types);

LLVMAddAttributeAtIndex(func, LLVMAttributeFunctionIndex, nounwind_attr);
#else
LLVMAddFunctionAttr(func, LLVMNoUnwindAttribute);
#endif
ast_t* child = ast_child(t->ast);
size_t i = 0;

while(child != NULL)
{
// A Bool in an intrinsic tuple return type is an i1, not an ibool.
if(is_bool(child))
e_types[i] = c->i1;

child = ast_sibling(child);
i++;
}

LLVMTypeRef r_type = LLVMStructTypeInContext(c->context, e_types, count,
false);
ponyint_pool_free_size(buf_size, e_types);
return r_type;
} else {
// An intrinsic that returns a Bool returns an i1, not an ibool.
if(intrinsic && is_bool(t->ast))
return c->i1;
else
return t->use_type;
}
}

static LLVMValueRef declare_ffi_vararg(compile_t* c, const char* f_name,
reach_type_t* t)
{
LLVMTypeRef r_type = ffi_return_type(c, t, false);
LLVMTypeRef f_type = LLVMFunctionType(r_type, NULL, 0, true);
LLVMValueRef func = LLVMAddFunction(c->module, f_name, f_type);

return func;
}

static LLVMValueRef declare_ffi(compile_t* c, const char* f_name,
reach_type_t* t, ast_t* args, bool err, bool intrinsic)
reach_type_t* t, ast_t* args, bool intrinsic)
{
ast_t* last_arg = ast_childlast(args);

if((last_arg != NULL) && (ast_id(last_arg) == TK_ELLIPSIS))
return declare_ffi_vararg(c, f_name, t, err);
return declare_ffi_vararg(c, f_name, t);

int count = (int)ast_childcount(args);
size_t buf_size = count * sizeof(LLVMTypeRef);
Expand Down Expand Up @@ -946,57 +975,10 @@ static LLVMValueRef declare_ffi(compile_t* c, const char* f_name,
arg = ast_sibling(arg);
}

LLVMTypeRef r_type;

if(t->underlying == TK_TUPLETYPE)
{
// Can't use the named type. Build an unnamed type with the same
// elements.
unsigned int count = LLVMCountStructElementTypes(t->use_type);
size_t buf_size = count * sizeof(LLVMTypeRef);
LLVMTypeRef* e_types = (LLVMTypeRef*)ponyint_pool_alloc_size(buf_size);
LLVMGetStructElementTypes(t->use_type, e_types);

if(intrinsic)
{
ast_t* child = ast_child(t->ast);
size_t i = 0;

while(child != NULL)
{
// A Bool in an intrinsic tuple return type is an i1, not an ibool.
if(is_bool(child))
e_types[i] = c->i1;

child = ast_sibling(child);
i++;
}
}

r_type = LLVMStructTypeInContext(c->context, e_types, count, false);
ponyint_pool_free_size(buf_size, e_types);
} else {
// An intrinsic that returns a Bool returns an i1, not an ibool.
if(intrinsic && is_bool(t->ast))
r_type = c->i1;
else
r_type = t->use_type;
}

LLVMTypeRef r_type = ffi_return_type(c, t, intrinsic);
LLVMTypeRef f_type = LLVMFunctionType(r_type, f_params, count, false);
LLVMValueRef func = LLVMAddFunction(c->module, f_name, f_type);

if(!err)
{
#if PONY_LLVM >= 309
LLVM_DECLARE_ATTRIBUTEREF(nounwind_attr, nounwind, 0);

LLVMAddAttributeAtIndex(func, LLVMAttributeFunctionIndex, nounwind_attr);
#else
LLVMAddFunctionAttr(func, LLVMNoUnwindAttribute);
#endif
}

ponyint_pool_free_size(buf_size, f_params);
return func;
}
Expand Down Expand Up @@ -1064,13 +1046,13 @@ LLVMValueRef gen_ffi(compile_t* c, ast_t* ast)
// Define using the declared types.
AST_GET_CHILDREN(decl, decl_id, decl_ret, decl_params, decl_err);
err = (ast_id(decl_err) == TK_QUESTION);
func = declare_ffi(c, f_name, t, decl_params, err, false);
} else if(!strncmp(f_name, "llvm.", 5)) {
func = declare_ffi(c, f_name, t, decl_params, false);
} else if(!strncmp(f_name, "llvm.", 5) || !strncmp(f_name, "internal.", 9)) {
// Intrinsic, so use the exact types we supply.
func = declare_ffi(c, f_name, t, args, err, true);
func = declare_ffi(c, f_name, t, args, true);
} else {
// Make it varargs.
func = declare_ffi_vararg(c, f_name, t, err);
func = declare_ffi_vararg(c, f_name, t);
}
}

Expand Down
47 changes: 43 additions & 4 deletions src/libponyc/expr/ffi.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "../type/subtype.h"
#include "../pkg/ifdef.h"
#include "ponyassert.h"
#include <string.h>

static bool void_star_param(ast_t* param_type, ast_t* arg_type)
{
Expand Down Expand Up @@ -54,6 +55,12 @@ static bool declared_ffi(pass_opt_t* opt, ast_t* call, ast_t* decl)

ast_t* a_type = ast_type(arg);

if(ast_id(a_type) == TK_TUPLETYPE)
{
ast_error(opt->check.errors, arg, "cannot pass tuples as FFI arguments");
return false;
}

errorframe_t info = NULL;
if((a_type != NULL) &&
!void_star_param(p_type, a_type) &&
Expand Down Expand Up @@ -98,6 +105,17 @@ static bool declared_ffi(pass_opt_t* opt, ast_t* call, ast_t* decl)
ast_t* call_ret_type = ast_child(call_ret_typeargs);
ast_t* decl_ret_type = ast_child(decl_ret_typeargs);

const char* f_name = ast_name(decl_name) + 1;
bool intrinsic = !strncmp(f_name, "llvm.", 5) ||
!strncmp(f_name, "internal.", 9);

if(!intrinsic && (ast_id(decl_ret_type) == TK_TUPLETYPE))
{
ast_error(opt->check.errors, decl_ret_type, "an FFI function cannot return "
"a tuple");
return false;
}

errorframe_t info = NULL;
if((call_ret_type != NULL) &&
!is_eqtype(call_ret_type, decl_ret_type, &info, opt))
Expand Down Expand Up @@ -134,11 +152,21 @@ bool expr_ffi(pass_opt_t* opt, ast_t* ast)
{
ast_t* a_type = ast_type(arg);

if((a_type != NULL) && is_type_literal(a_type))
if(a_type != NULL)
{
ast_error(opt->check.errors, arg,
"Cannot pass number literals as unchecked FFI arguments");
return false;
if(is_type_literal(a_type))
{
ast_error(opt->check.errors, arg,
"Cannot pass number literals as unchecked FFI arguments");
return false;
}

if(ast_id(a_type) == TK_TUPLETYPE)
{
ast_error(opt->check.errors, arg, "cannot pass tuples as FFI "
"arguments");
return false;
}
}
}

Expand All @@ -151,6 +179,17 @@ bool expr_ffi(pass_opt_t* opt, ast_t* ast)
return false;
}

const char* f_name = ast_name(name) + 1;
bool intrinsic = !strncmp(f_name, "llvm.", 5) ||
!strncmp(f_name, "internal.", 9);

if(!intrinsic && (ast_id(return_type) == TK_TUPLETYPE))
{
ast_error(opt->check.errors, return_type, "an FFI function cannot return a "
"tuple");
return false;
}

ast_settype(ast, return_type);

return true;
Expand Down
44 changes: 44 additions & 0 deletions test/libponyc/badpony.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -703,3 +703,47 @@ TEST_F(BadPonyTest, AsFromUninferredReference)
"argument not a subtype of parameter",
"cannot infer type of b");
}

TEST_F(BadPonyTest, FFIDeclaredTupleArgument)
{
const char* src =
"use @foo[None](x: (U8, U8))\n"

"actor Main\n"
" new create(env: Env) =>\n"
" @foo((0, 0))";

TEST_ERRORS_1(src, "cannot pass tuples as FFI arguments");
}

TEST_F(BadPonyTest, FFIUndeclaredTupleArgument)
{
const char* src =
"actor Main\n"
" new create(env: Env) =>\n"
" @foo[None]((U8(0), U8(0)))";

TEST_ERRORS_1(src, "cannot pass tuples as FFI arguments");
}

TEST_F(BadPonyTest, FFIDeclaredTupleReturn)
{
const char* src =
"use @foo[(U8, U8)]()\n"

"actor Main\n"
" new create(env: Env) =>\n"
" @foo()";

TEST_ERRORS_1(src, "an FFI function cannot return a tuple");
}

TEST_F(BadPonyTest, FFIUndeclaredTupleReturn)
{
const char* src =
"actor Main\n"
" new create(env: Env) =>\n"
" @foo[(U8, U8)]()";

TEST_ERRORS_1(src, "an FFI function cannot return a tuple");
}

0 comments on commit c658d1a

Please sign in to comment.