[NativeAot] Triage another 14 libs tests

[MichalStrehovsky]

Cc @dotnet/ilc-contrib

[ghost]

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

Issue Details

---

null

Author:	MichalStrehovsky
Assignees:	-
Labels:	area-Meta
Milestone:	-

[MichalStrehovsky]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jkotas]

Suggested change

	[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBuiltWithAggressiveTrimming))]
	[Fact]
	[DynamicDependency(DynamicallyAccessedMemberTypes.NonPublicFields, typeof(Process))]

[MichalStrehovsky]

This doesn't work. error IL2037: System.Diagnostics.Tests.ProcessTests.RefreshResetsAllRefreshableFields(): No members were resolved for 'NonPublicFields'. on WASM legs because of WarnAsError and illink trying to be "helpful".

NativeAOT is also still crashing with a NullRef.

I don't think there's enough value in this test to spend this much time on it trying to make it work.

[jkotas]

NativeAOT is also still crashing with a NullRef.

Why is that? Is it a bug or expected?

I agree that the test itself is not super valuable for AOT. However, I believe that there is a value in trying to use the annotations in cases like these to validate that they work as expected and to tech people how to annotate the private reflection patterns correctly.

[MichalStrehovsky]

Why is that? Is it a bug or expected?

Likely because of the reflection on System.Diagnostics.ProcessInfo below.

tech people how to annotate the private reflection patterns correctly.

There are only two places where DynamicDependency is an appropriate things to use:

• Internal reflection happening within the runtime (pretty much just corelib is eligible), or
• Other reflection happening outside trimming reach (pretty much just hosting scenarios).

It's not the appropriate thing to use anywhere else because it doesn't make the analysis warning go away. One needs to suppress the warnings with DynamicDependency and most people will get the suppressions wrong.

I wouldn't be able to rewrite this to use the correct pattern because uses of object.GetType can only be correctly done by placing DynamicallyAccessedMembers on the type itself, or making it sealed. But we don't want to mark Process as DynamicallyAccessedMembers or mark it sealed just so that the test can get to the internals.

[vitek-karas]

This is weird:

typeof(Process).GetField(fieldName, BindingFlags.NonPublic | BindingFlags.Instance)

Is basically going to do the same thing as

[DynamicDependency(DynamicallyAccessedMemberTypes.NonPublicFields, typeof(Process))]

So we should not need the DynamicDependency.

And we could rewrite:

typeof(Process).Assembly.GetType("System.Diagnostics.ProcessInfo")

To

Type.GetType("System.Diagnostics.ProcessInfo")

and that part should also work.

[MichalStrehovsky]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[MichalStrehovsky]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[MichalStrehovsky]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[MichalStrehovsky]

@VSadov I see some odd test failures in the Linux ARM64 legs (these are not new tests that I'm enabling here). Could "illegal instruction" be suspension related?

[SKIP] System.Net.Tests.HttpWebRequestTest_Async.DefaultMaximumResponseHeadersLength_SetAndGetLength_ValuesMatch
./RunTests.sh: line 168:    26 Illegal instruction     (core dumped) ./System.Net.Requests.Tests -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing -xml testResults.xml $RSP_FILE

[SKIP] System.Net.Http.Functional.Tests.SyncHttpHandler_DiagnosticsTest.SendAsync_ExpectedDiagnosticSynchronousExceptionActivityLogging
./RunTests.sh: line 168:    27 Illegal instruction     (core dumped) ./System.Net.Http.Functional.Tests -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing -xml testResults.xml $RSP_FILE

The CI captured crash dumps:

https://dev.azure.com/dnceng/public/_build/results?buildId=1928848&view=ms.vss-test-web.build-test-results-tab&runId=49869048&resultId=155732&paneView=dotnet-dnceng.dnceng-build-release-tasks.helix-test-information-tab

[VSadov]

“illegal instruction” would be unexpected. Anything can be a result of some stack/heap corruption, but it it is hard to see how, since suspension does not change code.
It could be caused by some corrupted branch, but that would likely be random, not favoring particular tests.
I will have to take a look at the dumps to have more clue.

[MichalStrehovsky]

Running into illegal instruction in https://github.com/dotnet/runtime/pull/73546/checks?check_run_id=7718877740 (different pull request) as well.

In any case - this looks new - I've ran the runtime-extra-platforms pipeline quite extensively last week as I was banging my head against the various tests and didn't see this, but today 2 out of 3 pull requests hit it somewhere.

[MichalStrehovsky]

The NativeAOT failures are in tests that I'm not newly enabling. @dotnet/ilc-contrib could someone review this?

[VSadov]

I've looked at two dumps and it looks like it somehow involves libcrypto.so.1.1, but the instruction at reported address does not look illegal - in both cases adr x16, #0x8000, perhaps it is not the actual address of the illegal instruction though.

[MichalStrehovsky]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[MichalStrehovsky]

@dotnet/ilc-contrib could someone have a look? This is just a bunch of test changes. The test results look good, just a bunch of device failures that fail pretty much always.