Prefer ExactSpelling=true on [DllImport] for known APIs

[elinor-fung]

On Windows, setting ExactSpelling=true on the DllImport attribute prevents the runtime from looking for alternate function names (suffix based on CharSet), giving a slight performance benefit by avoiding this search.

There is a fair amount of nuance associated with the behaviour of ExactSpelling and under which scenarios there would actually be a performance benefit. In order to avoid noise, this rule can be scoped to check for a database of Win32 APIs that use the suffixes and provide a warning and fix for those cases where it matters.

// Flag ExactSpelling=false
[DllImport("kernel32.dll", CharSet=CharSet.Unicode)]
private static extern bool SetEnvironmentVariableW(string name, string? value);

// OK - not in database of APIs
[DllImport("MyLibrary.dll", CharSet=CharSet.Unicode)]
private static extern void Foo();

Recommend:

[DllImport("kernel32.dll", CharSet=CharSet.Unicode, ExactSpelling = true)]
private static extern bool SetEnvironmentVariableW(string name, string? value);

Category: Performance
Default: Enabled

cc @terrajobst @stephentoub @AaronRobinsonMSFT @jkoritzinsky

[stephentoub]

Is there any concern here that someone following this guidance will introduce bugs, e.g. because they were actually relying on the behavior? Can an auto-fixer do more than just add "ExactSpelling = true", e.g. would it be able to update the function name appropriately in case adding ExactSpelling would be introducing a bug?

I expect we have a bunch of places where we don't follow this guidance in dotnet/runtime. What does a fix look like there, and how much overhead should we expect to see removed? If it's meaningful enough to add an analyzer for, it would more so seem to be something we should fix in dotnet/runtime for .NET 5.

[AaronRobinsonMSFT]

Is there any concern here that someone following this guidance will introduce bugs, e.g. because they were actually relying on the behavior?

The biggest one is for Win32 APIs that use the A/W suffix. Detecting that might be possible, but the heuristic would be fragile. We could have an exhaustive Win32 API list where this applies. I haven't observed new Win32 APIs applying these suffixes anymore so it might actually be stable.

[EgorBo]

I believe there is an issue somewhere to turn off A/W probing for non-windows OSs so ExactSpelling=true will be just a noise for them.

[elinor-fung]

Good call about non-Windows - certainly would not want that noise.

Looking more at how we look for an entry point, it does have a bit more subtlety than I had realized. On Windows, if ExactSpelling=false and:

• CharSet=CharSet.Unicode, we always look for both the specified name and the name with the W suffix
  • Even if the specified name is found as an exact match to an entry point, we look for the W suffix. If found, the W suffix function is used.
  • It may not be clear to the user that they are actually relying on the suffix-appending behaviour, so ExactSpelling=true, would also require adding the W suffix,
• CharSet=CharSet.Ansi, we look for the specified name (no suffix) first and only try the suffix if it is not found
  • If the specified name is already the exact entry point name, setting ExactSpelling=true wouldn't make any difference.
  • Setting ExactSpelling=true would only matter if the user is actually relying on the suffix-appending behaviour - and it would require updating to add the A suffix.

Given the above, I'm a little less convinced that this should warrant a rule/fixer.

Rule or not, I'd agree we should do a pass over dotnet/runtime. Aside from places that just don't need the suffix behaviour, I also see some places where functions explicitly have the W suffix in the entry point name (e.g. FooW), but don't set ExactSpelling=true, so the runtime always looks for a duplicated suffix (e.g. FooWW)...

[jkotas]

Given the above, I'm a little less convinced that this should warrant a rule/fixer.

You build the database of OS APIs and do the rule/fixer for those only.

[terrajobst]

Video

Looks good as proposed.

[Mrnikbobjeff]

Which dll's would we want in that api? I generated a brief list with the dumpbin tool for user32 and kernel32-dll - are there any others which should be added for now?

[AaronRobinsonMSFT]

@Mrnikbobjeff advapi32.dll and perhaps kernelbase.dll too?

I think the easiest way to do this is leverage dumpbin /exports on all DLLs in System32 and SysWoW64 and find those similar APIs that end with both a A and W.

Cases:
XxxxW exists AND XxxxA exists, then add DLL and both exports to database.
XxxxW exists but XxxxA doesn't exist, ignore.
XxxxA exists but XxxxW doesn't exist, ignore.

[jkotas]

I would think we want to have all entry-points in the database. Otherwise, we cannot tell the difference between an API that does not have W suffix and API that is missing in the database (e.g. available in new OS version only).

[AaronRobinsonMSFT]

I would think we want to have all entry-points in the database.

As in, only have Xxxx and not XxxxA and XxxxW?

[jkotas]

I think the database should have the full snapshot of the exports, ie all Xxxx, XxxxA, XxxxW if they are present.

When we see "Xxxx" in the entrypoint, the ExactSpelling autofixer needs to decide whether to suggest:

• "XxxxW" + ExactSpelling=true. This should be suggested when we see "XxxxW" in the database.
• "Xxxx" + ExactSpelling=true. This should be suggested when we see "Xxxx" and no "XxxxW" in the database
• Leave the API alone. This should be the case when neither "Xxxx" nor "XxxxW" is in the database

[AaronRobinsonMSFT]

@jkotas I agree with all of this. I wasn't trying to dictate design merely indicating a possible collection of exports for how to think about what the analyzer needs to "know". I believe the logic you laid out is properly described in the issue.

@Mrnikbobjeff Please let us know if something is unclear with @jkotas's statement and with the issue details.

[Mrnikbobjeff]

@AaronRobinsonMSFT thank you for the details. The last thing unclear to me is where to store the resulting database. Keeping it in memory seems kind of wasteful as the symbol list contains approximately 10k entries for the four explicitly listed dll's alone. Is there infrastructure already in place for these kind of databases?

[AaronRobinsonMSFT]

@Mrnikbobjeff I think generating a series of constant data structures in C# will work just fine. The term "database" here wasn't to imply we have a real SQL-esque or SQLite database file somewhere. The solution could also be a simple text file that ships with the analyzer - choose the format (JSON, XML, etc).

[Mrnikbobjeff]

I thought about it a little. At first I wrote a little utility to do what @jkotas wanted to scan all dll files in System32 which resulted in 120k methods. I tried storing it in a Dictionary<string (dll), HashSet (methods)> dictionary generated by the new generator feature, but the compile time for the resulting file is abysmal, I stopped the process after it took thrice the time required to compile the entire solution on my machine without completing. I settled on initializing the Dictionary on first access, which performs okay. I also thought it would be wise to include the entrypoint parameter into the analysis if it is present. Perhaps this is also the place to edit the Entrypoint name in case we change it?
Edit: There are only 650 methods for which this applies, perhaps we could reconsider not adding all the clutter?

[jkotas]

scan all dll files in System32 which resulted in 120k methods.

There are many private APIs in System32 .dlls. I agree that having database of everything in System32 does not sound right.

There are only 650 methods for which this applies, perhaps we could reconsider not adding all the clutter?

Do you mean that there are only 650 methods with W or A suffixes? If we had just these methods in the database, the analyzer may recommend false positives if there are new Windows APIs with W or A suffixes introduced. Maybe that's ok.

[Mrnikbobjeff]

There are only 650 methods which have a similar method with the same name + "W" in the same assembly. There are many more instances where there are methods which have both a W and A suffix but no base method without the suffix. These are currently not counted and included

[riverar]

@Mrnikbobjeff Are you using https://github.com/microsoft/win32metadata as opposed to manually scanning PE files?

[Mrnikbobjeff]

@riverar I used a small utility exe I wrote to parse dumpbin output for all dll files in the System32 directory. I uploaded the gist but it really is just a hacked together script and nothing production ready :)

[riverar]

@Mrnikbobjeff Thanks, I think my concern when I originally asked was that we should be using win32metadata as the authoritative source and not any ad-hoc scans. Scanning System32 is not a great idea due to windows componentization, you'll definitely miss something.

[jkoritzinsky]

@Mrnikbobjeff how are you doing with this work? Do you need any assistance?

[carlossanlop]

@Mrnikbobjeff I'll unassign this issue and move it to 8.0. If you still want to work on it, let us know.

[jkotas]

This was superseded by the interop source generator and LibraryImportAttribute that sets ExactSpelling=true by default.

@AaronRobinsonMSFT @jkoritzinsky Should we close this?

[AaronRobinsonMSFT]

I think so. I'm of the opinion that for .NET 7+, any analyzers for DllImport, unless security sensitive, should be retargeted to LibraryImport.