Testsuite: Ensure stack protection is off for GCC
Commit Message
Using -fstack-protector-strong will cause GDB to break on the wrong line
when placing a breakpoint on a function. This is caused by due to
inadequate dwarf line numbering, and is being tracked by the GCC bug
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88432
GCC (and Clang) provided by Debian/Ubuntu default to Stack Protector
being enabled.
So, ensure that when running the GDB testsuite, stack protector is always
turned off for GCC (option has existed since GCC 4.1.0).
Also, ensure that this does not cause infinite recursion due to
test_compiler_info having to compile a file itself.
Restore change in ovldbreak.exp which worked around the issue.
[Side note - this fix would be improved by checking for Ubuntu/Debian,
but I don't see any existing checks for that in the testsuite]
2019-01-07 Alan Hayward <alan.hayward@arm.com>
* gdb.cp/ovldbreak.exp: Only allow a single break line.
* lib/gdb.exp (get_compiler_info): Use getting_compiler_info option.
(gdb_compile): Remove stack protector for GCC and prevent recursion.
---
gdb/testsuite/gdb.cp/ovldbreak.exp | 2 +-
gdb/testsuite/lib/gdb.exp | 17 +++++++++++++++--
2 files changed, 16 insertions(+), 3 deletions(-)
Comments
Forgot to mention: this fixes ~60 tests when running make check on Ubuntu.
> On 7 Jan 2019, at 16:37, Alan Hayward <Alan.Hayward@arm.com> wrote:
>
> Using -fstack-protector-strong will cause GDB to break on the wrong line
> when placing a breakpoint on a function. This is caused by due to
> inadequate dwarf line numbering, and is being tracked by the GCC bug
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88432
>
> GCC (and Clang) provided by Debian/Ubuntu default to Stack Protector
> being enabled.
>
> So, ensure that when running the GDB testsuite, stack protector is always
> turned off for GCC (option has existed since GCC 4.1.0).
>
> Also, ensure that this does not cause infinite recursion due to
> test_compiler_info having to compile a file itself.
>
> Restore change in ovldbreak.exp which worked around the issue.
>
> [Side note - this fix would be improved by checking for Ubuntu/Debian,
> but I don't see any existing checks for that in the testsuite]
>
> 2019-01-07 Alan Hayward <alan.hayward@arm.com>
>
> * gdb.cp/ovldbreak.exp: Only allow a single break line.
> * lib/gdb.exp (get_compiler_info): Use getting_compiler_info option.
> (gdb_compile): Remove stack protector for GCC and prevent recursion.
> ---
> gdb/testsuite/gdb.cp/ovldbreak.exp | 2 +-
> gdb/testsuite/lib/gdb.exp | 17 +++++++++++++++--
> 2 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.cp/ovldbreak.exp b/gdb/testsuite/gdb.cp/ovldbreak.exp
> index 494f2a12e9..3ffd04209a 100644
> --- a/gdb/testsuite/gdb.cp/ovldbreak.exp
> +++ b/gdb/testsuite/gdb.cp/ovldbreak.exp
> @@ -209,7 +209,7 @@ for {set idx 0} {$idx < [llength $overloads]} {incr idx} {
>
> # Verify the breakpoints.
> set bptable "Num\[\t \]+Type\[\t \]+Disp Enb Address\[\t \]+What.*\[\r\n]+"
> -append bptable "\[0-9\]+\[\t \]+breakpoint\[\t \]+keep\[\t \]y\[\t \]+$hex\[\t \]+in main(\\((|void)\\))? at.*$srcfile:4\[89\]\[\r\n\]+"
> +append bptable "\[0-9\]+\[\t \]+breakpoint\[\t \]+keep\[\t \]y\[\t \]+$hex\[\t \]+in main(\\((|void)\\))? at.*$srcfile:49\[\r\n\]+"
> append bptable "\[\t \]+breakpoint already hit 1 time\[\r\n\]+."
> foreach ovld $overloads {
> append bptable [format "\[0-9\]+\[\t \]+breakpoint\[\t \]+keep y\[\t \]+$hex\[\t \]+in foo::overload1arg\\(%s\\) at.*$srcfile:%d\[\r\n\]+" $ovld \
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index ed9f89d5a7..7443638a72 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -3276,12 +3276,12 @@ proc get_compiler_info {{arg ""}} {
> # We have to use -E and -o together, despite the comments
> # above, because of how DejaGnu handles remote host testing.
> set ppout "$outdir/compiler.i"
> - gdb_compile "${ifile}" "$ppout" preprocess [list "$arg" quiet]
> + gdb_compile "${ifile}" "$ppout" preprocess [list "$arg" quiet getting_compiler_info]
> set file [open $ppout r]
> set cppout [read $file]
> close $file
> } else {
> - set cppout [ gdb_compile "${ifile}" "" preprocess [list "$arg" quiet] ]
> + set cppout [ gdb_compile "${ifile}" "" preprocess [list "$arg" quiet getting_compiler_info] ]
> }
> eval log_file $saved_log
>
> @@ -3519,6 +3519,7 @@ proc gdb_compile {source dest type options} {
> }
> set shlib_found 0
> set shlib_load 0
> + set getting_compiler_info 0
> foreach opt $options {
> if {[regexp {^shlib=(.*)} $opt dummy_var shlib_name]
> && $type == "executable"} {
> @@ -3549,11 +3550,23 @@ proc gdb_compile {source dest type options} {
> }
> } elseif { $opt == "shlib_load" && $type == "executable" } {
> set shlib_load 1
> + } elseif { $opt == "getting_compiler_info" } {
> + # If this is set, calling test_compiler_info will cause recursion.
> + set getting_compiler_info 1
> } else {
> lappend new_options $opt
> }
> }
>
> + # Ensure stack protector is disabled for GCC, as this will causes problems
> + # with DWARF line numbering.
> + # See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88432
> + # This option defaults to on for Debian/Ubuntu.
> + if { $getting_compiler_info == 0 && [test_compiler_info "gcc-*"] } {
> + # Put it at the front to not override any user-provided value
> + lappend new_options "early_flags=-fno-stack-protector"
> + }
> +
> # Because we link with libraries using their basename, we may need
> # (depending on the platform) to set a special rpath value, to allow
> # the executable to find the libraries it depends on.
> --
> 2.17.2 (Apple Git-113)
>
Ping.
Note there is a fix patch for GCC under review here:
https://gcc.gnu.org/ml/gcc-patches/2019-01/msg00543.html
Once it is in an upstream GCC release, I can expand the GDB change
to only disable stack protection on older GCCs.
Alan.
> On 7 Jan 2019, at 16:57, Alan Hayward <Alan.Hayward@arm.com> wrote:
>
> Forgot to mention: this fixes ~60 tests when running make check on Ubuntu.
>
>
>> On 7 Jan 2019, at 16:37, Alan Hayward <Alan.Hayward@arm.com> wrote:
>>
>> Using -fstack-protector-strong will cause GDB to break on the wrong line
>> when placing a breakpoint on a function. This is caused by due to
>> inadequate dwarf line numbering, and is being tracked by the GCC bug
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88432
>>
>> GCC (and Clang) provided by Debian/Ubuntu default to Stack Protector
>> being enabled.
>>
>> So, ensure that when running the GDB testsuite, stack protector is always
>> turned off for GCC (option has existed since GCC 4.1.0).
>>
>> Also, ensure that this does not cause infinite recursion due to
>> test_compiler_info having to compile a file itself.
>>
>> Restore change in ovldbreak.exp which worked around the issue.
>>
>> [Side note - this fix would be improved by checking for Ubuntu/Debian,
>> but I don't see any existing checks for that in the testsuite]
>>
>> 2019-01-07 Alan Hayward <alan.hayward@arm.com>
>>
>> * gdb.cp/ovldbreak.exp: Only allow a single break line.
>> * lib/gdb.exp (get_compiler_info): Use getting_compiler_info option.
>> (gdb_compile): Remove stack protector for GCC and prevent recursion.
>> ---
>> gdb/testsuite/gdb.cp/ovldbreak.exp | 2 +-
>> gdb/testsuite/lib/gdb.exp | 17 +++++++++++++++--
>> 2 files changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.cp/ovldbreak.exp b/gdb/testsuite/gdb.cp/ovldbreak.exp
>> index 494f2a12e9..3ffd04209a 100644
>> --- a/gdb/testsuite/gdb.cp/ovldbreak.exp
>> +++ b/gdb/testsuite/gdb.cp/ovldbreak.exp
>> @@ -209,7 +209,7 @@ for {set idx 0} {$idx < [llength $overloads]} {incr idx} {
>>
>> # Verify the breakpoints.
>> set bptable "Num\[\t \]+Type\[\t \]+Disp Enb Address\[\t \]+What.*\[\r\n]+"
>> -append bptable "\[0-9\]+\[\t \]+breakpoint\[\t \]+keep\[\t \]y\[\t \]+$hex\[\t \]+in main(\\((|void)\\))? at.*$srcfile:4\[89\]\[\r\n\]+"
>> +append bptable "\[0-9\]+\[\t \]+breakpoint\[\t \]+keep\[\t \]y\[\t \]+$hex\[\t \]+in main(\\((|void)\\))? at.*$srcfile:49\[\r\n\]+"
>> append bptable "\[\t \]+breakpoint already hit 1 time\[\r\n\]+."
>> foreach ovld $overloads {
>> append bptable [format "\[0-9\]+\[\t \]+breakpoint\[\t \]+keep y\[\t \]+$hex\[\t \]+in foo::overload1arg\\(%s\\) at.*$srcfile:%d\[\r\n\]+" $ovld \
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index ed9f89d5a7..7443638a72 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -3276,12 +3276,12 @@ proc get_compiler_info {{arg ""}} {
>> # We have to use -E and -o together, despite the comments
>> # above, because of how DejaGnu handles remote host testing.
>> set ppout "$outdir/compiler.i"
>> - gdb_compile "${ifile}" "$ppout" preprocess [list "$arg" quiet]
>> + gdb_compile "${ifile}" "$ppout" preprocess [list "$arg" quiet getting_compiler_info]
>> set file [open $ppout r]
>> set cppout [read $file]
>> close $file
>> } else {
>> - set cppout [ gdb_compile "${ifile}" "" preprocess [list "$arg" quiet] ]
>> + set cppout [ gdb_compile "${ifile}" "" preprocess [list "$arg" quiet getting_compiler_info] ]
>> }
>> eval log_file $saved_log
>>
>> @@ -3519,6 +3519,7 @@ proc gdb_compile {source dest type options} {
>> }
>> set shlib_found 0
>> set shlib_load 0
>> + set getting_compiler_info 0
>> foreach opt $options {
>> if {[regexp {^shlib=(.*)} $opt dummy_var shlib_name]
>> && $type == "executable"} {
>> @@ -3549,11 +3550,23 @@ proc gdb_compile {source dest type options} {
>> }
>> } elseif { $opt == "shlib_load" && $type == "executable" } {
>> set shlib_load 1
>> + } elseif { $opt == "getting_compiler_info" } {
>> + # If this is set, calling test_compiler_info will cause recursion.
>> + set getting_compiler_info 1
>> } else {
>> lappend new_options $opt
>> }
>> }
>>
>> + # Ensure stack protector is disabled for GCC, as this will causes problems
>> + # with DWARF line numbering.
>> + # See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88432
>> + # This option defaults to on for Debian/Ubuntu.
>> + if { $getting_compiler_info == 0 && [test_compiler_info "gcc-*"] } {
>> + # Put it at the front to not override any user-provided value
>> + lappend new_options "early_flags=-fno-stack-protector"
>> + }
>> +
>> # Because we link with libraries using their basename, we may need
>> # (depending on the platform) to set a special rpath value, to allow
>> # the executable to find the libraries it depends on.
>> --
>> 2.17.2 (Apple Git-113)
>>
>
On 01/07/2019 04:37 PM, Alan Hayward wrote:
> Using -fstack-protector-strong will cause GDB to break on the wrong line
> when placing a breakpoint on a function. This is caused by due to
> inadequate dwarf line numbering, and is being tracked by the GCC bug
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88432
>
> GCC (and Clang) provided by Debian/Ubuntu default to Stack Protector
> being enabled.
>
> So, ensure that when running the GDB testsuite, stack protector is always
> turned off for GCC (option has existed since GCC 4.1.0).
I think it'd still be good to add a GCC version check instead of
unconditionally adding the option to all GCCs. The version
of GCC we support building GDB with is not the same concept as which
GCC version the target uses/supports, considering cross debugging / bare
metal, etc. It's plausible some port may be using GCC 4.0, say. Or if
someone wants to test against an old GCC to see what it generated, etc.
Since it's easy to do, I think we should just do it.
A regex similar to the old_gcc variable in gdb.ada/arrayidx.exp should
do it?
>
> Also, ensure that this does not cause infinite recursion due to
> test_compiler_info having to compile a file itself.
>
> Restore change in ovldbreak.exp which worked around the issue.
Thanks for doing this. My suspicion was correct. :-)
>
> [Side note - this fix would be improved by checking for Ubuntu/Debian,
> but I don't see any existing checks for that in the testsuite]
Could you add a small test that tests the opposite? I.e., a smoke
testcase with "-fstack-protector-strong" force-enabled? Probably
test both force on and off for full coverage. Then xfail
the test on unfixed GCCs. Which means all GCCs for now.
Thanks,
Pedro Alves
@@ -209,7 +209,7 @@ for {set idx 0} {$idx < [llength $overloads]} {incr idx} {
# Verify the breakpoints.
set bptable "Num\[\t \]+Type\[\t \]+Disp Enb Address\[\t \]+What.*\[\r\n]+"
-append bptable "\[0-9\]+\[\t \]+breakpoint\[\t \]+keep\[\t \]y\[\t \]+$hex\[\t \]+in main(\\((|void)\\))? at.*$srcfile:4\[89\]\[\r\n\]+"
+append bptable "\[0-9\]+\[\t \]+breakpoint\[\t \]+keep\[\t \]y\[\t \]+$hex\[\t \]+in main(\\((|void)\\))? at.*$srcfile:49\[\r\n\]+"
append bptable "\[\t \]+breakpoint already hit 1 time\[\r\n\]+."
foreach ovld $overloads {
append bptable [format "\[0-9\]+\[\t \]+breakpoint\[\t \]+keep y\[\t \]+$hex\[\t \]+in foo::overload1arg\\(%s\\) at.*$srcfile:%d\[\r\n\]+" $ovld \
@@ -3276,12 +3276,12 @@ proc get_compiler_info {{arg ""}} {
# We have to use -E and -o together, despite the comments
# above, because of how DejaGnu handles remote host testing.
set ppout "$outdir/compiler.i"
- gdb_compile "${ifile}" "$ppout" preprocess [list "$arg" quiet]
+ gdb_compile "${ifile}" "$ppout" preprocess [list "$arg" quiet getting_compiler_info]
set file [open $ppout r]
set cppout [read $file]
close $file
} else {
- set cppout [ gdb_compile "${ifile}" "" preprocess [list "$arg" quiet] ]
+ set cppout [ gdb_compile "${ifile}" "" preprocess [list "$arg" quiet getting_compiler_info] ]
}
eval log_file $saved_log
@@ -3519,6 +3519,7 @@ proc gdb_compile {source dest type options} {
}
set shlib_found 0
set shlib_load 0
+ set getting_compiler_info 0
foreach opt $options {
if {[regexp {^shlib=(.*)} $opt dummy_var shlib_name]
&& $type == "executable"} {
@@ -3549,11 +3550,23 @@ proc gdb_compile {source dest type options} {
}
} elseif { $opt == "shlib_load" && $type == "executable" } {
set shlib_load 1
+ } elseif { $opt == "getting_compiler_info" } {
+ # If this is set, calling test_compiler_info will cause recursion.
+ set getting_compiler_info 1
} else {
lappend new_options $opt
}
}
+ # Ensure stack protector is disabled for GCC, as this will causes problems
+ # with DWARF line numbering.
+ # See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88432
+ # This option defaults to on for Debian/Ubuntu.
+ if { $getting_compiler_info == 0 && [test_compiler_info "gcc-*"] } {
+ # Put it at the front to not override any user-provided value
+ lappend new_options "early_flags=-fno-stack-protector"
+ }
+
# Because we link with libraries using their basename, we may need
# (depending on the platform) to set a special rpath value, to allow
# the executable to find the libraries it depends on.