Tracing multiple subprocesses doesn't work on Windows

ndmitchell commented 5 years ago

If I create a file twice.bat with contents:

cat foo.txt
cat bar.txt

Then it only records the first call to cat, not the second. Similarly, if I do gcc -c main.c then it traces the call to cc1 (which loads main.c and writes the .s file), but not the call to as (which writes main.o).

The problem (as best I can tell...) is that patchInstalled asks if this thread has previously installed a patch. But, if this thread previously installed a patch for a different process, it returns True even though the patch isn't valid. I "fixed" this by changing the value stored in the Tls to be the process of the thread passed to NtResumeThread. I have no real idea what I'm doing, but it fixes the problem, and tracing it seems to approximately work. Any advice?

ndmitchell commented 5 years ago

Concretely:

diff --git a/src/win/hooks.c b/src/win/hooks.c
index eb1e66a..529612b 100644
--- a/src/win/hooks.c
+++ b/src/win/hooks.c
@@ -3,6 +3,8 @@
 #endif
 #include <winternl.h>
 #include <limits.h>
+#include <processthreadsapi.h>
+

 #undef ASSERT
 #include "dbg.h"
@@ -213,7 +215,7 @@ static NTSTATUS NTAPI hNtQueryFullAttributesFile(POBJECT_ATTRIBUTES oa, PFILE_NE
 static NTSTATUS NTAPI hNtResumeThread(HANDLE th, PULONG sc) {
    NTSTATUS r;
    D;
-   if (!patchInstalled())
+   if (!patchInstalled(GetProcessIdOfThread(th)))
        injectThread(th);
    r = oNtResumeThread(th, sc);
    return r;
diff --git a/src/win/patch.c b/src/win/patch.c
index 6fdc26d..7b63f0a 100644
--- a/src/win/patch.c
+++ b/src/win/patch.c
@@ -95,12 +95,12 @@ void patchInstall(void *orig, void *hook, void **preal, const char *nm) {
    dbg("modules patched\n");
 }

-int patchInstalled() {
+int patchInstalled(DWORD old) {
    int ret;
    ASSERT(s_hooked);
-   ret = (int)(intptr_t)TlsGetValue(s_hooked);
-   CHK(TlsSetValue(s_hooked, (void *)1));
-   return ret;
+   ret = (DWORD)(intptr_t)TlsGetValue(s_hooked);
+   CHK(TlsSetValue(s_hooked, (void *)old));
+   return ret == old;
 }

 void patchInit() {
diff --git a/src/win/patch.h b/src/win/patch.h
index a9628ec..64ba3d0 100644
--- a/src/win/patch.h
+++ b/src/win/patch.h
@@ -1,5 +1,4 @@
 void patchInit();
 void patchTerm();
 void patchInstall(void *orig, void *hook, void **preal, const char *nm);
-int patchInstalled();
-
+int patchInstalled(DWORD old);