CA2101 false positive

[AArnott]

Analyzer

Diagnostic ID: CA2101: Specify marshaling for P/Invoke string arguments

Analyzer source

NuGet Package: Microsoft.CodeAnalysis.NetAnalyzers

Version: 5.0.3

Describe the bug

This code produces a CA2101 diagnostic.

        [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Ansi)]
        private static extern IntPtr GetProcAddress(IntPtr hModule, string lpProcName);

The code fix offered changes Ansi to Unicode, although this method is only exposed as ANSI, so the code fix would produce faulty code, and the analyzer should not tell me to specify the string marshaling when I already have.

[NN---]

Also in .NET 6

csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>

    <AnalysisLevel>latest</AnalysisLevel>
    <RunAnalyzers>True</RunAnalyzers>
    <EnableNETAnalyzers>True</EnableNETAnalyzers>
    <AnalysisMode>AllEnabledByDefault</AnalysisMode>
  </PropertyGroup>

</Project>

Program.cs

using System;
using System.Runtime.InteropServices;

[assembly: CLSCompliant(false)]

namespace A;

internal static class NativeMethods
{
    [DllImport("a", ExactSpelling = true, CallingConvention = CallingConvention.Cdecl, CharSet = CharSet.Ansi)]
    public static extern int f(string a);
}

public static class P { public static void Main() {} }

dotnet build result:

Microsoft (R) Build Engine version 17.0.0-preview-21501-01+bbcce1dff for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  All projects are up-to-date for restore.
  You are using a preview version of .NET. See: https://aka.ms/dotnet-core-preview
warning CA2101: Specify marshaling for P/Invoke string arguments

[NN---]

2886

5009

[dss539]

https://github.com/dotnet/roslyn-analyzers/issues/2886#issuecomment-1079426722

[Youssef1313]

This comment https://github.com/dotnet/roslyn-analyzers/issues/2886#issuecomment-1079426722 suggests that this is not a false positive, but the codefix is incorrect. The suggested code fix in that comment still produces a warning (which is a false positive if the comment is correct).

[AArnott]

I disagree, @Youssef1313. The message in the warning, and the docs for the diagnostic itself, indicate the problem happens when you set CharSet.Auto. I'm not doing that. Nowhere in the docs or message does it say that specifying Ansi is a problem. This is a false positive because per the message and the docs, the analyzer is telling me I have a problem that I don't have. The code fix is defective because it creates a problem.

Based on the comment you link to, it sounds like there is a new security problem that may warrant consideration. But that is outside the scope of this analyzer, at least as far as it is documented and the message it uses. If the analyzer was updated to notice these security issues, its message and docs should have been updated to match.

[hopperpl]

When is this issue being addressed? This is open for almost 1 year now.

Looking at the documentation, I have a strong feeling what went wrong.

https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.charset?view=net-6.0

None => This value is obsolete and has the same behavior as Ansi.

And then someone drew the conclusion, Ansi is the same as None. Which it is not! Ansi is a valid value. None and Auto are the only values the Roslyn analyzer can touch, any other value it must not touch or change.

[philipp-naused]

We have the same problem in .NET Framework 4.8 using Microsoft.CodeAnalysis.NetAnalyzers 6.0.0 [DllImport("example.dll", CharSet = CharSet.Ansi, BestFitMapping = false, ThrowOnUnmappableChar = true)] triggers CA2101

I think the analyzer was just implemented incorrectly. If I understand correctly the rule should only trigger if BestFitMapping is not set to false.

[philipp-naused]

I have checked the implementation of the rule: PInvokeDiagnosticAnalyzer.cs

Pseudocode:

if (DllImport.BestFitMapping != false) OR (MSBuildOptions.InvariantGlobalization != true)
    if (any parameter is String or StringBuilder) AND (DllImport.CharSet != Unicode)
        Report CA2101

The InvariantGlobalization seems to be the issue since this property is false by default, causing the analyzer to ignore our BestFitMapping settings.

I also found that none of the unit tests for CA2101 cover the BestFitMapping or InvariantGlobalization settings.

[mavasani]

@jkoritzinsky would you be able to take a look here? I think you have the most expertise in the analyzers in this space.

[dominoFire]

It would be grateful if this is fixed. Thanks for tracking this issue!

[yaakov-h]

@mavasani @jkoritzinsky I got fed up enough with this issue to file a PR yesterday, and only later came across the comments above that came to the same conclusion that I did. I've opened #6192 if you would like to use it as a starting point.

[jkoritzinsky]

@yaakov-h is this issue entirely fixed by your PR?

[yaakov-h]

@jkoritzinsky The code in the OP will still trigger it, but adding BestFitMapping = false will silence the diagnostic.

I tested this just now on Microsoft.CodeAnalysis.NetAnalyzers 7.0.0-preview1.22513.1.

One could call that "entirely fixed", but I'm tempted to try add BestFitMapping = false as an alternative code fix for CA2101, particularly when CharSet = Ansi is already explicitly specified.

[Cyberboss]

Triggered on my .net6.0 project, but only on CI builds for some reason, not in VS.

[DllImport("libsystemd.so.0", CharSet = CharSet.Ansi, BestFitMapping = false, ThrowOnUnmappableChar = true)]
public static extern int sd_notify(int unset_environment, [MarshalAs(UnmanagedType.LPUTF8Str)] string state);

PS S:\workspace\tgstation-server\src\Tgstation.Server.Host> dotnet --version
7.0.304
PS S:\workspace\tgstation-server\src\Tgstation.Server.Host> dotnet --list-sdks
3.1.100 [C:\Program Files\dotnet\sdk]
5.0.214 [C:\Program Files\dotnet\sdk]
6.0.118 [C:\Program Files\dotnet\sdk]
7.0.304 [C:\Program Files\dotnet\sdk]
PS S:\workspace\tgstation-server\src\Tgstation.Server.Host>