Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arithmetic overflow crashing host process #297

Closed
apobekiaris opened this issue May 25, 2020 · 21 comments
Closed

Arithmetic overflow crashing host process #297

apobekiaris opened this issue May 25, 2020 · 21 comments

Comments

@apobekiaris
Copy link

I am using a recompiled for net461 version at cceb7f5 15 days ago and when my patched assemblies are loaded from VS (x86) the process crash

image

switching back to 277c77b where monomod was not introduced i do not get any issues.

@pardeike
Copy link
Owner

Not enough information about your related patches. Adding @0x0ade anyway.

@apobekiaris
Copy link
Author

i am not sure what type of info is helpfull my patches are super simple

internal static partial class RxApp{
        static void CreateFrame(Frame __result){
            FramesSubject.OnNext(__result);
        }
    }

the initialization is

         var xafApplicationMethods = typeof(XafApplication).Methods();
            var createFrameMethodPatch = GetMethodInfo(nameof(CreateFrame));
            var frameMethods = new[]{
                xafApplicationMethods.First(info => info.Name == nameof(XafApplication.CreateNestedFrame)),
                xafApplicationMethods.First(info => info.Name == nameof(XafApplication.CreateFrame))
            };
            foreach (var frameMethod in frameMethods){
                harmony.Patch(frameMethod, finalizer: new HarmonyMethod(createFrameMethodPatch));
            }

and the methods i patch are

		public NestedFrame CreateNestedFrame(ViewItem detailViewItem, TemplateContext context) {
			return CreateNestedFrame(detailViewItem, context, null);
		}
		public Frame CreateFrame(TemplateContext context) {
			return new Frame(this, context, CreateControllers(typeof(Controller), true, null));
		}

also forgot to mention that the assemblies load fine with testrunners winapps and asp.net apps

if something else can help happy to provide

@pardeike
Copy link
Owner

Have you tried to replicate with a version build from master? There have been changes lately to MonoMod.Common and it helps to get the baseline. You can even try to clone master and go into Harmony.csproj and use the latest MonoMod.Common Version to build to make sure we are not dealing with an issue that is fixed and that is caused by me not keeping up with the lastest MMC updates.

@apobekiaris
Copy link
Author

although I am only 15 days back on both Harmony and MonoMod repos I will rebuild the latest and update u

@0x0ade
Copy link
Contributor

0x0ade commented May 25, 2020

It probably won't be caused by any patches in particular, but the environment you're patching in.

are loaded from VS (x86)

By chance, do you know if the process you're patching in is large address aware?
And if you're replicating it consistently, can you please provide from and delta as well?

@apobekiaris
Copy link
Author

hmm yeah it's not consistent probably scared of u guys, i let u know soon though cause is often.

By chance, do you know if the process you're patching in is large address aware?

i am not experience in this level but considering that my patching process should be the devenv.exe cause I am loading the assemblies thtough VS combonent designer are u looking for something like this https://stackoverflow.com/questions/9054469/how-to-check-if-exe-is-set-as-largeaddressaware/9056757

@0x0ade
Copy link
Contributor

0x0ade commented May 25, 2020

I've just pushed a possible fix for this issue to the MonoMod.Common repository: MonoMod/MonoMod.Common@1498092

MonoMod.Common 20.5.25.1 should be available on nuget.org shortly, after which either Harmony will need to update to it, or you'll need to build Harmony with that update yourself.

@pardeike
Copy link
Owner

Now master uses 20.5.25.1. @apobekiaris can you build it and test to verify the fix?

@apobekiaris
Copy link
Author

thnks will do, fyi the reason I get into trouble recompiling both repos is that assemblies are not strongly signed, hopefully at some point u will reconsider this decision

@pardeike
Copy link
Owner

The decision on signing framework and libraries like Harmony was discussed before (and even tested!) and it causes more problems than it solves. In the end it was concluded that users like you need to resign it themselves with the appropriate tools.

@apobekiaris
Copy link
Author

yeah I am aware of it, however it super easy to sing if there are no unsigned dependencies like MonoMod but if there are u need to recompile or patch the references.

@pardeike
Copy link
Owner

If your requirements are for things to be signed you probably want your own copy of repositories mirrored and build your binaries yourself. At least that’s what we do at my government job.

@apobekiaris
Copy link
Author

I now complied the latest Harmony for net461 and MonoMod for netstandard2.0 and although it never completes now the exception is different

image

i played around with the patched methods and I noticed that I can patch this method

		protected internal virtual Frame CreateFrame(TemplateContext context, View view) {
			if(OptimizeControllersCreationInMasterDetailMode) {
				return new Frame(this, context, CreateControllers(typeof(Controller), true, null, view));
			}
			return new Frame(this, context, CreateControllers(typeof(Controller), true, null, null));
		}

but not this one

		private ApplicationModulesManager CreateModuleManager(String[] moduleAssemblies) {
			Tracing.Tracer.LogSubSeparator("Initialize Modules Manager");
			ApplicationModulesManager modulesManager = CreateApplicationModulesManager(CreateControllersManager());
			if(modulesManager == null) {
				throw new InvalidOperationException(SystemExceptionLocalizer.GetExceptionMessage(ExceptionId.TheCreateApplicationModulesManagerReturnsNull));
			}
			modulesManager.Modules = modules;
			foreach(Type moduleType in GetDefaultModuleTypes()) {
				modulesManager.AddModule(moduleType);
			}
			modulesManager.AddModuleFromAssemblies(moduleAssemblies);
			return modulesManager;
		}

and if i move the method to class that i own there are no errors.

@apobekiaris
Copy link
Author

method patch is done like

var harmonyMethod = new HarmonyMethod(GetMethodInfo(nameof(CreateModuleManager)));
harmony.Patch(createModuleManager, finalizer: harmonyMethod);

and redirection is defined as

        static void CreateModuleManager(ApplicationModulesManager __result){
            ApplicationModulesManagerSubject.OnNext(__result);
        }

@apobekiaris
Copy link
Author

apobekiaris commented May 27, 2020

@0x0ade I found which is the last Monomod working commit. If I compile latest Harmony commit against Fix GetLdftnPointer, use it for virtual struct methods on .NET it works however compiling against the next one "Parse" some .NET JIT stubs, required to hook NGEN'd methods VS crashed. I also confirm that all non design-time tests of the working version passed

@apobekiaris
Copy link
Author

Hello,

As all my published packages (100+) depends on harmony it is of a major importance to be able to follow the latest version. So I wonder should I expect some fix on this? or is it anything else that I can provide that could help?

@pardeike
Copy link
Owner

pardeike commented Jun 7, 2020

I don't see how I can wait releasing Harmony 2.0.2 and have it contain a fix for this. So I am going to release 2.0.2 and we have to deal with this in a later version.

@apobekiaris
Copy link
Author

apobekiaris commented Jun 21, 2020

I am getting an arithmetic overflow again however this time is on runtime so unrelated to VS process and can share the repro sample.

Solution15.zip

Method virtual System.Object DevExpress.ExpressApp.Security.AuthenticationStandard::Authenticate(DevExpress.ExpressApp.IObjectSpace objectSpace) cannot be patched. Reason: Arithmetic operation resulted in an overflow.

   at HarmonyLib.PatchFunctions.UpdateWrapper(MethodBase original, PatchInfo patchInfo)
   at HarmonyLib.PatchProcessor.Patch()
   at HarmonyLib.Harmony.Patch(MethodBase original, HarmonyMethod prefix, HarmonyMethod postfix, HarmonyMethod transpiler, HarmonyMethod finalizer)
   at 

The patching is done XAfApplicationExtensions .cs file for the Authenticate method. The patching code I borrowed from another project of mine that works and is build for 4.7.2 as well, however Harmony used there is build against the commits I mentioned above.

The project uses Harmony.lib 2.0.1 from nuget.

this is a WIn app just hit F5 and it will throw

@pardeike
Copy link
Owner

pardeike commented Jun 21, 2020

I tested the project and when I start it, it shows a password login dialog. Is it crashing after or before that dialog comes up?
Edit: after entering dummy values, I get this error:

The method or operation is not implemented.
   at Solution15.Module.XAfApplicationExtensions.Authenticate(Object& __result, AuthenticationBase __instance, IObjectSpace objectSpace) in C:\Users\andre\Desktop\Solution15\Solution15.Win\XAfApplicationExtensions.cs:line 31
   at DevExpress.ExpressApp.Security.AuthenticationStandard.Authenticate_Patch1(AuthenticationStandard this, IObjectSpace objectSpace)
   at DevExpress.ExpressApp.Security.SecurityStrategyBase.Logon(IObjectSpace objectSpace)
   at DevExpress.ExpressApp.XafApplication.Logon(PopupWindowShowActionExecuteEventArgs logonWindowArgs)
   at DevExpress.ExpressApp.Win.WinApplication.Logon(PopupWindowShowActionExecuteEventArgs logonWindowArgs)

@apobekiaris
Copy link
Author

it throws b4 the dialog and a not implemented exception on accept implies that the patch worked. Further testing unfortunately revealed that this exception is not consistent in all environments

@pardeike
Copy link
Owner

Closed because of inactivity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants