Skip to content
This repository has been archived by the owner on Nov 1, 2020. It is now read-only.

CppCodeGen: AV in Enum.ToString() #4558

Closed
am11 opened this issue Sep 18, 2017 · 12 comments
Closed

CppCodeGen: AV in Enum.ToString() #4558

am11 opened this issue Sep 18, 2017 · 12 comments

Comments

@am11
Copy link
Member

am11 commented Sep 18, 2017

The runtime transpiled code in C++ for ToString looks like this:

namespace System_Private_CoreLib { namespace System { class Object {
    public:
        static MethodTable * __getMethodTable();
        intptr_t m_pEEType;
			
    typedef ::System_Private_CoreLib::System::String*(*__slot__ToString)(::System_Private_CoreLib::System::Object*);

    static __slot__ToString __getslot__ToString(void * pThis)
    {
        return (__slot__ToString)*((void **)(*((RawEEType **)pThis) + 1) + 0);
    };
...
...
}

Given the following code:

using System;
internal class Program
{
    enum State { On, Off }
    private static void Main(string[] args)
        => Console.WriteLine(State.On.ToString());
}

we get access violation when the getslot function tries to dereference ((RawEEType **)pThis), this part: (*((RawEEType **)pThis) + 1).

Stacktrace:

reproNativeCpp.exe!System_Private_CoreLib::System::Object::__getslot__ToString(void * pThis=0xcccccccc00000000) Line 7776	C++	Symbols loaded.
reproNativeCpp.exe!repro::Program::Main(__Array_A_::System_Private_CoreLib::System::String_V_ * args=0x0000029ce23128c0) Line 11	C++	Symbols loaded.
reproNativeCpp.exe!repro::_Module_::MainMethodWrapper(__Array_A_::System_Private_CoreLib::System::String_V_ * _a0=0x0000029ce23128c0)	C++	Non-user code. Symbols loaded.
reproNativeCpp.exe!repro::_Module_::StartupCodeMain(int _a0=1, __int64 _a1=2872806503888)	C++	Non-user code. Symbols loaded.
reproNativeCpp.exe!__managed__Main(int _a0=1, __int64 _a1=2872806503888) Line 16707570	C++	Symbols loaded.
reproNativeCpp.exe!wmain(int argc=1, wchar_t * * argv=0x0000029ce09c75d0) Line 332	C++	Symbols loaded.
reproNativeCpp.exe!invoke_main() Line 91	C++	Non-user code. Symbols loaded.
reproNativeCpp.exe!__scrt_common_main_seh() Line 283	C++	Non-user code. Symbols loaded.
reproNativeCpp.exe!__scrt_common_main() Line 326	C++	Non-user code. Symbols loaded.
reproNativeCpp.exe!wmainCRTStartup() Line 17	C++	Non-user code. Symbols loaded.
kernel32.dll!BaseThreadInitThunk�()	Unknown	Non-user code. Symbols loaded.
ntdll.dll!RtlUserThreadStart�()	Unknown	Non-user code. Symbols loaded.
@am11
Copy link
Member Author

am11 commented Sep 18, 2017

Without ToString():

using System;
internal class Program
{
    enum State { On, Off }
    private static void Main(string[] args)
        => Console.WriteLine(State.On);
}

The stacktrace looks bit different:

0000000000000000()	Unknown	Non-user code
reproNativeCpp.exe!System_Runtime_Extensions::System::IO::TextWriter::WriteLine_12(System_Runtime_Extensions::System::IO::TextWriter * ___this, System_Private_CoreLib::System::Object * value) Line 430	C++	Symbols loaded.
reproNativeCpp.exe!System_Console::System::IO::SyncTextWriter::WriteLine_12(System_Console::System::IO::SyncTextWriter * ___this, System_Private_CoreLib::System::Object * value) Line 289	C++	Symbols loaded.
reproNativeCpp.exe!System_Console::System::Console::WriteLine_11(System_Private_CoreLib::System::Object * value) Line 555	C++	Symbols loaded.
reproNativeCpp.exe!repro::Program::Main(__Array_A_::System_Private_CoreLib::System::String_V_ * args) Line 11	C++	Symbols loaded.
reproNativeCpp.exe!repro::_Module_::MainMethodWrapper(__Array_A_::System_Private_CoreLib::System::String_V_ * _a0)	C++	Non-user code. Symbols loaded.
reproNativeCpp.exe!repro::_Module_::StartupCodeMain(int _a0, __int64 _a1)	C++	Non-user code. Symbols loaded.
reproNativeCpp.exe!__managed__Main(int _a0, __int64 _a1) Line 16707570	C++	Symbols loaded.
reproNativeCpp.exe!wmain(int argc, wchar_t * * argv) Line 332	C++	Symbols loaded.
reproNativeCpp.exe!invoke_main() Line 91	C++	Non-user code. Symbols loaded.
reproNativeCpp.exe!__scrt_common_main_seh() Line 283	C++	Non-user code. Symbols loaded.
reproNativeCpp.exe!__scrt_common_main() Line 326	C++	Non-user code. Symbols loaded.
reproNativeCpp.exe!wmainCRTStartup() Line 17	C++	Non-user code. Symbols loaded.
kernel32.dll!BaseThreadInitThunk�()	Unknown	Non-user code. Symbols loaded.
ntdll.dll!RtlUserThreadStart�()	Unknown	Non-user code. Symbols loaded.

I assume this is because Console.WriteLine doesn't implicitly make the ToString() virtual call?

@hippiehunter, this second case might be related to #4556.

@hippiehunter
Copy link
Contributor

@am11 the first crash looks like the last of the rules here isn't being taken into account in ImportCall. I think i have a good handle on what needs to be done to fix it.

@hippiehunter
Copy link
Contributor

The second failure appears to be something interacting poorly between boxed enum types and interface methods. The call to FindInterfaceMethodImplementationTarget appears to be returning 0 for the requested function pointer.

@hippiehunter
Copy link
Contributor

@jkotas
it looks like boxed ints are castable to interfaces and their methods are callable, but it doesn't work for enums. I think the problem might be the lack of unbox_ wrappers for the interface methods, but I don't really understand why this doesn't break the plain ToString Case that I fixed in #4562

@am11
Copy link
Member Author

am11 commented Sep 18, 2017

@hippiehunter, I tested State.On.ToString() case with your master (#4562) branch and set a breakpoint at line 162:

public static unsafe object CheckCastClass(Object obj, void* pvTargetEEType)
{
// a null value can be cast to anything
if (obj == null)
return null;
object result = IsInstanceOfClass(obj, pvTargetEEType);
if (result == null)
{
// Throw the invalid cast exception defined by the classlib, using the input EEType*
// to find the correct classlib.
throw ((EEType*)pvTargetEEType)->GetClasslibException(ExceptionIDs.InvalidCast);

while debugging reproNativeCpp.vcxproj.

Except for the first time it hits this breakpoint (different code path from initial module loader), it throws invalid cast exception from line 169 as IsInstanceOfClass returns null. After the fourth hit, it aborts the operation followed by NYI for Method: RhpThrowEx.

It also throws same exception with constant int ToString():

using System;
internal class Program
{
    private static void Main(string[] args)
        => Console.WriteLine(42.ToString());
}

@hippiehunter
Copy link
Contributor

hippiehunter commented Sep 18, 2017

It looks like I cut my test down to far and was only really testing the boxing interface call portion of this issue. It looks like the failure happens with just this

System.Globalization.CultureInfo.CurrentCulture.GetFormat(typeof(System.Globalization.NumberFormatInfo))

Im guessing this is blocked on #2035 and #2884

@jkotas
Copy link
Member

jkotas commented Sep 19, 2017

Im guessing this is blocked on #2035 and #2884

typeof(Foo) should work even without reflection data (ie #2035). It should detect that there is no reflection data and then create RuntimeNoMetadataNamedTypeInfo. It maybe a good idea to figure out why it does not work without reflection data first.

@hippiehunter
Copy link
Contributor

@jkotas
Internal.Reflection.Execution.ExecutionEnvironmentImplementation.IsReflectionBlocked appears to be the primary offender here.

NativeFormatModuleInfo module = ModuleList.Instance.GetModuleInfoByHandle(moduleHandle);

Only works if the module has NativeFormatModuleInfo, otherwise it will throw when it attempts to cast. So basically the path that is supposed to return false and cause RuntimeNoMetadataNamedTypeInfo to be created, itself fails because there is no guard against the requested module being just a plain ModuleInfo instance.

I don't see a way to tell from the TypeManagerHandle if the type is going to be good or not. The header of GetModuleInfoByHandle is pretty clear that it only works for NativeFormatModuleInfo and it makes no attempt to guard against improper access. Is that wrong or is there some way to tell before we get into GetModuleInfoByHandle that there isn't going to be a NativeFormatModuleInfo result?

@hippiehunter
Copy link
Contributor

when i hard code IsReflectionBlocked => true, we fail anyway because ToString of an Enum looks like it requires at least FieldInfo level reflection in order to work

Hello.exe!System_Private_Reflection_Execution::System::SR::get_Reflection_InsufficientMetadata_EdbNeeded() Line 133	C++	Symbols loaded.
Hello.exe!System_Private_Reflection_Execution::Internal::Reflection::Execution::PayForPlayExperience::MissingMetadataExceptionCreator::Create_0(System_Private_CoreLib::System::Reflection::TypeInfo * pertainant) Line 26	C++	Symbols loaded.
Hello.exe!System_Private_Reflection_Execution::Internal::Reflection::Execution::ReflectionDomainSetupImplementation::CreateMissingMetadataException(System_Private_Reflection_Execution::Internal::Reflection::Execution::ReflectionDomainSetupImplementation * ___this, System_Private_CoreLib::System::Reflection::TypeInfo * pertainant) Line 34	C++	Symbols loaded.
Hello.exe!System_Private_Reflection_Core::Internal::Reflection::Core::Execution::ExecutionDomain::CreateMissingMetadataException_0(System_Private_Reflection_Core::Internal::Reflection::Core::Execution::ExecutionDomain * ___this, System_Private_CoreLib::System::Reflection::TypeInfo * pertainant) Line 293	C++	Symbols loaded.
Hello.exe!System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeNoMetadataNamedTypeInfo::get_AnchoringTypeDefinitionForDeclaredMembers(System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeNoMetadataNamedTypeInfo * ___this) Line 151	C++	Symbols loaded.
Hello.exe!System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo::CoreGetDeclaredFields(System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo * ___this, System_Private_Reflection_Core::System::Reflection::Runtime::BindingFlagSupport::NameFilter * optionalNameFilter, System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo * reflectedType) Line 94	C++	Symbols loaded.
Hello.exe!System_Private_Reflection_Core::System::Reflection::Runtime::BindingFlagSupport::FieldPolicies::CoreGetDeclaredMembers(System_Private_Reflection_Core::System::Reflection::Runtime::BindingFlagSupport::FieldPolicies * ___this, System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo * type, System_Private_Reflection_Core::System::Reflection::Runtime::BindingFlagSupport::NameFilter * optionalNameFilter, System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo * reflectedType) Line 23	C++	Symbols loaded.
Hello.exe!System_Private_Reflection_Core::System::Reflection::Runtime::BindingFlagSupport::QueriedMemberList_1_A__System_Private_CoreLib_System_Reflection_FieldInfo_V_::Create(System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo * type, System_Private_CoreLib::System::String * optionalNameFilter, unsigned char ignoreCase) Line 123	C++	Symbols loaded.
Hello.exe!System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo_TypeComponentsCache::GetQueriedMembers_0_A__System_Private_CoreLib_System_Reflection_FieldInfo_V_(System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo_TypeComponentsCache * ___this) Line 67	C++	Symbols loaded.
Hello.exe!System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo::Query_1_A__System_Private_CoreLib_System_Reflection_FieldInfo_V_(System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo * ___this, System_Private_CoreLib::System::String * optionalName, int bindingAttr, System_Private_CoreLib::System::Func_2_A__System_Private_CoreLib_System_Reflection_FieldInfo___System_Private_CoreLib_System_Boolean_V_ * optionalPredicate) Line 190	C++	Symbols loaded.
Hello.exe!System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo::Query_A__System_Private_CoreLib_System_Reflection_FieldInfo_V_(System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo * ___this, int bindingAttr) Line 171	C++	Symbols loaded.
Hello.exe!System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo::GetFields(System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo * ___this, int bindingAttr) Line 51	C++	Symbols loaded.
Hello.exe!System_Private_CoreLib::Internal::Runtime::Augments::EnumInfo::_ctor(System_Private_CoreLib::Internal::Runtime::Augments::EnumInfo * ___this, System_Private_CoreLib::System::Type * enumType) Line 24	C++	Symbols loaded.
Hello.exe!System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo_TypeComponentsCache::get_EnumInfo(System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo_TypeComponentsCache * ___this) Line 78	C++	Symbols loaded.
Hello.exe!System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo::get_EnumInfo(System_Private_Reflection_Core::System::Reflection::Runtime::TypeInfos::RuntimeTypeInfo * ___this) Line 615	C++	Symbols loaded.
Hello.exe!System_Private_Reflection_Core::System::Reflection::Runtime::General::ReflectionCoreCallbacksImplementation::GetEnumInfo(System_Private_Reflection_Core::System::Reflection::Runtime::General::ReflectionCoreCallbacksImplementation * ___this, System_Private_CoreLib::System::Type * type) Line 397	C++	Symbols loaded.
Hello.exe!System_Private_CoreLib::System::Enum::GetEnumInfo(System_Private_CoreLib::System::Type * enumType) Line 850	C++	Symbols loaded.
Hello.exe!System_Private_CoreLib::System::Enum::ToString_0(System_Private_CoreLib::System::Enum * ___this, System_Private_CoreLib::System::String * format) Line 821	C++	Symbols loaded.
Hello.exe!System_Private_CoreLib::System::Enum::ToString(System_Private_CoreLib::System::Enum * ___this) Line 808	C++	Symbols loaded.
Hello.exe!Hello::Hello::Program::Main(__Array_A_::System_Private_CoreLib::System::String_V_ * args) Line 23	C++	Symbols loaded.

@jkotas
Copy link
Member

jkotas commented Sep 20, 2017

Only works if the module has NativeFormatModuleInfo, otherwise it will throw

Fix this by using TryGetModuleInfoByHandle instead?

we fail anyway because ToString of an Enum

Yes, the enum.ToString is still expected to fail. int.ToString should work.

@hippiehunter
Copy link
Contributor

I didn't even think to look for a Try method. Anyway that works for the int case and I've made #4571, does the github bot run a larger set of the coreclr tests? Given this is a common area I would really like to not break the world again.

@jkotas
Copy link
Member

jkotas commented Oct 5, 2018

#6423 has more current discussion about Enum.ToString

@jkotas jkotas closed this as completed Oct 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants