Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Josh Poimboeuf 
On Mon, Sep 19, 2016 at 02:02:18PM -0700, Linus Torvalds wrote:
> On Mon, Sep 19, 2016 at 1:56 PM, Josh Poimboeuf  wrote:
> > On Mon, Sep 19, 2016 at 01:24:03PM -0700, Linus Torvalds wrote:
> >>
> >> because I certainly hope there is always a space there.
> >
> > No luck.  The Japanese translation uses an empty string:
> 
> Heh.
> 
> Ok, we don't want to *just* have the "./" pattern ever be replaced,
> because for all I know there could be a directory name ending with "."
> in there somewhere.
> 
> But I guess we could do it this way instead:
> 
>   --- a/scripts/faddr2line
>   +++ b/scripts/faddr2line
>   @@ -79,7 +79,7 @@ find_dir_prefix() {
>   return
>   fi
> 
>   -   DIR_PREFIX=$prefix
>   +   DIR_PREFIX="$prefix\(\./\)*"
>   return 0
>}
> 
> and thus just make it part of the auto-generated pattern string instead.

Actually, false alarm.  The empty translation string seems to mean "use
the original string":

  $ export LANG=ja_JP.utf8
  $ ./scripts/faddr2line ~/k/vmlinux type_show+0x10/45
  type_show+0x10/0x2d:
  type_show at drivers/video/backlight/backlight.c:213

So your previous patch which checks for a space looks good after all :-)

-- 
Josh

Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Josh Poimboeuf 
On Mon, Sep 19, 2016 at 02:02:18PM -0700, Linus Torvalds wrote:
> On Mon, Sep 19, 2016 at 1:56 PM, Josh Poimboeuf  wrote:
> > On Mon, Sep 19, 2016 at 01:24:03PM -0700, Linus Torvalds wrote:
> >>
> >> because I certainly hope there is always a space there.
> >
> > No luck.  The Japanese translation uses an empty string:
> 
> Heh.
> 
> Ok, we don't want to *just* have the "./" pattern ever be replaced,
> because for all I know there could be a directory name ending with "."
> in there somewhere.
> 
> But I guess we could do it this way instead:
> 
>   --- a/scripts/faddr2line
>   +++ b/scripts/faddr2line
>   @@ -79,7 +79,7 @@ find_dir_prefix() {
>   return
>   fi
> 
>   -   DIR_PREFIX=$prefix
>   +   DIR_PREFIX="$prefix\(\./\)*"
>   return 0
>}
> 
> and thus just make it part of the auto-generated pattern string instead.

Actually, false alarm.  The empty translation string seems to mean "use
the original string":

  $ export LANG=ja_JP.utf8
  $ ./scripts/faddr2line ~/k/vmlinux type_show+0x10/45
  type_show+0x10/0x2d:
  type_show at drivers/video/backlight/backlight.c:213

So your previous patch which checks for a space looks good after all :-)

-- 
Josh

Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Linus Torvalds 
On Mon, Sep 19, 2016 at 1:56 PM, Josh Poimboeuf  wrote:
> On Mon, Sep 19, 2016 at 01:24:03PM -0700, Linus Torvalds wrote:
>>
>> because I certainly hope there is always a space there.
>
> No luck.  The Japanese translation uses an empty string:

Heh.

Ok, we don't want to *just* have the "./" pattern ever be replaced,
because for all I know there could be a directory name ending with "."
in there somewhere.

But I guess we could do it this way instead:

  --- a/scripts/faddr2line
  +++ b/scripts/faddr2line
  @@ -79,7 +79,7 @@ find_dir_prefix() {
  return
  fi

  -   DIR_PREFIX=$prefix
  +   DIR_PREFIX="$prefix\(\./\)*"
  return 0
   }

and thus just make it part of the auto-generated pattern string instead.

 Linus

Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Linus Torvalds 
On Mon, Sep 19, 2016 at 1:56 PM, Josh Poimboeuf  wrote:
> On Mon, Sep 19, 2016 at 01:24:03PM -0700, Linus Torvalds wrote:
>>
>> because I certainly hope there is always a space there.
>
> No luck.  The Japanese translation uses an empty string:

Heh.

Ok, we don't want to *just* have the "./" pattern ever be replaced,
because for all I know there could be a directory name ending with "."
in there somewhere.

But I guess we could do it this way instead:

  --- a/scripts/faddr2line
  +++ b/scripts/faddr2line
  @@ -79,7 +79,7 @@ find_dir_prefix() {
  return
  fi

  -   DIR_PREFIX=$prefix
  +   DIR_PREFIX="$prefix\(\./\)*"
  return 0
   }

and thus just make it part of the auto-generated pattern string instead.

 Linus

Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Josh Poimboeuf 
On Mon, Sep 19, 2016 at 01:24:03PM -0700, Linus Torvalds wrote:
> On Mon, Sep 19, 2016 at 1:00 PM, Rabin Vincent  wrote:
> >
> > Note that addr2line has localized strings, so the regex with the " at "
> > won't match for everyone unless you invoke addr2line with LANG=C.
> 
> Ok, I'll make it match just on the space instead.
> 
> > __write_once_size på /home/rabinv/dev/linux/include/linux/compiler.h:248
> 
> That's an odd localization choice.
> 
> "på"? Wouldn't "i" (or perhaps "vid") be a better choice?
> 
> Anyway, this works for me in the Swedish locale too. Look ok?
> 
>-   addr2line -fpie $objfile $hexaddr | sed "s;$dir_prefix;;"
>+   addr2line -fpie $objfile $hexaddr |
>+   sed "s; $dir_prefix\(\./\)*; ;"
> 
> because I certainly hope there is always a space there.

No luck.  The Japanese translation uses an empty string:

  $ grep -A1 '" at "' binutils/po/ja.po
  msgid " at "
  msgstr ""

-- 
Josh

Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Josh Poimboeuf 
On Mon, Sep 19, 2016 at 01:24:03PM -0700, Linus Torvalds wrote:
> On Mon, Sep 19, 2016 at 1:00 PM, Rabin Vincent  wrote:
> >
> > Note that addr2line has localized strings, so the regex with the " at "
> > won't match for everyone unless you invoke addr2line with LANG=C.
> 
> Ok, I'll make it match just on the space instead.
> 
> > __write_once_size på /home/rabinv/dev/linux/include/linux/compiler.h:248
> 
> That's an odd localization choice.
> 
> "på"? Wouldn't "i" (or perhaps "vid") be a better choice?
> 
> Anyway, this works for me in the Swedish locale too. Look ok?
> 
>-   addr2line -fpie $objfile $hexaddr | sed "s;$dir_prefix;;"
>+   addr2line -fpie $objfile $hexaddr |
>+   sed "s; $dir_prefix\(\./\)*; ;"
> 
> because I certainly hope there is always a space there.

No luck.  The Japanese translation uses an empty string:

  $ grep -A1 '" at "' binutils/po/ja.po
  msgid " at "
  msgstr ""

-- 
Josh

Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Linus Torvalds 
On Mon, Sep 19, 2016 at 1:00 PM, Rabin Vincent  wrote:
>
> Note that addr2line has localized strings, so the regex with the " at "
> won't match for everyone unless you invoke addr2line with LANG=C.

Ok, I'll make it match just on the space instead.

> __write_once_size på /home/rabinv/dev/linux/include/linux/compiler.h:248

That's an odd localization choice.

"på"? Wouldn't "i" (or perhaps "vid") be a better choice?

Anyway, this works for me in the Swedish locale too. Look ok?

   -   addr2line -fpie $objfile $hexaddr | sed "s;$dir_prefix;;"
   +   addr2line -fpie $objfile $hexaddr |
   +   sed "s; $dir_prefix\(\./\)*; ;"

because I certainly hope there is always a space there.

  Linus

Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Linus Torvalds 
On Mon, Sep 19, 2016 at 1:00 PM, Rabin Vincent  wrote:
>
> Note that addr2line has localized strings, so the regex with the " at "
> won't match for everyone unless you invoke addr2line with LANG=C.

Ok, I'll make it match just on the space instead.

> __write_once_size på /home/rabinv/dev/linux/include/linux/compiler.h:248

That's an odd localization choice.

"på"? Wouldn't "i" (or perhaps "vid") be a better choice?

Anyway, this works for me in the Swedish locale too. Look ok?

   -   addr2line -fpie $objfile $hexaddr | sed "s;$dir_prefix;;"
   +   addr2line -fpie $objfile $hexaddr |
   +   sed "s; $dir_prefix\(\./\)*; ;"

because I certainly hope there is always a space there.

  Linus

Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Rabin Vincent 
On Mon, Sep 19, 2016 at 12:15:42PM -0700, Linus Torvalds wrote:
> Hmm. Would you mind if I change the
> 
> addr2line -fpie $objfile $hexaddr | sed "s;$dir_prefix;;"
> 
> into
> 
> addr2line -fpie $objfile $hexaddr |
> sed "s; at $dir_prefix\(\./\)*; at ;"
> 
> instead? There's two changes there: matching the " at " part (to just
> make the match stricter) but also matching any following "./" thing
> (which shows up for our include tree files, at least for me).

Note that addr2line has localized strings, so the regex with the " at "
won't match for everyone unless you invoke addr2line with LANG=C.

$ ../linux/scripts/faddr2line vmlinux free_reserved_area+61
free_reserved_area+61/0xe4:
__write_once_size på /home/rabinv/dev/linux/include/linux/compiler.h:248
(inline:ad av)set_page_count på 
/home/rabinv/dev/linux/include/linux/page_ref.h:76
(inline:ad av)init_page_count på 
/home/rabinv/dev/linux/include/linux/page_ref.h:87
(inline:ad av)__free_reserved_page på 
/home/rabinv/dev/linux/include/linux/mm.h:1818
(inline:ad av)free_reserved_page på 
/home/rabinv/dev/linux/include/linux/mm.h:1824
(inline:ad av)free_reserved_area på /home/rabinv/dev/linux/mm/page_alloc.c:6476

$ LANG=C ../linux/scripts/faddr2line vmlinux free_reserved_area+61
free_reserved_area+61/0xe4:
__write_once_size at include/linux/compiler.h:248
 (inlined by) set_page_count at include/linux/page_ref.h:76
 (inlined by) init_page_count at include/linux/page_ref.h:87
 (inlined by) __free_reserved_page at include/linux/mm.h:1818
 (inlined by) free_reserved_page at include/linux/mm.h:1824
 (inlined by) free_reserved_area at mm/page_alloc.c:6476

Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Rabin Vincent 
On Mon, Sep 19, 2016 at 12:15:42PM -0700, Linus Torvalds wrote:
> Hmm. Would you mind if I change the
> 
> addr2line -fpie $objfile $hexaddr | sed "s;$dir_prefix;;"
> 
> into
> 
> addr2line -fpie $objfile $hexaddr |
> sed "s; at $dir_prefix\(\./\)*; at ;"
> 
> instead? There's two changes there: matching the " at " part (to just
> make the match stricter) but also matching any following "./" thing
> (which shows up for our include tree files, at least for me).

Note that addr2line has localized strings, so the regex with the " at "
won't match for everyone unless you invoke addr2line with LANG=C.

$ ../linux/scripts/faddr2line vmlinux free_reserved_area+61
free_reserved_area+61/0xe4:
__write_once_size på /home/rabinv/dev/linux/include/linux/compiler.h:248
(inline:ad av)set_page_count på 
/home/rabinv/dev/linux/include/linux/page_ref.h:76
(inline:ad av)init_page_count på 
/home/rabinv/dev/linux/include/linux/page_ref.h:87
(inline:ad av)__free_reserved_page på 
/home/rabinv/dev/linux/include/linux/mm.h:1818
(inline:ad av)free_reserved_page på 
/home/rabinv/dev/linux/include/linux/mm.h:1824
(inline:ad av)free_reserved_area på /home/rabinv/dev/linux/mm/page_alloc.c:6476

$ LANG=C ../linux/scripts/faddr2line vmlinux free_reserved_area+61
free_reserved_area+61/0xe4:
__write_once_size at include/linux/compiler.h:248
 (inlined by) set_page_count at include/linux/page_ref.h:76
 (inlined by) init_page_count at include/linux/page_ref.h:87
 (inlined by) __free_reserved_page at include/linux/mm.h:1818
 (inlined by) free_reserved_page at include/linux/mm.h:1824
 (inlined by) free_reserved_area at mm/page_alloc.c:6476

Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Josh Poimboeuf 
On Mon, Sep 19, 2016 at 12:15:42PM -0700, Linus Torvalds wrote:
> On Mon, Sep 19, 2016 at 11:56 AM, Linus Torvalds
>  wrote:
> >>
> >> The only working (and fast) approach I could come up with was an ugly
> >> hack.  It assumes that start_kernel() is in init/main.c.
> >
> > That sounds entirely reasonable. Maybe somebody can come up with a
> > better and more general approach, but that sounds like a fine starting
> > point, and any incremental improvents can happen in the tree. So I'll
> > apply your patch (assuming it passes my basic testing, which I expect
> > it will).
> 
> Hmm. Would you mind if I change the
> 
> addr2line -fpie $objfile $hexaddr | sed "s;$dir_prefix;;"
> 
> into
> 
> addr2line -fpie $objfile $hexaddr |
> sed "s; at $dir_prefix\(\./\)*; at ;"
> 
> instead? There's two changes there: matching the " at " part (to just
> make the match stricter) but also matching any following "./" thing
> (which shows up for our include tree files, at least for me).
> 
> With that, all the cases I threw at it looked pretty good.
> 
> But the stricter matching might not matter, of course, and maybe there
> is some addr2line version that doesn't do that? So I'm checking here..

For some reason, I don't see the "./" for my include files.  But
regardless, your changes look good to me.

-- 
Josh

Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Josh Poimboeuf 
On Mon, Sep 19, 2016 at 12:15:42PM -0700, Linus Torvalds wrote:
> On Mon, Sep 19, 2016 at 11:56 AM, Linus Torvalds
>  wrote:
> >>
> >> The only working (and fast) approach I could come up with was an ugly
> >> hack.  It assumes that start_kernel() is in init/main.c.
> >
> > That sounds entirely reasonable. Maybe somebody can come up with a
> > better and more general approach, but that sounds like a fine starting
> > point, and any incremental improvents can happen in the tree. So I'll
> > apply your patch (assuming it passes my basic testing, which I expect
> > it will).
> 
> Hmm. Would you mind if I change the
> 
> addr2line -fpie $objfile $hexaddr | sed "s;$dir_prefix;;"
> 
> into
> 
> addr2line -fpie $objfile $hexaddr |
> sed "s; at $dir_prefix\(\./\)*; at ;"
> 
> instead? There's two changes there: matching the " at " part (to just
> make the match stricter) but also matching any following "./" thing
> (which shows up for our include tree files, at least for me).
> 
> With that, all the cases I threw at it looked pretty good.
> 
> But the stricter matching might not matter, of course, and maybe there
> is some addr2line version that doesn't do that? So I'm checking here..

For some reason, I don't see the "./" for my include files.  But
regardless, your changes look good to me.

-- 
Josh

Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Linus Torvalds 
On Mon, Sep 19, 2016 at 11:56 AM, Linus Torvalds
 wrote:
>>
>> The only working (and fast) approach I could come up with was an ugly
>> hack.  It assumes that start_kernel() is in init/main.c.
>
> That sounds entirely reasonable. Maybe somebody can come up with a
> better and more general approach, but that sounds like a fine starting
> point, and any incremental improvents can happen in the tree. So I'll
> apply your patch (assuming it passes my basic testing, which I expect
> it will).

Hmm. Would you mind if I change the

addr2line -fpie $objfile $hexaddr | sed "s;$dir_prefix;;"

into

addr2line -fpie $objfile $hexaddr |
sed "s; at $dir_prefix\(\./\)*; at ;"

instead? There's two changes there: matching the " at " part (to just
make the match stricter) but also matching any following "./" thing
(which shows up for our include tree files, at least for me).

With that, all the cases I threw at it looked pretty good.

But the stricter matching might not matter, of course, and maybe there
is some addr2line version that doesn't do that? So I'm checking here..

Linus

Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Linus Torvalds 
On Mon, Sep 19, 2016 at 11:56 AM, Linus Torvalds
 wrote:
>>
>> The only working (and fast) approach I could come up with was an ugly
>> hack.  It assumes that start_kernel() is in init/main.c.
>
> That sounds entirely reasonable. Maybe somebody can come up with a
> better and more general approach, but that sounds like a fine starting
> point, and any incremental improvents can happen in the tree. So I'll
> apply your patch (assuming it passes my basic testing, which I expect
> it will).

Hmm. Would you mind if I change the

addr2line -fpie $objfile $hexaddr | sed "s;$dir_prefix;;"

into

addr2line -fpie $objfile $hexaddr |
sed "s; at $dir_prefix\(\./\)*; at ;"

instead? There's two changes there: matching the " at " part (to just
make the match stricter) but also matching any following "./" thing
(which shows up for our include tree files, at least for me).

With that, all the cases I threw at it looked pretty good.

But the stricter matching might not matter, of course, and maybe there
is some addr2line version that doesn't do that? So I'm checking here..

Linus

Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Linus Torvalds 
On Mon, Sep 19, 2016 at 8:52 AM, Josh Poimboeuf  wrote:
>
> One caveat with #5 (relative path names).  Looking at DW_AT_comp_dir
> wouldn't work, because that's the *object* directory, not the source
> directory.
>
> The only working (and fast) approach I could come up with was an ugly
> hack.  It assumes that start_kernel() is in init/main.c.

That sounds entirely reasonable. Maybe somebody can come up with a
better and more general approach, but that sounds like a fine starting
point, and any incremental improvents can happen in the tree. So I'll
apply your patch (assuming it passes my basic testing, which I expect
it will).

Linus

Re: [PATCH v3] scripts: add script for translating stack dump function

2016-09-19 Thread Linus Torvalds 
On Mon, Sep 19, 2016 at 8:52 AM, Josh Poimboeuf  wrote:
>
> One caveat with #5 (relative path names).  Looking at DW_AT_comp_dir
> wouldn't work, because that's the *object* directory, not the source
> directory.
>
> The only working (and fast) approach I could come up with was an ugly
> hack.  It assumes that start_kernel() is in init/main.c.

That sounds entirely reasonable. Maybe somebody can come up with a
better and more general approach, but that sounds like a fine starting
point, and any incremental improvents can happen in the tree. So I'll
apply your patch (assuming it passes my basic testing, which I expect
it will).

Linus