[PATCH 0/2] Fix gdb.base/print-file-var.exp with Clang

[PATCH 0/2] Fix gdb.base/print-file-var.exp with Clang

[Gary Benson]

Hi all,

The C++ parts of gdb.base/print-file-var.exp failed to build with
Clang because the "-x c++" option added by gdb_compile caused the
compiler to attempt to parse .so files as C++.  Not only did this
fail to compile, it also resulted in a MAHOOSSIVE gdb.log of over
100Mb for that single test, as Clang happily dumped the "lines"
of the binary file it felt violated the C++ standard.

The first patch of this series adds code to gdb_compile to detect
this situation and report an error.  The second patch is the actual
fix: it separates the options for the compiler and linker into two
lists, and uses build_executable_from_specs to perform the build
as it can handle this separation.

Checked on Fedora 32 x86_64, with GCC and Clang.  Is this ok to
commit?

Cheers,
Gary

--
Gary Benson - he / him / his
Principal Software Engineer, Red Hat

[PATCH 1/2] Detect and report incompatible gdb_compile options

[Gary Benson]

In commits 221db974e653659edb280787af1b3efdd1615083 and
68d654afdfcff840ebb3ae432ed72dca0521d670, these patches:

    2020-06-24  Pedro Alves  <palves@redhat.com>

	* lib/gdb.exp (gdb_compile): Pass "-x c++" explicitly when
	compiling C++ programs.

    2020-09-25  Gary Benson <gbenson@redhat.com>

	* lib/gdb.exp (gdb_compile): Pass "-x c++" earlier, and only
	for .c files.

attempted to fix problems with testcases that compile .c files
using the C++ compiler.  These patches cause gdb_compile to add
"-x c++" to the compiler options when using Clang.  This fix does
not work for gdb.base/print-file-var.exp, however, which attempts
to compile a .c input file to an executable linked with shared
libraries: the resulting command caused the compiler to attempt
to parse the .so files as C++.  This patch causes gdb_compile
to reject this combination of options.

gdb/testsuite/ChangeLog:

	* lib/gdb.exp (gdb_compile): Inhibit passing "-x c++"
	for .c files compiled as C++ with Clang if any shared
	libraries are specified.
---
 gdb/testsuite/ChangeLog   |  6 ++++++
 gdb/testsuite/lib/gdb.exp | 16 ++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 6084699..999fd63 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3946,6 +3946,22 @@ proc gdb_compile {source dest type options} {
 	 && [lsearch -exact $options c++] != -1
 	 && [string match *.c $source] != 0
 	 && [test_compiler_info "clang-*"] } {
+
+	# gdb_compile cannot handle this combination of options, the
+	# result is a command like "clang -x c++ foo.c bar.so -o baz"
+	# which tells Clang to treat bar.so as C++.  The solution is
+	# to call gdb_compile twice--once to compile, once to link--
+	# either directly, or via build_executable_from_specs.
+	if { [lsearch $options shlib=*] != -1 } {
+	    set result "incompatible gdb_compile options"
+
+	    if {[lsearch $options quiet] < 0} {
+		clone_output "gdb compile failed, $result"
+	    }
+
+	    return $result
+	}
+
 	lappend new_options early_flags=-x\ c++
     }
 
-- 
1.8.3.1

[PATCH 2/2] Fix gdb.base/print-file-var.exp with Clang

[Gary Benson]

The C++ parts of gdb.base/print-file-var.exp failed to build with
Clang because the "-x c++" option added by gdb_compile caused the
compiler to attempt to parse .so files as C++.  This patch splits
the compiler and linker options into separate lists, and switches
to building via build_executable_from_specs which can accomodate
this separation.

gdb/testsuite/ChangeLog:

	* gdb.base/print-file-var.exp (test): Separate compiler and
	linker options, and build using build_executable_from_specs
	to accomodate this.
---
 gdb/testsuite/ChangeLog                   |  6 ++++++
 gdb/testsuite/gdb.base/print-file-var.exp | 18 ++++++++++--------
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
index 22df9f1..62e5f23 100644
--- a/gdb/testsuite/gdb.base/print-file-var.exp
+++ b/gdb/testsuite/gdb.base/print-file-var.exp
@@ -56,22 +56,24 @@ proc test {hidden dlopen version_id_main lang} {
 	return -1
     }
 
-    set main_opts [list debug shlib=${libobj1} $lang]
+    set main_opts [list debug $lang]
+    set link_opts [list debug shlib=${libobj1}]
 
     if {$dlopen} {
-	lappend main_opts "shlib_load" \
-	    "additional_flags=-DSHLIB_NAME=\"$libobj2\""
+	lappend main_opts "additional_flags=-DSHLIB_NAME=\"$libobj2\""
+	lappend link_opts "shlib_load"
     } else {
-	lappend main_opts "shlib=${libobj2}"
+	lappend link_opts "shlib=${libobj2}"
     }
 
     lappend main_opts "additional_flags=-DVERSION_ID_MAIN=$version_id_main"
 
-    if { [gdb_compile "${srcdir}/${subdir}/${main}.c" \
-	      [standard_output_file ${executable}] \
-	      executable \
+    if { [build_executable_from_specs ${main} \
+	      $executable \
+	      $link_opts \
+	      ${main}.c \
 	      $main_opts]
-	 != ""} {
+	 != 0 } {
 	return -1
     }
 
-- 
1.8.3.1

Re: [PATCH 1/2] Detect and report incompatible gdb_compile options

[Pedro Alves]

On 10/13/20 6:26 PM, Gary Benson via Gdb-patches wrote:

> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 6084699..999fd63 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -3946,6 +3946,22 @@ proc gdb_compile {source dest type options} {
>  	 && [lsearch -exact $options c++] != -1
>  	 && [string match *.c $source] != 0
>  	 && [test_compiler_info "clang-*"] } {
> +
> +	# gdb_compile cannot handle this combination of options, the
> +	# result is a command like "clang -x c++ foo.c bar.so -o baz"
> +	# which tells Clang to treat bar.so as C++.  The solution is
> +	# to call gdb_compile twice--once to compile, once to link--
> +	# either directly, or via build_executable_from_specs.
> +	if { [lsearch $options shlib=*] != -1 } {
> +	    set result "incompatible gdb_compile options"
> +
> +	    if {[lsearch $options quiet] < 0} {
> +		clone_output "gdb compile failed, $result"
> +	    }
> +
> +	    return $result
> +	}

I think it would be better to do this check outside
the [test_compiler_info "clang-*"] condition, so that people
don't  add more tests with the same problem without realizing.

I'd also think it would be better to make this a hard visible
error call blowing away the testcase, instead of just merely
a "failed to compile" regular return?

Re: [PATCH 2/2] Fix gdb.base/print-file-var.exp with Clang

[Pedro Alves]

On 10/13/20 6:26 PM, Gary Benson via Gdb-patches wrote:
> The C++ parts of gdb.base/print-file-var.exp failed to build with
> Clang because the "-x c++" option added by gdb_compile caused the
> compiler to attempt to parse .so files as C++.  This patch splits
> the compiler and linker options into separate lists, and switches
> to building via build_executable_from_specs which can accomodate

accomodate -> accommodate

> this separation.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/print-file-var.exp (test): Separate compiler and
> 	linker options, and build using build_executable_from_specs
> 	to accomodate this.

OK.

Re: [PATCH 1/2] Detect and report incompatible gdb_compile options

[Tom Tromey]

Gary> gdb/testsuite/ChangeLog:

Gary> 	* lib/gdb.exp (gdb_compile): Inhibit passing "-x c++"
Gary> 	for .c files compiled as C++ with Clang if any shared
Gary> 	libraries are specified.

I think this is ok.  Thank you.

Tom

Re: [PATCH 2/2] Fix gdb.base/print-file-var.exp with Clang

[Tom Tromey]

>>>>> "Gary" == Gary Benson via Gdb-patches <gdb-patches@sourceware.org> writes:

Gary> gdb/testsuite/ChangeLog:

Gary> 	* gdb.base/print-file-var.exp (test): Separate compiler and
Gary> 	linker options, and build using build_executable_from_specs
Gary> 	to accomodate this.

Looks good.  Thanks.

Tom

Re: [PATCH 1/2] Detect and report incompatible gdb_compile options

[Gary Benson]

Pedro Alves wrote:
> On 10/13/20 6:26 PM, Gary Benson via Gdb-patches wrote:
> > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> > index 6084699..999fd63 100644
> > --- a/gdb/testsuite/lib/gdb.exp
> > +++ b/gdb/testsuite/lib/gdb.exp
> > @@ -3946,6 +3946,22 @@ proc gdb_compile {source dest type options} {
> >  	 && [lsearch -exact $options c++] != -1
> >  	 && [string match *.c $source] != 0
> >  	 && [test_compiler_info "clang-*"] } {
> > +
> > +	# gdb_compile cannot handle this combination of options, the
> > +	# result is a command like "clang -x c++ foo.c bar.so -o baz"
> > +	# which tells Clang to treat bar.so as C++.  The solution is
> > +	# to call gdb_compile twice--once to compile, once to link--
> > +	# either directly, or via build_executable_from_specs.
> > +	if { [lsearch $options shlib=*] != -1 } {
> > +	    set result "incompatible gdb_compile options"
> > +
> > +	    if {[lsearch $options quiet] < 0} {
> > +		clone_output "gdb compile failed, $result"
> > +	    }
> > +
> > +	    return $result
> > +	}
> 
> I think it would be better to do this check outside
> the [test_compiler_info "clang-*"] condition, so that people
> don't  add more tests with the same problem without realizing.

Good idea!

> I'd also think it would be better to make this a hard visible
> error call blowing away the testcase, instead of just merely
> a "failed to compile" regular return?

Sure.

I've made both these changes and pushed the patch, as inlined below.
Thanks for reviewing this!

Cheers,
Gary

---
In commits 221db974e653659edb280787af1b3efdd1615083 and
68d654afdfcff840ebb3ae432ed72dca0521d670, these patches:

    2020-06-24  Pedro Alves  <palves@redhat.com>

	* lib/gdb.exp (gdb_compile): Pass "-x c++" explicitly when
	compiling C++ programs.

    2020-09-25  Gary Benson <gbenson@redhat.com>

	* lib/gdb.exp (gdb_compile): Pass "-x c++" earlier, and only
	for .c files.

attempted to fix problems with testcases that compile .c files
using the C++ compiler.  These patches cause gdb_compile to add
"-x c++" to the compiler options when using Clang.  This fix does
not work for gdb.base/print-file-var.exp, however, which attempts
to compile a .c input file to an executable linked with shared
libraries: the resulting command caused the compiler to attempt
to parse the .so files as C++.  This commit causes gdb_compile
to reject this combination of options.

gdb/testsuite/ChangeLog:

	* lib/gdb.exp (gdb_compile): Inhibit passing "-x c++"
	for .c files compiled as C++ with Clang if any shared
	libraries are specified.
---
 gdb/testsuite/ChangeLog   |  6 ++++++
 gdb/testsuite/lib/gdb.exp | 17 ++++++++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 003bd30..6c98ae0 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3960,9 +3960,20 @@ proc gdb_compile {source dest type options} {
     # explicitly force C++ language.
     if { [lsearch -exact $options getting_compiler_info] == -1
 	 && [lsearch -exact $options c++] != -1
-	 && [string match *.c $source] != 0
-	 && [test_compiler_info "clang-*"] } {
-	lappend new_options early_flags=-x\ c++
+	 && [string match *.c $source] != 0 } {
+
+	# gdb_compile cannot handle this combination of options, the
+	# result is a command like "clang -x c++ foo.c bar.so -o baz"
+	# which tells Clang to treat bar.so as C++.  The solution is
+	# to call gdb_compile twice--once to compile, once to link--
+	# either directly, or via build_executable_from_specs.
+	if { [lsearch $options shlib=*] != -1 } {
+	    error "incompatible gdb_compile options"
+	}
+
+	if {[test_compiler_info "clang-*"]} {
+	    lappend new_options early_flags=-x\ c++
+	}
     }
 
     # Place (and look for) Fortran `.mod` files in the output
-- 
1.8.3.1

Re: [PATCH 2/2] Fix gdb.base/print-file-var.exp with Clang

[Gary Benson]

Pedro Alves wrote:
> On 10/13/20 6:26 PM, Gary Benson via Gdb-patches wrote:
> > The C++ parts of gdb.base/print-file-var.exp failed to build with
> > Clang because the "-x c++" option added by gdb_compile caused the
> > compiler to attempt to parse .so files as C++.  This patch splits
> > the compiler and linker options into separate lists, and switches
> > to building via build_executable_from_specs which can accomodate
> 
> accomodate -> accommodate
> 
> > this separation.
> > 
> > gdb/testsuite/ChangeLog:
> > 
> > 	* gdb.base/print-file-var.exp (test): Separate compiler and
> > 	linker options, and build using build_executable_from_specs
> > 	to accomodate this.
> 
> OK.

Thanks.  I've updated the spelling of "accommodate" and pushed this.

Cheers,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat