[PATCH] Add type for $_tlb->process_environment_block->process_parameters

[Hannes Domani via gdb-patches]

 Am Mittwoch, 15. Januar 2020, 22:27:40 MEZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben:

> I was hoping that our Windows specialist would review this :), but I'll
> give it a look.
>
> On 2019-12-23 11:10 a.m., Hannes Domani via gdb-patches wrote:
> > The type then looks like this:
> >
> > (gdb) pt $_tlb->process_environment_block->process_parameters
> > type = struct rtl_user_process_parameters {
> >    DWORD32 maximum_length;
> >    DWORD32 length;
> >    DWORD32 flags;
> >    DWORD32 debug_flags;
> >    void *console_handle;
> >    DWORD32 console_flags;
> >    void *standard_input;
> >    void *standard_output;
> >    void *standard_error;
> >    unicode_string current_directory;
> >    void *current_directory_handle;
> >    unicode_string dll_path;
> >    unicode_string image_path_name;
> >    unicode_string command_line;
> >    void *environment;
> >    DWORD32 starting_x;
> >    DWORD32 starting_y;
> >    DWORD32 count_x;
> >    DWORD32 count_y;
> >    DWORD32 count_chars_x;
> >    DWORD32 count_chars_y;
> >    DWORD32 fill_attribute;
> >    DWORD32 window_flags;
> >    DWORD32 show_window_flags;
> >    unicode_string window_title;
> >    unicode_string desktop_info;
> >    unicode_string shell_info;
> >    unicode_string runtime_data;
> > } *
> >
> > It's mainly useful to get the current directory, or the full command line:
> >
> > (gdb) p $_tlb->process_environment_block->process_parameters->current_directory
> > $1 = {
> >  length = 26,
> >  maximum_length = 520,
> >  buffer = 0xe36c8 L"C:\\src\\tests\\"
> > }
> > (gdb) p $_tlb->process_environment_block->process_parameters->command_line
> > $2 = {
> >  length = 94,
> >  maximum_length = 96,
> >  buffer = 0xe32aa L"\"C:\\gdb\\build64\\gdb-git\\gdb\\gdb.exe\" access.exe"
> > }
>
> Could you add links in the commit message to the relevant documentation that describes these types?
>
> I am guessing this, for example:
>
> https://docs.microsoft.com/en-us/windows/win32/api/ntdef/ns-ntdef-_unicode_string

Yes, that's the one.

But for rtl_user_process_parameters only very little is officially documented:
https://docs.microsoft.com/en-us/windows/win32/api/winternl/ns-winternl-rtl_user_process_parameters

So I used this instead:
https://www.nirsoft.net/kernel_struct/vista/RTL_USER_PROCESS_PARAMETERS.html

Should I use that for the commit message?

>
> >
> > gdb/ChangeLog:
> >
> > 2019-12-23  Hannes Domani  <ssbssa@yahoo.de>
> >
> >     * windows-tdep.c (windows_get_tlb_type):
> >     Add rtl_user_process_parameters type.
> > ---
> >  gdb/windows-tdep.c | 65 +++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 64 insertions(+), 1 deletion(-)
> >
> > diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
> > index bb69a79996..83a80f2f80 100644
> > --- a/gdb/windows-tdep.c
> > +++ b/gdb/windows-tdep.c
> > @@ -114,6 +114,8 @@ windows_get_tlb_type (struct gdbarch *gdbarch)
> >    struct type *peb_type, *peb_ptr_type, *list_type;
> >    struct type *module_list_ptr_type;
> >    struct type *tib_type, *seh_type, *tib_ptr_type, *seh_ptr_type;
> > +  struct type *word_type, *wchar_type, *wchar_ptr_type;
> > +  struct type *uni_str_type, *rupp_type, *rupp_ptr_type;
> >
> >    /* Do not rebuild type if same gdbarch as last time.  */
> >    if (last_tlb_type && last_gdbarch == gdbarch)
> > @@ -123,7 +125,15 @@ windows_get_tlb_type (struct gdbarch *gdbarch)
> >                  1, "DWORD_PTR");
> >    dword32_type = arch_integer_type (gdbarch, 32,
> >                  1, "DWORD32");
> > +  word_type = arch_integer_type (gdbarch, 16,
> > +                1, "WORD");
> > +  wchar_type = arch_integer_type (gdbarch, 16,
> > +                  1, "wchar_t");
> >    void_ptr_type = lookup_pointer_type (builtin_type (gdbarch)->builtin_void);
> > +  wchar_ptr_type = arch_type (gdbarch, TYPE_CODE_PTR,
> > +                  TYPE_LENGTH (void_ptr_type) * TARGET_CHAR_BIT,
> > +                  NULL);
> > +  TYPE_TARGET_TYPE (wchar_ptr_type) = wchar_type;
>
> Could you use arch_pointer_type instead of setting the TYPE_TARGET_TYPE by hand?

I will try that.

>
> This function (windows_get_tlb_type) appears to be called everytime the $_tlb variable
> is used.  The arch_*_type functions allocate new types every time.  This would mean
> that new types are allocate every time $_tlb is used.  Am I reading this right?
>
> If so, it's a problem that predates your patch.  But I think the right way of doing this
> would be to create the types when the gdbarch is created, store them in the
> gdbarch_tdep structure, and use them here.

Actually, this is at the top of windows_get_tlb_type:
  /* Do not rebuild type if same gdbarch as last time.  */
  if (last_tlb_type && last_gdbarch == gdbarch)
    return last_tlb_type;

>
> >
> >    /* list entry */
> >
> > @@ -168,6 +178,59 @@ windows_get_tlb_type (struct gdbarch *gdbarch)
> >                  NULL);
> >    TYPE_TARGET_TYPE (peb_ldr_ptr_type) = peb_ldr_type;
> >
> > +  /* struct UNICODE_STRING */
> > +  uni_str_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
> > +  TYPE_NAME (uni_str_type) = xstrdup ("unicode_string");
> > +
> > +  append_composite_type_field (uni_str_type, "length", word_type);
> > +  append_composite_type_field (uni_str_type, "maximum_length", word_type);
> > +  append_composite_type_field_aligned (uni_str_type, "buffer",
> > +                      wchar_ptr_type,
>
> The actual name in the Windows API UNICODE_STRING (in capital letters), and
> the fields are in camel case, such as "MaximumLength".  Logically, I would
> expect GDB to name the types and fields the same way.
>
> However, I see that the existing types created by this function (that predate
> your patch) don't do this.  Do you have any idea why it was done like that?
>
> I suppose it's better to continue using the same format as the existing types
> (as you did), but still I find this odd.

I actually wondered about this myself, but I also just used the existing format.

>
>
> > +                      TYPE_LENGTH (wchar_ptr_type));
> > +
> > +  /* struct _RTL_USER_PROCESS_PARAMETERS */
> > +  rupp_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
> > +  TYPE_NAME (rupp_type) = xstrdup ("rtl_user_process_parameters");
>
>
> Couldn't you pass the name directly to arch_composite_type?

I will try that as well.


Regards
Hannes Domani