From: Changbin Du <[email protected]>
Inspired by gdb command 'list', show the code context of target lines.
Here is a example:
$ scripts/faddr2line vmlinux native_write_msr+0x6
native_write_msr+0x6/0x20:
arch_static_branch at arch/x86/include/asm/msr.h:105
100 return EAX_EDX_VAL(val, low, high);
101 }
102
103 static inline void notrace __wrmsr(unsigned int msr, u32 low, u32 high)
104 {
105 asm volatile("1: wrmsr\n"
106 "2:\n"
107 _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_wrmsr_unsafe)
108 : : "c" (msr), "a"(low), "d" (high) : "memory");
109 }
110
(inlined by) static_key_false at include/linux/jump_label.h:142
137 #define JUMP_TYPE_LINKED 2UL
138 #define JUMP_TYPE_MASK 3UL
139
140 static __always_inline bool static_key_false(struct static_key *key)
141 {
142 return arch_static_branch(key, false);
143 }
144
145 static __always_inline bool static_key_true(struct static_key *key)
146 {
147 return !arch_static_branch(key, true);
(inlined by) native_write_msr at arch/x86/include/asm/msr.h:150
145 static inline void notrace
146 native_write_msr(unsigned int msr, u32 low, u32 high)
147 {
148 __wrmsr(msr, low, high);
149
150 if (msr_tracepoint_active(__tracepoint_write_msr))
151 do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
152 }
153
154 /* Can be uninlined because referenced by paravirt */
155 static inline int notrace
Signed-off-by: Changbin Du <[email protected]>
---
scripts/faddr2line | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/scripts/faddr2line b/scripts/faddr2line
index 7721d5b..9e5735a 100755
--- a/scripts/faddr2line
+++ b/scripts/faddr2line
@@ -163,7 +163,17 @@ __faddr2line() {
# pass real address to addr2line
echo "$func+$offset/$sym_size:"
- ${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;"
+ local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
+ [[ -z $file_lines ]] && return
+
+ # show each line with context
+ echo "$file_lines" | while read -r line
+ do
+ echo $line
+ eval $(echo $line | awk -F "[ :]" '{printf("n1=%d;n2=%d;f=%s",$NF-5, $NF+5, $(NF-1))}')
+ awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") {printf("%d\t%s\n", NR, $0)}' $f
+ done
+
DONE=1
done < <(${NM} -n $objfile | awk -v fn=$func -v end=$file_end '$3 == fn { found=1; line=$0; start=$1; next } found == 1 { found=0; print line, "0x"$1 } END {if (found == 1) print line, end; }')
--
2.7.4
On Mon, Mar 19, 2018 at 03:23:25PM +0800, [email protected] wrote:
> From: Changbin Du <[email protected]>
>
> Inspired by gdb command 'list', show the code context of target lines.
> Here is a example:
>
> $ scripts/faddr2line vmlinux native_write_msr+0x6
> native_write_msr+0x6/0x20:
> arch_static_branch at arch/x86/include/asm/msr.h:105
> 100 return EAX_EDX_VAL(val, low, high);
> 101 }
> 102
> 103 static inline void notrace __wrmsr(unsigned int msr, u32 low, u32 high)
> 104 {
> 105 asm volatile("1: wrmsr\n"
> 106 "2:\n"
> 107 _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_wrmsr_unsafe)
> 108 : : "c" (msr), "a"(low), "d" (high) : "memory");
> 109 }
> 110
> (inlined by) static_key_false at include/linux/jump_label.h:142
> 137 #define JUMP_TYPE_LINKED 2UL
> 138 #define JUMP_TYPE_MASK 3UL
> 139
> 140 static __always_inline bool static_key_false(struct static_key *key)
> 141 {
> 142 return arch_static_branch(key, false);
> 143 }
> 144
> 145 static __always_inline bool static_key_true(struct static_key *key)
> 146 {
> 147 return !arch_static_branch(key, true);
> (inlined by) native_write_msr at arch/x86/include/asm/msr.h:150
> 145 static inline void notrace
> 146 native_write_msr(unsigned int msr, u32 low, u32 high)
> 147 {
> 148 __wrmsr(msr, low, high);
> 149
> 150 if (msr_tracepoint_active(__tracepoint_write_msr))
> 151 do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
> 152 }
> 153
> 154 /* Can be uninlined because referenced by paravirt */
> 155 static inline int notrace
Not a fan of this :-/ And you didn't even make it optional. Nor did you
Cc the original author of the tool.
On Tue, May 29, 2018 at 06:03:32PM +0200, Peter Zijlstra wrote:
> On Mon, Mar 19, 2018 at 03:23:25PM +0800, [email protected] wrote:
> > From: Changbin Du <[email protected]>
> >
> > Inspired by gdb command 'list', show the code context of target lines.
> > Here is a example:
> >
> > $ scripts/faddr2line vmlinux native_write_msr+0x6
> > native_write_msr+0x6/0x20:
> > arch_static_branch at arch/x86/include/asm/msr.h:105
> > 100 return EAX_EDX_VAL(val, low, high);
> > 101 }
> > 102
> > 103 static inline void notrace __wrmsr(unsigned int msr, u32 low, u32 high)
> > 104 {
> > 105 asm volatile("1: wrmsr\n"
> > 106 "2:\n"
> > 107 _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_wrmsr_unsafe)
> > 108 : : "c" (msr), "a"(low), "d" (high) : "memory");
> > 109 }
> > 110
> > (inlined by) static_key_false at include/linux/jump_label.h:142
> > 137 #define JUMP_TYPE_LINKED 2UL
> > 138 #define JUMP_TYPE_MASK 3UL
> > 139
> > 140 static __always_inline bool static_key_false(struct static_key *key)
> > 141 {
> > 142 return arch_static_branch(key, false);
> > 143 }
> > 144
> > 145 static __always_inline bool static_key_true(struct static_key *key)
> > 146 {
> > 147 return !arch_static_branch(key, true);
> > (inlined by) native_write_msr at arch/x86/include/asm/msr.h:150
> > 145 static inline void notrace
> > 146 native_write_msr(unsigned int msr, u32 low, u32 high)
> > 147 {
> > 148 __wrmsr(msr, low, high);
> > 149
> > 150 if (msr_tracepoint_active(__tracepoint_write_msr))
> > 151 do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
> > 152 }
> > 153
> > 154 /* Can be uninlined because referenced by paravirt */
> > 155 static inline int notrace
>
> Not a fan of this :-/ And you didn't even make it optional. Nor did you
> Cc the original author of the tool.
Something like so fixes it up and attempts to make the --list version
more readable.
---
scripts/faddr2line | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/scripts/faddr2line b/scripts/faddr2line
index 1876a741087c..a0149db00be7 100755
--- a/scripts/faddr2line
+++ b/scripts/faddr2line
@@ -56,7 +56,7 @@ command -v ${SIZE} >/dev/null 2>&1 || die "size isn't installed"
command -v ${NM} >/dev/null 2>&1 || die "nm isn't installed"
usage() {
- echo "usage: faddr2line <object file> <func+offset> <func+offset>..." >&2
+ echo "usage: faddr2line [--list] <object file> <func+offset> <func+offset>..." >&2
exit 1
}
@@ -166,15 +166,25 @@ __faddr2line() {
local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
[[ -z $file_lines ]] && return
+ if [[ $LIST = 0 ]]; then
+ echo "$file_lines" | while read -r line
+ do
+ echo $line
+ done
+ DONE=1;
+ return
+ fi
+
# show each line with context
echo "$file_lines" | while read -r line
do
+ echo
echo $line
n=$(echo $line | sed 's/.*:\([0-9]\+\).*/\1/g')
n1=$[$n-5]
n2=$[$n+5]
f=$(echo $line | sed 's/.*at \(.\+\):.*/\1/g')
- awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") {printf("%d\t%s\n", NR, $0)}' $f
+ awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") { if (NR=='$n') printf(">%d<", NR); else printf(" %d ", NR); printf("\t%s\n", $0)}' $f
done
DONE=1
@@ -185,6 +195,10 @@ __faddr2line() {
[[ $# -lt 2 ]] && usage
objfile=$1
+
+LIST=0
+[[ "$objfile" == "--list" ]] && LIST=1 && shift && objfile=$1
+
[[ ! -f $objfile ]] && die "can't find objfile $objfile"
shift
On Tue, May 29, 2018 at 06:26:36PM +0200, Peter Zijlstra wrote:
> On Tue, May 29, 2018 at 06:03:32PM +0200, Peter Zijlstra wrote:
> > On Mon, Mar 19, 2018 at 03:23:25PM +0800, [email protected] wrote:
> > > From: Changbin Du <[email protected]>
> > >
> > > Inspired by gdb command 'list', show the code context of target lines.
> > > Here is a example:
> > >
> > > $ scripts/faddr2line vmlinux native_write_msr+0x6
> > > native_write_msr+0x6/0x20:
> > > arch_static_branch at arch/x86/include/asm/msr.h:105
> > > 100 return EAX_EDX_VAL(val, low, high);
> > > 101 }
> > > 102
> > > 103 static inline void notrace __wrmsr(unsigned int msr, u32 low, u32 high)
> > > 104 {
> > > 105 asm volatile("1: wrmsr\n"
> > > 106 "2:\n"
> > > 107 _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_wrmsr_unsafe)
> > > 108 : : "c" (msr), "a"(low), "d" (high) : "memory");
> > > 109 }
> > > 110
> > > (inlined by) static_key_false at include/linux/jump_label.h:142
> > > 137 #define JUMP_TYPE_LINKED 2UL
> > > 138 #define JUMP_TYPE_MASK 3UL
> > > 139
> > > 140 static __always_inline bool static_key_false(struct static_key *key)
> > > 141 {
> > > 142 return arch_static_branch(key, false);
> > > 143 }
> > > 144
> > > 145 static __always_inline bool static_key_true(struct static_key *key)
> > > 146 {
> > > 147 return !arch_static_branch(key, true);
> > > (inlined by) native_write_msr at arch/x86/include/asm/msr.h:150
> > > 145 static inline void notrace
> > > 146 native_write_msr(unsigned int msr, u32 low, u32 high)
> > > 147 {
> > > 148 __wrmsr(msr, low, high);
> > > 149
> > > 150 if (msr_tracepoint_active(__tracepoint_write_msr))
> > > 151 do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
> > > 152 }
> > > 153
> > > 154 /* Can be uninlined because referenced by paravirt */
> > > 155 static inline int notrace
> >
> > Not a fan of this :-/ And you didn't even make it optional. Nor did you
> > Cc the original author of the tool.
>
> Something like so fixes it up and attempts to make the --list version
> more readable.
>
> ---
> scripts/faddr2line | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/faddr2line b/scripts/faddr2line
> index 1876a741087c..a0149db00be7 100755
> --- a/scripts/faddr2line
> +++ b/scripts/faddr2line
> @@ -56,7 +56,7 @@ command -v ${SIZE} >/dev/null 2>&1 || die "size isn't installed"
> command -v ${NM} >/dev/null 2>&1 || die "nm isn't installed"
>
> usage() {
> - echo "usage: faddr2line <object file> <func+offset> <func+offset>..." >&2
> + echo "usage: faddr2line [--list] <object file> <func+offset> <func+offset>..." >&2
> exit 1
> }
>
> @@ -166,15 +166,25 @@ __faddr2line() {
> local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
> [[ -z $file_lines ]] && return
>
> + if [[ $LIST = 0 ]]; then
> + echo "$file_lines" | while read -r line
> + do
> + echo $line
> + done
> + DONE=1;
> + return
> + fi
> +
> # show each line with context
> echo "$file_lines" | while read -r line
> do
> + echo
> echo $line
> n=$(echo $line | sed 's/.*:\([0-9]\+\).*/\1/g')
> n1=$[$n-5]
> n2=$[$n+5]
> f=$(echo $line | sed 's/.*at \(.\+\):.*/\1/g')
> - awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") {printf("%d\t%s\n", NR, $0)}' $f
> + awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") { if (NR=='$n') printf(">%d<", NR); else printf(" %d ", NR); printf("\t%s\n", $0)}' $f
> done
>
> DONE=1
> @@ -185,6 +195,10 @@ __faddr2line() {
> [[ $# -lt 2 ]] && usage
>
> objfile=$1
> +
> +LIST=0
> +[[ "$objfile" == "--list" ]] && LIST=1 && shift && objfile=$1
> +
> [[ ! -f $objfile ]] && die "can't find objfile $objfile"
> shift
Yeah, this change really should have been an optional arg. It hurt the
readability and compactness of the output. The above looks good to me.
Care to send a proper patch? If you send it to Linus he might apply it
directly as he did with my original patches.
--
Josh
On Tue, May 29, 2018 at 12:07:10PM -0500, Josh Poimboeuf wrote:
> Yeah, this change really should have been an optional arg. It hurt the
> readability and compactness of the output. The above looks good to me.
>
> Care to send a proper patch? If you send it to Linus he might apply it
> directly as he did with my original patches.
---
From: Peter Zijlstra (Intel) <[email protected]>
Commit 6870c0165feaa5 ("scripts/faddr2line: show the code context")
radically altered the output format of the faddr2line tool. And while
the new list output format might have merrit it broke my vim usage and
was hard to read.
Make the new format optional; using a '--list' argument and attempt to
make the output slightly easier to read by adding a little whitespace to
separate the different files and explicitly mark the line in question.
Cc: Changbin Du <[email protected]>
Acked-by: Josh Poimboeuf <[email protected]>
Fixes: 6870c0165feaa5 ("scripts/faddr2line: show the code context")
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
scripts/faddr2line | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/scripts/faddr2line b/scripts/faddr2line
index 1876a741087c..a0149db00be7 100755
--- a/scripts/faddr2line
+++ b/scripts/faddr2line
@@ -56,7 +56,7 @@ command -v ${SIZE} >/dev/null 2>&1 || die "size isn't installed"
command -v ${NM} >/dev/null 2>&1 || die "nm isn't installed"
usage() {
- echo "usage: faddr2line <object file> <func+offset> <func+offset>..." >&2
+ echo "usage: faddr2line [--list] <object file> <func+offset> <func+offset>..." >&2
exit 1
}
@@ -166,15 +166,25 @@ __faddr2line() {
local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
[[ -z $file_lines ]] && return
+ if [[ $LIST = 0 ]]; then
+ echo "$file_lines" | while read -r line
+ do
+ echo $line
+ done
+ DONE=1;
+ return
+ fi
+
# show each line with context
echo "$file_lines" | while read -r line
do
+ echo
echo $line
n=$(echo $line | sed 's/.*:\([0-9]\+\).*/\1/g')
n1=$[$n-5]
n2=$[$n+5]
f=$(echo $line | sed 's/.*at \(.\+\):.*/\1/g')
- awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") {printf("%d\t%s\n", NR, $0)}' $f
+ awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") { if (NR=='$n') printf(">%d<", NR); else printf(" %d ", NR); printf("\t%s\n", $0)}' $f
done
DONE=1
@@ -185,6 +195,10 @@ __faddr2line() {
[[ $# -lt 2 ]] && usage
objfile=$1
+
+LIST=0
+[[ "$objfile" == "--list" ]] && LIST=1 && shift && objfile=$1
+
[[ ! -f $objfile ]] && die "can't find objfile $objfile"
shift
On Tue, May 29, 2018 at 07:24:30PM +0200, Peter Zijlstra wrote:
> From: Peter Zijlstra (Intel) <[email protected]>
Shees, you'd figure I could type my own email address by now..
From: Peter Zijlstra (Intel) <[email protected]>
>
> Commit 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> radically altered the output format of the faddr2line tool. And while
> the new list output format might have merrit it broke my vim usage and
> was hard to read.
>
> Make the new format optional; using a '--list' argument and attempt to
> make the output slightly easier to read by adding a little whitespace to
> separate the different files and explicitly mark the line in question.
>
> Cc: Changbin Du <[email protected]>
> Acked-by: Josh Poimboeuf <[email protected]>
> Fixes: 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> scripts/faddr2line | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/faddr2line b/scripts/faddr2line
> index 1876a741087c..a0149db00be7 100755
> --- a/scripts/faddr2line
> +++ b/scripts/faddr2line
> @@ -56,7 +56,7 @@ command -v ${SIZE} >/dev/null 2>&1 || die "size isn't installed"
> command -v ${NM} >/dev/null 2>&1 || die "nm isn't installed"
>
> usage() {
> - echo "usage: faddr2line <object file> <func+offset> <func+offset>..." >&2
> + echo "usage: faddr2line [--list] <object file> <func+offset> <func+offset>..." >&2
> exit 1
> }
>
> @@ -166,15 +166,25 @@ __faddr2line() {
> local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
> [[ -z $file_lines ]] && return
>
> + if [[ $LIST = 0 ]]; then
> + echo "$file_lines" | while read -r line
> + do
> + echo $line
> + done
> + DONE=1;
> + return
> + fi
> +
> # show each line with context
> echo "$file_lines" | while read -r line
> do
> + echo
> echo $line
> n=$(echo $line | sed 's/.*:\([0-9]\+\).*/\1/g')
> n1=$[$n-5]
> n2=$[$n+5]
> f=$(echo $line | sed 's/.*at \(.\+\):.*/\1/g')
> - awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") {printf("%d\t%s\n", NR, $0)}' $f
> + awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") { if (NR=='$n') printf(">%d<", NR); else printf(" %d ", NR); printf("\t%s\n", $0)}' $f
> done
>
> DONE=1
> @@ -185,6 +195,10 @@ __faddr2line() {
> [[ $# -lt 2 ]] && usage
>
> objfile=$1
> +
> +LIST=0
> +[[ "$objfile" == "--list" ]] && LIST=1 && shift && objfile=$1
> +
> [[ ! -f $objfile ]] && die "can't find objfile $objfile"
> shift
>
On Tue, May 29 2018, Peter Zijlstra wrote:
> On Tue, May 29, 2018 at 12:07:10PM -0500, Josh Poimboeuf wrote:
>> Yeah, this change really should have been an optional arg. It hurt the
>> readability and compactness of the output. The above looks good to me.
>>
>> Care to send a proper patch? If you send it to Linus he might apply it
>> directly as he did with my original patches.
>
> ---
> From: Peter Zijlstra (Intel) <[email protected]>
>
> Commit 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> radically altered the output format of the faddr2line tool. And while
> the new list output format might have merrit it broke my vim usage and
> was hard to read.
>
> Make the new format optional; using a '--list' argument and attempt to
> make the output slightly easier to read by adding a little whitespace to
> separate the different files and explicitly mark the line in question.
Not commenting on your code but on the original patch.....
I've recently noticed that ADDR2LINE sometimes outputs
(discriminator 2)
or similar at the end of the line. This messes up the parsing.
I hacked it to work so I could keep debugging with
- local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
+ local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed -e "s; $dir_prefix\(\./\)*; ;" -e "s/(discriminator [0-9]*)//")
but someone should probably find out exactly what sort of messages
ADDR2LINE produces, and make sure they are all parsed correctly.
(maybe that someone should be me, but not today).
Thanks,
NeilBrown
>
> Cc: Changbin Du <[email protected]>
> Acked-by: Josh Poimboeuf <[email protected]>
> Fixes: 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> scripts/faddr2line | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/faddr2line b/scripts/faddr2line
> index 1876a741087c..a0149db00be7 100755
> --- a/scripts/faddr2line
> +++ b/scripts/faddr2line
> @@ -56,7 +56,7 @@ command -v ${SIZE} >/dev/null 2>&1 || die "size isn't installed"
> command -v ${NM} >/dev/null 2>&1 || die "nm isn't installed"
>
> usage() {
> - echo "usage: faddr2line <object file> <func+offset> <func+offset>..." >&2
> + echo "usage: faddr2line [--list] <object file> <func+offset> <func+offset>..." >&2
> exit 1
> }
>
> @@ -166,15 +166,25 @@ __faddr2line() {
> local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
> [[ -z $file_lines ]] && return
>
> + if [[ $LIST = 0 ]]; then
> + echo "$file_lines" | while read -r line
> + do
> + echo $line
> + done
> + DONE=1;
> + return
> + fi
> +
> # show each line with context
> echo "$file_lines" | while read -r line
> do
> + echo
> echo $line
> n=$(echo $line | sed 's/.*:\([0-9]\+\).*/\1/g')
> n1=$[$n-5]
> n2=$[$n+5]
> f=$(echo $line | sed 's/.*at \(.\+\):.*/\1/g')
> - awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") {printf("%d\t%s\n", NR, $0)}' $f
> + awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") { if (NR=='$n') printf(">%d<", NR); else printf(" %d ", NR); printf("\t%s\n", $0)}' $f
> done
>
> DONE=1
> @@ -185,6 +195,10 @@ __faddr2line() {
> [[ $# -lt 2 ]] && usage
>
> objfile=$1
> +
> +LIST=0
> +[[ "$objfile" == "--list" ]] && LIST=1 && shift && objfile=$1
> +
> [[ ! -f $objfile ]] && die "can't find objfile $objfile"
> shift
>
On Wed, May 30, 2018 at 08:01:48AM +1000, NeilBrown wrote:
> On Tue, May 29 2018, Peter Zijlstra wrote:
>
> > On Tue, May 29, 2018 at 12:07:10PM -0500, Josh Poimboeuf wrote:
> >> Yeah, this change really should have been an optional arg. It hurt the
> >> readability and compactness of the output. The above looks good to me.
> >>
> >> Care to send a proper patch? If you send it to Linus he might apply it
> >> directly as he did with my original patches.
> >
> > ---
> > From: Peter Zijlstra (Intel) <[email protected]>
> >
> > Commit 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> > radically altered the output format of the faddr2line tool. And while
> > the new list output format might have merrit it broke my vim usage and
> > was hard to read.
> >
> > Make the new format optional; using a '--list' argument and attempt to
> > make the output slightly easier to read by adding a little whitespace to
> > separate the different files and explicitly mark the line in question.
>
> Not commenting on your code but on the original patch.....
> I've recently noticed that ADDR2LINE sometimes outputs
> (discriminator 2)
> or similar at the end of the line. This messes up the parsing.
>
> I hacked it to work so I could keep debugging with
>
> - local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
> + local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed -e "s; $dir_prefix\(\./\)*; ;" -e "s/(discriminator [0-9]*)//")
>
> but someone should probably find out exactly what sort of messages
> ADDR2LINE produces, and make sure they are all parsed correctly.
> (maybe that someone should be me, but not today).
>
Hi, I have fixed it by commit 78eb0c6356 ("scripts/faddr2line: fix error when
addr2line output contains discriminator") and it is already in the mainline now.
Thank you!
> Thanks,
> NeilBrown
>
>
> >
> > Cc: Changbin Du <[email protected]>
> > Acked-by: Josh Poimboeuf <[email protected]>
> > Fixes: 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> > Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> > ---
> > scripts/faddr2line | 18 ++++++++++++++++--
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/faddr2line b/scripts/faddr2line
> > index 1876a741087c..a0149db00be7 100755
> > --- a/scripts/faddr2line
> > +++ b/scripts/faddr2line
> > @@ -56,7 +56,7 @@ command -v ${SIZE} >/dev/null 2>&1 || die "size isn't installed"
> > command -v ${NM} >/dev/null 2>&1 || die "nm isn't installed"
> >
> > usage() {
> > - echo "usage: faddr2line <object file> <func+offset> <func+offset>..." >&2
> > + echo "usage: faddr2line [--list] <object file> <func+offset> <func+offset>..." >&2
> > exit 1
> > }
> >
> > @@ -166,15 +166,25 @@ __faddr2line() {
> > local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; $dir_prefix\(\./\)*; ;")
> > [[ -z $file_lines ]] && return
> >
> > + if [[ $LIST = 0 ]]; then
> > + echo "$file_lines" | while read -r line
> > + do
> > + echo $line
> > + done
> > + DONE=1;
> > + return
> > + fi
> > +
> > # show each line with context
> > echo "$file_lines" | while read -r line
> > do
> > + echo
> > echo $line
> > n=$(echo $line | sed 's/.*:\([0-9]\+\).*/\1/g')
> > n1=$[$n-5]
> > n2=$[$n+5]
> > f=$(echo $line | sed 's/.*at \(.\+\):.*/\1/g')
> > - awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") {printf("%d\t%s\n", NR, $0)}' $f
> > + awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") { if (NR=='$n') printf(">%d<", NR); else printf(" %d ", NR); printf("\t%s\n", $0)}' $f
> > done
> >
> > DONE=1
> > @@ -185,6 +195,10 @@ __faddr2line() {
> > [[ $# -lt 2 ]] && usage
> >
> > objfile=$1
> > +
> > +LIST=0
> > +[[ "$objfile" == "--list" ]] && LIST=1 && shift && objfile=$1
> > +
> > [[ ! -f $objfile ]] && die "can't find objfile $objfile"
> > shift
> >
--
Thanks,
Changbin Du
On Tue, May 29, 2018 at 06:03:32PM +0200, Peter Zijlstra wrote:
> On Mon, Mar 19, 2018 at 03:23:25PM +0800, [email protected] wrote:
> > From: Changbin Du <[email protected]>
> >
> > Inspired by gdb command 'list', show the code context of target lines.
> > Here is a example:
> >
> > $ scripts/faddr2line vmlinux native_write_msr+0x6
> > native_write_msr+0x6/0x20:
> > arch_static_branch at arch/x86/include/asm/msr.h:105
> > 100 return EAX_EDX_VAL(val, low, high);
> > 101 }
> > 102
> > 103 static inline void notrace __wrmsr(unsigned int msr, u32 low, u32 high)
> > 104 {
> > 105 asm volatile("1: wrmsr\n"
> > 106 "2:\n"
> > 107 _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_wrmsr_unsafe)
> > 108 : : "c" (msr), "a"(low), "d" (high) : "memory");
> > 109 }
> > 110
> > (inlined by) static_key_false at include/linux/jump_label.h:142
> > 137 #define JUMP_TYPE_LINKED 2UL
> > 138 #define JUMP_TYPE_MASK 3UL
> > 139
> > 140 static __always_inline bool static_key_false(struct static_key *key)
> > 141 {
> > 142 return arch_static_branch(key, false);
> > 143 }
> > 144
> > 145 static __always_inline bool static_key_true(struct static_key *key)
> > 146 {
> > 147 return !arch_static_branch(key, true);
> > (inlined by) native_write_msr at arch/x86/include/asm/msr.h:150
> > 145 static inline void notrace
> > 146 native_write_msr(unsigned int msr, u32 low, u32 high)
> > 147 {
> > 148 __wrmsr(msr, low, high);
> > 149
> > 150 if (msr_tracepoint_active(__tracepoint_write_msr))
> > 151 do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
> > 152 }
> > 153
> > 154 /* Can be uninlined because referenced by paravirt */
> > 155 static inline int notrace
>
> Not a fan of this :-/ And you didn't even make it optional. Nor did you
> Cc the original author of the tool.
Yeah, understand your compatibility concern, and thanks for your improvment.
I only added people from 'scripts/get_maintainer.pl'.
--
Thanks,
Changbin Du