[1/2] Fix reverse stepping multiple contiguous PC ranges over the line table.

  Simon, GDB maintainers:

Per the comments on version 4 for the gdb.reverse/func-map-to-same-
line.exp, I have added support to proc gdb_compile to enable or disable
generating line information as part of the debug information.  The two
new options are column-info and no-column-info.  

This patch implements the new options for gdb_compile.

These options have been tested with patch 2 of 2 on PowerPC with the
GCC and clang compilers.

Please let me know if the patch is acceptable for mainline.   Thanks.

                       Carl 

-------------------------------------------------------------

Add gdb_compile options column-info and no-column-info

This patch adds two new options to gdb_compile to specify if the compile
should or should not generate the line table information.  The
options are supported on clang and gcc version 7 and newer.

Patch has been tested on PowerPC with both gcc and clang.
---
 gdb/testsuite/lib/gdb.exp | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
  

On 5/16/23 18:54, Carl Love wrote:
> Simon, GDB maintainers:
> 
> Per the comments on version 4 for the gdb.reverse/func-map-to-same-
> line.exp, I have added support to proc gdb_compile to enable or disable
> generating line information as part of the debug information.  The two
> new options are column-info and no-column-info.  
> 
> This patch implements the new options for gdb_compile.
> 
> These options have been tested with patch 2 of 2 on PowerPC with the
> GCC and clang compilers.
> 
> Please let me know if the patch is acceptable for mainline.   Thanks.
> 
>                        Carl 
> 
> -------------------------------------------------------------
> 
> Add gdb_compile options column-info and no-column-info
> 
> This patch adds two new options to gdb_compile to specify if the compile
> should or should not generate the line table information.  The
> options are supported on clang and gcc version 7 and newer.
> 
> Patch has been tested on PowerPC with both gcc and clang.
> ---
>  gdb/testsuite/lib/gdb.exp | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index aed7e2d043c..e993fddf4c7 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -4794,6 +4794,8 @@ proc quote_for_host { args } {
>  #     debug information
>  #   - text_segment=addr: Tell the linker to place the text segment at ADDR.
>  #   - build-id: Ensure the final binary includes a build-id.
> +#   - no-column-info: Disable generation of column table information.
> +#   - column-info: Enable generation of column table information.
>  #
>  # And here are some of the not too obscure options understood by DejaGnu that
>  # influence the compilation:
> @@ -5003,6 +5005,34 @@ proc gdb_compile {source dest type options} {
>              } else {
>                  error "Don't know how to handle text_segment option."
>              }
> +	} elseif { $opt == "column-info" } {
> +	    if {[test_compiler_info {gcc-*}]} {
> +		if {[test_compiler_info {gcc-[1-6]-*}]} {
> +		    error "gdb_compile option no-column-info not supported."

I think this path should return the equivalent of "failed to compile",
instead of throwing an error.  Control will go back to the test, which
will generally skip the portion of the test that requires that
binary.

> +		}
> +		lappend new_options "additional_flags=-gcolumn-info"
> +
> +	    } elseif {[test_compiler_info {clang-*}]} {
> +		lappend new_options "additional_flags=-gcolumn-info"
> +
> +	    } else {
> +		error "Don't know how to handle gcolumn-info option."

I think it's ok to throw an error in this path.  If you are testing
against a compiler that we don't know about, it will produce errors that
are easy to spot, and you'll be able to add support for your compiler
here.

Simon
  

Simon:

On Mon, 2023-06-19 at 13:11 -0400, Simon Marchi wrote:
> > --- a/gdb/testsuite/lib/gdb.exp
> > +++ b/gdb/testsuite/lib/gdb.exp
> > @@ -4794,6 +4794,8 @@ proc quote_for_host { args } {
> >   #     debug information
> >   #   - text_segment=addr: Tell the linker to place the text
> > segment at ADDR.
> >   #   - build-id: Ensure the final binary includes a build-id.
> > +#   - no-column-info: Disable generation of column table
> > information.
> > +#   - column-info: Enable generation of column table information.
> >   #
> >   # And here are some of the not too obscure options understood by
> > DejaGnu that
> >   # influence the compilation:
> > @@ -5003,6 +5005,34 @@ proc gdb_compile {source dest type options}
> > {
> >               } else {
> >                   error "Don't know how to handle text_segment
> > option."
> >               }
> > +     } elseif { $opt == "column-info" } {
> > +         if {[test_compiler_info {gcc-*}]} {
> > +             if {[test_compiler_info {gcc-[1-6]-*}]} {
> > +                 error "gdb_compile option no-column-info not
> > supported."
> 
> I think this path should return the equivalent of "failed to
> compile",
> instead of throwing an error.  Control will go back to the test,
> which
> will generally skip the portion of the test that requires that
> binary.

Not entirely sure how to accomplish what you are looking for.

I change:
  error "gdb_compile option no-column-info not supported."
to
  set result "option no-column-info not supported."
  clone_output "gdb compile failed, $result"
  return 1

When I force the if {[test_compiler_info...]} tp be true to test this,
I get:

   get_compiler_info: gcc-12-2-1
   gdb compile failed, option no-column-info not supported.
   UNTESTED: gdb.reverse/func-map-to-same-line.exp:
   with_column_info=yes: failed t\
   o prepare
   testcase /home/carll/GDB/build-reverse-multiple-
   contiguous/gdb/testsuite/../../\
   ../binutils-gdb-reverse-multiple-
   contiguous/gdb/testsuite/gdb.reverse/func-map-\
   to-same-line.exp completed in 0 seconds

                   === gdb Summary ===

   # of untested testcases         1

The test case doesn't have any part of the test that doesn't require
compiling so it is not clear if that would work with this fix.  Anyway,
wanted to run that by you to see if this is an appropriate fix?  I am
really not sure about it.  Thanks.

                     Carl
  

On 6/22/23 12:52, Carl Love wrote:
> 
> Simon:
> 
> On Mon, 2023-06-19 at 13:11 -0400, Simon Marchi wrote:
>>> --- a/gdb/testsuite/lib/gdb.exp
>>> +++ b/gdb/testsuite/lib/gdb.exp
>>> @@ -4794,6 +4794,8 @@ proc quote_for_host { args } {
>>>   #     debug information
>>>   #   - text_segment=addr: Tell the linker to place the text
>>> segment at ADDR.
>>>   #   - build-id: Ensure the final binary includes a build-id.
>>> +#   - no-column-info: Disable generation of column table
>>> information.
>>> +#   - column-info: Enable generation of column table information.
>>>   #
>>>   # And here are some of the not too obscure options understood by
>>> DejaGnu that
>>>   # influence the compilation:
>>> @@ -5003,6 +5005,34 @@ proc gdb_compile {source dest type options}
>>> {
>>>               } else {
>>>                   error "Don't know how to handle text_segment
>>> option."
>>>               }
>>> +     } elseif { $opt == "column-info" } {
>>> +         if {[test_compiler_info {gcc-*}]} {
>>> +             if {[test_compiler_info {gcc-[1-6]-*}]} {
>>> +                 error "gdb_compile option no-column-info not
>>> supported."
>>
>> I think this path should return the equivalent of "failed to
>> compile",
>> instead of throwing an error.  Control will go back to the test,
>> which
>> will generally skip the portion of the test that requires that
>> binary.
> 
> Not entirely sure how to accomplish what you are looking for.
> 
> I change:
>   error "gdb_compile option no-column-info not supported."
> to
>   set result "option no-column-info not supported."
>   clone_output "gdb compile failed, $result"
>   return 1
> 
> When I force the if {[test_compiler_info...]} tp be true to test this,
> I get:
> 
>    get_compiler_info: gcc-12-2-1
>    gdb compile failed, option no-column-info not supported.
>    UNTESTED: gdb.reverse/func-map-to-same-line.exp:
>    with_column_info=yes: failed t\
>    o prepare
>    testcase /home/carll/GDB/build-reverse-multiple-
>    contiguous/gdb/testsuite/../../\
>    ../binutils-gdb-reverse-multiple-
>    contiguous/gdb/testsuite/gdb.reverse/func-map-\
>    to-same-line.exp completed in 0 seconds
> 
>                    === gdb Summary ===
> 
>    # of untested testcases         1
> 
> The test case doesn't have any part of the test that doesn't require
> compiling so it is not clear if that would work with this fix.  Anyway,
> wanted to run that by you to see if this is an appropriate fix?  I am
> really not sure about it.  Thanks.

I think that's the expected behavior.  The UNTESTED is emitted by
build_executable_from_specs, I think.  If the test used gdb_compile, I
think we wouldn't see an UNTESTED.  But as far as your addition is
concerned, I think it's fine.

I just thought of a simpler alternative though.  Just remove the version
check.  If we build with an older gcc, there will simply be a message
that says that the flag is not recognized, and the result should be just
the same.  I just hacked it locally and changed the flag name to be
wrong (I don't have a gcc <= 6 on hand to test).  It looks like:

    Executing on host: gcc  -fno-stack-protector  -fdiagnostics-color=never -gcolumn-info-foo -c -g  -o /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.reverse/func-map-to-same-line/func-map-to-same-line0.o /home/simark/src/binutils-gdb/gdb/testsuite/gdb.reverse/func-map-to-same-line.c    (timeout = 300)
    builtin_spawn -ignore SIGHUP gcc -fno-stack-protector -fdiagnostics-color=never -gcolumn-info-foo -c -g -o /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.reverse/func-map-to-same-line/func-map-to-same-line0.o /home/simark/src/binutils-gdb/gdb/testsuite/gdb.reverse/func-map-to-same-line.c

    gcc: error: unrecognized debug output level 'column-info-foo'

    compiler exited with status 1
    output is:
    gcc: error: unrecognized debug output level 'column-info-foo'


    gdb compile failed, gcc: error: unrecognized debug output level 'column-info-foo'
    UNTESTED: gdb.reverse/func-map-to-same-line.exp: with_column_info=yes: failed to prepare

I then thought about the "no-column-info" case.  Currently, you error
out for gccs <= 6.  However, shouldn't we just compile without any
special flag in that case, since there just wasn't any support for
column-info back then?

Simon
  

On Fri, 2023-06-23 at 13:44 -0400, Simon Marchi wrote:
> 
> On 6/22/23 12:52, Carl Love wrote:
> > Simon:
> > 
> > On Mon, 2023-06-19 at 13:11 -0400, Simon Marchi wrote:
> > > > --- a/gdb/testsuite/lib/gdb.exp
> > > > +++ b/gdb/testsuite/lib/gdb.exp
> > > > @@ -4794,6 +4794,8 @@ proc quote_for_host { args } {
> > > >   #     debug information
> > > >   #   - text_segment=addr: Tell the linker to place the text
> > > > segment at ADDR.
> > > >   #   - build-id: Ensure the final binary includes a build-id.
> > > > +#   - no-column-info: Disable generation of column table
> > > > information.
> > > > +#   - column-info: Enable generation of column table
> > > > information.
> > > >   #
> > > >   # And here are some of the not too obscure options understood
> > > > by
> > > > DejaGnu that
> > > >   # influence the compilation:
> > > > @@ -5003,6 +5005,34 @@ proc gdb_compile {source dest type
> > > > options}
> > > > {
> > > >               } else {
> > > >                   error "Don't know how to handle text_segment
> > > > option."
> > > >               }
> > > > +     } elseif { $opt == "column-info" } {
> > > > +         if {[test_compiler_info {gcc-*}]} {
> > > > +             if {[test_compiler_info {gcc-[1-6]-*}]} {
> > > > +                 error "gdb_compile option no-column-info not
> > > > supported."
> > > 
> > > I think this path should return the equivalent of "failed to
> > > compile",
> > > instead of throwing an error.  Control will go back to the test,
> > > which
> > > will generally skip the portion of the test that requires that
> > > binary.
> > 
> > Not entirely sure how to accomplish what you are looking for.
> > 
> > I change:
> >   error "gdb_compile option no-column-info not supported."
> > to
> >   set result "option no-column-info not supported."
> >   clone_output "gdb compile failed, $result"
> >   return 1
> > 
> > When I force the if {[test_compiler_info...]} tp be true to test
> > this,
> > I get:
> > 
> >    get_compiler_info: gcc-12-2-1
> >    gdb compile failed, option no-column-info not supported.
> >    UNTESTED: gdb.reverse/func-map-to-same-line.exp:
> >    with_column_info=yes: failed t\
> >    o prepare
> >    testcase /home/carll/GDB/build-reverse-multiple-
> >    contiguous/gdb/testsuite/../../\
> >    ../binutils-gdb-reverse-multiple-
> >    contiguous/gdb/testsuite/gdb.reverse/func-map-\
> >    to-same-line.exp completed in 0 seconds
> > 
> >                    === gdb Summary ===
> > 
> >    # of untested testcases         1
> > 
> > The test case doesn't have any part of the test that doesn't
> > require
> > compiling so it is not clear if that would work with this
> > fix.  Anyway,
> > wanted to run that by you to see if this is an appropriate fix?  I
> > am
> > really not sure about it.  Thanks.
> 
> I think that's the expected behavior.  The UNTESTED is emitted by
> build_executable_from_specs, I think.  If the test used gdb_compile,
> I
> think we wouldn't see an UNTESTED.  But as far as your addition is
> concerned, I think it's fine.
> 
> I just thought of a simpler alternative though.  Just remove the
> version
> check.  If we build with an older gcc, there will simply be a message
> that says that the flag is not recognized, and the result should be
> just
> the same.  I just hacked it locally and changed the flag name to be
> wrong (I don't have a gcc <= 6 on hand to test).  It looks like:

Yea, hacking the if {[test_compiler_info {gcc-[...]}  is how I have
been testing it as well.  :-)
> 
>     Executing on host: gcc  -fno-stack-protector  -fdiagnostics-
> color=never -gcolumn-info-foo -c -g  -o /home/simark/build/binutils-
> gdb/gdb/testsuite/outputs/gdb.reverse/func-map-to-same-line/func-map-
> to-same-line0.o /home/simark/src/binutils-
> gdb/gdb/testsuite/gdb.reverse/func-map-to-same-line.c    (timeout =
> 300)
>     builtin_spawn -ignore SIGHUP gcc -fno-stack-protector
> -fdiagnostics-color=never -gcolumn-info-foo -c -g -o
> /home/simark/build/binutils-
> gdb/gdb/testsuite/outputs/gdb.reverse/func-map-to-same-line/func-map-
> to-same-line0.o /home/simark/src/binutils-
> gdb/gdb/testsuite/gdb.reverse/func-map-to-same-line.c
> 
>     gcc: error: unrecognized debug output level 'column-info-foo'
> 
>     compiler exited with status 1
>     output is:
>     gcc: error: unrecognized debug output level 'column-info-foo'
> 
> 
>     gdb compile failed, gcc: error: unrecognized debug output level
> 'column-info-foo'
>     UNTESTED: gdb.reverse/func-map-to-same-line.exp:
> with_column_info=yes: failed to prepare

Yes, that seems to give us the desired result.
> 
> I then thought about the "no-column-info" case.  Currently, you error
> out for gccs <= 6.  However, shouldn't we just compile without any
> special flag in that case, since there just wasn't any support for
> column-info back then?

OK, but seems like we should also warn the user that the option is not
supported and we are ignoring it.  I put 

         # In this case, don't add the compile line option and       
         # the result will be the same as using no-column-info       
         # on a version that supports the option.                    
         warning "gdb_compile option no-column-info not supported, ignoring."

in for this case.

                               Carl