Re: [PATCH v3] Implement debugging of WOW64 processes

["Hannes Domani via gdb-patches" <gdb-patches@sourceware.org>]

 Am Mittwoch, 4. März 2020, 21:07:53 MEZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben:

> On 2020-03-04 2:46 p.m., Hannes Domani via gdb-patches wrote:
> >  Am Mittwoch, 4. März 2020, 20:32:52 MEZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben:
> >
> >> On 2020-03-04 2:16 p.m., Hannes Domani via gdb-patches wrote:
> >>> @@ -99,7 +99,5 @@ void _initialize_amd64_windows_nat ();
> >>>   void
> >>>   _initialize_amd64_windows_nat ()
> >>>   {
> >>> -  windows_set_context_register_offsets (mappings);
> >>> -  windows_set_segment_register_p (amd64_windows_segment_register_p);
> >>>     x86_set_debug_register_length (8);
> >>>   }
> >>
> >> Does x86_set_debug_register_length need to be adjusted based on the type of the
> >> process?  With the current code, it will be set to 8 even if debugging a WOW64
> >> process, is that what we want?
> >
> > I don't really know what this is for, but I haven't had any problem so far like this.
>
> It looks like it should be set to the size of the DR registers of the debugee.
>
> When debugging a standard x86-64 process, those registers are 8 bytes long (DWORD64):
>
>   https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-context
>
> But when debugging a WOW64 process, they appear to be 4 bytes long (DWORD):
>
>   https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-wow64_context
>
> So it looks wrong that there is a single global value for this.  If you were debugging
> one 64 bits and one 32 bits process, I guess you would want one value per inferior.
>
> If you search for uses of the x86_dr_low.debug_register_length field (most of them
> are hidden behind macros), you'll see that this is used for setting watchpoints
> properly.  So if something should break, it would be in that area.

I tested watchpoints with WOW64 processes as well, didn't have any problems.


> I'm fine with leaving this as-is in the current patch, since it would require a good
> refactor of x86-nat/x86-dregs to make that value per inferior (if that's even the
> right thing to do).
>
> Note that the x86-linux-nat target also sets this value just once in its _initialize:
>
>   x86_set_debug_register_length (sizeof (void *));
>
> I don't know what happens then when debugging a 32 bits process from a 64 bits GDB
> (or vice versa) on x86/Linux.
>
> Anyway, this version of the patch LGTM, thanks.

Pushed, thanks


Regards
Hannes Domani