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

[CallerMustBeUnsafe] type attribute #8663

Closed
benaadams opened this issue Feb 13, 2016 · 10 comments
Closed

[CallerMustBeUnsafe] type attribute #8663

benaadams opened this issue Feb 13, 2016 · 10 comments

Comments

@benaadams
Copy link
Member

From https://github.com/dotnet/coreclr/issues/3143

To mark a function to be only be able to be called in an unsafe block.

It came up as an issue with having an IntPtr based api for Vector.Copy be equally could apply to something like a .ctor where you are passing an internal buffer to use (e.g. https://github.com/dotnet/coreclr/issues/3142)

Or risk of use of buffers with overlapped I/O tasks and dispose dotnet/corefx#5954 (comment)

To indicate that the caller is aware there are risks and to be careful. What I am suggesting is something where .ctor 2 is forced to be unsafe in the same way .ctor 3 is:

public BufferedThing(int bufferSize){}

[CallerMustBeUnsafe]
public BufferedThing(byte[] internalBuffer){}

public BufferedThing(byte* internalBuffer, int bufferLength){}

e.g.

var buffer0 = new BufferedThing(10); // fine

var buffer1 = new BufferedThing(new byte[10]); // compile error

unsafe {
    var buffer2 = new BufferedThing(new byte[10]); // fine
}

unsafe {
    var buffer = new byte[10];
    fixed (byte* pBuffer = &buffer[0]) {
        var buffer3 = new BufferedThing(pBuffer, 10); // fine
    }
}
@morganbr
Copy link

I like this for a couple reasons:

  1. Compared to SecurityCriticalAttribute, it's a compile-time check with no runtime overhead -- this is especially important for runtimes like .NET Native and CoreRT that don't implement transparent/critical code.
  2. It makes it clear that even if you're not directly using pointers, what you're doing isn't safe.
  3. By default, all code is critical, so calling critical methods doesn't necessary attract special attention.

A real-world example of this would be methods on the Marshal class such as Copy and StructureToPtr.

@benaadams
Copy link
Member Author

Also for some of the Span<T> types

@miloush
Copy link

miloush commented Feb 23, 2016

I am not sure I understand this. You already have to be called from unsafe context to do unsafe things, which manipulating IntPtr is not from the point of view of CLR. That's the whole point of unsafe vs IntPtr.

What can you do that is not safe without unsafe? Looks like your "safe" is different from CLR's "safe" and you want to use the latter to enforce the former...

@benaadams
Copy link
Member Author

What can you do that is not safe without unsafe?

Operating over a window of larger data; so have buffer of 1000 bytes, return an ArraySegmentto say your data starts at offset 10 and is 20 bytes long; but you can easily read both before it and after it - so in a way it is unsafe for the caller if they don't respect that.

@miloush
Copy link

miloush commented Feb 23, 2016

Yes, but that is not CLR unsafe. If you want to behave it that way, what prevents you declaring the constructor as unsafe?

@benaadams
Copy link
Member Author

An unsafe method can be happily called by safe code; this is an attribute to say the methods/class can only be used in an unsafe block - i.e. to propagate the constraint to the caller.

@miloush
Copy link

miloush commented Feb 23, 2016

Well as happily as any other unsafe code, i.e. you still need to be in full trust, otherwise you get VerificationException. What do you expect the benefits would be?

@benaadams
Copy link
Member Author

Full trust doesn't exist in coreclr or native as far as I'm aware. Also as a "user beware" do you accept the risks marker.

I have no objection to any code calling these psuedo-"unsafe" methods; unlike the the Full trust system - its more to make the caller aware there are risks.

@benaadams
Copy link
Member Author

Also full trust vs not full trust is quite broad brush; rather than at specific method level (like unsafe)

@srivatsn
Copy link
Contributor

Moved to the roslyn-analyzers repo - dotnet/roslyn-analyzers#972

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants