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

Extension of IL verification rules to address ref returns #17

Merged
merged 2 commits into from
Jan 22, 2018

Conversation

VSadov
Copy link
Member

@VSadov VSadov commented Oct 26, 2017

A proposal for extension of IL verification rules that would allow byref returns in verifiable code and validate that they are used in type-safe manner.


Motivation -

There seems to be enough interest in making typesafe code emitted by current compilers formally verifiable, so we need a central location where we discuss and agree on the rules.

The other areas that would need to be addressed are: readonly references (type-safe code may not be verifiable according to current rules) and ref-structs (opposite: current rules are insufficient to detect type-safety violations).

However, we need to start with something, so let’s start with ref returns.

Immediately actionable pieces:

  • Who should be on board when we take decision on this?
  • Do we agree on the overall proposal?

…w byref returns and validate that they are used in type-safe manner.
@VSadov VSadov changed the title Added proposal for extension of IL verification rules addressing ref returns. A proposal for extension of IL verification rules addressing ref returns. Oct 26, 2017
@terrajobst terrajobst changed the title A proposal for extension of IL verification rules addressing ref returns. Extension of IL verification rules to address ref returns Oct 26, 2017
@dotnet dotnet deleted a comment from dnfclas Nov 1, 2017

## Metadata encoding of returnable local slots ##

Local slots can be marked as returnable by applying `modopt[System.Runtime.CompilerServices.IsReturnableAttribute]` in the local signature.
Copy link
Member

Choose a reason for hiding this comment

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

Is there a particular reason to use an Attribute suffix here? This will serve similar purpose as System.Runtime.CompilerServices.IsVolatile or IsConst. Those are not attributes either.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, no reason to be an attribute.

In the parallel case with readonly references, it seems appropriate to use IsReadOnlyAttribute since that is used in other places to express readonliness.
I could not think of a similar existing type for "returnable", so I suggest IsReturnableAttribute just for similarity, but you are right - it does not need to be an attribute or even a new type.

Any alternative ideas here would be appreciated, but we need to pick some type.

- `LDFLDA` when the receiver is a returnable ref (see: C# rule #4)
- `LDARG` of a byref parameter (see: C# rule #2, #3),
except for arg0 in a struct method (see: C# rule #5).
- `CALL`, `CALLVIRT` when all reference arguments are returnable. (see: C# rule 6)
Copy link
Member

Choose a reason for hiding this comment

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

Should this also apply to CALLI?

Copy link
Member Author

Choose a reason for hiding this comment

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

The rule could include CALLI for completeness. However CALLI cannot be present in verifiable code so generalization would not make much difference.

Copy link
Member

Choose a reason for hiding this comment

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

However CALLI cannot be present in verifiable code

Why not? ECMA-335 defines verification rules for when CALLI is verifiable. It's not like JMP (whose verification rules are simply "The jmp instruction is never verifiable").

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it? Perhaps I think of it as unverifiable because it invariably involves pointers.
Anyways, no problem with including it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think, I should mention varargs/arglist case as well.

C# compiler does consider them for the purpose of "any ref argument can potentially come back as a ref return"

@VSadov
Copy link
Member Author

VSadov commented Jan 22, 2018

Merging this, since this has been opened for a long time, and informally approved by interested parties.

NOTE: The proposal does not have direct impact on the language, but has effect on the compiler.
It codifies the metadata shape which has direct impact on interop/verification scenarios.
(that is why it fits in csharplang repo)

@VSadov VSadov merged commit 2099ba7 into dotnet:master Jan 22, 2018
@MichalStrehovsky
Copy link
Member

@VSadov Based on your own comments "I think, I should mention varargs/arglist case as well.", the question around verification of CALLI, and not giving this Attribute suffix (that can lead someone to believe they can apply this as an attribute on somethings), I thought this is not fully fleshed out yet...

@VSadov
Copy link
Member Author

VSadov commented Jan 24, 2018

@MichalStrehovsky - will make the update regarding CALLI, I thought I did...

As for the type to mark returnable local slots - any good ideas?

Ideally that would be a preexisting type that exists everywhere - like IsConst or even System.Object, but there is a danger that it is already used by somebody to modopt locals.

@MichalStrehovsky
Copy link
Member

As for the type to mark returnable local slots - any good ideas?

I think we need a new type (IsConst is only available starting NetStandard 1.3 and I can't think of anything existing that wouldn't be confusing). I would be fine with System.Runtime.CompilerServices.IsReturnable. If we would like to eventually make it part of the framework so that the compiler doesn't need to always emit it, we might want to run it through a CoreFX API review. (This is the part where making this a custom attribute would be weird because users might think they can apply it to things and something will happen as a result.)

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

Successfully merging this pull request may close these issues.

3 participants