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) <pet...@infraded.org>
> >
> > 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 <changbin...@intel.com>
> > Acked-by: Josh Poimboeuf <jpoim...@redhat.com>
> > Fixes: 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> > Signed-off-by: Peter Zijlstra (Intel) <pet...@infradead.org>
> > ---
> > 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
