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.

Sign up for GitHub

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

Jump to bottom

Disallow Unsafe Declarations #229

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Disallow Unsafe Declarations #229

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fdd94f3
Add Dip, Disallow Unsafe Declarations
skyline131313 Jun 15, 2022
45aaecf
Add name and contact info.
skyline131313 Jun 19, 2022
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
55 changes: 55 additions & 0 deletions DIPs/DIP1045.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# Disallow Unsafe Declarations

| Field | Value |
|-----------------|-----------------------------------------------------------------|
| DIP: | |
| Review Count: | |
| Author: | [Mark]([email protected]) |
| Implementation: | |
| Status: | Draft |

## Abstract

Disallow extern C and C++ function declarations from being marked `@safe` or `@trusted`.

## Contents
* [Rationale](#rationale)
* [Prior Work](#prior-work)
* [Description](#description)
* [Breaking Changes and Deprecations](#breaking-changes-and-deprecations)
* [Reference](#reference)
* [Copyright & License](#copyright--license)
* [Reviews](#reviews)

## Rationale

Foreign code that is interfaced cannot be guaranteed to be safe. It should always be assumed as unsafe. Especially C declarations as the types used are not mangled into the name of the function. It would be possible to link to a C function with the incorrect parameter types.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all extern C/C++ functions are foreign code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless there is a body it is foreign code. If you use C or C++ extern and access the function through a declaration instead of importing it, I don't think that should be a valid use case to consider. You can just import it instead or mark it as a D function and have a different wrapper for C/C++ if you need that.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not true:

// foo.di
extern(C) foo(int i);

// foo.d
extern(C) foo(int i) {
    // look ma, I have a body here
    return i * 2;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should always be assumed as unsafe.

That's the thesis of the DIP, the rationale section should explain in detail why that assumption is useful.

It would be possible to link to a C function with the incorrect parameter types.

You can also link a D function with different signature by giving a false mangle.

Once the OS / build system / foreign code is compromised, there's nothing the programming language can do, so I would say it's best to assume those behave well, because if they don't, all bets are off.

See also: https://forum.dlang.org/post/[email protected]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the OS / build system / foreign code is compromised, there's nothing the programming language can do, so I would say it's best to assume those behave well, because if they don't, all bets are off.

I’d use the last part to come to the very opposite conclusion. @safe means that the compiler checked it up to @trusted calls. Ideally, this means functions with pragma(mangle,…) should not be allowed to be marked @safe because it simply may not be. If your implementation is @safe, make a shallow wrapper like this:

pragma(mangle, "body")
extern(C) void body_func() @trusted => safe_body();
void safe_body() @safe { … }

It is a little lengthy, but it’s the shortest way which is honest.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to think a function definition should always be allowed to be @safe. Only a declaration without a body should be limited to @trusted or @system, if it has non-D ABI or custom mangling. Binary mangling of @trusted and @safe should be merged so they would link to each other even in D mangling.

It still does mean that a false @safe D mangling could be given at the definition site without using @trusted, but that's a small enough hole that i think it's better than requiring the safe_body workaround. If we require the workaround, people are likely to just implement body_func with a @trusted body.


## Prior Work

Rust declares all foreign interfaces as unsafe, they cannot be explicitly marked as safe as there is only an `unsafe` keyword. [Rust avoids the issue of marking `unsafe` foreign declarations as `safe` entirely by design](https://doc.rust-lang.org/nomicon/ffi.html).

## Description

Any C or C++ function declaration with the `@safe` or `@trusted` attributes will now become an error.

```D
@safe extern(C) foo(); // Error: extern(C) declaration cannot be marked as `@safe`.
@trusted extern(C++) bar(); // Error: extern(C++) declaration cannot be marked as `@trusted`.
```

## Breaking Changes and Deprecations

Any extern C and C++ declarations that are marked with `@trusted` or `@safe` will cause breaking changes.

## Reference


## Copyright & License

Copyright (c) 2022 by the D Language Foundation

Licensed under [Creative Commons Zero 1.0](https://creativecommons.org/publicdomain/zero/1.0/legalcode.txt)

## Reviews