[libcc1] Improve detection of triplet on compiler names

[libcc1] Improve detection of triplet on compiler names

[Sergio Durigan Junior]

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.

The GCC patch improves the libcc1::compiler_triplet_regexp::find and
libcp1::compiler_triplet_regexp::find methods by first trying to match
the triplet in the compiler name and correctly discarding the triplet
part of the regexp if the matching succeeds.  I've had to do a few
modifications on the way the regexp's are built, but I'll explain them
in the patch itself.

The GDB patch is very simple: it adds the trailing "-" in the triplet
regexp.  Therefore, we will have a regexp that truly matches the full
triplet (e.g., "^(x86_64|i.86)(-[^-]*)?-linux(-gnu)?-") instead of one
that leaves the trailing "-" match to libcc1.

I've tested this patch both on my Fedora and my Debian machines, and
both now work as expected, independently of the presence of the triplet
string in the compiler name.  I am sorry about the cross-post, but these
patches are really dependent on one another.

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/

[PATCH 1/2] [GCC] Improve regexp handling on libc[cp]1::compiler_triplet_regexp::find

[Sergio Durigan Junior]

This is the GCC patch.

It adjusts the behaviour of make_regexp in order to not add the "-" at
the end of the triplet regexp.  This is now GDB's responsibility.

It also improves the 'find' methods of both
libcc1::compiler_triplet_regexp and libcp1::compiler_triplet_regexp
classes by implementing a detection of a triplet string in the compiler
name.  If it finds the triplet there, then the pristine compiler name is
used to match a compiler in the system, without using the initial
triplet regexp provided by GDB.  If the compiler name doesn't start with
a triplet, then the old behaviour is maintained and we'll still search
for compilers using the triplet regexp.

With this patch it is possible to include triplets in compiler names
(via the --program-prefix configure option), like Debian does, for
example.  I chose not to worry too much about possible suffixes (which
can be specified via --program-suffix) because even with them it is
still possible to find the compiler in the system.  It is important to
note that Debian uses suffixes as well.

It is still perfectly possible to find compilers without
prefixes/suffixes, like "gcc" (this is how Fedora names its GCC, by the
way).

OK to apply?

-- 
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-08-23  Sergio Durigan Junior  <sergiodj@redhat.com>

	* libcc1.cc (make_regexp): Don't add dash after triplet regexp.
	Handle case when COMPILER is empty.
	(libcc1::compiler_triplet_regexp::find): Detect when
	C_COMPILER_NAME already contains a triplet.
	* libcp1.cc (make_regexp): Don't add dash after triplet regexp.
	Handle case when COMPILER is empty.
	(libcp1::compiler_triplet_regexp::find): Detect when
	CP_COMPILER_NAME already contains a triplet.

diff --git a/libcc1/libcc1.cc b/libcc1/libcc1.cc
index 0ef6c112dae..41a6e3cca77 100644
--- a/libcc1/libcc1.cc
+++ b/libcc1/libcc1.cc
@@ -336,31 +336,34 @@ make_regexp (const char *triplet_regexp, const char *compiler)
 {
   std::stringstream buf;
 
-  buf << "^" << triplet_regexp << "-";
+  buf << "^" << triplet_regexp;
 
-  // Quote the compiler name in case it has something funny in it.
-  for (const char *p = compiler; *p; ++p)
+  if (compiler[0] != '\0')
     {
-      switch (*p)
+      // Quote the compiler name in case it has something funny in it.
+      for (const char *p = compiler; *p; ++p)
 	{
-	case '.':
-	case '^':
-	case '$':
-	case '*':
-	case '+':
-	case '?':
-	case '(':
-	case ')':
-	case '[':
-	case '{':
-	case '\\':
-	case '|':
-	  buf << '\\';
-	  break;
+	  switch (*p)
+	    {
+	    case '.':
+	    case '^':
+	    case '$':
+	    case '*':
+	    case '+':
+	    case '?':
+	    case '(':
+	    case ')':
+	    case '[':
+	    case '{':
+	    case '\\':
+	    case '|':
+	      buf << '\\';
+	      break;
+	    }
+	  buf << *p;
 	}
-      buf << *p;
+      buf << "$";
     }
-  buf << "$";
 
   return buf.str ();
 }
@@ -382,12 +385,30 @@ libcc1::compiler::find (std::string &compiler ATTRIBUTE_UNUSED) const
 char *
 libcc1::compiler_triplet_regexp::find (std::string &compiler) const
 {
-  std::string rx = make_regexp (triplet_regexp_.c_str (), C_COMPILER_NAME);
+  regex_t triplet;
+  bool c_compiler_has_triplet = false;
+
+  // Some distros, like Debian, choose to use a prefix and a suffix
+  // in their C_COMPILER_NAME, so we try to check if that's the case
+  // here and adjust the regexp's accordingly.
+  std::string triplet_rx = make_regexp (triplet_regexp_.c_str (), "");
+  int code = regcomp (&triplet, triplet_rx.c_str (), REG_EXTENDED | REG_NOSUB);
+
+  if (code == 0)
+    {
+      if (regexec (&triplet, C_COMPILER_NAME, 0, NULL, 0) == 0)
+	c_compiler_has_triplet = true;
+
+      regfree (&triplet);
+    }
+
+  std::string rx = make_regexp (c_compiler_has_triplet
+				? "" : triplet_regexp_.c_str (),
+				C_COMPILER_NAME);
   if (self_->verbose)
     fprintf (stderr, _("searching for compiler matching regex %s\n"),
 	     rx.c_str());
-  regex_t triplet;
-  int code = regcomp (&triplet, rx.c_str (), REG_EXTENDED | REG_NOSUB);
+  code = regcomp (&triplet, rx.c_str (), REG_EXTENDED | REG_NOSUB);
   if (code != 0)
     {
       size_t len = regerror (code, &triplet, NULL, 0);
diff --git a/libcc1/libcp1.cc b/libcc1/libcp1.cc
index bbd8488af93..5bac6e4e37c 100644
--- a/libcc1/libcp1.cc
+++ b/libcc1/libcp1.cc
@@ -360,31 +360,34 @@ make_regexp (const char *triplet_regexp, const char *compiler)
 {
   std::stringstream buf;
 
-  buf << "^" << triplet_regexp << "-";
+  buf << "^" << triplet_regexp;
 
-  // Quote the compiler name in case it has something funny in it.
-  for (const char *p = compiler; *p; ++p)
+  if (compiler[0] != '\0')
     {
-      switch (*p)
+      // Quote the compiler name in case it has something funny in it.
+      for (const char *p = compiler; *p; ++p)
 	{
-	case '.':
-	case '^':
-	case '$':
-	case '*':
-	case '+':
-	case '?':
-	case '(':
-	case ')':
-	case '[':
-	case '{':
-	case '\\':
-	case '|':
-	  buf << '\\';
-	  break;
+	  switch (*p)
+	    {
+	    case '.':
+	    case '^':
+	    case '$':
+	    case '*':
+	    case '+':
+	    case '?':
+	    case '(':
+	    case ')':
+	    case '[':
+	    case '{':
+	    case '\\':
+	    case '|':
+	      buf << '\\';
+	      break;
+	    }
+	  buf << *p;
 	}
-      buf << *p;
+      buf << "$";
     }
-  buf << "$";
 
   return buf.str ();
 }
@@ -406,12 +409,30 @@ libcp1::compiler::find (std::string &compiler ATTRIBUTE_UNUSED) const
 char *
 libcp1::compiler_triplet_regexp::find (std::string &compiler) const
 {
-  std::string rx = make_regexp (triplet_regexp_.c_str (), CP_COMPILER_NAME);
+  regex_t triplet;
+  bool cp_compiler_name_has_triplet = false;
+
+  // Some distros, like Debian, choose to use a prefix and a suffix
+  // in their CP_COMPILER_NAME, so we try to check if that's the case
+  // here and adjust the regexp's accordingly.
+  std::string triplet_rx = make_regexp (triplet_regexp_.c_str (), "");
+  int code = regcomp (&triplet, triplet_rx.c_str (), REG_EXTENDED | REG_NOSUB);
+
+  if (code == 0)
+    {
+      if (regexec (&triplet, CP_COMPILER_NAME, 0, NULL, 0) == 0)
+	cp_compiler_name_has_triplet = true;
+
+      regfree (&triplet);
+    }
+
+  std::string rx = make_regexp (cp_compiler_name_has_triplet
+				? "" : triplet_regexp_.c_str (),
+				CP_COMPILER_NAME);
   if (self_->verbose)
     fprintf (stderr, _("searching for compiler matching regex %s\n"),
 	     rx.c_str());
-  regex_t triplet;
-  int code = regcomp (&triplet, rx.c_str (), REG_EXTENDED | REG_NOSUB);
+  code = regcomp (&triplet, rx.c_str (), REG_EXTENDED | REG_NOSUB);
   if (code != 0)
     {
       size_t len = regerror (code, &triplet, NULL, 0);

[PATCH 2/2] [GDB] Add trailing dash on triplet regexp

[Sergio Durigan Junior]

This is the GDB patch.

It is very simple, and just a necessary adjustment needed because of the
modifications made in the "make_regexp" functions on libcc1.

Now, GDB will provide a full regexp for triplet names, including the
trailing dash ("-").  Therefore, we will have a regexp that truly
matches the full triplet (e.g., "^(x86_64|i.86)(-[^-]*)?-linux(-gnu)?-")
instead of one that leaves the trailing "-" match to libcc1.

OK to apply?

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

gdb/ChangeLog:
2017-08-23  Sergio Durigan Junior  <sergiodj@redhat.com>

	* compile/compile.c (compile_to_object): Add trailing dash on
	triplet regexp.

diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 91e084f89f..0ce77a8b95 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -509,7 +509,7 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
   arch_rx = gdbarch_gnu_triplet_regexp (gdbarch);
 
   /* Allow triplets with or without vendor set.  */
-  triplet_rx = concat (arch_rx, "(-[^-]*)?-", os_rx, (char *) NULL);
+  triplet_rx = concat (arch_rx, "(-[^-]*)?-", os_rx, "-", (char *) NULL);
   make_cleanup (xfree, triplet_rx);
 
   /* Set compiler command-line arguments.  */

Re: [libcc1] Improve detection of triplet on compiler names

[Pedro Alves]

On 08/23/2017 05:17 AM, Sergio Durigan Junior wrote:
> The GCC patch improves the libcc1::compiler_triplet_regexp::find and
> libcp1::compiler_triplet_regexp::find methods by first trying to match
> the triplet in the compiler name and correctly discarding the triplet
> part of the regexp if the matching succeeds.  I've had to do a few
> modifications on the way the regexp's are built, but I'll explain them
> in the patch itself.
> 
> The GDB patch is very simple: it adds the trailing "-" in the triplet
> regexp.  Therefore, we will have a regexp that truly matches the full
> triplet (e.g., "^(x86_64|i.86)(-[^-]*)?-linux(-gnu)?-") instead of one
> that leaves the trailing "-" match to libcc1.
> 
> I've tested this patch both on my Fedora and my Debian machines, and
> both now work as expected, independently of the presence of the triplet
> string in the compiler name.  I am sorry about the cross-post, but these
> patches are really dependent on one another.

Is there a backward/forward compatibility impact?
Does new GDB work with old GCC?
Does old GDB work with new GCC?

Thanks,
Pedro Alves

Re: [libcc1] Improve detection of triplet on compiler names

[Sergio Durigan Junior]

On Wednesday, August 23 2017, Pedro Alves wrote:

> On 08/23/2017 05:17 AM, Sergio Durigan Junior wrote:
>> The GCC patch improves the libcc1::compiler_triplet_regexp::find and
>> libcp1::compiler_triplet_regexp::find methods by first trying to match
>> the triplet in the compiler name and correctly discarding the triplet
>> part of the regexp if the matching succeeds.  I've had to do a few
>> modifications on the way the regexp's are built, but I'll explain them
>> in the patch itself.
>> 
>> The GDB patch is very simple: it adds the trailing "-" in the triplet
>> regexp.  Therefore, we will have a regexp that truly matches the full
>> triplet (e.g., "^(x86_64|i.86)(-[^-]*)?-linux(-gnu)?-") instead of one
>> that leaves the trailing "-" match to libcc1.
>> 
>> I've tested this patch both on my Fedora and my Debian machines, and
>> both now work as expected, independently of the presence of the triplet
>> string in the compiler name.  I am sorry about the cross-post, but these
>> patches are really dependent on one another.
>
> Is there a backward/forward compatibility impact?

Unfortunately, yes.

> Does new GDB work with old GCC?

No.  On Fedora systems, you would get:

  Could not find a compiler matching "^(x86_64|i.86)(-[^-]*)?-linux(-gnu)?--gcc$"

And on Debian:

  Could not find a compiler matching "^(x86_64|i.86)(-[^-]*)?-linux(-gnu)?--x86_64-linux-gnu-gcc-7$"

> Does old GDB work with new GCC?

The situation would be the inverse of what's currently happening.
Old Fedora GDBs would now be broken:

  Could not find a compiler matching "^(x86_64|i.86)(-[^-]*)?-linux(-gnu)?gcc$"

But old Debian GDBs would now start working.

As can be seen, these failures are now happening because of the trailing
dash that is now included in the triplet regexp by GDB.  I don't know if
that warrants a change in the API, though.

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/

Re: [libcc1] Improve detection of triplet on compiler names

[Pedro Alves]

On 08/23/2017 02:07 PM, Sergio Durigan Junior wrote:
> On Wednesday, August 23 2017, Pedro Alves wrote:
> 
>> On 08/23/2017 05:17 AM, Sergio Durigan Junior wrote:
>>> The GCC patch improves the libcc1::compiler_triplet_regexp::find and
>>> libcp1::compiler_triplet_regexp::find methods by first trying to match
>>> the triplet in the compiler name and correctly discarding the triplet
>>> part of the regexp if the matching succeeds.  I've had to do a few
>>> modifications on the way the regexp's are built, but I'll explain them
>>> in the patch itself.
>>>
>>> The GDB patch is very simple: it adds the trailing "-" in the triplet
>>> regexp.  Therefore, we will have a regexp that truly matches the full
>>> triplet (e.g., "^(x86_64|i.86)(-[^-]*)?-linux(-gnu)?-") instead of one
>>> that leaves the trailing "-" match to libcc1.
>>>
>>> I've tested this patch both on my Fedora and my Debian machines, and
>>> both now work as expected, independently of the presence of the triplet
>>> string in the compiler name.  I am sorry about the cross-post, but these
>>> patches are really dependent on one another.
>>
>> Is there a backward/forward compatibility impact?
> 
> Unfortunately, yes.
> 
>> Does new GDB work with old GCC?
> 
> No.  On Fedora systems, you would get:
> 
>   Could not find a compiler matching "^(x86_64|i.86)(-[^-]*)?-linux(-gnu)?--gcc$"
> 

That's a problem then.  Please read this:

 https://sourceware.org/gdb/wiki/GCCCompileAndExecute#How_to_extend_the_gdb.2BAC8-gcc_interface

> As can be seen, these failures are now happening because of the trailing
> dash that is now included in the triplet regexp by GDB.  I don't know if
> that warrants a change in the API, though.

Thanks,
Pedro Alves

Re: [libcc1] Improve detection of triplet on compiler names

[Sergio Durigan Junior]

On Wednesday, August 23 2017, Pedro Alves wrote:

> On 08/23/2017 02:07 PM, Sergio Durigan Junior wrote:
>> On Wednesday, August 23 2017, Pedro Alves wrote:
>> 
>>> On 08/23/2017 05:17 AM, Sergio Durigan Junior wrote:
>>>> The GCC patch improves the libcc1::compiler_triplet_regexp::find and
>>>> libcp1::compiler_triplet_regexp::find methods by first trying to match
>>>> the triplet in the compiler name and correctly discarding the triplet
>>>> part of the regexp if the matching succeeds.  I've had to do a few
>>>> modifications on the way the regexp's are built, but I'll explain them
>>>> in the patch itself.
>>>>
>>>> The GDB patch is very simple: it adds the trailing "-" in the triplet
>>>> regexp.  Therefore, we will have a regexp that truly matches the full
>>>> triplet (e.g., "^(x86_64|i.86)(-[^-]*)?-linux(-gnu)?-") instead of one
>>>> that leaves the trailing "-" match to libcc1.
>>>>
>>>> I've tested this patch both on my Fedora and my Debian machines, and
>>>> both now work as expected, independently of the presence of the triplet
>>>> string in the compiler name.  I am sorry about the cross-post, but these
>>>> patches are really dependent on one another.
>>>
>>> Is there a backward/forward compatibility impact?
>> 
>> Unfortunately, yes.
>> 
>>> Does new GDB work with old GCC?
>> 
>> No.  On Fedora systems, you would get:
>> 
>>   Could not find a compiler matching "^(x86_64|i.86)(-[^-]*)?-linux(-gnu)?--gcc$"
>> 
>
> That's a problem then.  Please read this:
>
>  https://sourceware.org/gdb/wiki/GCCCompileAndExecute#How_to_extend_the_gdb.2BAC8-gcc_interface

OK, thanks, I'll follow the advices from the wiki.

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

Re: [libcc1] Improve detection of triplet on compiler names

[Pedro Alves]

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?

Thanks,
Pedro Alves

Re: [libcc1] Improve detection of triplet on compiler names

[Sergio Durigan Junior]

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".

Thanks for summarizing the discussion.

I tend to agree with the rationale, and having C_COMPILER_NAME as "gcc"
is also a valid fix for the original problem.

As I said, I'd also like to hear others' opinions about the issue.  I
can then submit a proper patch.

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/

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

[Sergio Durigan Junior]

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;

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

[Sergio Durigan Junior]

Ping.

On Friday, September 01 2017, I wrote:

> 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;

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

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

[Sergio Durigan Junior]

Ping^2.

On Friday, September 15 2017, I wrote:

> Ping.
>
> On Friday, September 01 2017, I wrote:
>
>> 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;
>
> -- 
> Sergio
> GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
> Please send encrypted e-mail if possible
> http://sergiodj.net/

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

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

[Sergio Durigan Junior]

On Tuesday, September 26 2017, I wrote:

> Ping^2.

Ping^3.

I'm sending the updated ChangeLog/patch.  I'm also removing gdb-patches
from the Cc list.

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.


> On Friday, September 15 2017, I wrote:
>
>> Ping.
>>
>> On Friday, September 01 2017, I wrote:
>>
>>> 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/

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;

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

[Alexandre Oliva]

On Nov 13, 2017, Sergio Durigan Junior <sergiodj@redhat.com> wrote:

> On Tuesday, September 26 2017, I wrote:
>> Ping^2.

> Ping^3.

> I'm sending the updated ChangeLog/patch.  I'm also removing gdb-patches
> from the Cc list.

> 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.

I agree with Pedro's rationale.

I'm not sure I have authority to approve changes to libcc1, but since
this hasn't got a review for so long, and I'm the one who last touched
it, I kind of feel responsible and able, so I suggest that, if nobody
objects till Thursday, this is ok to install.

Thanks!

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

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

[Jim Wilson]

On 11/13/2017 01:10 PM, Sergio Durigan Junior wrote:
> On Tuesday, September 26 2017, I wrote:
> 
>> Ping^2.
> 
> Ping^3.
> 
> I'm sending the updated ChangeLog/patch.  I'm also removing gdb-patches
> from the Cc list.
> 
> 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.

OK.

This is a gcc plugin for gdb, so it makes sense that gdb developers 
should be allowed to decide how it should work.

Jim

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

[Sergio Durigan Junior]

On Wednesday, November 15 2017, Jim Wilson wrote:

> On 11/13/2017 01:10 PM, Sergio Durigan Junior wrote:
>> On Tuesday, September 26 2017, I wrote:
>>
>>> Ping^2.
>>
>> Ping^3.
>>
>> I'm sending the updated ChangeLog/patch.  I'm also removing gdb-patches
>> from the Cc list.
>>
>> 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.
>
> OK.
>
> This is a gcc plugin for gdb, so it makes sense that gdb developers
> should be allowed to decide how it should work.

Thanks Jim and Alex for the review.

I don't have permission to push to the GCC repository, so if one of you
guys could do it for me I'd appreciate.

Thank you,

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

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

[Jeff Law]

On 11/15/2017 09:12 PM, Sergio Durigan Junior wrote:
> On Wednesday, November 15 2017, Jim Wilson wrote:
> 
>> On 11/13/2017 01:10 PM, Sergio Durigan Junior wrote:
>>> On Tuesday, September 26 2017, I wrote:
>>>
>>>> Ping^2.
>>>
>>> Ping^3.
>>>
>>> I'm sending the updated ChangeLog/patch.  I'm also removing gdb-patches
>>> from the Cc list.
>>>
>>> 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.
>>
>> OK.
>>
>> This is a gcc plugin for gdb, so it makes sense that gdb developers
>> should be allowed to decide how it should work.
> 
> Thanks Jim and Alex for the review.
> 
> I don't have permission to push to the GCC repository, so if one of you
> guys could do it for me I'd appreciate.
Done.
jeff