[release/8.0-staging] [LibraryImportGenerator] Use basic forwarder in down-level support if any parameters can't be marshalled

[elinor-fung]

Backport of #104416 to release/8.0-staging

cc @dotnet/ncl for WinHttpHandler

Customer Impact

• Customer reported
• Found internally

Libraries built out of dotnet/runtime using LibraryImport targeting .NET 6 can end up with some parameters that are marshalled by generated source and some that are simply forwarded to a DllImport, such that the generated DllImport signature is still not blittable. This can result in some parameters being incorrectly marshalled, as the generated DllImport does not have all the metadata/attributes needed for those parameters. In the WinHttpHandler package, this hit an issue where StringBuilder was no longer marshalled as expected and using the function fails at runtime.

This fix makes it so that if any parameter cannot be marshalled by the source generator, it simply forwards everything to a DllImport. It eliminates there is no scenario where a stub is generated that marshals some parameters and forwards the rest (without erroring).

Regression

• Yes
• No

This is a regression when using the LibraryImport source generator to target .NET 6 (down-level support that only exists for the dotnet/runtime repo) building out of the .NET 8 branch.

Testing

Manual verification. Automated tests added.

Risk

Low. This is down-level support that exists for dotnet/runtime only. While it could affect anything using the LibraryImport targeting .NET 6, the switch to just forwarding to DllImport (instead of generating something that only marshals some parameters) is low risk.

Other

• Updated ServicingVersion for System.Net.Http.WinHttpHandlers

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

[elinor-fung]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[agocke]

LGTM thanks!

[agocke]

Is there any additional/different testing that we want to do in the 8.0 branch, since my recollection is that is where the downlevel targeting is supported? Or are the current tests sufficient?

[rokonec]

I have discussed it with NCL team and changes in WinHttpHandler.csproj are correct

[jeffschwMSFT]

lgtm. we will take for consideration in 8.0.x

[elinor-fung]

Is there any additional/different testing that we want to do in the 8.0 branch, since my recollection is that is where the downlevel targeting is supported? Or are the current tests sufficient?

For 8.0, the only additional testing I did was inspect the IL for the library built for 6.0 to validate it is simply a DllImport now for p/invokes where we could only marshal some parameters and that p/invokes where we could marshal all parameters were not affected (that is, we still generate the stub to handle the marshalling). For automated tests, I think what is added in this change is sufficient (the down-level support from the perspective of LibraryImport generator itself is the same in main - it is just that the repo doesn't build libraries that use it for 6.0 anymore)

[carlossanlop]

Friendly reminder that Monday July 15th is Code Complete day, that's the deadline to get this included in the August Release.