Re: [PATCH v2] Testsuite: Ensure stack protection is off for GCC

[Sergio Durigan Junior <sergiodj at redhat dot com>]

On Friday, January 18 2019, 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 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.
>
> Ensure that when running the GDB testsuite, stack protector is always
> turned off for GCC 4.1.0 (when stack protector was added) and above.
>
> Ensure that this does not cause infinite recursion due to
> test_compiler_info having to compile a file itself.
>
> Add a test to explicitly test breakpoints with various levels of stack
> protection on both GCC and Clang, with xfail for the known errors.
>
> Restore change in ovldbreak.exp which worked around the issue.

Hi Alan,

This commit has caused a problem when we run the testsuite in parallel
mode.  For some reason, the "=== gdb Summary ===" line is not being
generated for a few testcases.  This is causing the consolidation script
that runs at the end of the testsuite to fail, which then causes gdb.log
to be empty.

The BuildBot is encountering this error (because we always run the tests
in parallel there); you can see an example of it here:

  https://gdb-build.sergiodj.net/builders/Fedora-x86_64-m64/builds/11967/steps/test%20gdb/logs/stdio

Go to the end of the page and look for

  outputs/gdb.base/fullpath-expand/gdb.log: no recognised summary line

I can reproduce this locally by doing:

  make -k check TESTS=gdb.base/*.exp FORCE_PARALLEL=1 -j4

Initially I don't know what could cause this problem, because your
changes seem very harmless.  Either way, I'd appreciate if you could
take a look at this as well.

Thanks,

> gdb/testsuite/ChangeLog:
>
> 2019-01-18  Alan Hayward  <alan.hayward@arm.com>
>
> 	* gdb.base/stack-protector.c: New test.
> 	* gdb.base/stack-protector.exp: New file.
>         * 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.base/stack-protector.c   | 27 +++++++++
>  gdb/testsuite/gdb.base/stack-protector.exp | 68 ++++++++++++++++++++++
>  gdb/testsuite/gdb.cp/ovldbreak.exp         |  2 +-
>  gdb/testsuite/lib/gdb.exp                  | 20 ++++++-
>  4 files changed, 114 insertions(+), 3 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/stack-protector.c
>  create mode 100644 gdb/testsuite/gdb.base/stack-protector.exp
>
> diff --git a/gdb/testsuite/gdb.base/stack-protector.c b/gdb/testsuite/gdb.base/stack-protector.c
> new file mode 100644
> index 0000000000..5e048859a7
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/stack-protector.c
> @@ -0,0 +1,27 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> +   Copyright 2019 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/>.  */
> +
> +int foo (int a)
> +{
> +  return a + 7; /* break here.  */
> +}
> +
> +int
> +main ()
> +{
> +  return foo (5);
> +}
> diff --git a/gdb/testsuite/gdb.base/stack-protector.exp b/gdb/testsuite/gdb.base/stack-protector.exp
> new file mode 100644
> index 0000000000..ad8c15de9f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/stack-protector.exp
> @@ -0,0 +1,68 @@
> +# Copyright (C) 2019 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/>.
> +
> +# Test breakpoints work correctly when stack protector is used.
> +
> +# Note on Debian/Ubuntu, stack protector is on by default, and is
> +# currently force disabled in gdb_compile due to the XFAIL below.
> +
> +# Ensure the compiler is gcc/clang and is new enough to support stack
> +# protection.
> +if { !([test_compiler_info "gcc-*"] || [test_compiler_info "clang-*"])
> +     || [test_compiler_info {gcc-[0-3]-*}]
> +     || [test_compiler_info {gcc-4-0-*}]
> +     || [test_compiler_info {clang-[0-4]=*}] } {
> +    return 0
> +}
> +
> +standard_testfile
> +
> +set protector_options { "-fno-stack-protector" "-fstack-protector" \
> +			"-fstack-protector-all" "-fstack-protector-strong" }
> +
> +proc simple_func_break_test { protection } {
> +    global testfile
> +    global srcfile
> +    global binfile
> +
> +    set options debug
> +    lappend options additional_flags=$protection
> +
> +    if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
> +	return -1
> +    }
> +
> +    clean_restart ${binfile}
> +
> +    if { ![runto_main] } then {
> +	fail "can't run to main"
> +	return -1
> +    }
> +
> +    # Break on function foo and ensure it stops on the first line of code.
> +    gdb_breakpoint "foo"
> +
> +    if { $protection == "-fstack-protector-all"
> +         && [test_compiler_info "gcc-*"] } {
> +	setup_xfail "gcc/88432" "*-*-linux*"
> +    }
> +    gdb_continue_to_breakpoint "foo" ".*break here.*"
> +
> +    return 1
> +}
> +
> +foreach_with_prefix protection $protector_options {
> +    simple_func_break_test $protection
> +}
> 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..c1b22609a6 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,26 @@ 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-*-*}]
> +	 && !([test_compiler_info {gcc-[0-3]-*}]
> +	      || [test_compiler_info {gcc-4-0-*}]) } {
> +        # 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)

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/