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

[Pedro Alves <palves@redhat.com>]

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(-)

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

base-commit: a8a566853a0fc7f57159e55436ff6f395e499568
-- 
2.14.5