[windows] patch to set breakpoint in a dll

[windows] patch to set breakpoint in a dll

[asmwarrior]

[-- Attachment #1: Type: text/plain, Size: 2197 bytes --]

Hi, all. Several months ago, I have propose and discuss this issue, but now I see no progress, so I just give a "Ping" like post.

The patch is quite simple. (see the attachment), currently, the patch is include in the MinGW's official gdb 7.4 release.

Why we need this patch? It can let us debug dll(the dll is build with relative file path under GCC)
The reason is below: this is my modified code

int
find_and_open_source (const char *filename,
		      const char *dirname,
		      char **fullname)
{
   char *path = source_path;
   const char *p;
   int result;
   char *lpath;

   /* Quick way out if we already know its full name.  */

   if (*fullname)
     {
       /* The user may have requested that source paths be rewritten
          according to substitution rules he provided.  If a substitution
          rule applies to this path, then apply it.  */
       char *rewritten_fullname = rewrite_source_path (*fullname);

       if (rewritten_fullname != NULL)
         {
           xfree (*fullname);
           *fullname = rewritten_fullname;
         }

       result = open (*fullname, OPEN_MODE);
       if (result >= 0)
       {
         lpath = gdb_realpath(*fullname);
         xfree(*fullname);
         *fullname = lpath;
         return result;
       }
       /* Didn't work -- free old one, try again.  */
       xfree (*fullname);
       *fullname = NULL;
     }

Here,
the *fullname = E:\code\cb\wx\wxWidgets-2.8.12\build\msw/../../src/common/string.cpp

And you set the break point by using this command:
break "E:/code/cb/wx/wxWidgets-2.8.12/src/common/string.cpp:164"
Without the patch, GDB can not set the breakpoint, no file name matches.

Luckily, the second break command works:
break E:\code\cb\wx\wxWidgets-2.8.12\build\msw/../../src/common/string.cpp:164
But I think no body will wrote such kind of file path specification.


Now, In my patch, I add a function "gdb_realpath" which internally use Windows API GetFullPathName() to calculate the canonized path. So, with this patch, the first break command works.

We have many discussion before:
http://sourceware.org/ml/gdb/2011-06/msg00074.html

Thanks.

Yuanhui Zhang
Codeblocks forum ID(ollydbg)











[-- Attachment #2: dll_bp.patch --]
[-- Type: text/x-patch, Size: 791 bytes --]

 gdb/source.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/gdb/source.c b/gdb/source.c
index cfdf81b..cd87e34 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -979,6 +979,7 @@ find_and_open_source (const char *filename,
   char *path = source_path;
   const char *p;
   int result;
+  char *lpath;
 
   /* Quick way out if we already know its full name.  */
 
@@ -997,7 +998,12 @@ find_and_open_source (const char *filename,
 
       result = open (*fullname, OPEN_MODE);
       if (result >= 0)
-	return result;
+      {
+        lpath = gdb_realpath(*fullname);
+        xfree(*fullname);
+        *fullname = lpath;
+        return result;
+      }
       /* Didn't work -- free old one, try again.  */
       xfree (*fullname);
       *fullname = NULL;

Re: [windows] patch to set breakpoint in a dll

[Doug Evans]

On Mon, Mar 5, 2012 at 7:10 AM, asmwarrior <asmwarrior@gmail.com> wrote:
> Hi, all. Several months ago, I have propose and discuss this issue, but now
> I see no progress, so I just give a "Ping" like post.
>
> The patch is quite simple. (see the attachment), currently, the patch is
> include in the MinGW's official gdb 7.4 release.
>
> Why we need this patch? It can let us debug dll(the dll is build with
> relative file path under GCC)
> The reason is below: this is my modified code
>
> int
> find_and_open_source (const char *filename,
>                      const char *dirname,
>                      char **fullname)
> {
>  char *path = source_path;
>  const char *p;
>  int result;
>  char *lpath;
>
>  /* Quick way out if we already know its full name.  */
>
>  if (*fullname)
>    {
>      /* The user may have requested that source paths be rewritten
>         according to substitution rules he provided.  If a substitution
>         rule applies to this path, then apply it.  */
>      char *rewritten_fullname = rewrite_source_path (*fullname);
>
>      if (rewritten_fullname != NULL)
>        {
>          xfree (*fullname);
>          *fullname = rewritten_fullname;
>        }
>
>      result = open (*fullname, OPEN_MODE);
>      if (result >= 0)
>      {
>        lpath = gdb_realpath(*fullname);
>        xfree(*fullname);
>        *fullname = lpath;
>        return result;
>      }
>      /* Didn't work -- free old one, try again.  */
>      xfree (*fullname);
>      *fullname = NULL;
>    }
>
> Here,
> the *fullname =
> E:\code\cb\wx\wxWidgets-2.8.12\build\msw/../../src/common/string.cpp
>
> And you set the break point by using this command:
> break "E:/code/cb/wx/wxWidgets-2.8.12/src/common/string.cpp:164"
> Without the patch, GDB can not set the breakpoint, no file name matches.
>
> Luckily, the second break command works:
> break
> E:\code\cb\wx\wxWidgets-2.8.12\build\msw/../../src/common/string.cpp:164
> But I think no body will wrote such kind of file path specification.
>
>
> Now, In my patch, I add a function "gdb_realpath" which internally use
> Windows API GetFullPathName() to calculate the canonized path. So, with this
> patch, the first break command works.
>
> We have many discussion before:
> http://sourceware.org/ml/gdb/2011-06/msg00074.html
>
> Thanks.

The patch has a few nits that need to be fixed, but I like the idea:
have find_and_open_source consistently return the canonicalized path.
However, openp (called by find_and_open_source later on) uses
xfullpath, so for consistency I think that should be used here too
instead of gdb_realpath.
What do others think?

Re: [windows] patch to set breakpoint in a dll

[asmwarrior]

On 2012-3-8 2:46, Doug Evans wrote:
> The patch has a few nits that need to be fixed, but I like the idea:
Hi, Doug, Thanks for the reply.
> have find_and_open_source consistently return the canonicalized path.
> However, openp (called by find_and_open_source later on) uses
> xfullpath, so for consistency I think that should be used here too
> instead of gdb_realpath.
> What do others think?
By reading the function body of xfullpath, I see that it do more checks.
And transfer from gdb_realpath to xfullpath will adding an extra xmalloc and xfree call.

BTW: are there any simple way we can quickly check a path is cononicalized or not, because even gdb_realpath have internally call xmalloc and xfree. but once the *fullname is already cononicalized(after the first call of find_and_open_source()), I think we can avoid calling any xmalloc and xfree. Am I right?

Yuanhui Zhang

Re: [windows] patch to set breakpoint in a dll

[Doug Evans]

On Wed, Mar 7, 2012 at 6:18 PM, asmwarrior <asmwarrior@gmail.com> wrote:
> On 2012-3-8 2:46, Doug Evans wrote:
>>
>> The patch has a few nits that need to be fixed, but I like the idea:
>
> Hi, Doug, Thanks for the reply.
>
>> have find_and_open_source consistently return the canonicalized path.
>> However, openp (called by find_and_open_source later on) uses
>> xfullpath, so for consistency I think that should be used here too
>> instead of gdb_realpath.
>> What do others think?
>
> By reading the function body of xfullpath, I see that it do more checks.
> And transfer from gdb_realpath to xfullpath will adding an extra xmalloc and
> xfree call.
>
> BTW: are there any simple way we can quickly check a path is cononicalized
> or not, because even gdb_realpath have internally call xmalloc and xfree.
> but once the *fullname is already cononicalized(after the first call of
> find_and_open_source()), I think we can avoid calling any xmalloc and xfree.
> Am I right?

No such quick check is available, but I think we can ignore the cost
of the xmalloc/xfree for this particular case until the data shows
it's a problem.

Re: [windows] patch to set breakpoint in a dll

[asmwarrior]

[-- Attachment #1: Type: text/plain, Size: 306 bytes --]

On 2012-3-9 6:39, Doug Evans wrote:
> No such quick check is available, but I think we can ignore the cost
> of the xmalloc/xfree for this particular case until the data shows
> it's a problem.
Ok, this is the modified patch.(simply replace gdb_fullpath with xfullpath)
It works Ok under mingw+windowsXP.


[-- Attachment #2: dll_bp.patch --]
[-- Type: text/x-c++, Size: 788 bytes --]

 gdb/source.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/gdb/source.c b/gdb/source.c
index cfdf81b..cd87e34 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -979,6 +979,7 @@ find_and_open_source (const char *filename,
   char *path = source_path;
   const char *p;
   int result;
+  char *lpath;
 
   /* Quick way out if we already know its full name.  */
 
@@ -997,7 +998,12 @@ find_and_open_source (const char *filename,
 
       result = open (*fullname, OPEN_MODE);
       if (result >= 0)
-	return result;
+      {
+        lpath = xfullpath(*fullname);
+        xfree(*fullname);
+        *fullname = lpath;
+        return result;
+      }
       /* Didn't work -- free old one, try again.  */
       xfree (*fullname);
       *fullname = NULL;

[windows] patch to set breakpoint in a dll

[Doug Evans]

On Fri, Mar 9, 2012 at 8:40 PM, asmwarrior <asmwarrior@gmail.com> wrote:
> On 2012-3-9 6:39, Doug Evans wrote:
>>
>> No such quick check is available, but I think we can ignore the cost
>> of the xmalloc/xfree for this particular case until the data shows
>> it's a problem.
>
> Ok, this is the modified patch.(simply replace gdb_fullpath with xfullpath)
> It works Ok under mingw+windowsXP.

Thanks.
This needs a ChangeLog entry of course.
Something like

2012-03-14  Zhang Yuanhui  <asmwarrior@gmail.com>

	* source.c (find_and_open_source): Consistently pass resulting
	full path through xfullpath.

Further comments inline with the patch.

  diff --git a/gdb/source.c b/gdb/source.c
  index cfdf81b..cd87e34 100644
  --- a/gdb/source.c
  +++ b/gdb/source.c
  @@ -979,6 +979,7 @@ find_and_open_source (const char *filename,
     char *path = source_path;
     const char *p;
     int result;
  +  char *lpath;

Move this declaration into the block where it is used.
   
     /* Quick way out if we already know its full name.  */
   
  @@ -997,7 +998,12 @@ find_and_open_source (const char *filename,
   
         result = open (*fullname, OPEN_MODE);
         if (result >= 0)
  -	return result;
  +      {

Add a comment here explaining why we're calling xfullpath.
E.g.,
           /* Call xfullpath here to be consistent with openp
              which we use below.  */

  +        lpath = xfullpath(*fullname);
  +        xfree(*fullname);

Space before '('.
Plus, for consistency, I think the indentation here should use tabs.

  +        *fullname = lpath;
  +        return result;
  +      }
         /* Didn't work -- free old one, try again.  */
         xfree (*fullname);
         *fullname = NULL;


Ok with those changes.
Thanks!

Re: [windows] patch to set breakpoint in a dll

[asmwarrior]

[-- Attachment #1: Type: text/plain, Size: 2424 bytes --]

On 2012-3-15 2:26, Doug Evans wrote:
> On Fri, Mar 9, 2012 at 8:40 PM, asmwarrior<asmwarrior@gmail.com>  wrote:
>> On 2012-3-9 6:39, Doug Evans wrote:
>>> No such quick check is available, but I think we can ignore the cost
>>> of the xmalloc/xfree for this particular case until the data shows
>>> it's a problem.
>> Ok, this is the modified patch.(simply replace gdb_fullpath with xfullpath)
>> It works Ok under mingw+windowsXP.
> Thanks.
> This needs a ChangeLog entry of course.
> Something like
>
> 2012-03-14  Zhang Yuanhui<asmwarrior@gmail.com>
>
> 	* source.c (find_and_open_source): Consistently pass resulting
> 	full path through xfullpath.
>
> Further comments inline with the patch.
>
>    diff --git a/gdb/source.c b/gdb/source.c
>    index cfdf81b..cd87e34 100644
>    --- a/gdb/source.c
>    +++ b/gdb/source.c
>    @@ -979,6 +979,7 @@ find_and_open_source (const char *filename,
>       char *path = source_path;
>       const char *p;
>       int result;
>    +  char *lpath;
>
> Move this declaration into the block where it is used.
>
>       /* Quick way out if we already know its full name.  */
>
>    @@ -997,7 +998,12 @@ find_and_open_source (const char *filename,
>
>           result = open (*fullname, OPEN_MODE);
>           if (result>= 0)
>    -	return result;
>    +      {
>
> Add a comment here explaining why we're calling xfullpath.
> E.g.,
>             /* Call xfullpath here to be consistent with openp
>                which we use below.  */
>
>    +        lpath = xfullpath(*fullname);
>    +        xfree(*fullname);
>
> Space before '('.
> Plus, for consistency, I think the indentation here should use tabs.
>
>    +        *fullname = lpath;
>    +        return result;
>    +      }
>           /* Didn't work -- free old one, try again.  */
>           xfree (*fullname);
>           *fullname = NULL;
>
>
> Ok with those changes.
> Thanks!
Ok, this is the new patch.(I have problem using the TortoiseGit to generate a diff between two revisions.you can see, the file name is long and dirty)

Also, the log message is what you wrote:


2012-03-14  Zhang Yuanhui <asmwarrior@gmail.com>

     * source.c (find_and_open_source): Consistently pass resulting
     full path through xfullpath.


My family name is "Zhang", so I'm not sure it should be put before the given name or after the given name. The Chinese name and English name have different formats.


Thanks.









[-- Attachment #2: bp.patch --]
[-- Type: text/x-c++, Size: 704 bytes --]

diff --git "a/C:\\Temp\\sou4C.tmp\\source-765c91-left.c" "b/C:\\Temp\\sou49.tmp\\source-e835db-right.c"
index cfdf81b..1293b38 100644
--- "a/C:\\Temp\\sou4C.tmp\\source-765c91-left.c"
+++ "b/C:\\Temp\\sou49.tmp\\source-e835db-right.c"
@@ -996,8 +996,16 @@ find_and_open_source (const char *filename,
         }
 
       result = open (*fullname, OPEN_MODE);
+      /* Call xfullpath here to be consistent with openp
+         which we use below.  */
       if (result >= 0)
+      {
+	char *lpath;
+	lpath = xfullpath (*fullname);
+	xfree (*fullname);
+	*fullname = lpath;
 	return result;
+      }
       /* Didn't work -- free old one, try again.  */
       xfree (*fullname);
       *fullname = NULL;

Re: [windows] patch to set breakpoint in a dll

[Doug Evans]

On Thu, Mar 15, 2012 at 6:52 PM, asmwarrior <asmwarrior@gmail.com> wrote:
> Ok, this is the new patch.(I have problem using the TortoiseGit to generate
> a diff between two revisions.you can see, the file name is long and dirty)
>
>
> Also, the log message is what you wrote:
>
>
> 2012-03-14  Zhang Yuanhui <asmwarrior@gmail.com>
>
>    * source.c (find_and_open_source): Consistently pass resulting
>    full path through xfullpath.
>
>
> My family name is "Zhang", so I'm not sure it should be put before the given
> name or after the given name. The Chinese name and English name have
> different formats.

I say it's your choice.  I just copied from an existing example in ChangeLog.

Heh.  One last nit.  Blank line between these two lines.

+       char *lpath;
+       lpath = xfullpath (*fullname);

[or combine them, but then you still need a blank line afterwards]

Can you check the patch in?  I'm happy to for you if necessary (at
which point I can just insert the blank line before checkin).

Re: [windows] patch to set breakpoint in a dll

[asmwarrior]

On 2012-3-17 6:23, Doug Evans wrote:
> On Thu, Mar 15, 2012 at 6:52 PM, asmwarrior<asmwarrior@gmail.com>  wrote:
>> >  Ok, this is the new patch.(I have problem using the TortoiseGit to generate
>> >  a diff between two revisions.you can see, the file name is long and dirty)
>> >
>> >
>> >  Also, the log message is what you wrote:
>> >
>> >
>> >  2012-03-14  Zhang Yuanhui<asmwarrior@gmail.com>
>> >
>> >      * source.c (find_and_open_source): Consistently pass resulting
>> >      full path through xfullpath.
>> >
>> >
>> >  My family name is "Zhang", so I'm not sure it should be put before the given
>> >  name or after the given name. The Chinese name and English name have
>> >  different formats.
> I say it's your choice.  I just copied from an existing example in ChangeLog.
OK, please use the conversion Yuanhui Zhang which is similar to some gdb developers like: Hui Zhu and Yao Qi.
> Heh.  One last nit.  Blank line between these two lines.
>
> +       char *lpath;
> +       lpath = xfullpath (*fullname);
>
> [or combine them, but then you still need a blank line afterwards]
>
> Can you check the patch in?  I'm happy to for you if necessary (at
> which point I can just insert the blank line before checkin).
Thanks, I have no cvs write access, so please help me to check in by fixing the tiny nit.

Yuanhui Zhang

PS: resend the email to gdb patches maillist by using the plain text format.

Re: [windows] patch to set breakpoint in a dll

[asmwarrior]

On 2012-3-16 9:52, asmwarrior wrote:
> Ok, this is the new patch.
Ping. Any comments about this?

Re: [windows] patch to set breakpoint in a dll

[Doug Evans]

On Wed, Apr 11, 2012 at 7:23 AM, asmwarrior <asmwarrior@gmail.com> wrote:
> On 2012-3-16 9:52, asmwarrior wrote:
>>
>> Ok, this is the new patch.
>
> Ping. Any comments about this?

Hi.
Sorry for the delay!

Thanks for the ping, tweaked and committed.