[PATCH 0/1] MR112: Revert "dbghelp: Don't bother trying to initialize loader backend if we can't...

[Jinoh Kang (＠iamahuman)]

Revert "dbghelp: Don't bother trying to initialize loader backend if we can't get debug base address from PEB."

This reverts commit 3047385437c7ef36996d0418ac378677f3e9d67c.

ntdll.so puts the base address of the main Unix executable, called the
"debug base address," inside an unused field of PEB.

When debugging a WoW64 target process, obtaining the debug base address
fails.  This is because the architecture of PE modules differs from that
of Unix modules.  Specifically, dbghelp attempts to fetch it from the
WoW64 PEB although it actually resides in the native PEB.

However, this shouldn't be a reason to give up initializing dbghelp
entirely.  We do not expect a "user-mode" debugger to be able to debug
Unix-side modules in a WoW64 process anyway.

Signed-off-by: Jinoh Kang

[PATCH 1/1] Revert "dbghelp: Don't bother trying to initialize loader backend if we can't get debug base address from PEB."

[Jinoh Kang]

From: Jinoh Kang

This reverts commit 3047385437c7ef36996d0418ac378677f3e9d67c.

ntdll.so puts the base address of the main Unix executable, called the
"debug base address," inside an unused field of PEB.

When debugging a WoW64 target process, obtaining the debug base address
fails.  This is because the architecture of PE modules differs from that
of Unix modules.  Specifically, dbghelp attempts to fetch it from the
WoW64 PEB although it actually resides in the native PEB.

However, this shouldn't be a reason to give up initializing dbghelp
entirely.  We do not expect a "user-mode" debugger to be able to debug
Unix-side modules in a WoW64 process anyway.

Signed-off-by: Jinoh Kang 
---
 dlls/dbghelp/dbghelp.c      | 2 --
 dlls/dbghelp/macho_module.c | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/dlls/dbghelp/dbghelp.c b/dlls/dbghelp/dbghelp.c
index c154d8d9713..6202c75381b 100644
--- a/dlls/dbghelp/dbghelp.c
+++ b/dlls/dbghelp/dbghelp.c
@@ -407,8 +407,6 @@ static BOOL check_live_target(struct process* pcs, BOOL wow64, BOOL child_wow64)
         free(buf);
     }

-    if (!base) return FALSE;
-
     TRACE("got debug info address %#Ix from PEB %p\n", base, pbi.PebBaseAddress);
     if (!elf_read_wine_loader_dbg_info(pcs, base) && !macho_read_wine_loader_dbg_info(pcs, base))
         WARN("couldn't load process debug info at %#Ix\n", base);
diff --git a/dlls/dbghelp/macho_module.c b/dlls/dbghelp/macho_module.c
index b90f248b15d..3ed77c3b1b8 100644
--- a/dlls/dbghelp/macho_module.c
+++ b/dlls/dbghelp/macho_module.c
@@ -1832,7 +1832,7 @@ static BOOL macho_search_loader(struct process* pcs, struct macho_info* macho_in
         len = sizeof(image_infos.infos64);
     else
         len = sizeof(image_infos.infos32);
-    if (read_process_memory(pcs, pcs->dbg_hdr_addr, &image_infos, len))
+    if (pcs->dbg_hdr_addr && read_process_memory(pcs, pcs->dbg_hdr_addr, &image_infos, len))
     {
         if (pcs->is_64bit)
             len = sizeof(image_info.info64);

[eric pouech (＠epo)]

> of Unix modules.  Specifically, dbghelp attempts to fetch it from the
> WoW64 PEB although it actually resides in the native PEB.

Actually, getting back to the correct 'base address' should be taken care of in previous lines of check_live_target
So I'd say we'd better fix the reason for not getting to the base address (if needed).

can you describe further:
- are you trying to debug an active process (or not)?
- is debuggee 32bit or 64bit?
- is dbghelp 32 or 64bit?

Notes:
- 32bit dbghelp will not be able to handle 64bit processes, so I hope it's not what you're trying to do
- 64bit dbgjelp is able to load 32bit process only if SYMOPT_INCLUDE_32BIT_MODULES is set in options before loading the relevant modules (so before SymInitialize when fInvade is TRUE)
> However, this shouldn't be a reason to give up initializing dbghelp
> entirely.  We do not expect a "user-mode" debugger to be able to debug
> Unix-side modules in a WoW64 process anyway.

why wouldn't we (at least for the 64bit version of dbghelp)?

[Jinoh Kang (＠iamahuman)]

On Mon May 23 12:04:35 2022 +0000, eric pouech wrote:
> > of Unix modules.  Specifically, dbghelp attempts to fetch it from the
> > WoW64 PEB although it actually resides in the native PEB.
>
> Actually, getting back to the correct 'base address' should be taken
> care of in previous lines of check_live_target
> So I'd say we'd better fix the reason for not getting to the base
> address (if needed).
> can you describe further:
> - are you trying to debug an active process (or not)?
> - is debuggee 32bit or 64bit?
> - is dbghelp 32 or 64bit?
>
> Notes:
> - 32bit dbghelp will not be able to handle 64bit processes, so I hope
>
> it's not what you're trying to do
> - 64bit dbgjelp is able to load 32bit process only if
>
> SYMOPT_INCLUDE_32BIT_MODULES is set in options before loading the
> relevant modules (so before SymInitialize when fInvade is TRUE)
> > However, this shouldn't be a reason to give up initializing dbghelp
> > entirely.  We do not expect a "user-mode" debugger to be able to debug
> > Unix-side modules in a WoW64 process anyway.
>
> why wouldn't we (at least for the 64bit version of dbghelp)?
> are you trying to debug an active process (or not)?

Yes, I'm trying to debug an active process.
> is debuggee 32bit or 64bit?

32bit, Wow64 *proper*.  (Of course, this means that the bug cannot be triggered without modification to upstream code.)
> is dbghelp 32 or 64bit?

64bit.

[eric pouech (＠epo)]

On Mon May 23 12:04:35 2022 +0000, Jinoh Kang wrote:
> > are you trying to debug an active process (or not)?
>
> Yes, I'm trying to debug an active process.
> > is debuggee 32bit or 64bit?
>
> 32bit, Wow64 *proper*.  (Of course, this means that the bug cannot be
> triggered without modification to upstream code.)
> > is dbghelp 32 or 64bit?
>
> 64bit.

ok, then we need to fix the lookup of the dbg_hdr (and/or its exposure in the PEB:s)
(for the record, 32bit dbghelp is not ready to handle a 64bit module, and likely will never be... so your patch would open a large can of worms :-( )

[Jinoh Kang (＠iamahuman)]

On Mon May 23 12:38:38 2022 +0000, eric pouech wrote:
> ok, then we need to fix the lookup of the dbg_hdr (and/or its exposure
> in the PEB:s)
> (for the record, 32bit dbghelp is not ready to handle a 64bit module,
> and likely will never be... so your patch would open a large can of
> worms :-( )

I managed to fix dbg_hdr lookup and make it all work.  It turns out the debugger isn't very happy with 64-bit libraries loaded on what is supposed to be a 32-bit process.  I suppose recognizing Unix libraries probably isn't what we want at all...

[Jinoh Kang (＠iamahuman)]

On Mon May 23 13:46:45 2022 +0000, Jinoh Kang wrote:
> I managed to fix dbg_hdr lookup and make it all work.  It turns out the
> debugger isn't very happy with 64-bit libraries loaded on what is
> supposed to be a 32-bit process.  I suppose recognizing Unix libraries
> probably isn't what we want at all...
> 32bit dbghelp will not be able to handle 64bit processes

AFAICT it already isn't, and my patch won't change that fact.