Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't allow FFI calls in default methods or behaviors #4065

Merged
merged 1 commit into from
Mar 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions .release-notes/fix_3997.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
## Don't allow FFI calls in default methods or behaviors

In Pony, when you `use` an external library, you import the public types defined by that library. An exception to this rule is FFI declarations: any FFI declarations in a library will not be visible by external users. Given that the compiler only allows a single FFI declaration per package for any given function, if external FFI declarations were imported, deciding which declaration to use would be ambiguous.

When added to the fact that every FFI function must have a matching declaration, a consequence of not importing external declarations is that if a library contains a public interface or trait with default methods (or behaviors), performing an FFI call on those methods will render them unusable to external consumers.

Consider the following code:

```pony
// src/lib/lib.pony
use @printf[I32](fmt: Pointer[None] tag, ...)

trait Foo
fun apply() =>
@printf("Hello from trait Foo\n".cstring())

// src/main.pony
use "./lib"

actor Main is Foo
new create(env: Env) =>
this.apply()
```

Up until ponyc 0.49.1, the above failed to compile, because `main.pony` never defined an FFI declaration for `printf`. One way of making the above code compile is by moving the call to `@printf` to a separate type:

```pony
// src/lib/lib.pony
use @printf[I32](fmt: Pointer[None] tag, ...)

trait Foo
fun apply() =>
Printf("Hello from trait Foo\n")

primitive Printf
fun apply(str: String) =>
@printf(str.cstring())

// src/main.pony
use "./lib"

actor Main is Foo
new create(env: Env) =>
this.apply()
```

Given that the original error is not immediately obvious, starting from this release, it is now a compile error to call FFI functions in default methods (or behaviors). However, this means that code that used to be legal will now fail with a compiler error. For example:

```pony
use @printf[I32](fmt: Pointer[None] tag, ...)

actor Main is _PrivateFoo
new create(env: Env) =>
this.apply()

trait _PrivateFoo
fun apply() =>
// Error: can't call an FFI function in a default method or behavior.
@printf("Hello from private trait _PrivateFoo\n".cstring())
```

As mentioned above, to fix the code above, you will need to move any FFI calls to external function:

```pony
use @printf[I32](fmt: Pointer[None] tag, ...)

actor Main is _PrivateFoo
new create(env: Env) =>
this.apply()

trait _PrivateFoo
fun apply() =>
// OK
Printf("Hello from private trait _PrivateFoo\n")

primitive Printf
fun apply(str: String) =>
@printf(str.cstring())
```

We believe that the impact of this change should be small, given the usability problems outlined above.
10 changes: 9 additions & 1 deletion packages/files/_test.pony
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ primitive \nodoc\ _FileHelper
end
top.path

primitive \nodoc\ _GetUID
fun apply(): U32 =>
ifdef not windows then
@getuid()
else
compile_error "getuid not available on windows"
end

trait \nodoc\ iso _NonRootTest is UnitTest

fun apply_as_non_root(h: TestHelper) ?
Expand All @@ -102,7 +110,7 @@ trait \nodoc\ iso _NonRootTest is UnitTest
true
else
ifdef not windows then
@getuid() == 0
_GetUID() == 0
else
false
end
Expand Down
48 changes: 46 additions & 2 deletions src/libponyc/pass/syntax.c
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,50 @@ static ast_result_t syntax_ffi(pass_opt_t* opt, ast_t* ast,
}


static ast_result_t syntax_ffi_decl(pass_opt_t* opt, ast_t* ast)
{
pony_assert(ast_id(ast) == TK_FFIDECL);
return syntax_ffi(opt, ast, true);
}

static ast_result_t syntax_ffi_call(pass_opt_t* opt, ast_t* ast)
{
pony_assert(ast_id(ast) == TK_FFICALL);
ast_result_t r = AST_OK;

if (ast_id(opt->check.frame->method) == TK_BE ||
ast_id(opt->check.frame->method )== TK_FUN)
{
ast_t* parent = ast_parent(ast);
switch(ast_id(opt->check.frame->method))
{
case TK_BE:
case TK_FUN:
while(parent != NULL)
{
switch(ast_id(parent)) {
case TK_INTERFACE:
case TK_TRAIT:
ast_error(opt->check.errors, ast,
"Can't call an FFI function in a default method or behavior");
r = AST_ERROR;
break;
default: {}
}

parent = ast_parent(parent);
}
default: {}
}
}

if(syntax_ffi(opt, ast, false) == AST_ERROR)
r = AST_ERROR;

return r;
}


static ast_result_t syntax_ellipsis(pass_opt_t* opt, ast_t* ast)
{
pony_assert(ast != NULL);
Expand Down Expand Up @@ -1417,8 +1461,8 @@ ast_result_t pass_syntax(ast_t** astp, pass_opt_t* options)
case TK_TUPLETYPE: r = syntax_tupletype(options, ast); break;
case TK_NOMINAL: r = syntax_nominal(options, ast); break;
case TK_MATCH: r = syntax_match(options, ast); break;
case TK_FFIDECL: r = syntax_ffi(options, ast, true); break;
case TK_FFICALL: r = syntax_ffi(options, ast, false); break;
case TK_FFIDECL: r = syntax_ffi_decl(options, ast); break;
case TK_FFICALL: r = syntax_ffi_call(options, ast); break;
case TK_ELLIPSIS: r = syntax_ellipsis(options, ast); break;
case TK_CONSUME: r = syntax_consume(options, ast); break;
case TK_RETURN:
Expand Down
68 changes: 68 additions & 0 deletions test/libponyc/badpony.cc
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,74 @@ TEST_F(BadPonyTest, FFIDeclaredTupleReturn)
TEST_ERRORS_1(src, "an FFI function cannot return a tuple");
}

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

"interface Foo\n"
" fun apply() =>\n"
" @foo()\n"

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

TEST_ERRORS_1(src,
"Can't call an FFI function in a default method or behavior");
}

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

"interface Foo\n"
" be apply() =>\n"
" @foo()\n"

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

TEST_ERRORS_1(src,
"Can't call an FFI function in a default method or behavior");
}

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

"trait Foo\n"
" fun apply() =>\n"
" @foo()\n"

"actor Main is Foo\n"
" new create(env: Env) =>\n"
" None";

TEST_ERRORS_1(src,
"Can't call an FFI function in a default method or behavior");
}

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

"trait Foo\n"
" be apply() =>\n"
" @foo()\n"

"actor Main is Foo\n"
" new create(env: Env) =>\n"
" None";

TEST_ERRORS_1(src,
"Can't call an FFI function in a default method or behavior");
}

TEST_F(BadPonyTest, FFIDeclaredTupleReturnAtCallSite)
{
const char* src =
Expand Down