PR30000 - introduce new srcfiles tool

[Aaron Merey]

Hi Housam,

On Thu, Sep 28, 2023 at 11:00 AM Housam Alamour via Elfutils-devel
<elfutils-devel@sourceware.org> wrote:
>
> Here is the new srcfiles tool ready for review.

Thanks for working on this.

I was not able to apply the patch from your email due to some line
breaks that your email client may have inserted.  `git format-patch` and
`git send-email` are useful for emailing patches with the right format.
However I was able to test the patch using your try-pr30000 git branch.

> diff --git a/doc/srcfiles.1 b/doc/srcfiles.1
> new file mode 100644
> index 00000000..245c2d27
> --- /dev/null
> +++ b/doc/srcfiles.1

srcfile.1 does not install since it's missing from doc/Makefile.am.

> @@ -0,0 +1,125 @@
> +.\" Copyright 2023 Red Hat Inc.
> +.\" Mon 2023-Sept 23 Housam Alamour <halamour@redhat.com>
> +.\" Contact elfutils-devel@sourceware.org to correct errors or typos.
> +.TH EU-SRCFILES 1 "2023-Sept-25" "elfutils"
> +
> +.de SAMPLE
> +.br
> +.RS 0
> +.nf
> +.nh
> +\fB
> +..
> +.de ESAMPLE
> +\fP
> +.hy
> +.fi
> +.RE
> +..
> +
> +.SH "NAME"
> +eu-srcfiles \- Lists the source files of a dwarf/elf file.

DWARF and ELF are usually capitalized.

> +
> +.SH "SYNOPSIS"
> +eu-srcfiles [\fB\-0\fR|\fB\-\-null\fR] [\fB\-c\fR|\fB\-\-cu\-only\fR]
> [\fB\-v\fR|\fB\-\-verbose\fR] INPUT

This description of the arg format differs from `eu-srcfiles --help`

    $ eu-srcfiles --help
    Usage: eu-srcfiles [OPTION...] [FILE]
    [...]

The --help description implies that `eu-srcfiles ./prog` should work
for some executable named prog but this is not the case.
The man page is correct that I have to include -e or --core for this
to work and that '-e a.out' is used as a default file name.

> +
> +.SH "DESCRIPTION"
> +\fBeu-srcfiles\fR lists the source files of a given \s-1dwarf/elf\s0
> +file.  This list is based on a search of the dwarf debuginfo, which
> +may be automatically fetched by debuginfod if applicable.  The target
> +file may be an executable, a coredump, a process, or even the running
> +kernel.  The default is the file 'a.out'.  The source file names are
> +made unique and printed to standard output.
> +
> +.SH "OPTIONS"
> +The long and short forms of options, shown here as alternatives, are
> +equivalent.
> +
> +.SS "Input Options"
> +
> +Only one INPUT option may be used.  The default is '-e a.out'.

There are some input options, such as -k, that are listed in --help but
not included in the man page.

> diff --git a/src/srcfiles.cxx b/src/srcfiles.cxx
> new file mode 100644
> index 00000000..c948269d
> [...]
> +int
> +main (int argc, char *argv[])
> +{
> +  int remaining;
> +
> +  /* Parse and process arguments.  This includes opening the modules.  */
> +  argp_children[0].argp = dwfl_standard_argp ();
> +  argp_children[0].group = 1;
> +
> +  Dwfl *dwfl = NULL;
> +  (void) argp_parse (&argp, argc, argv, 0, &remaining, &dwfl);
> +  assert (dwfl != NULL);
> +  // Process all loaded modules - probably just one, except if -K or -p is
> used.

The use of /* */ and // for comments is inconsistent.  Although to be
fair it's inconsistent in parts of elfutils too!

> diff --git a/tests/run-srcfiles-self.sh b/tests/run-srcfiles-self.sh
> new file mode 100755
> index 00000000..075ff7d2
> --- /dev/null
> +++ b/tests/run-srcfiles-self.sh
> @@ -0,0 +1,45 @@
> +#! /bin/sh
> +# Copyright (C) 2023 Red Hat, Inc.
> +# This file is part of elfutils.
> +#
> +# This file is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# elfutils is distributed in the hope that it will be useful, but
> +# WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +. $srcdir/test-subr.sh
> +
> +# Test different command line combinations on the srcfiles binary itself.
> +ET_EXEC="${abs_top_builddir}/src/srcfiles"
> +ET_PID=$$
> +
> +SRC_NAME="srcfiles.cxx"
> +
> +for null_arg in --null ""; do
> +  for verbose_arg in --verbose ""; do
> +    testrun $ET_EXEC $null_arg $verbose_arg -p $ET_PID > /dev/null
> +
> +    # Ensure that the output contains srclines.cxx
> +    cu_only=$(testrun $ET_EXEC $null_arg $verbose_arg -c -e $ET_EXEC)
> +    default=$(testrun $ET_EXEC $null_arg $verbose_arg -e $ET_EXEC)
> +    result1=$(echo "$cu_only" | grep "$SRC_NAME")
> +    result2=$(echo "$default" | grep "$SRC_NAME")
> +    if [ -z "$result1" ] || [-z "$result2"]; then
> +      exit 1
> +    fi
> +
> +    # Ensure that the output with the cu-only option contains less source
> files
> +    if [ "$cu_only" -gt "$default" ]; then
> +      exit 1
> +    fi
> +  done
> +done
> +

These tests pass for me on F38 x86_64.

It looks like this only tests whether different command line options
generate the same output or not.  Would it be possible to add a test
that checks whether a particular source file actually appears in the
output as intended?  For example, you could test whether src/srcfiles.cxx
appears in the output of "$ET_EXEC -e $ET_EXEC".

Aaron