W/ Clang, compile/link C++ test programs with "-x c++"

Message ID 50b54268-e8aa-9ad6-1812-ec4647c1e3f3@redhat.com
State New
Headers show
Series
  • W/ Clang, compile/link C++ test programs with "-x c++"
Related show

Commit Message

Jose E. Marchesi via Gdb-patches June 19, 2020, 3:42 p.m.
On 6/19/20 2:49 PM, Gary Benson via Gdb-patches wrote:
> Pedro Alves wrote:

>> On 5/29/20 5:43 PM, Gary Benson via Gdb-patches wrote:

>>> Clang fails to compile the file, with the following error:

>>>   fatal error: 'iostream' file not found

>>>

>>> This prevents the following testcase from executing:

>>>   gdb.compile/compile-cplus.exp

>>>

>>> The testcase sets additional_flags when building with GCC, which

>>> this commit causes to also be set when building with clang.  This

>>> makes the testcase fail to build with a different error:

>>>   warning: treating 'c' input as 'c++' when in C++ mode, this behavior

>>>   is deprecated [-Wdeprecated]

>>> so this commit adds -Wno-deprecated in two places to sidestep this.

>>> Note that, while allowing the testcase to build, this commit reveals

>>> failures when the testsuite is built using clang.

>>>

>>> gdb/testsuite/ChangeLog:

>>>

>>> 	* gdb.compile/compile-cplus.exp (additional_flags): Also

>>> 	set when building with clang.

>>> 	(additional_flags, srcfilesoptions): Pass -Wno-deprecated

>>> 	when building with clang.

>>> ---

>>>  gdb/testsuite/ChangeLog                     |  7 +++++++

>>>  gdb/testsuite/gdb.compile/compile-cplus.exp | 15 +++++++++++++--

>>>  2 files changed, 20 insertions(+), 2 deletions(-)

>>>

>>> diff --git a/gdb/testsuite/gdb.compile/compile-cplus.exp b/gdb/testsuite/gdb.compile/compile-cplus.exp

>>> index cca5b20..85b2f20 100644

>>> --- a/gdb/testsuite/gdb.compile/compile-cplus.exp

>>> +++ b/gdb/testsuite/gdb.compile/compile-cplus.exp

>>> @@ -19,11 +19,16 @@ standard_testfile .c compile-shlib.c compile-constvar.S compile-nodebug.c

>>>  

>>>  get_compiler_info

>>>  set options {}

>>> -if [test_compiler_info gcc*] {

>>> +if { [test_compiler_info gcc*] || [test_compiler_info clang*] } {

>>>      lappend options additional_flags=-g3

>>>      lappend options additional_flags=-std=gnu++11

>>>      lappend options c++

>>>  }

>>> +if [test_compiler_info clang*] {

>>> +    # Treating C input as C++ is deprecated in Clang, so

>>> +    # the build will fail without disabling -Wdeprecated.

>>> +    lappend options additional_flags=-Wno-deprecated

>>> +}

>>

>> I think

>>

>>  lappend options "additional_flags=-x c++"

>>

>> in the previous "if" would be better.

> 

> [i'm replying to this out-of-order]

> 

>> Even better would be to tweak gdb_compile or thereabouts to make the

>> "c++" option automatically add "-x c++" to the compiler options with

>> clang.  That would handle everywhere this could be an issue all at

>> once.  WDYT?

> 

> I disagree with adding global support for this, for these reasons:

> 

> 1) It only affects this testcase.


That's not true.  For example:

 $ grep -rn "foreach" |grep "c+"
 gdb.base/whatis-ptype-typedefs.exp:304:foreach_with_prefix lang {"c" "c++"} {
 gdb.cp/wide_char_types.exp:178:    foreach_with_prefix lang {"c" "c++03" "c++11"} {
 gdb.cp/local-static.exp:271:foreach lang {"c" "c++"} {

And running gdb.base/whatis-ptype-typedefs.exp shows:

 $ make check TESTS="gdb.base/whatis-ptype-typedefs.exp" RUNTESTFLAGS="CC_FOR_TARGET=clang CXX_FOR_TARGET=clang++" 
 ...
 Running /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.base/whatis-ptype-typedefs.exp ...
 gdb compile failed, clang-5.0: warning: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated [-Wdeprecated]

                 === gdb Summary ===

 # of expected passes            2383
 # of untested testcases         1


> 

> 2) This testcase is one big hack, afaict.  It's a copy of (one/some

>    of?) the original C-language testcases, with some stuff at the

>    start to force the C sources through the C++ compiler, and to force

>    GDB to recognise the result as C++. Force force force.


Well, it's just a filename.  If it compiles with a C++ compiler, then
it's valid C++ input, regardless of the filename.

> 

> 3) Clang is warning that compiling C as C++ is deprecated.  Deprecated,

>    as in, the support is there now, but don't count on it being there

>    forever.  We should not add global support for testcases to do

>    deprecated things.


They probably want to get to a point where a single driver handles all
input sources, so if you compile with either:

  "clang++ foo.c"
  "random-driver-name foo.c"

the compiler treats the input a C.  Maybe they end up doing that,
but do you honestly see them removing support for the _explicit_
-x option?  That would be quite silly, IMNSHO.  It would
break the GDB build too, since we build GDB with that option, given
our sources are still named '.c'.

> 

> In terms of fixing this locally:

>> I think

>>

>>  lappend options "additional_flags=-x c++"

>>

>> in the previous "if" would be better.

> 

> It doesn't work, one of the sources is assembler.  I tried adding it

> to the other sources individually, but I couldn't make that work

> either,

> 


We just need to NOT pass "c++" and all the other related options when
building that file.

> a) There's some quoting issue, I had to do:

>      lappend options "additional_flags=-x"

>      lappend options "additional_flags=c++"

>    (I'm assuming there's a better way, but I don't know it)


I was surprised by this too.  Figured out that escaping the space works.

> 

> b) There's $srcfile3, $srcfile4, but I couldn't figure out how to

>    apply the additional flags to the first srcfile.


Not sure what you mean here -- the first srcfile is $srcfile, so its
options are in this line:

 set srcfilesoptions [list ${srcfile} ${options}]

The second source file is $srcfile2, and so on.

> 

>> BTW, it looks like a bug that appending the "c++" option is inside

>> the if, rather than outside.

> 

> Maybe. I don't know.  I doubt it's ever been run other than with GCC

> as the testcase compiler, the testcase is testing GDB using GCC to

> compile code for injection after all.

> 

> Let me know what to do with this testcase.  How important is it to

> not have -Wno-deprecated in there?  I've probably spent close to

> three hours on this issue now fwiw :(

It's not that -Wno-deprecated is very harmful (though it could hide
other deprecated things), it's that we've already told gdb_compile to
compile in C++ mode with the "c++" option, so it looks like a bug that
we need to pass more options to actually mean it.  There's a pattern
here that we can address in one single central place and forget about it.

The patch below works for me with gdb.compile/compile-cplus.exp, and
also fixes gdb.base/whatis-ptype-typedefs.exp too at least.  I've not run
the whole testsuite.

From 4f5629d25a305a93b26c3a7ecc001bf7c0fae9ae Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>

Date: Fri, 19 Jun 2020 15:43:53 +0100
Subject: [PATCH] W/ Clang, compile/link C++ test programs with "-x c++"

Some testcases want to compile .c files with a C++ compiler.  So they
pass the "c++" option to gdb_compile.  That works fine with GCC, but
with Clang, it results in:

  gdb compile failed, clang-5.0: warning: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated [-Wdeprecated]

and the testcase is skipped with UNTESTED.

A previous patch fixed a case like that in
gdb.compile/compile-cplus.exp, by adding -Wno-deprecated to the build
options.  However, there are other testcases that use the same
pattern.  For example, gdb.base/whatis-ptype-typedefs.exp.

Fix this in a central place, within gdb_compile, by passing "-x c++"
to the compiler driver when we're compiling/linking C++.

This revealed that gdb.compile/compile-cplus.exp is compiling an
assembly file with the "c++" option, which would now fail to compile,
with the C++ compiler not grokking the assemly, of course.  We just
need to not pass "c++" and all the other related C++ options when
building that assembly file.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.compile/compile-cplus.exp: Don't compile $srcfile3 with
	$options, since it's an assembly file.  Remove -Wno-deprecated.
	* lib/gdb.exp (gdb_compile): Pass "-x c++" explicitly when
	compiling C++ programs.
---
 gdb/testsuite/gdb.compile/compile-cplus.exp | 12 +-----------
 gdb/testsuite/lib/gdb.exp                   |  8 ++++++++
 2 files changed, 9 insertions(+), 11 deletions(-)


base-commit: a8a566853a0fc7f57159e55436ff6f395e499568
-- 
2.14.5

Comments

Jose E. Marchesi via Gdb-patches June 24, 2020, 10:30 p.m. | #1
On 6/19/20 4:42 PM, Pedro Alves via Gdb-patches wrote:

> The patch below works for me with gdb.compile/compile-cplus.exp, and

> also fixes gdb.base/whatis-ptype-typedefs.exp too at least.  I've not run

> the whole testsuite.


I've run the whole testsuite now, and only gdb.arch/amd64-entry-value-paramref.exp
had a similar issue with compiling an .S file as C++.  Fixed in the version
below, which I've now pushed.

This makes these testcases compile and start running, at least:

 gdb.base/info-types-c++.exp
 gdb.base/max-depth-c++.exp
 gdb.base/msym-lang.exp
 gdb.base/whatis-ptype-typedefs.exp
 gdb.btrace/rn-dl-bind.exp

From: Pedro Alves <palves@redhat.com>

Date: 2020-06-24 23:18:19 +0100

W/ Clang, compile/link C++ test programs with "-x c++"

Some testcases want to compile .c files with a C++ compiler.  So they
pass the "c++" option to gdb_compile.  That works fine with GCC, but
with Clang, it results in:

  gdb compile failed, clang-5.0: warning: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated [-Wdeprecated]

and the testcase is skipped with UNTESTED.

A previous patch fixed a case like that in
gdb.compile/compile-cplus.exp, by adding -Wno-deprecated to the build
options.  However, there are other testcases that use the same
pattern, and all fail for the same reason.  For example:

 gdb.base/info-types-c++.exp
 gdb.base/max-depth-c++.exp
 gdb.base/msym-lang.exp
 gdb.base/whatis-ptype-typedefs.exp
 gdb.btrace/rn-dl-bind.exp

Fix this in a central place, within gdb_compile, by passing "-x c++"
to the compiler driver when we're compiling/linking C++.

This revealed that gdb.compile/compile-cplus.exp and
gdb.arch/amd64-entry-value-paramref.exp tests are compiling an
assembly file with the "c++" option, which would now fail to compile,
with the C++ compiler not grokking the assembly, of course.  We just
need to not pass "c++" and all the other related C++ options when
compiling an assembly file.

gdb/testsuite/ChangeLog:
2020-06-24  Pedro Alves  <palves@redhat.com>

	* gdb.arch/amd64-entry-value-paramref.exp: Use
	prepare_for_testing_full and don't pass "c++" for the .S file
	build spec.
	* gdb.compile/compile-cplus.exp: Don't compile $srcfile3 with
	$options, since it's an assembly file.  Remove -Wno-deprecated.
	* lib/gdb.exp (gdb_compile): Pass "-x c++" explicitly when
	compiling C++ programs.
---

 gdb/testsuite/ChangeLog                            |   10 ++++++++++
 .../gdb.arch/amd64-entry-value-paramref.exp        |    3 ++-
 gdb/testsuite/gdb.compile/compile-cplus.exp        |   12 +-----------
 gdb/testsuite/lib/gdb.exp                          |    8 ++++++++
 4 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 6f4d99d3902..1b77459bce3 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,13 @@
+2020-06-24  Pedro Alves  <palves@redhat.com>
+
+	* gdb.arch/amd64-entry-value-paramref.exp: Use
+	prepare_for_testing_full and don't pass "c++" for the .S file
+	build spec.
+	* gdb.compile/compile-cplus.exp: Don't compile $srcfile3 with
+	$options, since it's an assembly file.  Remove -Wno-deprecated.
+	* lib/gdb.exp (gdb_compile): Pass "-x c++" explicitly when
+	compiling C++ programs.
+
 2020-06-24  Pedro Alves  <palves@redhat.com>
 
 	* lib/gdb.exp (gdb_compile): Update intro comment.  If C/C++ with
diff --git a/gdb/testsuite/gdb.arch/amd64-entry-value-paramref.exp b/gdb/testsuite/gdb.arch/amd64-entry-value-paramref.exp
index be60e2554d7..cbb69f46b81 100644
--- a/gdb/testsuite/gdb.arch/amd64-entry-value-paramref.exp
+++ b/gdb/testsuite/gdb.arch/amd64-entry-value-paramref.exp
@@ -20,7 +20,8 @@ if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
     return
 }
 
-if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} "c++"] } {
+if { [prepare_for_testing_full "failed to prepare" \
+	  [list $testfile "c++" $srcfile {}]] } {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.compile/compile-cplus.exp b/gdb/testsuite/gdb.compile/compile-cplus.exp
index 85b2f20a8fa..f794e5a1439 100644
--- a/gdb/testsuite/gdb.compile/compile-cplus.exp
+++ b/gdb/testsuite/gdb.compile/compile-cplus.exp
@@ -24,11 +24,6 @@ if { [test_compiler_info gcc*] || [test_compiler_info clang*] } {
     lappend options additional_flags=-std=gnu++11
     lappend options c++
 }
-if [test_compiler_info clang*] {
-    # Treating C input as C++ is deprecated in Clang, so
-    # the build will fail without disabling -Wdeprecated.
-    lappend options additional_flags=-Wno-deprecated
-}
 
 if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
     verbose "Skipping x86_64 LOC_CONST test."
@@ -37,14 +32,9 @@ if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
 
 set srcfilesoptions [list ${srcfile} ${options}]
 if { $srcfile3 != "" } {
-    lappend srcfilesoptions $srcfile3 ${options}
+    lappend srcfilesoptions $srcfile3 {}
 }
 set srcfile4options "nodebug c++"
-if [test_compiler_info clang*] {
-    # Treating C input as C++ is deprecated in Clang, so
-    # the build will fail without disabling -Wdeprecated.
-    set srcfile4options "$srcfile4options additional_flags=-Wno-deprecated"
-}
 lappend srcfilesoptions $srcfile4 $srcfile4options
 if { [eval build_executable_from_specs ${testfile}.exp $testfile {$options} ${srcfilesoptions}] } {
     return -1
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 6b4f71be588..8dbfa7e7a94 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3868,6 +3868,14 @@ proc gdb_compile {source dest type options} {
 	lappend new_options "additional_flags=-Wno-unknown-warning-option"
     }
 
+    # Treating .c input files as C++ is deprecated in Clang, so
+    # explicitly force C++ language.
+    if { [lsearch -exact $options getting_compiler_info] == -1
+	 && [lsearch -exact $options c++] != -1
+	 && [test_compiler_info "clang-*"]} {
+	lappend new_options additional_flags=-x\ c++
+    }
+
     # Place (and look for) Fortran `.mod` files in the output
     # directory for this specific test.
     if {[lsearch -exact $options f77] != -1 \
Jose E. Marchesi via Gdb-patches July 2, 2020, 9:58 a.m. | #2
Pedro Alves wrote:
> 2020-06-24  Pedro Alves  <palves@redhat.com>

> 

> 	* gdb.arch/amd64-entry-value-paramref.exp: Use

> 	prepare_for_testing_full and don't pass "c++" for the .S file

> 	build spec.

> 	* gdb.compile/compile-cplus.exp: Don't compile $srcfile3 with

> 	$options, since it's an assembly file.  Remove -Wno-deprecated.

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

> 	compiling C++ programs.


Thanks Pedro, I ran the full testsuite with this last week and it's
working well.

Cheers,
Gary

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

Patch

diff --git a/gdb/testsuite/gdb.compile/compile-cplus.exp b/gdb/testsuite/gdb.compile/compile-cplus.exp
index 85b2f20a8fa..f794e5a1439 100644
--- a/gdb/testsuite/gdb.compile/compile-cplus.exp
+++ b/gdb/testsuite/gdb.compile/compile-cplus.exp
@@ -24,11 +24,6 @@  if { [test_compiler_info gcc*] || [test_compiler_info clang*] } {
     lappend options additional_flags=-std=gnu++11
     lappend options c++
 }
-if [test_compiler_info clang*] {
-    # Treating C input as C++ is deprecated in Clang, so
-    # the build will fail without disabling -Wdeprecated.
-    lappend options additional_flags=-Wno-deprecated
-}
 
 if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
     verbose "Skipping x86_64 LOC_CONST test."
@@ -37,14 +32,9 @@  if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
 
 set srcfilesoptions [list ${srcfile} ${options}]
 if { $srcfile3 != "" } {
-    lappend srcfilesoptions $srcfile3 ${options}
+    lappend srcfilesoptions $srcfile3 {}
 }
 set srcfile4options "nodebug c++"
-if [test_compiler_info clang*] {
-    # Treating C input as C++ is deprecated in Clang, so
-    # the build will fail without disabling -Wdeprecated.
-    set srcfile4options "$srcfile4options additional_flags=-Wno-deprecated"
-}
 lappend srcfilesoptions $srcfile4 $srcfile4options
 if { [eval build_executable_from_specs ${testfile}.exp $testfile {$options} ${srcfilesoptions}] } {
     return -1
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 480af7052f7..d1ad8ba6f3f 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3859,6 +3859,14 @@  proc gdb_compile {source dest type options} {
 	lappend new_options "additional_flags=-J${mod_path}"
     }
 
+    if { [lsearch -exact $options getting_compiler_info] == -1
+	 && [lsearch -exact $options c++] != -1
+	 && [test_compiler_info clang*]} {
+	# Treating .c input files as C++ is deprecated in Clang, so
+	# explicitly force C++ language.
+	lappend new_options additional_flags=-x\ c++
+    }
+
     set shlib_found 0
     set shlib_load 0
     set getting_compiler_info 0