[1/2,ver,2] Fix reverse stepping multiple contiguous PC ranges over the line table.
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
Commit Message
Simon, GDB maintainers:
Version 2, updated the compiler check and handling for gcc version 6
and earlier. Retested on Power 10.
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 | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
Comments
On 07/08/2023 21:03, Carl Love wrote:
> Simon, GDB maintainers:
>
> Version 2, updated the compiler check and handling for gcc version 6
> and earlier. Retested on Power 10.
>
> 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 | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 36bf738c667..bffbbf38b09 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -4896,6 +4896,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:
> @@ -5105,6 +5107,38 @@ proc gdb_compile {source dest type options} {
> } else {
> error "Don't know how to handle text_segment option."
> }
> + } elseif { $opt == "column-info" } {
> + # If GCC or clang does not support column-info, compilation
> + # will fail and the usupported column-info option will be
> + # reported as such.
> + if {[test_compiler_info {gcc-*}]} {
I think you missed a bit on an old comment from simon. Way back in may,
in this email
https://sourceware.org/pipermail/gdb-patches/2023-May/199523.html, he
mentioned:
For instance, if you used no-column-info with gcc 6
(which doesn't support column info at all), gdb_compile should succeed,
even if there isn't an option to disable column info with that compiler.
If you used column-info with gcc 6, gdb_compile would fail.
So I think this bit should throw an error if it detects gcc-[1-6].
> + lappend new_options "additional_flags=-gcolumn-info"
> +
> + } elseif {[test_compiler_info {clang-*}]} {
I did some digging, and column-info were added in llvm back in october
2012 (commit a2f7eb7c52cdc), which seems to mean support was added in
llvm 3.2, but I don't see any mention in the release notes. In my
opinion, this is old enough that we don't need to have a special case,
but I wanted to mention, in case some maintainer thinks it should be
dealt with.
If we should, before then, it seems that clang WOULD add column info by
default, so it should compile with a warning here, and fail if the user
requested no column info
Guinevere:
On Tue, 2023-08-08 at 12:04 +0200, Guinevere Larsen wrote:
> >
<snip>
> On 07/08/2023 21:03, Carl Love wrote:
> >
> > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> > index 36bf738c667..bffbbf38b09 100644
> > --- a/gdb/testsuite/lib/gdb.exp
> > +++ b/gdb/testsuite/lib/gdb.exp
> > @@ -4896,6 +4896,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:
> > @@ -5105,6 +5107,38 @@ proc gdb_compile {source dest type options}
> > {
> > } else {
> > error "Don't know how to handle text_segment
> > option."
> > }
> > + } elseif { $opt == "column-info" } {
> > + # If GCC or clang does not support column-info, compilation
> > + # will fail and the usupported column-info option will be
> > + # reported as such.
> > + if {[test_compiler_info {gcc-*}]} {
>
> I think you missed a bit on an old comment from simon. Way back in
> may,
> in this email
> https://sourceware.org/pipermail/gdb-patches/2023-May/199523.html ,
> he
> mentioned:
>
> For instance, if you used no-column-info with gcc 6
> (which doesn't support column info at all), gdb_compile should
> succeed,
> even if there isn't an option to disable column info with that
> compiler.
> If you used column-info with gcc 6, gdb_compile would fail.
>
> So I think this bit should throw an error if it detects gcc-[1-6].
It has been awhile, but as I recall, we decided that the we would
specify column-info and if the compiler doesn't support it then the
compiler will complain (i.e. fail) and we will let the failure be
handled by the normal compiler failure path. I think that will work
fine? If there is some concern that is not sufficient, I would be
happy to put in the test if {[test_compiler_info {gcc-[1-6]-*}]} for
the $opt == "column-info" to have the script flag the error.
Thoughts?
In the case where the compiler doesn't handle the no-column-info flag,
i.e. gcc 1-6, we handle that case by not adding the flag so the
compiler will not flag the error and fail. In that case, it isn't
going to generate the column info anyways so we don't need to specify
no-column info.
>
> > + lappend new_options "additional_flags=-gcolumn-info"
> > +
> > + } elseif {[test_compiler_info {clang-*}]} {
>
> I did some digging, and column-info were added in llvm back in
> october
> 2012 (commit a2f7eb7c52cdc), which seems to mean support was added
> in
> llvm 3.2, but I don't see any mention in the release notes. In my
> opinion, this is old enough that we don't need to have a special
> case,
> but I wanted to mention, in case some maintainer thinks it should be
> dealt with.
>
> If we should, before then, it seems that clang WOULD add column info
> by
> default, so it should compile with a warning here, and fail if the
> user
> requested no column info
>
Carl
On 08/08/2023 17:38, Carl Love wrote:
> Guinevere:
>
> On Tue, 2023-08-08 at 12:04 +0200, Guinevere Larsen wrote:
> <snip>
>
>> On 07/08/2023 21:03, Carl Love wrote:
>>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>>> index 36bf738c667..bffbbf38b09 100644
>>> --- a/gdb/testsuite/lib/gdb.exp
>>> +++ b/gdb/testsuite/lib/gdb.exp
>>> @@ -4896,6 +4896,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:
>>> @@ -5105,6 +5107,38 @@ proc gdb_compile {source dest type options}
>>> {
>>> } else {
>>> error "Don't know how to handle text_segment
>>> option."
>>> }
>>> + } elseif { $opt == "column-info" } {
>>> + # If GCC or clang does not support column-info, compilation
>>> + # will fail and the usupported column-info option will be
>>> + # reported as such.
>>> + if {[test_compiler_info {gcc-*}]} {
>> I think you missed a bit on an old comment from simon. Way back in
>> may,
>> in this email
>> https://sourceware.org/pipermail/gdb-patches/2023-May/199523.html ,
>> he
>> mentioned:
>>
>> For instance, if you used no-column-info with gcc 6
>> (which doesn't support column info at all), gdb_compile should
>> succeed,
>> even if there isn't an option to disable column info with that
>> compiler.
>> If you used column-info with gcc 6, gdb_compile would fail.
>>
>> So I think this bit should throw an error if it detects gcc-[1-6].
> It has been awhile, but as I recall, we decided that the we would
> specify column-info and if the compiler doesn't support it then the
> compiler will complain (i.e. fail) and we will let the failure be
> handled by the normal compiler failure path. I think that will work
> fine? If there is some concern that is not sufficient, I would be
> happy to put in the test if {[test_compiler_info {gcc-[1-6]-*}]} for
> the $opt == "column-info" to have the script flag the error.
> Thoughts?
oh, I see. I must have misread when looking back at that conversation.
Sorry for the noise.
Also, disregard my comments about LLVM. I checked on IRC and the oldest
GCC we support compiling/using for usptream stuff is form 2014, so I
think we dont have to worry about 2012 clang :)
All in all, this patch looks good to go in. Reviewed-By: Guinevere
Larsen <blarsen@redhat.com>
I hope the maintainers approve this series soon, it is a long time coming!
@@ -4896,6 +4896,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:
@@ -5105,6 +5107,38 @@ proc gdb_compile {source dest type options} {
} else {
error "Don't know how to handle text_segment option."
}
+ } elseif { $opt == "column-info" } {
+ # If GCC or clang does not support column-info, compilation
+ # will fail and the usupported column-info option will be
+ # reported as such.
+ if {[test_compiler_info {gcc-*}]} {
+ 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."
+ }
+
+ } elseif { $opt == "no-column-info" } {
+ if {[test_compiler_info {gcc-*}]} {
+ if {[test_compiler_info {gcc-[1-6]-*}]} {
+ # 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."
+ } else {
+ lappend new_options "additional_flags=-gno-column-info"
+ }
+
+ } elseif {[test_compiler_info {clang-*}]} {
+ lappend new_options "additional_flags=-gno-column-info"
+
+ } else {
+ error "Don't know how to handle gno-column-info option."
+ }
+
} else {
lappend new_options $opt
}