Skip to content

Commit

Permalink
Maintenace + CI: Enable debug logging on 'Debug' build configs (#175)
Browse files Browse the repository at this point in the history
This enables 'Debug' build configs to build the debug output and breakpoints,
and fixes all mistakes that slipped in printf format specifiers.
  • Loading branch information
sylveon authored Mar 1, 2021
1 parent 7f33ae3 commit 259ad41
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 80 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ jobs:
matrix:
os: [windows-2019, windows-2016]
arch: [x86, x64, x64_arm, x64_arm64]
conf: [Release, Debug]

steps:
- name: Clone Repository
Expand All @@ -44,10 +45,13 @@ jobs:
env:
# Tell detours what process to target
DETOURS_TARGET_PROCESSOR: ${{ env.VSCMD_ARG_TGT_ARCH }}
DETOURS_CONFIG: ${{ matrix.conf }}
run: nmake

- name: Run unit tests for ${{ matrix.arch }} on ${{ matrix.os }}
id: run-unit-tests
env:
DETOURS_CONFIG: ${{ matrix.conf }}
run: cd tests && nmake test
if: ${{ matrix.arch == 'x86' || matrix.arch == 'x64' }}

Expand Down
2 changes: 1 addition & 1 deletion samples/common.mak
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ CLIB=/MT
!ENDIF

AFLAGS=/nologo /Zi /c /Fl
CFLAGS=/nologo /Zi $(CLIB) /Gm- /W4 /WX /we4777 /we4800 /Od
CFLAGS=/nologo /Zi $(CLIB) /Gm- /W4 /WX /we4777 /we4800 /Od /DDETOUR_DEBUG=$(DETOURS_DEBUG)

!IF $(DETOURS_SOURCE_BROWSING)==1
CFLAGS=$(CFLAGS) /FR
Expand Down
2 changes: 1 addition & 1 deletion src/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ DETOURS_SOURCE_BROWSING = 0

#######################/#######################################################
##
CFLAGS=/nologo /W4 /WX /we4777 /we4800 /Zi /MT /Gy /Gm- /Zl /Od
CFLAGS=/nologo /W4 /WX /we4777 /we4800 /Zi /MT /Gy /Gm- /Zl /Od /DDETOUR_DEBUG=$(DETOURS_DEBUG)

!IF $(DETOURS_SOURCE_BROWSING)==1
CFLAGS=$(CFLAGS) /FR
Expand Down
80 changes: 40 additions & 40 deletions src/creatwth.cpp

Large diffs are not rendered by default.

26 changes: 13 additions & 13 deletions src/detours.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ inline void detour_find_jmp_bounds(PBYTE pbCode,
// We have to place trampolines within +/- 2GB of code.
ULONG_PTR lo = detour_2gb_below((ULONG_PTR)pbCode);
ULONG_PTR hi = detour_2gb_above((ULONG_PTR)pbCode);
DETOUR_TRACE(("[%p..%p..%p]\n", lo, pbCode, hi));
DETOUR_TRACE(("[%p..%p..%p]\n", (PVOID)lo, pbCode, (PVOID)hi));

// And, within +/- 2GB of relative jmp targets.
if (pbCode[0] == 0xe9) { // jmp +imm32
Expand All @@ -198,7 +198,7 @@ inline void detour_find_jmp_bounds(PBYTE pbCode,
else {
lo = detour_2gb_below((ULONG_PTR)pbNew);
}
DETOUR_TRACE(("[%p..%p..%p] +imm32\n", lo, pbCode, hi));
DETOUR_TRACE(("[%p..%p..%p] +imm32\n", (PVOID)lo, pbCode, (PVOID)hi));
}

*ppLower = (PDETOUR_TRAMPOLINE)lo;
Expand Down Expand Up @@ -399,7 +399,7 @@ inline void detour_find_jmp_bounds(PBYTE pbCode,
// We have to place trampolines within +/- 2GB of code.
ULONG_PTR lo = detour_2gb_below((ULONG_PTR)pbCode);
ULONG_PTR hi = detour_2gb_above((ULONG_PTR)pbCode);
DETOUR_TRACE(("[%p..%p..%p]\n", lo, pbCode, hi));
DETOUR_TRACE(("[%p..%p..%p]\n", (PVOID)lo, pbCode, (PVOID)hi));

// And, within +/- 2GB of relative jmp vectors.
if (pbCode[0] == 0xff && pbCode[1] == 0x25) { // jmp [+imm32]
Expand All @@ -411,7 +411,7 @@ inline void detour_find_jmp_bounds(PBYTE pbCode,
else {
lo = detour_2gb_below((ULONG_PTR)pbNew);
}
DETOUR_TRACE(("[%p..%p..%p] [+imm32]\n", lo, pbCode, hi));
DETOUR_TRACE(("[%p..%p..%p] [+imm32]\n", (PVOID)lo, pbCode, (PVOID)hi));
}
// And, within +/- 2GB of relative jmp targets.
else if (pbCode[0] == 0xe9) { // jmp +imm32
Expand All @@ -423,7 +423,7 @@ inline void detour_find_jmp_bounds(PBYTE pbCode,
else {
lo = detour_2gb_below((ULONG_PTR)pbNew);
}
DETOUR_TRACE(("[%p..%p..%p] +imm32\n", lo, pbCode, hi));
DETOUR_TRACE(("[%p..%p..%p] +imm32\n", (PVOID)lo, pbCode, (PVOID)hi));
}

*ppLower = (PDETOUR_TRAMPOLINE)lo;
Expand Down Expand Up @@ -818,7 +818,7 @@ inline void detour_find_jmp_bounds(PBYTE pbCode,
// We have to place trampolines within +/- 2GB of code.
ULONG_PTR lo = detour_2gb_below((ULONG_PTR)pbCode);
ULONG_PTR hi = detour_2gb_above((ULONG_PTR)pbCode);
DETOUR_TRACE(("[%p..%p..%p]\n", lo, pbCode, hi));
DETOUR_TRACE(("[%p..%p..%p]\n", (PVOID)lo, pbCode, (PVOID)hi));

*ppLower = (PDETOUR_TRAMPOLINE)lo;
*ppUpper = (PDETOUR_TRAMPOLINE)hi;
Expand Down Expand Up @@ -1124,7 +1124,7 @@ inline void detour_find_jmp_bounds(PBYTE pbCode,

ULONG_PTR lo = detour_2gb_below((ULONG_PTR)pbCode);
ULONG_PTR hi = detour_2gb_above((ULONG_PTR)pbCode);
DETOUR_TRACE(("[%p..%p..%p]\n", lo, pbCode, hi));
DETOUR_TRACE(("[%p..%p..%p]\n", (PVOID)lo, pbCode, (PVOID)hi));

*ppLower = (PDETOUR_TRAMPOLINE)lo;
*ppUpper = (PDETOUR_TRAMPOLINE)hi;
Expand Down Expand Up @@ -1237,7 +1237,7 @@ static PVOID detour_alloc_region_from_lo(PBYTE pbLo, PBYTE pbHi)
break;
}

DETOUR_TRACE((" Try %p => %p..%p %6x\n",
DETOUR_TRACE((" Try %p => %p..%p %6lx\n",
pbTry,
mbi.BaseAddress,
(PBYTE)mbi.BaseAddress + mbi.RegionSize - 1,
Expand Down Expand Up @@ -1287,7 +1287,7 @@ static PVOID detour_alloc_region_from_hi(PBYTE pbLo, PBYTE pbHi)
break;
}

DETOUR_TRACE((" Try %p => %p..%p %6x\n",
DETOUR_TRACE((" Try %p => %p..%p %6lx\n",
pbTry,
mbi.BaseAddress,
(PBYTE)mbi.BaseAddress + mbi.RegionSize - 1,
Expand Down Expand Up @@ -1704,7 +1704,7 @@ LONG WINAPI DetourTransactionCommitEx(_Out_opt_ PVOID **pppFailedPointer)
#endif // DETOURS_ARM
}
else {
DETOUR_TRACE(("detours: pbTramp =%p, pbRemain=%p, pbDetour=%p, cbRestore=%d\n",
DETOUR_TRACE(("detours: pbTramp =%p, pbRemain=%p, pbDetour=%p, cbRestore=%x\n",
o->pTrampoline,
o->pTrampoline->pbRemain,
o->pTrampoline->pbDetour,
Expand Down Expand Up @@ -1996,13 +1996,13 @@ LONG WINAPI DetourAttachEx(_Inout_ PVOID *ppPointer,
}

if (s_nPendingThreadId != (LONG)GetCurrentThreadId()) {
DETOUR_TRACE(("transaction conflict with thread id=%d\n", s_nPendingThreadId));
DETOUR_TRACE(("transaction conflict with thread id=%ld\n", s_nPendingThreadId));
return ERROR_INVALID_OPERATION;
}

// If any of the pending operations failed, then we don't need to do this.
if (s_nPendingError != NO_ERROR) {
DETOUR_TRACE(("pending transaction error=%d\n", s_nPendingError));
DETOUR_TRACE(("pending transaction error=%ld\n", s_nPendingError));
return s_nPendingError;
}

Expand Down Expand Up @@ -2183,7 +2183,7 @@ LONG WINAPI DetourAttachEx(_Inout_ PVOID *ppPointer,
pTrampoline->rAlign[n].obTrampoline == 0) {
break;
}
DETOUR_TRACE((" %d/%d",
DETOUR_TRACE((" %x/%x",
pTrampoline->rAlign[n].obTarget,
pTrampoline->rAlign[n].obTrampoline
));
Expand Down
14 changes: 7 additions & 7 deletions src/modules.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ PVOID WINAPI DetourFindFunction(_In_ LPCSTR pszModule,
DETOUR_TRACE(("DetourFindFunction(%hs, %hs)\n", pszModule, pszFunction));
PDETOUR_SYM_INFO pSymInfo = DetourLoadImageHlp();
if (pSymInfo == NULL) {
DETOUR_TRACE(("DetourLoadImageHlp failed: %d\n",
DETOUR_TRACE(("DetourLoadImageHlp failed: %lx\n",
GetLastError()));
return NULL;
}
Expand All @@ -169,7 +169,7 @@ PVOID WINAPI DetourFindFunction(_In_ LPCSTR pszModule,
(PCHAR)pszModule, NULL,
(DWORD64)hModule, 0) == 0) {
if (ERROR_SUCCESS != GetLastError()) {
DETOUR_TRACE(("SymLoadModule64(%p) failed: %d\n",
DETOUR_TRACE(("SymLoadModule64(%p) failed: %lx\n",
pSymInfo->hProcess, GetLastError()));
return NULL;
}
Expand All @@ -181,24 +181,24 @@ PVOID WINAPI DetourFindFunction(_In_ LPCSTR pszModule,
ZeroMemory(&modinfo, sizeof(modinfo));
modinfo.SizeOfStruct = sizeof(modinfo);
if (!pSymInfo->pfSymGetModuleInfo64(pSymInfo->hProcess, (DWORD64)hModule, &modinfo)) {
DETOUR_TRACE(("SymGetModuleInfo64(%p, %p) failed: %d\n",
DETOUR_TRACE(("SymGetModuleInfo64(%p, %p) failed: %lx\n",
pSymInfo->hProcess, hModule, GetLastError()));
return NULL;
}

hrRet = StringCchCopyA(szFullName, sizeof(szFullName)/sizeof(CHAR), modinfo.ModuleName);
if (FAILED(hrRet)) {
DETOUR_TRACE(("StringCchCopyA failed: %08x\n", hrRet));
DETOUR_TRACE(("StringCchCopyA failed: %08lx\n", hrRet));
return NULL;
}
hrRet = StringCchCatA(szFullName, sizeof(szFullName)/sizeof(CHAR), "!");
if (FAILED(hrRet)) {
DETOUR_TRACE(("StringCchCatA failed: %08x\n", hrRet));
DETOUR_TRACE(("StringCchCatA failed: %08lx\n", hrRet));
return NULL;
}
hrRet = StringCchCatA(szFullName, sizeof(szFullName)/sizeof(CHAR), pszFunction);
if (FAILED(hrRet)) {
DETOUR_TRACE(("StringCchCatA failed: %08x\n", hrRet));
DETOUR_TRACE(("StringCchCatA failed: %08lx\n", hrRet));
return NULL;
}

Expand All @@ -215,7 +215,7 @@ PVOID WINAPI DetourFindFunction(_In_ LPCSTR pszModule,
#endif

if (!pSymInfo->pfSymFromName(pSymInfo->hProcess, szFullName, &symbol)) {
DETOUR_TRACE(("SymFromName(%hs) failed: %d\n", szFullName, GetLastError()));
DETOUR_TRACE(("SymFromName(%hs) failed: %lx\n", szFullName, GetLastError()));
return NULL;
}

Expand Down
36 changes: 18 additions & 18 deletions src/uimports.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ static BOOL UPDATE_IMPORTS_XX(HANDLE hProcess,
if (!ReadProcessMemory(hProcess, pbModule, &idh, sizeof(idh), &cbRead)
|| cbRead < sizeof(idh)) {

DETOUR_TRACE(("ReadProcessMemory(idh@%p..%p) failed: %d\n",
DETOUR_TRACE(("ReadProcessMemory(idh@%p..%p) failed: %lx\n",
pbModule, pbModule + sizeof(idh), GetLastError()));

finish:
Expand All @@ -51,7 +51,7 @@ static BOOL UPDATE_IMPORTS_XX(HANDLE hProcess,

if (!ReadProcessMemory(hProcess, pbModule + idh.e_lfanew, &inh, sizeof(inh), &cbRead)
|| cbRead < sizeof(inh)) {
DETOUR_TRACE(("ReadProcessMemory(inh@%p..%p) failed: %d\n",
DETOUR_TRACE(("ReadProcessMemory(inh@%p..%p) failed: %lx\n",
pbModule + idh.e_lfanew,
pbModule + idh.e_lfanew + sizeof(inh),
GetLastError()));
Expand Down Expand Up @@ -82,14 +82,14 @@ static BOOL UPDATE_IMPORTS_XX(HANDLE hProcess,
sizeof(ish), &cbRead)
|| cbRead < sizeof(ish)) {

DETOUR_TRACE(("ReadProcessMemory(ish@%p..%p) failed: %d\n",
DETOUR_TRACE(("ReadProcessMemory(ish@%p..%p) failed: %lx\n",
pbModule + dwSec + sizeof(ish) * i,
pbModule + dwSec + sizeof(ish) * (i + 1),
GetLastError()));
goto finish;
}

DETOUR_TRACE(("ish[%d] : va=%08x sr=%d\n", i, ish.VirtualAddress, ish.SizeOfRawData));
DETOUR_TRACE(("ish[%lx] : va=%08lx sr=%lx\n", i, ish.VirtualAddress, ish.SizeOfRawData));

// If the linker didn't suggest an IAT in the data directories, the
// loader will look for the section of the import directory to be used
Expand Down Expand Up @@ -117,7 +117,7 @@ static BOOL UPDATE_IMPORTS_XX(HANDLE hProcess,
do {
IMAGE_IMPORT_DESCRIPTOR ImageImport;
if (!ReadProcessMemory(hProcess, pImageImport, &ImageImport, sizeof(ImageImport), NULL)) {
DETOUR_TRACE(("ReadProcessMemory failed: %u\n", GetLastError()));
DETOUR_TRACE(("ReadProcessMemory failed: %lx\n", GetLastError()));
goto finish;
}
inh.IMPORT_DIRECTORY.Size += sizeof(IMAGE_IMPORT_DESCRIPTOR);
Expand All @@ -136,8 +136,8 @@ static BOOL UPDATE_IMPORTS_XX(HANDLE hProcess,
}

DETOUR_TRACE((" Imports: %p..%p\n",
(DWORD_PTR)pbModule + inh.IMPORT_DIRECTORY.VirtualAddress,
(DWORD_PTR)pbModule + inh.IMPORT_DIRECTORY.VirtualAddress +
pbModule + inh.IMPORT_DIRECTORY.VirtualAddress,
pbModule + inh.IMPORT_DIRECTORY.VirtualAddress +
inh.IMPORT_DIRECTORY.Size));

DWORD nOldDlls = inh.IMPORT_DIRECTORY.Size / sizeof(IMAGE_IMPORT_DESCRIPTOR);
Expand Down Expand Up @@ -186,23 +186,23 @@ static BOOL UPDATE_IMPORTS_XX(HANDLE hProcess,

if (inh.IMPORT_DIRECTORY.VirtualAddress != 0) {
// Read the old import directory if it exists.
DETOUR_TRACE(("IMPORT_DIRECTORY perms=%x\n", dwProtect));
DETOUR_TRACE(("IMPORT_DIRECTORY perms=%lx\n", dwProtect));

if (!ReadProcessMemory(hProcess,
pbModule + inh.IMPORT_DIRECTORY.VirtualAddress,
&piid[nDlls],
nOldDlls * sizeof(IMAGE_IMPORT_DESCRIPTOR), &cbRead)
|| cbRead < nOldDlls * sizeof(IMAGE_IMPORT_DESCRIPTOR)) {

DETOUR_TRACE(("ReadProcessMemory(imports) failed: %d\n", GetLastError()));
DETOUR_TRACE(("ReadProcessMemory(imports) failed: %lx\n", GetLastError()));
goto finish;
}
}

for (n = 0; n < nDlls; n++) {
HRESULT hrRet = StringCchCopyA((char*)pbNew + obStr, cbNew - obStr, plpDlls[n]);
if (FAILED(hrRet)) {
DETOUR_TRACE(("StringCchCopyA failed: %d\n", GetLastError()));
DETOUR_TRACE(("StringCchCopyA failed: %lx\n", GetLastError()));
goto finish;
}

Expand All @@ -211,7 +211,7 @@ static BOOL UPDATE_IMPORTS_XX(HANDLE hProcess,
cbNew - obStr,
DETOURS_STRINGIFY(DETOURS_BITS_XX));
if (FAILED(hrRet)) {
DETOUR_TRACE(("ReplaceOptionalSizeA failed: %d\n", GetLastError()));
DETOUR_TRACE(("ReplaceOptionalSizeA failed: %lx\n", GetLastError()));
goto finish;
}

Expand Down Expand Up @@ -253,14 +253,14 @@ static BOOL UPDATE_IMPORTS_XX(HANDLE hProcess,
#endif

if (!WriteProcessMemory(hProcess, pbNewIid, pbNew, obStr, NULL)) {
DETOUR_TRACE(("WriteProcessMemory(iid) failed: %d\n", GetLastError()));
DETOUR_TRACE(("WriteProcessMemory(iid) failed: %lx\n", GetLastError()));
goto finish;
}

DETOUR_TRACE(("obBaseBef = %08x..%08x\n",
DETOUR_TRACE(("obBaseBef = %08lx..%08lx\n",
inh.IMPORT_DIRECTORY.VirtualAddress,
inh.IMPORT_DIRECTORY.VirtualAddress + inh.IMPORT_DIRECTORY.Size));
DETOUR_TRACE(("obBaseAft = %08x..%08x\n", obBase, obBase + obStr));
DETOUR_TRACE(("obBaseAft = %08lx..%08lx\n", obBase, obBase + obStr));

// In this case the file didn't have an import directory in first place,
// so we couldn't fix the missing IAT above. We still need to explicitly
Expand All @@ -278,20 +278,20 @@ static BOOL UPDATE_IMPORTS_XX(HANDLE hProcess,
//
if (!DetourVirtualProtectSameExecuteEx(hProcess, pbModule, inh.OptionalHeader.SizeOfHeaders,
PAGE_EXECUTE_READWRITE, &dwProtect)) {
DETOUR_TRACE(("VirtualProtectEx(inh) write failed: %d\n", GetLastError()));
DETOUR_TRACE(("VirtualProtectEx(inh) write failed: %lx\n", GetLastError()));
goto finish;
}

inh.OptionalHeader.CheckSum = 0;

if (!WriteProcessMemory(hProcess, pbModule, &idh, sizeof(idh), NULL)) {
DETOUR_TRACE(("WriteProcessMemory(idh) failed: %d\n", GetLastError()));
DETOUR_TRACE(("WriteProcessMemory(idh) failed: %lx\n", GetLastError()));
goto finish;
}
DETOUR_TRACE(("WriteProcessMemory(idh:%p..%p)\n", pbModule, pbModule + sizeof(idh)));

if (!WriteProcessMemory(hProcess, pbModule + idh.e_lfanew, &inh, sizeof(inh), NULL)) {
DETOUR_TRACE(("WriteProcessMemory(inh) failed: %d\n", GetLastError()));
DETOUR_TRACE(("WriteProcessMemory(inh) failed: %lx\n", GetLastError()));
goto finish;
}
DETOUR_TRACE(("WriteProcessMemory(inh:%p..%p)\n",
Expand All @@ -300,7 +300,7 @@ static BOOL UPDATE_IMPORTS_XX(HANDLE hProcess,

if (!VirtualProtectEx(hProcess, pbModule, inh.OptionalHeader.SizeOfHeaders,
dwProtect, &dwProtect)) {
DETOUR_TRACE(("VirtualProtectEx(idh) restore failed: %d\n", GetLastError()));
DETOUR_TRACE(("VirtualProtectEx(idh) restore failed: %lx\n", GetLastError()));
goto finish;
}

Expand Down
6 changes: 6 additions & 0 deletions system.mak
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,12 @@ DETOURS_OPTION_BITS=64

##############################################################################
##
!IF "$(DETOURS_CONFIG)" == "Debug"
DETOURS_DEBUG=1
!ELSE
DETOURS_DEBUG=0
!ENDIF

INCD = $(ROOT)\include
LIBD = $(ROOT)\lib.$(DETOURS_TARGET_PROCESSOR)$(DETOURS_CONFIG)
BIND = $(ROOT)\bin.$(DETOURS_TARGET_PROCESSOR)$(DETOURS_CONFIG)
Expand Down

0 comments on commit 259ad41

Please sign in to comment.