[v2,1/2] compile: set debug compile: Display GCC driver filename
Commit Message
Hi,
in the mail thread
https://sourceware.org/ml/gdb-patches/2015-04/msg00804.html
the idea of breaking libcc1.so compatibility was rejected.
Therefore this patch series implements full backward/forward GCC/GDB ABI
compatibility.
As discussed in
How to use compile & execute function in GDB
https://sourceware.org/ml/gdb/2015-04/msg00026.html
GDB currently searches for /usr/bin/ARCH-OS-gcc and chooses one but it does not
display which one. It cannot, GCC method set_arguments() does not yet know
whether 'set debug compile' is enabled or not.
Jan
gdb/ChangeLog
2015-04-23 Jan Kratochvil <jan.kratochvil@redhat.com>
* compile/compile.c (compile_to_object): Conditionally pass
compile_debug to GCC's set_arguments.
include/ChangeLog
2015-04-23 Jan Kratochvil <jan.kratochvil@redhat.com>
* gcc-interface.h (enum gcc_base_api_version): Add GCC_FE_VERSION_1.
(struct gcc_base_vtable): Rename set_arguments to set_arguments_v0.
Update comment for compile. New method set_arguments.
---
gdb/compile/compile.c | 10 ++++++++--
include/gcc-interface.h | 48 ++++++++++++++++++++++++++++++-----------------
2 files changed, 39 insertions(+), 19 deletions(-)
Comments
On 04/23/2015 09:34 PM, Jan Kratochvil wrote:
> Hi,
>
> in the mail thread
> https://sourceware.org/ml/gdb-patches/2015-04/msg00804.html
> the idea of breaking libcc1.so compatibility was rejected.
>
> Therefore this patch series implements full backward/forward GCC/GDB ABI
> compatibility.
Thanks.
>
> As discussed in
> How to use compile & execute function in GDB
> https://sourceware.org/ml/gdb/2015-04/msg00026.html
>
> GDB currently searches for /usr/bin/ARCH-OS-gcc and chooses one but it does not
> display which one. It cannot, GCC method set_arguments() does not yet know
> whether 'set debug compile' is enabled or not.
>
Hmm, but does it really make sense to add a "verbose" flag to
particular methods incrementally?
It seems to me that "set debug compile" should enable verbosity in
in the plugin, for all methods. Which in turn suggests to me
that we should have a separate method in the plugin for toggling
verbosity?
Thanks,
Pedro Alves
On Mon, 27 Apr 2015 17:31:18 +0200, Pedro Alves wrote:
> Hmm, but does it really make sense to add a "verbose" flag to
> particular methods incrementally?
>
> It seems to me that "set debug compile" should enable verbosity in
> in the plugin, for all methods. Which in turn suggests to me
> that we should have a separate method in the plugin for toggling
> verbosity?
Are these questions or directions? I do not have an answer for the questions.
Jan
On 04/27/2015 05:47 PM, Jan Kratochvil wrote:
> On Mon, 27 Apr 2015 17:31:18 +0200, Pedro Alves wrote:
>> Hmm, but does it really make sense to add a "verbose" flag to
>> particular methods incrementally?
>>
>> It seems to me that "set debug compile" should enable verbosity in
>> in the plugin, for all methods. Which in turn suggests to me
>> that we should have a separate method in the plugin for toggling
>> verbosity?
>
> Are these questions or directions? I do not have an answer for the questions.
Both. I was curious on the reason why this was added as a flag
to a method, and on your thoughts, because, as I said, I seems to me
that that's not the best direction. So if there's no good reason,
I think it should indeed be made a separate method.
Thanks,
Pedro Alves
On Mon, 27 Apr 2015 19:19:14 +0200, Pedro Alves wrote:
> Both. I was curious on the reason why this was added as a flag
> to a method,
It looks to me as a most simple and working change to the API.
> and on your thoughts,
I do not have any thoughts about GDB, I make minimal changes.
Even these design changes are still wrong because of the incorrect principle
of search for the GCC driver. In LLDB this already works for years better,
properly designed and with compatible licensing.
> I think it should indeed be made a separate method.
OK, I will post a new API for approval so that I do not have to rework the
2 patch series for the 3rd times as this reviewing method is very expensive.
Thanks,
Jan
On 04/27/2015 06:52 PM, Jan Kratochvil wrote:
> I do not have any thoughts about GDB, I make minimal changes.
> Even these design changes are still wrong because of the incorrect principle
> of search for the GCC driver. In LLDB this already works for years better,
> properly designed and with compatible licensing.
"wrong" is subjective. This split is a conscious design decision,
that has advantages like insulating the debugger from compiler ICEs.
And please don't tell me that LLDB/Clang don't crash. ;-)
>> I think it should indeed be made a separate method.
>
> OK, I will post a new API for approval so that I do not have to rework the
> 2 patch series for the 3rd times as this reviewing method is very expensive.
Thanks. Maybe in these cases, we should be sending a single series
with both lists CCed.
Thanks,
Pedro Alves
On Mon, 27 Apr 2015 20:53:46 +0200, Pedro Alves wrote:
> This split is a conscious design decision,
> that has advantages like insulating the debugger from compiler ICEs.
It would be useful if it was optional. This way it complicates the debugging
a lot. AFAIK it is not optional due to packaging reasons, cc1 would need to
be -fPIC which has some performance hit. OTOH that performance hit on x86_64
is say 1% (just my rough guess) but the performance hit of using GCC instead
of Clang is 119%. So I do not see why the -fPIC is not acceptable for GCC.
> And please don't tell me that LLDB/Clang don't crash. ;-)
I find it offtopic here but when you ask all software has bugs, for LLDB/clang:
* clang has not yet crashed for me on Linux during its daily use.
(gcc did but this is just due to using gcc longer / more extensively.)
* lldb has not crashed for me on OSX but I did not test it much there.
* lldb Linux port did crash for me but I do not consider that port finished.
Jan
On Mon, 27 Apr 2015 20:53:46 +0200, Pedro Alves wrote:
> "wrong" is subjective. This split is a conscious design decision,
> that has advantages like insulating the debugger from compiler ICEs.
I haven't looked it up now but IIRC that is not a "conscious design decision"
but just a workaround of buggy GCC which cannot recover/re-run from
compilation errors in the same instance. The ICE resistance was only
a side-effect.
Jan
On 27/04/15 21:36, Jan Kratochvil wrote:
> On Mon, 27 Apr 2015 20:53:46 +0200, Pedro Alves wrote:
>> "wrong" is subjective. This split is a conscious design decision,
>> that has advantages like insulating the debugger from compiler ICEs.
>
> I haven't looked it up now but IIRC that is not a "conscious design decision"
> but just a workaround of buggy GCC which cannot recover/re-run from
> compilation errors in the same instance. The ICE resistance was only
> a side-effect.
>
It's not buggy. We are pushing GCC in new ways. It's a side effect of
change. GCC was designed, and has only run, AFAIK, in this way with
the recent changes of libcc1. The state of GCC has never needed to be
preserved and/or reset (say with GDB and cleanups) as if it
encountered a problem it just exited. Change is good.
But this is not the GCC list. And we probably should not further
exhaust GDB'ers patience with GCC internals. If a plan is set, then
lets just go with it and see if it works?
Otherwise we can just explore until one works ;)
Cheers
Phil
@@ -492,8 +492,14 @@ compile_to_object (struct command_line *cmd, char *cmd_string,
get_args (compiler, gdbarch, &argc, &argv);
make_cleanup_freeargv (argv);
- error_message = compiler->fe->ops->set_arguments (compiler->fe, triplet_rx,
- argc, argv);
+ if (compiler->fe->ops->version >= GCC_FE_VERSION_1)
+ error_message = compiler->fe->ops->set_arguments (compiler->fe, triplet_rx,
+ argc, argv,
+ compile_debug);
+ else
+ error_message = compiler->fe->ops->set_arguments_v0 (compiler->fe,
+ triplet_rx, argc,
+ argv);
if (error_message != NULL)
{
make_cleanup (xfree, error_message);
@@ -44,7 +44,10 @@ struct gcc_base_context;
enum gcc_base_api_version
{
- GCC_FE_VERSION_0 = 0
+ GCC_FE_VERSION_0 = 0,
+
+ /* Parameter verbose has been moved from compile to set_arguments. */
+ GCC_FE_VERSION_1 = 1,
};
/* The operations defined by the GCC base API. This is the vtable for
@@ -64,20 +67,13 @@ struct gcc_base_vtable
unsigned int version;
- /* Set the compiler's command-line options for the next compilation.
- TRIPLET_REGEXP is a regular expression that is used to match the
- configury triplet prefix to the compiler.
- The arguments are copied by GCC. ARGV need not be
- NULL-terminated. The arguments must be set separately for each
- compilation; that is, after a compile is requested, the
- previously-set arguments cannot be reused.
-
- This returns NULL on success. On failure, returns a malloc()d
- error message. The caller is responsible for freeing it. */
+ /* Deprecated GCC_FE_VERSION_0 variant of the GCC_FE_VERSION_1
+ set_arguments method. GCC_FE_VERSION_0 version did not have the
+ verbose parameter. */
- char *(*set_arguments) (struct gcc_base_context *self,
- const char *triplet_regexp,
- int argc, char **argv);
+ char *(*set_arguments_v0) (struct gcc_base_context *self,
+ const char *triplet_regexp,
+ int argc, char **argv);
/* Set the file name of the program to compile. The string is
copied by the method implementation, but the caller must
@@ -94,9 +90,9 @@ struct gcc_base_vtable
void *datum);
/* Perform the compilation. FILENAME is the name of the resulting
- object file. VERBOSE can be set to cause GCC to print some
- information as it works. Returns true on success, false on
- error. */
+ object file. VERBOSE should be the same value as passed
+ to gcc_base_vtable::set_arguments. Returns true on success, false
+ on error. */
int /* bool */ (*compile) (struct gcc_base_context *self,
const char *filename,
@@ -105,6 +101,24 @@ struct gcc_base_vtable
/* Destroy this object. */
void (*destroy) (struct gcc_base_context *self);
+
+ /* Set the compiler's command-line options for the next compilation.
+ TRIPLET_REGEXP is a regular expression that is used to match the
+ configury triplet prefix to the compiler.
+ The arguments are copied by GCC. ARGV need not be
+ NULL-terminated. The arguments must be set separately for each
+ compilation; that is, after a compile is requested, the
+ previously-set arguments cannot be reused. VERBOSE can be set
+ to cause GCC to print some information as it works.
+
+ This returns NULL on success. On failure, returns a malloc()d
+ error message. The caller is responsible for freeing it.
+
+ This method is only available since GCC_FE_VERSION_1. */
+
+ char *(*set_arguments) (struct gcc_base_context *self,
+ const char *triplet_regexp,
+ int argc, char **argv, int /* bool */ verbose);
};
/* The GCC object. */