71 / scoop-better-shimexe

A better shim.exe file for Scoop.
MIT License
72 stars 29 forks source link

Crash when handle parameters with shim binary #9

Open kiennq opened 4 years ago

kiennq commented 4 years ago

I notice that Emacs shim which's using this shim.exe crash when running emacsclient -a="" -nw. It seems that there're memory early release error somewhere, thus leads to access violation.

Side note: I've convert this to Cpp with better memory management notation (using smart pointer and c++17 features), let me know if you think it's appropriate to be able to merged back here. https://github.com/kiennq/scoop-better-shimexe

71 commented 4 years ago

Sorry I didn't reply until now.

I wanted an extremely lightweight program, both in compile-time and runtime dependencies. For such a small program, I don't think those features are necessarily useful. Unless switching to a more sound language gets requested, I won't change it.

Personal note: I really don't like C (and at the very least, I prefer C++), but I wanted something very easy to set up and use. If it was up to me, this would be in Zig (fast, but unpopular) or Rust (popular, but requires a ton of dependencies). I also made this at the time in order to learn C.

As for the issue, does your repo fix it?

kiennq commented 4 years ago

As for the issue, does your repo fix it?

Yes

71 commented 4 years ago

Weirdly, I can't reproduce on my system. The only problem I can possibly see is that the PROCESS_INFORMATION would keep some reference to cmd, which I free before the end of the program. If you don't mind, can you try running the shim with this line removed?

kiennq commented 4 years ago

Can you try to delete FaultTolerantHeap entry from Computer\HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows NT\CurrentVersion\AppCompatFlags\Layers? https://stackoverflow.com/questions/5020418/how-do-i-turn-off-the-fault-tolerant-heap

When it's heap issue and it crashed too much, OS may turn on fault tolerant heap automatically.

71 commented 4 years ago

No entry for the shim in Layers, and disabling FTH didn't change anything either.

kiennq commented 4 years ago

Do you familiar with WinDbg and TTD trace?

I've taken the trace and the symbol of compiled binary shimexe attached here. Here is the crash stacks:

0:000> k
 # Child-SP          RetAddr           Call Site
00 00000057`66efc358 00007ffb`53aadf10 ntdll!NtTerminateProcess+0x12
01 00000057`66efc360 00007ffb`53a3b686 ntdll!RtlIsZeroMemory+0x160
02 00000057`66efc390 00007ffb`53a500ef ntdll!_C_specific_handler+0x96
03 00000057`66efc400 00007ffb`539fb474 ntdll!_chkstk+0x11f
04 00000057`66efc430 00007ffb`539fb1c5 ntdll!RtlRaiseException+0x434
05 00000057`66efcb40 00007ffb`53aadec9 ntdll!RtlRaiseException+0x185
06 00000057`66efd3b0 00007ffb`53aade93 ntdll!RtlIsZeroMemory+0x119
07 00000057`66efd400 00007ffb`53ab6c32 ntdll!RtlIsZeroMemory+0xe3
08 00000057`66efd4f0 00007ffb`53ab6f1a ntdll!RtlpNtSetValueKey+0x4b2
09 00000057`66efd520 00007ffb`53abc941 ntdll!RtlpNtSetValueKey+0x79a
0a 00000057`66efd550 00007ffb`539caa2f ntdll!RtlpNtSetValueKey+0x61c1
0b 00000057`66efd580 00007ffb`539c73d4 ntdll!RtlAllocateHeap+0x3daf
0c 00000057`66efd7d0 00007ffb`53a2e9be ntdll!RtlAllocateHeap+0x754
0d 00000057`66efd8f0 00007ffb`537ae305 ntdll!RtlpEnsureBufferSize+0x4e
0e 00000057`66efd920 00007ffb`537adecf KERNEL32!BasepConstructSxsCreateProcessMessage+0x625
0f 00000057`66efd9c0 00007ffb`515f8bf9 KERNEL32!BasepConstructSxsCreateProcessMessage+0x1ef
10 00000057`66efdcc0 00007ffb`515f6ab6 KERNELBASE!CreateProcessInternalW+0x1459
11 00000057`66eff1d0 00007ffb`537bcb54 KERNELBASE!CreateProcessW+0x66
12 00000057`66eff240 00007ff7`284077fc KERNEL32!CreateProcessW+0x54
13 00000057`66eff2a0 00007ff7`2840abac emacsclient!main+0x78c [C:\Data\projects\vs\scoop-shimexe1\shim.c @ 196] 
14 (Inline Function) --------`-------- emacsclient!invoke_main+0x22 [d:\agent\_work\5\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl @ 78] 
15 00000057`66effc40 00007ffb`537b6fd4 emacsclient!__scrt_common_main_seh+0x10c [d:\agent\_work\5\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl @ 288] 
16 00000057`66effc80 00007ffb`539fcec1 KERNEL32!BaseThreadInitThunk+0x14
17 00000057`66effcb0 00000000`00000000 ntdll!RtlUserThreadStart+0x21

emacsclient.zip

Also, which Windows version are you using?

71 commented 4 years ago

I can try looking into this. What happens when you remove the calls to free?

By the way, which Scoop package is that? I can't find emacsclient (only emacs and emacsclientw), so I had to create a shim manually.

My Windows version is 10.0.18362.

kiennq commented 4 years ago

Please use this, I thought the official one is also providing emacsclient, sorry. Even remove the free doesn't help. From the call stack, the exception is being throw is

0:002> !error 80000003
Error code: (HRESULT) 0x80000003 (2147483651) - One or more arguments are invalid

It crash/become undefined around here image

If you run this under a debugger, you will see the message saying Heap block at <addr> modified at <addr> past requested size of 88, which indicates some memory is wrongly released or used

71 commented 4 years ago

I still can't figure out what's wrong with my code, and what makes it different from yours. I still don't have any issue, even with your Scoop bucket.

Your C++ code is cleaner so I'd be inclined to use it instead of my C, but it does add a dependency to C++. How big is the generated binary with optimizations?

kiennq commented 4 years ago

Around 122K with 32 bit binary and Oz flag