Runtime: Analyzer for MarshalAs usage

Created on 10 Apr 2020  路  6Comments  路  Source: dotnet/runtime

Defining how types should be marshaled via a P/Invoke or COM interop is difficult to get correct and many bad practices have been developed over the years. The Native interoperability best practices page should be converted into an analyzer to help developers do the right thing.

Related to https://github.com/dotnet/runtime/issues/33808

Additional thoughts:

  • If a value type is in a P/Invoke definition analyze it proactively for ideal layout and suggested optimizations. See https://github.com/dotnet/runtime/issues/33620 involving COM-RPC.
  • Types marked with ComImport should be scrutinized.

/cc @terrajobst @jkoritzinsky @elinor-fung

api-suggestion area-Interop-coreclr code-analyzer

Most helpful comment

There is an existing P/Invoke analyzer, which could be a fitting home for some of this.

Some marshalling-related items that could be reasonable rules from the guidance doc:

  • Specify CharSet on DllImport when strings/characters are present

    • CA2101 seems to partly cover this (does string and StringBuilder, not char), although it is under Globalization(?) and the doc talks about security / partial trust

  • Prefer ExactSpelling=true on DllImport (Performance)
  • Do not use [Out] string (Interoperability)
  • Avoid StringBuilder parameters (Performance)
  • Specify marshalling for booleans (Interoperability)

    • CA1414 exists, but is disabled for Core (also only for p/invokes, so won't hit fields on types used in p/invokes)

  • Specify SizeConst when marshalling field as ByValArray (Interoperability)
  • Do not use Delegate or MulticastDelegate fields in marshalled structs (Interoperability).

All 6 comments

There may be more in
https://github.com/dotnet/runtime/blob/master/docs/coding-guidelines/interop-guidelines.md
https://github.com/dotnet/runtime/blob/master/docs/coding-guidelines/interop-pinvokes.md (which I'll merge into the former)

Of course, anything unique in those would be candidates for adding to the official doc as well.

There is an existing P/Invoke analyzer, which could be a fitting home for some of this.

Some marshalling-related items that could be reasonable rules from the guidance doc:

  • Specify CharSet on DllImport when strings/characters are present

    • CA2101 seems to partly cover this (does string and StringBuilder, not char), although it is under Globalization(?) and the doc talks about security / partial trust

  • Prefer ExactSpelling=true on DllImport (Performance)
  • Do not use [Out] string (Interoperability)
  • Avoid StringBuilder parameters (Performance)
  • Specify marshalling for booleans (Interoperability)

    • CA1414 exists, but is disabled for Core (also only for p/invokes, so won't hit fields on types used in p/invokes)

  • Specify SizeConst when marshalling field as ByValArray (Interoperability)
  • Do not use Delegate or MulticastDelegate fields in marshalled structs (Interoperability).

@elinor-fung Here is another issue that should get a rule: https://github.com/dotnet/runtime/issues/9944

Some initial thoughts:

  • Having _both_ [ComVisible] and [ComImport] on an interface should be incorrect, right?
  • For COM interfaces we should consider always requiring [MarshalAs] for string and object as they are often _not_ BSTR and VARIANT

@elinor-fung Is this aggregated issue being handled by the various other issues being filed or should we keep this one open?

Closing this and using #37039 as the tracking issue that will point to the separate issues we file for each rule.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

yahorsi picture yahorsi  路  3Comments

omariom picture omariom  路  3Comments

GitAntoinee picture GitAntoinee  路  3Comments

jamesqo picture jamesqo  路  3Comments

omajid picture omajid  路  3Comments