[mono][aot] Loader tests failing on CI with Mono LLVM FullAOT #86327
Comments
ivanpovazan
added
blocking-clean-ci
|
Tagging subscribers to this area: @directhex Issue DetailsSeen when running
Build InformationBuild: https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_build/results?buildId=274202 Error MessageFill the error message using known issues guidance. {
"ErrorPattern": "(Loader).+(InlineArrayInvalid|StartupHookTests)(.sh\s\[FAIL\])",
"ExcludeConsoleLog": false
}
|
|
@ivanpovazan I can take a look at |
|
Sure, thanks! UPDATE: I fixed the |
|
I just fixed the StartupHookTests one with #86269 |
|
Initially, the To fix this issue, AOT compiler can avoid setting the runtime/src/mono/mono/metadata/class.c Lines 2960 to 2963 in 4db4bb5 and runtime/src/mono/mono/metadata/class.c Lines 4722 to 4725 in 4db4bb5 By making these changes, the errors will not be thrown during the AOT compilation. In AOT runtime, when loading structs with invalid attribute, it should throw a type load error. Since runtime/src/mono/mono/mini/aot-runtime.c Lines 4487 to 4488 in 4db4bb5 Even without error propagation, the runtime will throw an exception during type load for It could be that I missed something important. Any ideas on how to proceed? In general, the current approach with reporting issues during mono_class_layout_fields and mono_class_setup_fields could be challenging in AOT scenarios. @vargaz @lambdageek It is possible that I've missed some important details, so it would be helpful to gather additional information. In general, the current approach of throwing exceptions during class init in |
@kotlarmilos Here's an idea (I'm not confident yet - I'm just making stuff up) We could add a bit to the MonoClass (and to the cached class info) that says "the AOT compiler noticed this class has a failure, but it was not fatal enough that AOT compilation had to be aborted". In that case the AOT cache loading function could ignore the cached info and let the runtime go on the slow path of trying to setup the class layout at runtime. At that point it will fail again and we can throw the proper TLE. I'm assuming that if we just ignore the attribute during AOT compilation and pretend it's a single normal-sized field, we can continue with the AOT-time class setup enough to get somewhat usable code? Maybe we can wrap the manipulation of the bit and setting with a nice wrapper. So instead of we can have which will:
|
ghost
added
the
in-pr
Thanks for the feedback, it looks like a good approach! I've created a PR #86554 that addresses the issue as described. In the AOT compilation, the function typedef gboolean (*TypeLoadFailureCallback)(MonoClass *klass, const char * fmt, ...);
TypeLoadFailureCallback type_load_failure_callback;
gboolean
mono_class_set_type_load_deferred_failure (MonoClass *klass, const char * fmt, ...)
{
if (!mono_class_has_deferred_failure (klass)) {
va_list args;
va_start (args, fmt);
g_warning ("Warning: %s", fmt, args);
va_end (args);
mono_class_set_deferred_failure (klass);
}
return FALSE;
}
gboolean
mono_class_set_type_load_failure (MonoClass *klass, const char * fmt, ...)
{
// error handling
return TRUE;
}
void set_failure_type(FailureType failure_type) {
if (failure_type == DEFERRED_FAILURE) {
type_load_failure_callback = mono_class_set_type_load_deferred_failure;
} else if (failure_type == IMMEDIATE_FAILURE) {
type_load_failure_callback = mono_class_set_type_load_failure;
} else {
g_assert_not_reached ();
}
}Such an approach allows for extensibility in similar scenarios where failures should be deferred.
Correct. However, the fix cannot be applied in cases where the AOT compiler optimizes unused code or adds const instructions. For example, in the following code snippet the AOT compiler removes the body since it is not used. Assert.Throws<TypeLoadException>(() => { var t = typeof(Explicit); });Here, the AOT compiler adds struct size to the instruction during AOT compilation. Assert.Throws<TypeLoadException>(() => { return sizeof(Explicit); });To address these cases, I suggest adding an instruction (or using the existing one) that will force type loading instead of the actual instruction, ensuring that an exception is thrown during runtime, which would help ensure that the code is executed as intended. Do you think it is a good approach to handle these cases? |
|
Imho it would be easier to just disable these tests. Running apps with type load errors/invalid IL does not need to be supported in full aot mode. |
|
If it is not a customer requirement, it is definitely easier to disable the tests. I believe it is expected to have an error message during either AOT compilation or runtime. Currently, the AOT compiler avoids the compilation of specific methods, resulting in an AOT module with missing methods. Is there an option to stop the AOT compilation if such errors occur? |
|
The PR #86554 is ready for review. In the AOT nollvm configuration, the test is passing. However, in the AOT llvm configuration, it fails during To ensure that the test passes, it may be necessary to replace the exception handling generic method by a simple try/catch block.
As the test has been disabled in the AOT llvm configuration, we have to decide whether the support for deferred calls in AOT compiler is needed or not. It can be useful when a failure has to be deferred for the runtime but it introduces additional complexity. |
I think we should have the same behavior as NativeAOT. I'm not sure about InlineArrayAttribute, but there are some other APIs that are specced to fail at runtime, not during AOT. So it would be nice to have a mechanism to avoid that |
|
I agree. From developer's perspective, it would make sense to have sensible warning/error during the compilation and runtime that something is not supported. On the other hand, I understand that it introduces additional complexity for "edge" cases. |
|
@vargaz your feedback/review required to proceed. |
ghost
removed
the
in-pr
ghost
locked as resolved and limited conversation to collaborators
Seen when running
runtime-extra-platformsonlinux-x64 Release AllSubsets_Mono_LLVMFullAot_RuntimeTests llvmfullaot.Loader tests failures detected on:
Build Information
Build: https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_build/results?buildId=274202
Build error leg or test failing: Loader.classloader.WorkItemExecution
Pull request: #86266
Error Message
Fill the error message using known issues guidance.
{ "ErrorPattern": "(Loader).+(InlineArrayInvalid|StartupHookTests)(.sh\\s\\[FAIL\\])", "ExcludeConsoleLog": false }Report
Summary
The text was updated successfully, but these errors were encountered: