Re: [PATCH] Fix macro info lookup for binaries containing DWARFv5 line table

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

On 2021-05-12 1:16 p.m., Sourabh Singh Tomar via Gdb-patches wrote:
> Consider following C test case:
> ```
> cat test.c
> int main() {
>         int a = OUT;
> }
> ```
> Compiled as:
> `clang -gdwarf-5 -fdebug-macro test.c`
> 
> Loading this bin into GDB and trying to access macro info,
> GDB is unable to extract and display macro info correctly.
> ```
> $gdb a.out
> (gdb) start
> (gdb) info macro OUT
> Temporary breakpoint 1 at 0x2016a8: file test.c, line 3.
> Starting program: /home/a.out
> (gdb) info macro OUT
> The symbol `OUT' has no definition as a C/C++ preprocessor macro
> at /home/test.c:-1
> ```
> 
> The problems starts when processing file and dir indexes from DWARFv5
> line table.  DWARFv5 specifies:
>   - The current directory is explicitly present in the
>     directories field.
>   - The current compilation filename is explicitly present and
>     has index 0.
> dir entry 0 contains the absoute path(Not including source
> filename).

absoute -> absolute

You could say that it contains the "compilation directory", typically
the current working directory of the compiler.

> diff --git a/gdb/testsuite/gdb.base/dwarf5-macro.c b/gdb/testsuite/gdb.base/dwarf5-macro.c
> new file mode 100644
> index 00000000000..0a7dc98c620
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/dwarf5-macro.c
> @@ -0,0 +1,3 @@
> +#define HELLO_GDB 1
> +int main() {
> +}

Please add a copyright notice, like in the other .c files.

> diff --git a/gdb/testsuite/gdb.base/dwarf5-macro.exp b/gdb/testsuite/gdb.base/dwarf5-macro.exp
> new file mode 100644
> index 00000000000..cf622cebe32
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/dwarf5-macro.exp
> @@ -0,0 +1,43 @@
> +# Copyright 2020-2021 Free Software Foundation, Inc.
> +
> +# This program 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.
> +#
> +# This program 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/>.

For others in the future to be able to understand why this test exists,
please add a comment here explaining what this test case is about, what
is it specifically meant to check.  It would be good to note that for
the bug to reproduce, the source file must be supplied as a relative
path, so that the file_names[0] entry in .debug_line ends up relative
and not absolute.  That will help justify why we do a copy below, and
why we pass such an hardcoded path to gdb_compile.  This is unusual, so
somebody might be tempted to "clean this up", breaking the purpose of
the test.

> +
> +standard_testfile
> +set additional_flags {debug}
> +file copy -force $srcdir/$subdir/$srcfile [standard_output_file $srcfile]
> +gdb_compile outputs/gdb.base/dwarf5-macro/dwarf5-macro.c $binfile executable {debug}

We typically check for gdb_compile failures, check for examples
elsewhere in the testsuite.

As Tom said, please comment adequatly to explain why we do it this way.

It would perhaps make sense to move this test to gdb.dwarf2?  Also, the
name "dwarf5-macro" is a bit broad.  If we want to add other tests
related to macro handling in DWARF5, we'll be a bit stuck (we'll have to
rename this one).  I would suggest giving it a more specific name, like
dwarf5-macro-relative-path", something like that.

> +
> +get_compiler_info
> +if { [test_compiler_info "gcc-*"] } {
> +    set options "additional_flags=-g3"
> +} elseif { [test_compiler_info "clang-*"] } {
> +    set options "additional_flags=-gdwarf-5 -fdebug-macro"
> +}
> +
> +if { [prepare_for_testing "failed to prepare" \

prepare_for_testing builds the executable, doesn't it?  And it ends up
overwriting the file you compiled above, don't you see that?  The test
fails for me because of this, I think.

Simon