From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by sourceware.org (Postfix) with ESMTP id 773423851C24 for ; Fri, 19 Jun 2020 15:42:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 773423851C24 Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-217-ovgAZTQROhqwBBsL8QNBTg-1; Fri, 19 Jun 2020 11:42:25 -0400 X-MC-Unique: ovgAZTQROhqwBBsL8QNBTg-1 Received: by mail-wr1-f71.google.com with SMTP id j5so2036228wro.6 for ; Fri, 19 Jun 2020 08:42:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=DySrn99oSaIviQdML2Wb61xzlmH74lCw0I0B2uIDsMo=; b=gsIjSTbnd+agyuZ8dFpM4mPabcOFx+bZeDcPCD5xTL4/ExISJ483KbaBQbuBmREeZ5 ZcGmMLX03AG0H6X4zi1G2v5vMrG0dTtGaRuDJNzOLckExZAlpYylSnXaTfFpxLnmS2dF qFkJbJhcJcvrt+uaq55wKlsFRhTNzqUMuUs53l1rSN5Ea1vxftGAAJvJH+QtV65pdFUF M5AFqKr9Vu6otZ4u85VvKIVKoFwDBtz6XH/mWXsm03x27UYRJRSL/wlngt5VGVkT/Dcn hzlF2Al/uiuj6uFUIsPz3ueYsvhDtrB7/Ol/xrET8VC0/vz7DLBVKjLdEOMtjZAW5cSt h1Sw== X-Gm-Message-State: AOAM531BlBFLwMf3NdibSbHYiGbR0KaIHrfjMT6/6iRCGuAisgqmjF1g oXVh0NTTVC7GgiI/w3x8orcI3MYvdJvX7zM0Cwi4/T4YvUhP6EcfM0Zwst3eewQFGe407uYu7Qr bkNokjiErCRgG40fkkfA1Wg== X-Received: by 2002:adf:a452:: with SMTP id e18mr4629393wra.349.1592581343623; Fri, 19 Jun 2020 08:42:23 -0700 (PDT) X-Google-Smtp-Source: ABdhPJy1C9/YTfSQXtDS5zTK+uF69peWfitjMNnH3F3JBsTJzI3YZzQ1eP6JodrfpgVzKaF9/GJzGQ== X-Received: by 2002:adf:a452:: with SMTP id e18mr4629366wra.349.1592581343287; Fri, 19 Jun 2020 08:42:23 -0700 (PDT) Received: from ?IPv6:2001:8a0:f922:c400:56ee:75ff:fe8d:232b? ([2001:8a0:f922:c400:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id k64sm7280204wmf.34.2020.06.19.08.42.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 19 Jun 2020 08:42:22 -0700 (PDT) Subject: [PATCH] W/ Clang, compile/link C++ test programs with "-x c++" To: Gary Benson References: <1590770634-26360-1-git-send-email-gbenson@redhat.com> <7b5505a6-68ac-fc94-a747-ffec3c7bf086@redhat.com> <20200619134946.GA31823@blade.nx> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <50b54268-e8aa-9ad6-1812-ec4647c1e3f3@redhat.com> Date: Fri, 19 Jun 2020 16:42:19 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20200619134946.GA31823@blade.nx> Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-8.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Jun 2020 15:42:30 -0000 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 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 * 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