CompiledName allows invalid input, leading to PeVerify to fail
- Dominant language
- F#
- Stars
- 4.3k
- Forks
- 876
- Avg merge
- 4d 11h
- Merged PRs (30d)
- 131
Description
Using `CompiledName` can lead to invalid IL that fails PeVerify. A name in IL is basically a pointer to an item in the #Strings table and any Unicode character is allowed, however, there three limitations that IL poses on this:
* The string it points to may not be empty
* The string it points to may not exceed 1023 characters for MethodName and the like
* The length of the string plus the length of the complete namespace may not exceed 1023 characters. This is true for top-level types, nested types do not have this restriction (but are bound to a fixed limit of 1023).
Also, names must be unique, but this is enforced by the 2nd compile stage. Though the error does not remember the actual position in the source file, it is quite clear:
> FSC: error FS2014: A problem occurred writing the binary 'D:\Tests\SomeTests.dll': Error in pass2 for type SomeTests.Tests compiler, error: Error in pass2 for type other name, error: duplicate entry 'F' in method table
Few other observations:
* The `CompilationSourceName` is not set on nested types (under a module) nor on top-level types. It feels like it should, as it does so for any other type of identifier.
* Setting `CompiledName` on a module has no effect. This is probably by design, but could be mentioned in the docs and/or a warning could be given.
**Repro steps**
Example:
```f#
[]
type MyType =
[]
static member Foo() = 2 + 3
[]
static member Bar() = 2 + 5
```
**Expected behavior**
I'd expect at some stage an error for the empty name and perhaps also for the name that exceeds the combined length (though considering this is very rare in practice, it leads to all kinds of errors with tooling, for instance, Mono.Cecil totally crashes with an NRE and test runners cannot find the tests).
If an error is too hard, a warning would help. Or at the very least, we can update the documentation on `CompiledName`. But I would reckon that we shouldn't allow PeVerify-invalid assemblies, so my preference is to catch this in the 2nd stage as well.
**Actual behavior**
The code compiles fine, but shouldn't and PeVerify gives errors on the resulting IL assembly:
```text
Microsoft (R) .NET Framework PE Verifier. Version 4.0.30319.0
Copyright (c) Microsoft Corporation. All rights reserved.
[MD]: Error: TypeDef has no name. [token:0x02000057]
[MD]: Error: TypeDef is marked Nested but has no enclosing type. [token:0x02000057]
[MD]: Error: Method has no name. [token:0x0600016D]
[MD]: Error: Full name length exceeds maximum allowed (length: 1111; max: 1023). [token:0x0600016E]
4 Error(s) Verifying SomeTests.dll
```
Note that the error "TypeDef is marked Nested but has no enclosing type." follows from a nested type not having a name.
**Known workarounds**
Just be careful with `CompiledNameAttribute`. I happened upon this by accident when I copied an empty attribute around to fill in later, which I then forgot, and suddenly a lot of tests disappeared (I use it sometimes with test names when other options are not practical).
Contributor guide
Assessment
This issue has not been assessed yet.