[PATCH v2] [libcc1] Rename C{,P}_COMPILER_NAME and remove triplet from them

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

On Wednesday, August 23 2017, Pedro Alves wrote:

> On 08/23/2017 05:17 AM, Sergio Durigan Junior wrote:
>> Hi there,
>> 
>> This is a series of two patches, one for GDB and one for GCC, which aims
>> to improve the detection and handling of triplets present on compiler
>> names.  The motivation for this series was mostly the fact that GDB's
>> "compile" command is broken on Debian unstable, as can be seen here:
>> 
>>   <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=851146>
>> 
>> The reason for the failure is the fact that Debian compiles GCC using
>> the --program-{prefix,suffix} options from configure in order to name
>> the compiler using the full triplet (i.e., Debian's GCC is not merely
>> named "gcc", but e.g. "x86_64-linux-gnu-gcc-7"), which end up naming the
>> C_COMPILER_NAME and CP_COMPILER_NAME defines with the specified prefix
>> and suffix.  Therefore, the regexp being used to match the compiler name
>> is wrong because it doesn't take into account the fact that the defines
>> may already contain the triplets.
>
> As discussed on IRC, I think the problem is that C_COMPILER_NAME
> in libcc1 includes the full triplet in the first place.  I think
> that it shouldn't.  I think that C_COMPILER_NAME should always
> be "gcc".
>
> The problem is in bootstrapping code, before there's a plugin
> yet -- i.e.., in the code that libcc1 uses to find the compiler (which
> then loads a plugin that libcc1 talks with).
>
> Please bear with me while I lay down my rationale, so that we're
> in the same page.
>
> C_COMPILER_NAME seems to include the prefix currently in an attempt
> to support cross debugging, or more generically, --enable-targets=all
> in gdb, but the whole thing doesn't really work as intended if
> C_COMPILER_NAME already includes a target prefix.
>
> IIUC the libcc1/plugin design, a single "libcc1.so" (what gdb loads,
> not the libcc1plugin compiler plugin) should work with any compiler in
> the PATH, in case you have several in the system.  E.g., one for
> each arch.
>
> Let me expand.
>
> The idea is that gdb always dlopens "libcc1.so", by that name exactly.
> Usually that'll open the libcc1.so installed in the system, e.g.,
> "/usr/lib64/libcc1.so", which for convenience was originally built from the
> same source tree as the systems's compiler was built.  You could force gdb to
> load some other libcc1.so, e.g., by tweaking LD_LIBRARY_PATH of course,
> but you shouldn't need to.
>
> libcc1.so is responsible for finding a compiler that targets the
> architecture of the inferior that the user is debugging in gdb.
> E.g., say you're cross debugging for arm-none-eabi, on a
> x86-64 Fedora host.  GDB knows the target inferior's architecture, and passes
> down to (the system) libcc1 a triplet regex like "arm*-*eabi*" or
> similar to libcc1,.  libcc1 appends "-" + C_COMPILER_NAME to that regex,
> generating something like "arm*-*eabi*-gcc", and then looks for binaries
> in PATH that match that regex.  When one is found, e.g., "arm-none-eabi-gcc",
> libcc1 forks/execs that compiler, passing it "-fplugin=libcc1plugin".
> libcc1 then communicates with that compiler's libcc1plugin plugin
> via a socket.
>
> In this scheme, "libcc1.so", the library that gdb loads, has no
> target-specific logic at all.  It should work with any compiler
> in the system, for any target/arch.  All it does is marshall the gcc/gdb
> interface between the gcc plugin and gdb, it is not linked against gcc.
> That boundary is versioned, and ABI-stable.  So as long as the
> libcc1.so that gdb loads understands the same API version of the gcc/gdb
> interface API as gdb understands, it all should work.  (The APIs
> are always extended keeping backward compatibility.)
>
> So in this scheme, having the "C_COMPILER_NAME" macro in libcc1
> include the target prefix for the --target that the plugin that
> libcc1 is built along with, seems to serve no real purpose, AFAICT.
> It's just getting in the way.
>
> I.e., something like:
>
>   "$gdb_specified_triplet_re" + "-" + C_COMPILER_NAME
>
> works if C_COMPILER_NAME is exactly "gcc", but not if C_COMPILER_NAME is already:
>
>   "$whatever_triplet_libcc1_happened_to_be_built_with" + "-gcc"
>
> because we end up with:
>
>   "$gdb_specified_triplet_re" + "-" "$whatever_triplet_libcc1_happened_to_be_built_with" +  "-gcc"
>
> which is the problem case.
>
> In sum, I think the libcc1.so (not the plugin) should _not_ have baked
> in target awareness, and thus C_COMPILER_NAME should always be "gcc", and
> then libcc1's regex should be adjusted to also tolerate a suffix in
> the final compiler binary name regex.
>
> WDYT?

As I replied before, I agree with Pedro's rationale here and his idea
actually makes my initial patch much simpler.  By renaming
C_COMPILER_NAME (and the new CP_COMPILER_NAME) to just "gcc" (or "g++"),
the Debian bug I was fixing is solved and we don't have to bother with
breaking compatibility with older gdb's packaged by the distros, because
they will also keep working with this change.

So I would like to propose this new patch, only for GCC, which makes
C_COMPILER_NAME and CP_COMPILER_NAME have "gcc" and "g++" as hardcoded
names, respectively.  In the commit log, I intend to include Pedro's
rationale (above).

What do you guys think of this new version?  It doesn't need any GDB
patch, and works with both Fedora and Debian.

Thanks,

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

libcc1/ChangeLog:
2017-09-01  Sergio Durigan Junior  <sergiodj@redhat.com>
	    Pedro Alves  <palves@redhat.com>

	* Makefile.am: Remove references to c-compiler-name.h and
	cp-compiler-name.h
	* Makefile.in: Regenerate.
	* compiler-name.hh: New file.
	* libcc1.cc: Don't include c-compiler-name.h.  Include
	compiler-name.hh.
	* libcp1.cc: Don't include cp-compiler-name.h.  Include
	compiler-name.hh.

diff --git a/libcc1/Makefile.am b/libcc1/Makefile.am
index 5e61a92a26b..cdba5a50b23 100644
--- a/libcc1/Makefile.am
+++ b/libcc1/Makefile.am
@@ -45,24 +45,6 @@ plugin_LTLIBRARIES = libcc1plugin.la libcp1plugin.la
 cc1lib_LTLIBRARIES = libcc1.la
 endif
 
-BUILT_SOURCES = c-compiler-name.h cp-compiler-name.h
-MOSTLYCLEANFILES = c-compiler-name.h cp-compiler-name.h
-
-# Put this in a header so we don't run sed for each compilation.  This
-# is also simpler to debug as one can easily see the constant.
-# FIXME: compute it in configure.ac and output it in config.status, or
-# introduce timestamp files for some indirection to avoid rebuilding it
-# every time.
-c-compiler-name.h: Makefile
-	-rm -f $@T
-	echo "#define C_COMPILER_NAME \"`echo gcc | sed '$(transform)'`\"" > $@T
-	mv $@T $@ # $(SHELL) $(srcdir)/../move-if-change $@T $@
-
-cp-compiler-name.h: Makefile
-	-rm -f $@T
-	echo "#define CP_COMPILER_NAME \"`echo g++ | sed '$(transform)'`\"" > $@T
-	mv $@T $@ # $(SHELL) $(srcdir)/../move-if-change $@T $@
-
 shared_source = callbacks.cc callbacks.hh connection.cc connection.hh \
     marshall.cc marshall.hh rpc.hh status.hh
 
diff --git a/libcc1/Makefile.in b/libcc1/Makefile.in
index 54babb02a49..47be10025ad 100644
--- a/libcc1/Makefile.in
+++ b/libcc1/Makefile.in
@@ -307,8 +307,6 @@ plugindir = $(libdir)/gcc/$(target_noncanonical)/$(gcc_version)/plugin
 cc1libdir = $(libdir)/$(libsuffix)
 @ENABLE_PLUGIN_TRUE@plugin_LTLIBRARIES = libcc1plugin.la libcp1plugin.la
 @ENABLE_PLUGIN_TRUE@cc1lib_LTLIBRARIES = libcc1.la
-BUILT_SOURCES = c-compiler-name.h cp-compiler-name.h
-MOSTLYCLEANFILES = c-compiler-name.h cp-compiler-name.h
 shared_source = callbacks.cc callbacks.hh connection.cc connection.hh \
     marshall.cc marshall.hh rpc.hh status.hh
 
@@ -344,7 +342,7 @@ libcc1_la_LINK = $(LIBTOOL) --tag=CXX $(AM_LIBTOOLFLAGS) \
 	$(LIBTOOLFLAGS) --mode=link $(CXXLD) $(AM_CXXFLAGS) \
 	$(CXXFLAGS) $(libcc1_la_LDFLAGS) $(LTLDFLAGS) -o $@
 
-all: $(BUILT_SOURCES) cc1plugin-config.h
+all: cc1plugin-config.h
 	$(MAKE) $(AM_MAKEFLAGS) all-am
 
 .SUFFIXES:
@@ -567,15 +565,13 @@ GTAGS:
 distclean-tags:
 	-rm -f TAGS ID GTAGS GRTAGS GSYMS GPATH tags
 check-am: all-am
-check: $(BUILT_SOURCES)
-	$(MAKE) $(AM_MAKEFLAGS) check-am
+check: check-am
 all-am: Makefile $(LTLIBRARIES) cc1plugin-config.h
 installdirs:
 	for dir in "$(DESTDIR)$(cc1libdir)" "$(DESTDIR)$(plugindir)"; do \
 	  test -z "$$dir" || $(MKDIR_P) "$$dir"; \
 	done
-install: $(BUILT_SOURCES)
-	$(MAKE) $(AM_MAKEFLAGS) install-am
+install: install-am
 install-exec: install-exec-am
 install-data: install-data-am
 uninstall: uninstall-am
@@ -595,7 +591,6 @@ install-strip:
 	    "INSTALL_PROGRAM_ENV=STRIPPROG='$(STRIP)'" install; \
 	fi
 mostlyclean-generic:
-	-test -z "$(MOSTLYCLEANFILES)" || rm -f $(MOSTLYCLEANFILES)
 
 clean-generic:
 
@@ -606,7 +601,6 @@ distclean-generic:
 maintainer-clean-generic:
 	@echo "This command is intended for maintainers to use"
 	@echo "it deletes files that may require special tools to rebuild."
-	-test -z "$(BUILT_SOURCES)" || rm -f $(BUILT_SOURCES)
 clean: clean-am
 
 clean-am: clean-cc1libLTLIBRARIES clean-generic clean-libtool \
@@ -681,7 +675,7 @@ ps-am:
 
 uninstall-am: uninstall-cc1libLTLIBRARIES uninstall-pluginLTLIBRARIES
 
-.MAKE: all check install install-am install-strip
+.MAKE: all install-am install-strip
 
 .PHONY: CTAGS GTAGS all all-am am--refresh check check-am clean \
 	clean-cc1libLTLIBRARIES clean-generic clean-libtool \
@@ -702,21 +696,6 @@ uninstall-am: uninstall-cc1libLTLIBRARIES uninstall-pluginLTLIBRARIES
 override CXXFLAGS := $(filter-out -fsanitize=address,$(CXXFLAGS))
 override LDFLAGS := $(filter-out -fsanitize=address,$(LDFLAGS))
 
-# Put this in a header so we don't run sed for each compilation.  This
-# is also simpler to debug as one can easily see the constant.
-# FIXME: compute it in configure.ac and output it in config.status, or
-# introduce timestamp files for some indirection to avoid rebuilding it
-# every time.
-c-compiler-name.h: Makefile
-	-rm -f $@T
-	echo "#define C_COMPILER_NAME \"`echo gcc | sed '$(transform)'`\"" > $@T
-	mv $@T $@ # $(SHELL) $(srcdir)/../move-if-change $@T $@
-
-cp-compiler-name.h: Makefile
-	-rm -f $@T
-	echo "#define CP_COMPILER_NAME \"`echo g++ | sed '$(transform)'`\"" > $@T
-	mv $@T $@ # $(SHELL) $(srcdir)/../move-if-change $@T $@
-
 # Tell versions [3.59,3.63) of GNU make to not export all variables.
 # Otherwise a system limit (for SysV at least) may be exceeded.
 .NOEXPORT:
diff --git a/libcc1/compiler-name.hh b/libcc1/compiler-name.hh
new file mode 100644
index 00000000000..30cdc41838b
--- /dev/null
+++ b/libcc1/compiler-name.hh
@@ -0,0 +1,29 @@
+/* The names of the compilers we use.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC 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, or (at your option) any later
+version.
+
+GCC 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 GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef COMPILER_NAME_H
+#define COMPILER_NAME_H
+
+// C compiler name.
+#define C_COMPILER_NAME "gcc"
+
+// C++ compiler name.
+#define CP_COMPILER_NAME "g++"
+
+#endif // ! COMPILER_NAME_H
diff --git a/libcc1/libcc1.cc b/libcc1/libcc1.cc
index 0ef6c112dae..1a20dd941a4 100644
--- a/libcc1/libcc1.cc
+++ b/libcc1/libcc1.cc
@@ -37,7 +37,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "libiberty.h"
 #include "xregex.h"
 #include "findcomp.hh"
-#include "c-compiler-name.h"
+#include "compiler-name.hh"
 #include "intl.h"
 
 struct libcc1;
diff --git a/libcc1/libcp1.cc b/libcc1/libcp1.cc
index bbd8488af93..b4f9710e0e2 100644
--- a/libcc1/libcp1.cc
+++ b/libcc1/libcp1.cc
@@ -37,7 +37,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "libiberty.h"
 #include "xregex.h"
 #include "findcomp.hh"
-#include "cp-compiler-name.h"
+#include "compiler-name.hh"
 #include "intl.h"
 
 struct libcp1;