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  <sergi...@redhat.com>
>>  Pedro Alves  <pal...@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

[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  <sergi...@redhat.com>
Pedro Alves  <pal...@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".

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 -

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:

[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 ba

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/

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

[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/

[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  <sergi...@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.  */

[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  <sergi...@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  
ATTRIBUTE_UNUSED) const
 char *
 libcc1::compiler_triplet_regexp::find (std::string ) 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_rx.c_str (), REG_EXTENDED | REG_NOSUB);
+
+  if (code == 0)
+{
+  if (regexec (, C_COMPILER_NAME, 0, NULL, 0) == 0)
+   c_compiler_has_triplet = true;
+
+  regfree ();
+}
+
+  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 (, rx.c_str (), REG_EXTENDED | REG_NOSUB);
+  code = regcomp (, rx.c_str (), REG_EXTENDED | REG_NOSUB);
   if (code != 0)
 {
   size_t len = regerror (code, , 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 << "^" <&l

[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:

  

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/

Re: [PATCH] Make dg-extract-results.sh explicitly treat .{sum,log} files as text

[Sergio Durigan Junior]

On Tuesday, December 16 2014, Jakub Jelinek wrote:

 On Tue, Dec 16, 2014 at 09:36:33AM +, Pedro Alves wrote:
 On 12/15/2014 11:00 PM, Sergio Durigan Junior wrote:
  +# Check if grep supports the '--text' option.
  +
  +GREP_TEXT_OPT=--text
  +if grep --text 21 | grep unrecognized option  /dev/null 21 ; then
  +  GREP_TEXT_OPT=
  +fi
  +
 
 That assumes all greps output unrecognized option on
 unrecognized options.  I don't think that can be counted on being
 portable?  ISTM reversing the logic of the test would be better.
 IOW, test that --text actually works.  Something like this perhaps:
 
 # If grep supports the '--text' option, use it.
 GREP_TEXT_OPT=
 if echo foo | grep --text foo  /dev/null 21 ; then
   GREP_TEXT_OPT=--text
 fi

 Even better include some non-text bytes around the foo string in the input 
 you feed to grep.
 Also, perhaps better would be to use a GREP variable, initialized to
 GREP=grep
 or
 GREP=grep --text
 and just invoke $GREP.

Hm, right.  Thank you Pedro and Jakub for the comments.  I will work on
another version of the patch.

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH] Make dg-extract-results.sh explicitly treat .{sum,log} files as text

[Sergio Durigan Junior]

On Tuesday, December 16 2014, Mike Stump wrote:

 So, either, the tool should not generate 0 in the output, which, is rather 
 anti-social, or one should strip the funny characters in a more portable 
 fashion.

 tr and cat -v come to mind; both should be pretty portable.

 OK to apply?

 Try cat | cat -v in there instead.

Thanks, Mike.  WDYT of the attached patch?  It tries to use grep
--text if that is available, and falls back to using cat -v
otherwise.  Unfortunately, it is necessary to provide the filenames to
grep here:

  CNT=`$GREP '=== .* tests ===' $SUM_FILES | $AWK '{ print $3 }' | sort -u | wc 
-l`

so I couldn't use cat -v $SUM_FILES directly.

Thanks,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

diff --git a/contrib/dg-extract-results.sh b/contrib/dg-extract-results.sh
index a83c8e8..0ddf25b 100755
--- a/contrib/dg-extract-results.sh
+++ b/contrib/dg-extract-results.sh
@@ -127,13 +127,28 @@ do
 done
 test $ERROR -eq 0 || exit 1
 
+# Test if grep supports the '--text' option
+
+GREP=grep
+
+if echo -e '\x00foo\x00' | $GREP --text foo  /dev/null 21 ; then
+  GREP=grep --text
+else
+  # Our grep does not recognize the '--text' option.  We have to
+  # treat our files in order to remove any non-printable character.
+  for file in $SUM_FILES ; do
+mv $file ${file}.orig
+cat -v ${file}.orig  $file
+  done
+fi
+
 if [ -z $TOOL ]; then
   # If no tool was specified, all specified summary files must be for
   # the same tool.
 
-  CNT=`grep '=== .* tests ===' $SUM_FILES | $AWK '{ print $3 }' | sort -u | wc 
-l`
+  CNT=`$GREP '=== .* tests ===' $SUM_FILES | $AWK '{ print $3 }' | sort -u | 
wc -l`
   if [ $CNT -eq 1 ]; then
-TOOL=`grep '=== .* tests ===' $FIRST_SUM | $AWK '{ print $2 }'`
+TOOL=`$GREP '=== .* tests ===' $FIRST_SUM | $AWK '{ print $2 }'`
   else
 msg ${PROGNAME}: sum files are for multiple tools, specify a tool
 msg 
@@ -144,7 +159,7 @@ else
   # Ignore the specified summary files that are not for this tool.  This
   # should keep the relevant files in the same order.
 
-  SUM_FILES=`grep -l === $TOOL $SUM_FILES`
+  SUM_FILES=`$GREP -l === $TOOL $SUM_FILES`
   if test -z $SUM_FILES ; then
 msg ${PROGNAME}: none of the specified files are results for $TOOL
 exit 1
@@ -233,7 +248,7 @@ else
   VARIANTS=
   for VAR in $VARS
   do
-grep Running target $VAR $SUM_FILES  /dev/null  VARIANTS=$VARIANTS 
$VAR
+$GREP Running target $VAR $SUM_FILES  /dev/null  VARIANTS=$VARIANTS 
$VAR
   done
 fi
 
@@ -435,6 +450,6 @@ cat ${TMP}/var-* | $AWK -f $TOTAL_AWK
 # This is ugly, but if there's version output from the compiler under test
 # at the end of the file, we want it.  The other thing that might be there
 # is the final summary counts.
-tail -2 $FIRST_SUM | grep '^#'  /dev/null || tail -2 $FIRST_SUM
+tail -2 $FIRST_SUM | $GREP '^#'  /dev/null || tail -2 $FIRST_SUM
 
 exit 0

Re: [PATCH] Make dg-extract-results.sh explicitly treat .{sum,log} files as text

[Sergio Durigan Junior]

On Tuesday, December 16 2014, Mike Stump wrote:

 Well, I’d still say that ‘\0’ in the output of tools is antisocial and that 
 is the real bug that needs to be fixed.

I agree with you, and I am working on a patch to fix GDB, too.

 That aside, it is reasonable to protect testing from poorly behaving tools…  
 if you really want to.

That is also my rationale for keep wanting to commit this patch.

 Ok.

Thank you.  Despite having had patches accepted for GCC before, I am
still not in the Commit After Approval list (shame!).  Can you commit
this one for me, please?

Thanks,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

[PATCH] Make dg-extract-results.sh explicitly treat .{sum,log} files as text

[Sergio Durigan Junior]

This weekend I was running GDB's testsuite with many options enabled,
and I noticed that, for some specific configurations (specifically
when testing gdbserver), I was getting the following error:

 dg-extract-results.sh: sum files are for multiple tools, specify a tool

I remembered seeing this a lot before, so I spent some time
investigating the cause...

First, I found the line on dg-extract-results.sh that printed this
error message.  The code does:

  CNT=`grep '=== .* tests ===' $SUM_FILES --text | $AWK '{ print $3 }' | sort 
-u | wc -l`
  if [ $CNT -eq 1 ]; then
TOOL=`grep '=== .* tests ===' $FIRST_SUM --text | $AWK '{ print $2 }'`
  else
msg ${PROGNAME}: sum files are for multiple tools, specify a tool
msg 
usage
exit 1
  fi

So, the first thing to do was to identify why $CNT was not 1.  When I
ran the command that generated the result for CNT, I found:

  $ grep '=== .* tests ===' `find outputs -name gdb.log -print` \
 | awk '{ print $3 }' | sort -u | wc -l
  7

Hm, strange.  So, removing the wc command, the output was:

  gdb
  outputs/gdb.base/gdb-sigterm/gdb.log
  outputs/gdb.threads/non-ldr-exc-1/gdb.log
  outputs/gdb.threads/non-ldr-exc-2/gdb.log
  outputs/gdb.threads/non-ldr-exc-3/gdb.log
  outputs/gdb.threads/non-ldr-exc-4/gdb.log
  outputs/gdb.threads/thread-execl/gdb.log

And, when I used only the grep command, without the awk and the sort,
I saw that the majority of the lines were like this:

  outputs/gdb.trace/tfind/gdb.log:=== gdb tests ===

Which would generated the first line in the output above, gdb.  But,
for the other 6 files above, I saw:

  Binary file outputs/gdb.base/gdb-sigterm/gdb.log matches

Right, the problem is that grep is assuming those 6 files are binary,
not text.  This happens because of this code, in grep:

  http://git.savannah.gnu.org/cgit/grep.git/tree/src/grep.c#n526

  static enum textbin
  buffer_textbin (char *buf, size_t size)
  {
if (eolbyte  memchr (buf, '\0', size))
  return TEXTBIN_BINARY;
  ...

If one looks at those 6 files, one will find that they contain the NUL
byte there.  They are all printed by the same message, by gdbserver's
code:

  input_interrupt, count = 0 c = 0 ('^@')

(The ^@ above is the NUL byte.)

Maybe the right fix would be to improve input_interrupt in
gdbserver/remote-utils.c (see PR server/16359), but I decided to go
the easier route and adjust the dg-extract-results.sh to be more
robust when dealing with the sum and log files.  To do that, I am
suggest passing the '--text' option to grep, which overrides grep's
machinery to identify if the file is binary and forces it to treat
every file as text.  For me, it makes sense to do that because sum and
log files will always be text, no matter what happens.  It is also
worth noticing that the Python version of dg-extract-results already
takes care of binary files.

OK to apply?

2014-12-14  Sergio Durigan Junior  sergi...@redhat.com

* dg-extract-results.sh: Pass '--text' option to grep when
filtering .{sum,log} files, which may contain binary data.
---
 contrib/dg-extract-results.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/dg-extract-results.sh b/contrib/dg-extract-results.sh
index a83c8e8..2a85ad4 100755
--- a/contrib/dg-extract-results.sh
+++ b/contrib/dg-extract-results.sh
@@ -131,9 +131,9 @@ if [ -z $TOOL ]; then
   # If no tool was specified, all specified summary files must be for
   # the same tool.
 
-  CNT=`grep '=== .* tests ===' $SUM_FILES | $AWK '{ print $3 }' | sort -u | wc 
-l`
+  CNT=`grep '=== .* tests ===' $SUM_FILES --text | $AWK '{ print $3 }' | sort 
-u | wc -l`
   if [ $CNT -eq 1 ]; then
-TOOL=`grep '=== .* tests ===' $FIRST_SUM | $AWK '{ print $2 }'`
+TOOL=`grep '=== .* tests ===' $FIRST_SUM --text | $AWK '{ print $2 }'`
   else
 msg ${PROGNAME}: sum files are for multiple tools, specify a tool
 msg 
@@ -144,7 +144,7 @@ else
   # Ignore the specified summary files that are not for this tool.  This
   # should keep the relevant files in the same order.
 
-  SUM_FILES=`grep -l === $TOOL $SUM_FILES`
+  SUM_FILES=`grep -l === $TOOL $SUM_FILES --text`
   if test -z $SUM_FILES ; then
 msg ${PROGNAME}: none of the specified files are results for $TOOL
 exit 1
@@ -233,7 +233,7 @@ else
   VARIANTS=
   for VAR in $VARS
   do
-grep Running target $VAR $SUM_FILES  /dev/null  VARIANTS=$VARIANTS 
$VAR
+grep Running target $VAR $SUM_FILES --text  /dev/null  
VARIANTS=$VARIANTS $VAR
   done
 fi
 
-- 
1.9.3

Re: [PATCH] Make dg-extract-results.sh explicitly treat .{sum,log} files as text

[Sergio Durigan Junior]

On Monday, December 15 2014, Jakub Jelinek wrote:

 I'd be surprised if all versions of grep supported --text option (e.g. POSIX
 doesn't mention the -a nor --text options), guess
 you'd need to check for that first (early in the script) and add it only if
 it works.

Thanks for the review, Jakub.

Right, it makes sense to check that indeed.  Take a look at the attached
patch please.

 Also, supposedly the options should come before the regexp and
 list of files.

Right, fixed.

 Why isn't the python version used in your case btw?

Because GDB has not yet merged the new Python version into our codebase.
I am working on it now, too.

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

diff --git a/contrib/dg-extract-results.sh b/contrib/dg-extract-results.sh
index a83c8e8..ebf93bf 100755
--- a/contrib/dg-extract-results.sh
+++ b/contrib/dg-extract-results.sh
@@ -127,13 +127,20 @@ do
 done
 test $ERROR -eq 0 || exit 1
 
+# Check if grep supports the '--text' option.
+
+GREP_TEXT_OPT=--text
+if grep --text 21 | grep unrecognized option  /dev/null 21 ; then
+  GREP_TEXT_OPT=
+fi
+
 if [ -z $TOOL ]; then
   # If no tool was specified, all specified summary files must be for
   # the same tool.
 
-  CNT=`grep '=== .* tests ===' $SUM_FILES | $AWK '{ print $3 }' | sort -u | wc 
-l`
+  CNT=`grep $GREP_TEXT_OPT '=== .* tests ===' $SUM_FILES | $AWK '{ print $3 }' 
| sort -u | wc -l`
   if [ $CNT -eq 1 ]; then
-TOOL=`grep '=== .* tests ===' $FIRST_SUM | $AWK '{ print $2 }'`
+TOOL=`grep $GREP_TEXT_OPT '=== .* tests ===' $FIRST_SUM | $AWK '{ print $2 
}'`
   else
 msg ${PROGNAME}: sum files are for multiple tools, specify a tool
 msg 
@@ -144,7 +151,7 @@ else
   # Ignore the specified summary files that are not for this tool.  This
   # should keep the relevant files in the same order.
 
-  SUM_FILES=`grep -l === $TOOL $SUM_FILES`
+  SUM_FILES=`grep $GREP_TEXT_OPT -l === $TOOL $SUM_FILES`
   if test -z $SUM_FILES ; then
 msg ${PROGNAME}: none of the specified files are results for $TOOL
 exit 1
@@ -233,7 +240,7 @@ else
   VARIANTS=
   for VAR in $VARS
   do
-grep Running target $VAR $SUM_FILES  /dev/null  VARIANTS=$VARIANTS 
$VAR
+grep $GREP_TEXT_OPT Running target $VAR $SUM_FILES  /dev/null  
VARIANTS=$VARIANTS $VAR
   done
 fi
 

Re: [PATCH] Implement stap probe on ARM's unwinder

[Sergio Durigan Junior]

Sergio Durigan Junior sergi...@redhat.com writes:

 Sergio Durigan Junior sergi...@redhat.com writes:

 Bernd Schmidt ber...@codesourcery.com writes:

 On 12/01/11 13:01, Ramana Radhakrishnan wrote:
 Sergio: Other than a few minor tweaks to the Changelog it largely
 looks obvious to me.
 
 Bernd, could you take another look at this since this is now shared
 with the c6x backend ?

 Doesn't look like it would cause problems. I have no idea what
 builtin_frob_return_addr does but it appears to exist everywhere.

 Thanks for the reviews.  I guess I'll leave the call to
 builtin_frob_return_addr there.  So, after addressing Ramana's
 suggestions to ChangeLog, is this patch OK to go in?

 Ping.

Ping ^2.

Re: [PATCH] Implement stap probe on ARM's unwinder

[Sergio Durigan Junior]

Sergio Durigan Junior sergi...@redhat.com writes:

 Bernd Schmidt ber...@codesourcery.com writes:

 On 12/01/11 13:01, Ramana Radhakrishnan wrote:
 Sergio: Other than a few minor tweaks to the Changelog it largely
 looks obvious to me.
 
 Bernd, could you take another look at this since this is now shared
 with the c6x backend ?

 Doesn't look like it would cause problems. I have no idea what
 builtin_frob_return_addr does but it appears to exist everywhere.

 Thanks for the reviews.  I guess I'll leave the call to
 builtin_frob_return_addr there.  So, after addressing Ramana's
 suggestions to ChangeLog, is this patch OK to go in?

Ping.

Re: [PATCH] Implement stap probe on ARM's unwinder

[Sergio Durigan Junior]

Bernd Schmidt ber...@codesourcery.com writes:

 On 12/01/11 13:01, Ramana Radhakrishnan wrote:
 Sergio: Other than a few minor tweaks to the Changelog it largely
 looks obvious to me.
 
 Bernd, could you take another look at this since this is now shared
 with the c6x backend ?

 Doesn't look like it would cause problems. I have no idea what
 builtin_frob_return_addr does but it appears to exist everywhere.

Thanks for the reviews.  I guess I'll leave the call to
builtin_frob_return_addr there.  So, after addressing Ramana's
suggestions to ChangeLog, is this patch OK to go in?

Thanks.

Re: [PATCH] Implement stap probe on ARM's unwinder

[Sergio Durigan Junior]

Ramana Radhakrishnan ramana.radhakrish...@linaro.org writes:

 Sergio: Other than a few minor tweaks to the Changelog it largely
 looks obvious to me.

Hello Ramana,

Thanks for the review.  Here is the updated version of the patch.

I asked Tom Tromey to commit it for me, since I don't have write
permission on the repository.

Thank you again,

Sergio.


diff --git a/libgcc/ChangeLog b/libgcc/ChangeLog
index e7f18e2..0901cae 100644
--- a/libgcc/ChangeLog
+++ b/libgcc/ChangeLog
@@ -1,3 +1,14 @@
+2011-12-02  Sergio Durigan Junior  sergi...@redhat.com
+
+   * unwind-arm-common.inc: Include `tconfig.h', `tsystem.h' and
+   `sys/sdt.h'.
+   (_Unwind_DebugHook): New function.
+   (uw_restore_core_regs): New define.
+   (unwind_phase2): Use uw_restore_core_regs instead of
+   restore_core_regs.
+   (unwind_phase2_forced): Likewise.
+   (__gnu_Unwind_Resume): Likewise.
+
 2011-11-30  John David Anglin  dave.ang...@nrc-cnrc.gc.ca
 
PR other/51272
diff --git a/libgcc/unwind-arm-common.inc b/libgcc/unwind-arm-common.inc
index 0713056..bf16902 100644
--- a/libgcc/unwind-arm-common.inc
+++ b/libgcc/unwind-arm-common.inc
@@ -21,8 +21,15 @@
see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
http://www.gnu.org/licenses/.  */
 
+#include tconfig.h
+#include tsystem.h
 #include unwind.h
 
+/* Used for SystemTap unwinder probe.  */
+#ifdef HAVE_SYS_SDT_H
+#include sys/sdt.h
+#endif
+
 /* We add a prototype for abort here to avoid creating a dependency on
target headers.  */
 extern void abort (void);
@@ -105,6 +112,44 @@ static inline _uw selfrel_offset31 (const _uw *p);
 
 static _uw __gnu_unwind_get_pr_addr (int idx);
 
+static void _Unwind_DebugHook (void *, void *)
+  __attribute__ ((__noinline__, __used__, __noclone__));
+
+/* This function is called during unwinding.  It is intended as a hook
+   for a debugger to intercept exceptions.  CFA is the CFA of the
+   target frame.  HANDLER is the PC to which control will be
+   transferred.  */
+
+static void
+_Unwind_DebugHook (void *cfa __attribute__ ((__unused__)),
+  void *handler __attribute__ ((__unused__)))
+{
+  /* We only want to use stap probes starting with v3.  Earlier
+ versions added too much startup cost.  */
+#if defined (HAVE_SYS_SDT_H)  defined (STAP_PROBE2)  _SDT_NOTE_TYPE = 3
+  STAP_PROBE2 (libgcc, unwind, cfa, handler);
+#else
+  asm ();
+#endif
+}
+
+/* This is a wrapper to be called when we need to restore core registers.
+   It will call `_Unwind_DebugHook' before restoring the registers, thus
+   making it possible to intercept and debug exceptions.
+
+   When calling `_Unwind_DebugHook', the first argument (the CFA) is zero
+   because we are not interested in it.  However, it must be there (even
+   being zero) because GDB expects to find it when using the probe.  */
+
+#define uw_restore_core_regs(TARGET, CORE)   \
+  do \
+{\
+  void *handler = __builtin_frob_return_addr ((void *) VRS_PC (TARGET));  \
+  _Unwind_DebugHook (0, handler);\
+  restore_core_regs (CORE);
  \
+}\
+  while (0)
+
 /* Perform a binary search for RETURN_ADDRESS in TABLE.  The table contains
NREC entries.  */
 
@@ -253,8 +298,8 @@ unwind_phase2 (_Unwind_Control_Block * ucbp, phase2_vrs * 
vrs)
   
   if (pr_result != _URC_INSTALL_CONTEXT)
 abort();
-  
-  restore_core_regs (vrs-core);
+
+  uw_restore_core_regs (vrs, vrs-core);
 }
 
 /* Perform phase2 forced unwinding.  */
@@ -339,7 +384,7 @@ unwind_phase2_forced (_Unwind_Control_Block *ucbp, 
phase2_vrs *entry_vrs,
   return _URC_FAILURE;
 }
 
-  restore_core_regs (saved_vrs.core);
+  uw_restore_core_regs (saved_vrs, saved_vrs.core);
 }
 
 /* This is a very limited implementation of _Unwind_GetCFA.  It returns
@@ -450,7 +495,7 @@ __gnu_Unwind_Resume (_Unwind_Control_Block * ucbp, 
phase2_vrs * entry_vrs)
 {
 case _URC_INSTALL_CONTEXT:
   /* Upload the registers to enter the landing pad.  */
-  restore_core_regs (entry_vrs-core);
+  uw_restore_core_regs (entry_vrs, entry_vrs-core);
 
 case _URC_CONTINUE_UNWIND:
   /* Continue unwinding the next frame.  */

Re: [PATCH] Implement stap probe on ARM's unwinder

[Sergio Durigan Junior]

Sergio Durigan Junior sergi...@redhat.com writes:

 Hello,

 This is the implementation of
 http://gcc.gnu.org/ml/gcc-patches/2011-01/msg01016.html for the ARM
 unwinder.  Since ARM has a different unwinder, I basically replicated
 the existing code (on unwind-dw2.c) into it, with a few modifications in
 order to gather the necessary information for the probe.  This feature
 is pretty useful for GDB, which uses the probe inserted here to
 implement the next over throw feature.

Ping.

[PATCH] Implement stap probe on ARM's unwinder

[Sergio Durigan Junior]

Hello,

This is the implementation of
http://gcc.gnu.org/ml/gcc-patches/2011-01/msg01016.html for the ARM
unwinder.  Since ARM has a different unwinder, I basically replicated
the existing code (on unwind-dw2.c) into it, with a few modifications in
order to gather the necessary information for the probe.  This feature
is pretty useful for GDB, which uses the probe inserted here to
implement the next over throw feature.

I built and regtested it on a Beagle board, and found no regressions.
Is it OK to apply?

Thanks,

Sergio.

diff --git a/libgcc/ChangeLog b/libgcc/ChangeLog
index 305e8ad..f6e9dec 100644
--- a/libgcc/ChangeLog
+++ b/libgcc/ChangeLog
@@ -1,3 +1,15 @@
+2011-11-22  Sergio Durigan Junior  sergi...@redhat.com
+
+   Implement ARM Unwinder SystemTap probe.
+   * unwind-arm-common.inc: Include `tconfig.h', `tsystem.h' and
+   `sys/sdt.h'.
+   (_Unwind_DebugHook): New function.
+   (uw_restore_core_regs): New define.
+   (unwind_phase2): Use `uw_restore_core_regs' instead of
+   `restore_core_regs'.
+   (unwind_phase2_forced): Likewise.
+   (__gnu_Unwind_Resume): Likewise.
+
 2011-11-22  Iain Sandoe  ia...@gcc.gnu.org
 
* config/darwin-crt-tm.c: New file.
diff --git a/libgcc/unwind-arm-common.inc b/libgcc/unwind-arm-common.inc
index 0713056..bf16902 100644
--- a/libgcc/unwind-arm-common.inc
+++ b/libgcc/unwind-arm-common.inc
@@ -21,8 +21,15 @@
see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
http://www.gnu.org/licenses/.  */
 
+#include tconfig.h
+#include tsystem.h
 #include unwind.h
 
+/* Used for SystemTap unwinder probe.  */
+#ifdef HAVE_SYS_SDT_H
+#include sys/sdt.h
+#endif
+
 /* We add a prototype for abort here to avoid creating a dependency on
target headers.  */
 extern void abort (void);
@@ -105,6 +112,44 @@ static inline _uw selfrel_offset31 (const _uw *p);
 
 static _uw __gnu_unwind_get_pr_addr (int idx);
 
+static void _Unwind_DebugHook (void *, void *)
+  __attribute__ ((__noinline__, __used__, __noclone__));
+
+/* This function is called during unwinding.  It is intended as a hook
+   for a debugger to intercept exceptions.  CFA is the CFA of the
+   target frame.  HANDLER is the PC to which control will be
+   transferred.  */
+
+static void
+_Unwind_DebugHook (void *cfa __attribute__ ((__unused__)),
+  void *handler __attribute__ ((__unused__)))
+{
+  /* We only want to use stap probes starting with v3.  Earlier
+ versions added too much startup cost.  */
+#if defined (HAVE_SYS_SDT_H)  defined (STAP_PROBE2)  _SDT_NOTE_TYPE = 3
+  STAP_PROBE2 (libgcc, unwind, cfa, handler);
+#else
+  asm ();
+#endif
+}
+
+/* This is a wrapper to be called when we need to restore core registers.
+   It will call `_Unwind_DebugHook' before restoring the registers, thus
+   making it possible to intercept and debug exceptions.
+
+   When calling `_Unwind_DebugHook', the first argument (the CFA) is zero
+   because we are not interested in it.  However, it must be there (even
+   being zero) because GDB expects to find it when using the probe.  */
+
+#define uw_restore_core_regs(TARGET, CORE)   \
+  do \
+{\
+  void *handler = __builtin_frob_return_addr ((void *) VRS_PC (TARGET));  \
+  _Unwind_DebugHook (0, handler);\
+  restore_core_regs (CORE);
  \
+}\
+  while (0)
+
 /* Perform a binary search for RETURN_ADDRESS in TABLE.  The table contains
NREC entries.  */
 
@@ -253,8 +298,8 @@ unwind_phase2 (_Unwind_Control_Block * ucbp, phase2_vrs * 
vrs)
   
   if (pr_result != _URC_INSTALL_CONTEXT)
 abort();
-  
-  restore_core_regs (vrs-core);
+
+  uw_restore_core_regs (vrs, vrs-core);
 }
 
 /* Perform phase2 forced unwinding.  */
@@ -339,7 +384,7 @@ unwind_phase2_forced (_Unwind_Control_Block *ucbp, 
phase2_vrs *entry_vrs,
   return _URC_FAILURE;
 }
 
-  restore_core_regs (saved_vrs.core);
+  uw_restore_core_regs (saved_vrs, saved_vrs.core);
 }
 
 /* This is a very limited implementation of _Unwind_GetCFA.  It returns
@@ -450,7 +495,7 @@ __gnu_Unwind_Resume (_Unwind_Control_Block * ucbp, 
phase2_vrs * entry_vrs)
 {
 case _URC_INSTALL_CONTEXT:
   /* Upload the registers to enter the landing pad.  */
-  restore_core_regs (entry_vrs-core);
+  uw_restore_core_regs (entry_vrs, entry_vrs-core);
 
 case _URC_CONTINUE_UNWIND:
   /* Continue unwinding the next frame.  */