Not compatible with Windows Insider Build 20150

[koppor]

Version 0.4.9:

Version 1.0.0a1 does not start at all. cmd.exe hangs during start.

Microsoft Windows 10 Pro Insider Preview 
10.0.20150 Build 20150

[vladimir-poleh]

It seems that you are using it with ConEmu. You need to copy (not rename) clink_x64.dll to clink_dll_x64.dll and clink_x86.dll to clink_dll_x86.dll. This will resolve your problem.

[koppor]

Clink v0.4.9 offers clink_dll_x86.dll and clink_dll_x64.dll. I copied them to the new DLL names, still no success:

With the most recent release, I had to apply your "patch" resulting in a hang during start:

[return42]

Tested Clink v0.4.9 with ConEmu v20.06.15 on Microsoft Windows [Version 10.0.19041.329]

For me it works, did you read instal-doc: https://conemu.github.io/en/TabCompletion.html#ConEmu_and_clink

[koppor]

In "old" Windows versions, it worked fine for me (I'm using clink since more than two years now). Note that I explicitly provided my Windows version. It is higher than the version provided by you.

Build 19041 was released in May 2020, my build 20150 is an insider preview released on June 17th, 2020. I am well aware that using insider previews might break things. I fear that fixing a broken clink is not on Microsoft's priority list. One reason might be that PowerShell offers Emacs keybindings, too.

$ ver

Microsoft Windows [Version 10.0.20150.1000]

@return42 Would you mind to update to the "Beta Channel" and report whether it is broken, for you, too? And if it still works, could you update to the "Dev Channel"?

[return42]

Note that I explicitly provided my Windows version.

Now, with renamed title it is more clear that you are not asking about Version 2004 :)

@return42 Would you mind to update to the "Beta Channel" and report whether it is broken, for you, too? And if it still works, could you update to the "Dev Channel"?

No please not .. costs me 1/2 day to get the 2004 update running .. I am glad that after the first start-up difficulties it is now working. I better not change anything, otherwise this MS-win crap will collapse again :-o

Sorry, can't help.

[Lunchb0ne]

Note that I explicitly provided my Windows version.

Now, with renamed title it is more clear that you are not asking about Version 2004 :)

@return42 Would you mind to update to the "Beta Channel" and report whether it is broken, for you, too? And if it still works, could you update to the "Dev Channel"?

No please not .. costs me 1/2 day to get the 2004 update running .. I am glad that after the first start-up difficulties it is now working. I better not change anything, otherwise this MS-win crap will collapse again :-o

Sorry, can't help.

Works in beta channel for me, but not in dev.

[refack]

From what I'm seeing it's a bit more subtle. It seems to me that it does finish loading, but some of the pipes are broken.
Specifically it handles key presses, executes the commands, and does pipe the output.
It doesn't seem to pipe the prompt to the console, or hook the magic keys (like arrow up etc.)

(I can confirm the regression started with build 20150 and is still happening in build 20161)

Maybe @bitcrazed or @craigaloewen can help triage this?

[Congee]

i'm using Windows Terminal and have the same issue

[talmo]

Can confirm having the same issue using Windows Terminal or Cmder on Windows build 20170.

[Congee]

@mridgers is this project still alive?

[nikitalita]

Found the underlying issue; KernelBase.dll changed in the Insider build.
Log on Windows Build 20221 (insider):

21316 do_inject                  185 System: ver=6.2 0.0 arch=9 cpus=16 cpu_type=8664 page_size=4096
21316 do_inject                  190 Version: 0.4.9
21316 do_inject                  191 DLL: C:\tools\Cmder\vendor\clink\clink_dll_x64.dll
21316 do_inject                  193 Parent pid: 7680
21316 check_dll_version           52 DLL version: 00000004 0009b2fe
21316 do_inject_impl             283 Creating remote thread at 00007FF8A33E0390 with parameter 00000196B4FA0000
 7680 set_rl_readline_name        58 Setting rl_readline_name to 'cmd.exe'
 7680 hook_trap_veh              120 VEH hit - caller is 00007FF6276F09F5.
 7680 hook_jmp                   408 Attemping jump hook.
 7680 hook_jmp                   409 Target is kernelbase.dll, ReadConsoleW @ 00007FF8A1C581B0
 7680 hook_jmp_impl              351 Attempting to hook at 00007FF8A1C581B0 with 00007FF8700363A0
 7680 get_instruction_length     316 Matched prolog F1845FE9 (mask = 000000FF)
 7680 write_trampoline_out       202 No nop slide or int3 block detected prior to hook target.
 7680 hook_jmp_impl              387 Failed to write trampoline out.
 7680 hook_jmp                   415 Jump hook failed.
 7680 apply_hook_jmp              73 Unable to hook ReadConsoleW in kernelbase.dll
 7680 hook_trap_veh              125 Hook trap 00007FF8700362D4 failed.

KernelBase.dll from windows build 19041 (i.e. non-insider, working)

KernelBase.dll from windows build 20221 (i.e. insider, not working)

It checks for the 'cc' instructions that precede ReadConsoleW so that it knows it's safe to patch into:
clink\shared\hook.c

However, since it appears that those are just garbage instructions that never get reached, we can just comment out the return NULL; and it works fine.

[nikitalita]

patched version is here: https://github.com/nikitalita/clink/tree/v0.4.9-insider-fix_mridgers

[Lunchb0ne]

Wow that's great news

[chrisant996]

Thank you @nikitalita for identifying the issue and a workaround.

The 0x90/0xcc test is intended to ensure the patch doesn't overwrite real code, and it currently is hard coded to use the 5 preceding bytes. For a long term solution I'll make it scan nearby for 5 consecutive bytes that are safe to use, rather than using a hard-coded location.

[nikitalita]

excellent! Thanks @chrisant996 !

[nikitalita]

Apologies, I seem to have included the wrong screenshot for the Windows Insider build 20221, that screenshot was for ReadConsoleInputW, not ReadConsoleW

[chrisant996]

Apologies, I seem to have included the wrong screenshot for the Windows Insider build 20221, that screenshot was for ReadConsoleInputW, not ReadConsoleW

Oh, and there's only a 2 byte "safe" region available preceding ReadConsoleW.

Is there a 5+ byte region anywhere within 125 bytes before the beginning of ReadConsoleW? Or with 125 in either direction?

I might need to set up an Insiders Dev channel VM after all.

[nikitalita]

Looks like one right here, after the function jumps because of the if/else:

[nikitalita]

decompiled function:

[chrisant996]

I'm evaluating using Microsoft Detours for hooking APIs. It handles a lot more edge cases/etc than what's currently in clink (and even supports multiple chipsets, though that's not currently needed by clink). Detours 4.x uses the MIT License.

Detours should resolve this whole class of issues once and for all. That's attractive. 😉

[koppor]

@nikitalita You can click on the three dots and then choose "Edit" to change the markdown of a comment (and or instance update the screenshot).

Another thing @nikitalita: Could you provide a binary? I did not have success building the thing on GitHub (chrisant996/clink#4 (comment)).

[chrisant996]

Merged PR from @nikitalita, and will investigate switching to using Detours moving forward.
Merged PR from @koppor to enable builds produced by github.

[nikitalita]

@koppor here you go: https://github.com/nikitalita/clink/releases/tag/v1.1.0.72181d

[nikitalita]

@koppor BTW, as for fixing clink builds on github actions, take a look at what @cmderdev is doing with their appveyor setup: https://github.com/cmderdev/clink/tree/v0.4.9

[nikitalita]

BTW, here's a build of v0.4.9 with the fix backported:
https://github.com/nikitalita/clink/releases/tag/v0.4.9-insiderfix

[nikitalita]

aw shit, it broke again on 20246. Investigating why.

[nikitalita]

Looks like in the newest builds it no longer has that int3 block it had previously. @chrisant996 have you implemented detours in your fork yet?

[chrisant996]

Not yet, but it sounds like that needs to be a priority.

I was deferring it because it looks like there are some bugs in Detours that people have fixed in forks, and I'll need to collect + review fixes and determine what state of the code base to integrate into Clink.

[chrisant996]

Looks like in the newest builds it no longer has that int3 block it had previously.

I think my original change walked past adjacent functions, and I think you restricted it to only search in the immediately adjacent space (which I agree is safer). Am I remembering correctly? Does removing that restriction enable it to work again at least for the moment?

[nikitalita]

Looks like in the newest builds it no longer has that int3 block it had previously.

I think my original change walked past adjacent functions, and I think you restricted it to only search in the immediately adjacent space (which I agree is safer). Am I remembering correctly? Does removing that restriction enable it to work again at least for the moment?

I had just restricted the backwards search to stop searching backwards if it hit a RET, I didn't restrict fowards searching. Lifting that wouldn't help here, as the closest int3 block started at exactly 126 bytes forward AND after another function. I just lifted the search size from 125 bytes to 135 bytes and it's working now. I'll submit a patch, but yeah, these kinds of fixes seem increasingly fragile.

[nikitalita]

new v0.4.9 build with this fix:
https://github.com/nikitalita/clink/releases/tag/v0.4.9-insiderfixtwo

[chrisant996]

The reason I restricted it to less than 127 in either direction is it seemed to be producing a 2 byte relative jump that can go up to 127 bytes in either direction.

How is the jump being successful when the distance is greater than 7 bits?

Or did you change the instruction set?

[nikitalita]

The long jump instruction is being written starting at offset byte 126. The two-byte relative jump can just barely make it there. I've adjusted the patch accordingly; It'll only check 127 bytes backwards, and 131 bytes forwards.

[chrisant996]

Ok great, thank you. I'll start working on Detours sometime soon. Get Outlook for Android<https://aka.ms/ghei36>

…

________________________________ From: nikitalita <notifications@github.com> Sent: Tuesday, November 3, 2020 9:32:22 AM To: mridgers/clink <clink@noreply.github.com> Cc: Chris Antos <sparrowhawk996@gmail.com>; Mention <mention@noreply.github.com> Subject: Re: [mridgers/clink] Not compatible with Windows Insider Build 20150 (#543) The long jump instruction is being written starting at offset byte 126. The two-byte relative jump can just barely make it there. I've adjusted the patch accordingly; It'll only check 127 bytes backwards, and 131 bytes forwards. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#543 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AEFB4NY5C4YXR5OI7RZQ5BTSOA5CNANCNFSM4OKQHR4Q>.

[nikitalita]

BTW, for people still getting bit by this issue, I recommend using chrisant996's version of clink instead of my patch: https://github.com/chrisant996/clink