public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename
@ 2015-04-23 20:34 Jan Kratochvil
  2015-04-23 20:34 ` [PATCH v2 2/2] compile: Add 'set compile-gcc' Jan Kratochvil
  2015-04-27 15:31 ` [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename Pedro Alves
  0 siblings, 2 replies; 14+ messages in thread
From: Jan Kratochvil @ 2015-04-23 20:34 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

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(-)

diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 90cfc36..7f4c11d 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -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);
diff --git a/include/gcc-interface.h b/include/gcc-interface.h
index df7db6e..c11b7a1 100644
--- a/include/gcc-interface.h
+++ b/include/gcc-interface.h
@@ -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.  */

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 2/2] compile: Add 'set compile-gcc'
  2015-04-23 20:34 [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename Jan Kratochvil
@ 2015-04-23 20:34 ` Jan Kratochvil
  2015-04-23 21:08   ` [PATCH v3 " Jan Kratochvil
  2015-04-27 15:31 ` [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename Pedro Alves
  1 sibling, 1 reply; 14+ messages in thread
From: Jan Kratochvil @ 2015-04-23 20:34 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

Hi,

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 one cannot
override which one.

This patch does not change the libcc1 API as it overloads the triplet_regexp
parameter of GCC's set_arguments.  GCC decides on the meaning of the
triplet_regexp parameter according to the success of:

+  if (access (triplet_regexp, X_OK) == 0)


Jan


gdb/ChangeLog
2015-04-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* compile/compile.c (compile_gcc, show_compile_gcc): New.
	(compile_to_object): Implement compile_gcc.
	(_initialize_compile): Install "set compile-gcc".  Initialize
	compile_gcc.

gdb/doc/ChangeLog
2015-04-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* doc/gdb.texinfo (Compiling and Injecting Code): Add to subsection
	Compiler search for the compile command descriptions of set
	compile-gcc and show compile-gcc.

include/ChangeLog
2015-04-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gcc-interface.h (enum gcc_base_api_version): Add comment to
	GCC_FE_VERSION_1.
	(struct gcc_base_vtable): Describe triplet_regexp parameter overload
	for set_arguments.
---
 gdb/compile/compile.c   |   40 +++++++++++++++++++++++++++++++++-------
 gdb/doc/gdb.texinfo     |   33 ++++++++++++++++++++++++++++-----
 include/gcc-interface.h |    7 +++++--
 3 files changed, 66 insertions(+), 14 deletions(-)

diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 7f4c11d..c5267ba 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -299,6 +299,19 @@ append_args (int *argcp, char ***argvp, int argc, char **argv)
   (*argvp)[(*argcp)] = NULL;
 }
 
+/* String for 'set compile-gcc' and 'show compile-gcc'.  */
+static char *compile_gcc;
+
+/* Implement 'show compile-gcc'.  */
+
+static void
+show_compile_gcc (struct ui_file *file, int from_tty,
+		  struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Compile command GCC driver filename is \"%s\".\n"),
+		    value);
+}
+
 /* Return DW_AT_producer parsed for get_selected_frame () (if any).
    Return NULL otherwise.
 
@@ -430,8 +443,6 @@ compile_to_object (struct command_line *cmd, char *cmd_string,
   int ok;
   FILE *src;
   struct gdbarch *gdbarch = get_current_arch ();
-  const char *os_rx;
-  const char *arch_rx;
   char *triplet_rx;
   char *error_message;
 
@@ -481,12 +492,17 @@ compile_to_object (struct command_line *cmd, char *cmd_string,
   if (compile_debug)
     fprintf_unfiltered (gdb_stdout, "debug output:\n\n%s", code);
 
-  os_rx = osabi_triplet_regexp (gdbarch_osabi (gdbarch));
-  arch_rx = gdbarch_gnu_triplet_regexp (gdbarch);
+  if (compile_gcc[0] != 0)
+    triplet_rx = compile_gcc;
+  else
+    {
+      const char *os_rx = osabi_triplet_regexp (gdbarch_osabi (gdbarch));
+      const char *arch_rx = gdbarch_gnu_triplet_regexp (gdbarch);
 
-  /* Allow triplets with or without vendor set.  */
-  triplet_rx = concat (arch_rx, "(-[^-]*)?-", os_rx, (char *) NULL);
-  make_cleanup (xfree, triplet_rx);
+      /* Allow triplets with or without vendor set.  */
+      triplet_rx = concat (arch_rx, "(-[^-]*)?-", os_rx, (char *) NULL);
+      make_cleanup (xfree, triplet_rx);
+    }
 
   /* Set compiler command-line arguments.  */
   get_args (compiler, gdbarch, &argc, &argv);
@@ -688,4 +704,14 @@ String quoting is parsed like in shell, for example:\n\
 			 " -fno-stack-protector"
   );
   set_compile_args (compile_args, 0, NULL);
+
+  add_setshow_string_cmd ("compile-gcc", class_support,
+			  &compile_gcc,
+			  _("Set compile command GCC driver filename"),
+			  _("Show compile command GCC driver filename"),
+			  _("\
+It should be absolute pathname to the gcc executable.\n\
+If empty the default target triplet will be searched in $PATH."),
+			  NULL, show_compile_gcc, &setlist, &showlist);
+  compile_gcc = xstrdup ("");
 }
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0410702..8f3777d 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17437,11 +17437,13 @@ will print to the console.
 
 @value{GDBN} needs to find @value{NGCC} for the inferior being debugged which
 may not be obvious for remote targets of different architecture than where
-@value{GDBN} is running.  Environment variable @code{PATH} (@code{PATH} from
-shell that executed @value{GDBN}, not the one set by @value{GDBN}
-command @code{set environment}).  @xref{Environment}.  @code{PATH} on
-@value{GDBN} host is searched for @value{NGCC} binary matching the
-target architecture and operating system.
+@value{GDBN} is running.  By default the environment variable
+@code{PATH} (@code{PATH} from shell that executed @value{GDBN}, not the
+one set by @value{GDBN} command @code{set environment}).
+@xref{Environment}.  @code{PATH} on @value{GDBN} host is searched for
+@value{NGCC} binary matching the target architecture and operating
+system.  This search can be overriden by @code{set compile-gcc} @value{GDBN}
+command below.
 
 Specifically @code{PATH} is searched for binaries matching regular expression
 @code{@var{arch}(-[^-]*)?-@var{os}-gcc} according to the inferior target being
@@ -17451,6 +17453,27 @@ example both @code{i386} and @code{x86_64} targets look for pattern
 for pattern @code{s390x?}.  @var{os} is currently supported only for
 pattern @code{linux(-gnu)?}.
 
+Besides the compiler driver @value{GDBN} needs also shared library
+@file{libcc1.so}.  It is searched in default shared library search path
+(overridable with usual environment variable @code{LD_LIBRARY_PATH}),
+unrelated to @code{PATH} or @code{set compile-gcc} settings.  Contrary to it
+@file{libcc1plugin.so} is found according to the installation of the found
+compiler --- as possibly specified by the @code{set compile-gcc} command.
+
+@table @code
+@item set compile-gcc
+@cindex compile command driver filename override
+Set compilation command used for compiling and injecting code with the
+@code{compile} commands.  If this option is not set (it is set to
+an empty string), the search described above will occur --- that is the
+default.
+
+@item show compile-gcc
+Displays the current compile command @value{NGCC} driver filename.
+If set, it is the main command @code{gcc}, found usually for example
+under name @code{x86_64-linux-gnu-gcc}.
+@end table
+
 @node GDB Files
 @chapter @value{GDBN} Files
 
diff --git a/include/gcc-interface.h b/include/gcc-interface.h
index c11b7a1..81925b1 100644
--- a/include/gcc-interface.h
+++ b/include/gcc-interface.h
@@ -46,7 +46,9 @@ enum gcc_base_api_version
 {
   GCC_FE_VERSION_0 = 0,
 
-  /* Parameter verbose has been moved from compile to set_arguments.  */
+  /* Parameter verbose has been moved from compile to set_arguments.
+     Parameter triplet_regexp of set_arguments can be also gcc driver
+     executable.  */
   GCC_FE_VERSION_1 = 1,
 };
 
@@ -104,7 +106,8 @@ struct gcc_base_vtable
 
   /* 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.
+     configury triplet prefix to the compiler; TRIPLET_REGEXP can be
+     also absolute filename  to the computer.
      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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 2/2] compile: Add 'set compile-gcc'
  2015-04-23 20:34 ` [PATCH v2 2/2] compile: Add 'set compile-gcc' Jan Kratochvil
@ 2015-04-23 21:08   ` Jan Kratochvil
  2015-04-27 15:47     ` Pedro Alves
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Kratochvil @ 2015-04-23 21:08 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon


[-- Attachment #0: Type: message/rfc822, Size: 7783 bytes --]

From: Jan Kratochvil <jan.kratochvil@redhat.com>
Subject: [PATCH] compile: Add 'set compile-gcc'
Date: Thu, 23 Apr 2015 21:39:55 +0200

Hi,

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 one cannot
override which one.

This patch does not change the libcc1 API as it overloads the triplet_regexp
parameter of GCC's set_arguments.  GCC decides on the meaning of the
triplet_regexp parameter according to the success of:

+  if (access (triplet_regexp, X_OK) == 0)


Jan


gdb/ChangeLog
2015-04-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* compile/compile.c (compile_gcc, show_compile_gcc): New.
	(compile_to_object): Implement compile_gcc.
	(_initialize_compile): Install "set compile-gcc".  Initialize
	compile_gcc.

gdb/doc/ChangeLog
2015-04-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* doc/gdb.texinfo (Compiling and Injecting Code): Add to subsection
	Compiler search for the compile command descriptions of set
	compile-gcc and show compile-gcc.

include/ChangeLog
2015-04-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gcc-interface.h (enum gcc_base_api_version): Add comment to
	GCC_FE_VERSION_1.
	(struct gcc_base_vtable): Describe triplet_regexp parameter overload
	for set_arguments.
---
 gdb/compile/compile.c   | 40 +++++++++++++++++++++++++++++++++-------
 gdb/doc/gdb.texinfo     | 36 ++++++++++++++++++++++++++++++------
 include/gcc-interface.h |  7 +++++--
 3 files changed, 68 insertions(+), 15 deletions(-)

diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 7f4c11d..a4ca7db 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -299,6 +299,19 @@ append_args (int *argcp, char ***argvp, int argc, char **argv)
   (*argvp)[(*argcp)] = NULL;
 }
 
+/* String for 'set compile-gcc' and 'show compile-gcc'.  */
+static char *compile_gcc;
+
+/* Implement 'show compile-gcc'.  */
+
+static void
+show_compile_gcc (struct ui_file *file, int from_tty,
+		  struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Compile command GCC driver filename is \"%s\".\n"),
+		    value);
+}
+
 /* Return DW_AT_producer parsed for get_selected_frame () (if any).
    Return NULL otherwise.
 
@@ -430,8 +443,6 @@ compile_to_object (struct command_line *cmd, char *cmd_string,
   int ok;
   FILE *src;
   struct gdbarch *gdbarch = get_current_arch ();
-  const char *os_rx;
-  const char *arch_rx;
   char *triplet_rx;
   char *error_message;
 
@@ -481,12 +492,17 @@ compile_to_object (struct command_line *cmd, char *cmd_string,
   if (compile_debug)
     fprintf_unfiltered (gdb_stdout, "debug output:\n\n%s", code);
 
-  os_rx = osabi_triplet_regexp (gdbarch_osabi (gdbarch));
-  arch_rx = gdbarch_gnu_triplet_regexp (gdbarch);
+  if (compile_gcc[0] != 0)
+    triplet_rx = compile_gcc;
+  else
+    {
+      const char *os_rx = osabi_triplet_regexp (gdbarch_osabi (gdbarch));
+      const char *arch_rx = gdbarch_gnu_triplet_regexp (gdbarch);
 
-  /* Allow triplets with or without vendor set.  */
-  triplet_rx = concat (arch_rx, "(-[^-]*)?-", os_rx, (char *) NULL);
-  make_cleanup (xfree, triplet_rx);
+      /* Allow triplets with or without vendor set.  */
+      triplet_rx = concat (arch_rx, "(-[^-]*)?-", os_rx, (char *) NULL);
+      make_cleanup (xfree, triplet_rx);
+    }
 
   /* Set compiler command-line arguments.  */
   get_args (compiler, gdbarch, &argc, &argv);
@@ -688,4 +704,14 @@ String quoting is parsed like in shell, for example:\n\
 			 " -fno-stack-protector"
   );
   set_compile_args (compile_args, 0, NULL);
+
+  add_setshow_filename_cmd ("compile-gcc", class_support,
+			    &compile_gcc,
+			    _("Set compile command GCC driver filename"),
+			    _("Show compile command GCC driver filename"),
+			    _("\
+It should be absolute pathname to the gcc executable.\n\
+If empty the default target triplet will be searched in $PATH."),
+			    NULL, show_compile_gcc, &setlist, &showlist);
+  compile_gcc = xstrdup ("");
 }
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0410702..cbcc2ee 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17435,13 +17435,15 @@ will print to the console.
 
 @subsection Compiler search for the @code{compile} command
 
-@value{GDBN} needs to find @value{NGCC} for the inferior being debugged which
-may not be obvious for remote targets of different architecture than where
-@value{GDBN} is running.  Environment variable @code{PATH} (@code{PATH} from
-shell that executed @value{GDBN}, not the one set by @value{GDBN}
-command @code{set environment}).  @xref{Environment}.  @code{PATH} on
+@value{GDBN} needs to find @value{NGCC} for the inferior being debugged
+which may not be obvious for remote targets of different architecture
+than where @value{GDBN} is running.  Environment variable @code{PATH} on
 @value{GDBN} host is searched for @value{NGCC} binary matching the
-target architecture and operating system.
+target architecture and operating system.  This search can be overriden
+by @code{set compile-gcc} @value{GDBN} command below.  @code{PATH} is
+taken from shell that executed @value{GDBN}, it is not the value set by
+@value{GDBN} command @code{set environment}).  @xref{Environment}.
+
 
 Specifically @code{PATH} is searched for binaries matching regular expression
 @code{@var{arch}(-[^-]*)?-@var{os}-gcc} according to the inferior target being
@@ -17451,6 +17453,28 @@ example both @code{i386} and @code{x86_64} targets look for pattern
 for pattern @code{s390x?}.  @var{os} is currently supported only for
 pattern @code{linux(-gnu)?}.
 
+On Posix hosts the compiler driver @value{GDBN} needs to find also
+shared library @file{libcc1.so} from the compiler.  It is searched in
+default shared library search path (overridable with usual environment
+variable @code{LD_LIBRARY_PATH}), unrelated to @code{PATH} or @code{set
+compile-gcc} settings.  Contrary to it @file{libcc1plugin.so} is found
+according to the installation of the found compiler --- as possibly
+specified by the @code{set compile-gcc} command.
+
+@table @code
+@item set compile-gcc
+@cindex compile command driver filename override
+Set compilation command used for compiling and injecting code with the
+@code{compile} commands.  If this option is not set (it is set to
+an empty string), the search described above will occur --- that is the
+default.
+
+@item show compile-gcc
+Displays the current compile command @value{NGCC} driver filename.
+If set, it is the main command @code{gcc}, found usually for example
+under name @code{x86_64-linux-gnu-gcc}.
+@end table
+
 @node GDB Files
 @chapter @value{GDBN} Files
 
diff --git a/include/gcc-interface.h b/include/gcc-interface.h
index c11b7a1..81925b1 100644
--- a/include/gcc-interface.h
+++ b/include/gcc-interface.h
@@ -46,7 +46,9 @@ enum gcc_base_api_version
 {
   GCC_FE_VERSION_0 = 0,
 
-  /* Parameter verbose has been moved from compile to set_arguments.  */
+  /* Parameter verbose has been moved from compile to set_arguments.
+     Parameter triplet_regexp of set_arguments can be also gcc driver
+     executable.  */
   GCC_FE_VERSION_1 = 1,
 };
 
@@ -104,7 +106,8 @@ struct gcc_base_vtable
 
   /* 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.
+     configury triplet prefix to the compiler; TRIPLET_REGEXP can be
+     also absolute filename  to the computer.
      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
-- 
2.1.0

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename
  2015-04-23 20:34 [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename Jan Kratochvil
  2015-04-23 20:34 ` [PATCH v2 2/2] compile: Add 'set compile-gcc' Jan Kratochvil
@ 2015-04-27 15:31 ` Pedro Alves
  2015-04-27 16:48   ` Jan Kratochvil
  1 sibling, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2015-04-27 15:31 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/2] compile: Add 'set compile-gcc'
  2015-04-23 21:08   ` [PATCH v3 " Jan Kratochvil
@ 2015-04-27 15:47     ` Pedro Alves
  2015-04-27 17:54       ` Jan Kratochvil
  0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2015-04-27 15:47 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

On 04/23/2015 10:08 PM, Jan Kratochvil wrote:

> @@ -688,4 +704,14 @@ String quoting is parsed like in shell, for example:\n\
>  			 " -fno-stack-protector"
>    );
>    set_compile_args (compile_args, 0, NULL);
> +
> +  add_setshow_filename_cmd ("compile-gcc", class_support,
> +			    &compile_gcc,
> +			    _("Set compile command GCC driver filename"),
> +			    _("Show compile command GCC driver filename"),
> +			    _("\
> +It should be absolute pathname to the gcc executable.\n\
> +If empty the default target triplet will be searched in $PATH."),
> +			    NULL, show_compile_gcc, &setlist, &showlist);
> +  compile_gcc = xstrdup ("");

...

> +@table @code
> +@item set compile-gcc
> +@cindex compile command driver filename override
> +Set compilation command used for compiling and injecting code with the
> +@code{compile} commands.  If this option is not set (it is set to
> +an empty string), the search described above will occur --- that is the
> +default.
> +

IIUC, gdb will always apply the same search as when this is set
empty?  That is, the user can also set this to a regex.  So it seems to me
that the documentation (manual and help) doesn't match the implementation?

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename
  2015-04-27 15:31 ` [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename Pedro Alves
@ 2015-04-27 16:48   ` Jan Kratochvil
  2015-04-27 17:19     ` Pedro Alves
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Kratochvil @ 2015-04-27 16:48 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename
  2015-04-27 16:48   ` Jan Kratochvil
@ 2015-04-27 17:19     ` Pedro Alves
  2015-04-27 17:52       ` Jan Kratochvil
  0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2015-04-27 17:19 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches, Phil Muldoon

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename
  2015-04-27 17:19     ` Pedro Alves
@ 2015-04-27 17:52       ` Jan Kratochvil
  2015-04-27 19:17         ` Pedro Alves
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Kratochvil @ 2015-04-27 17:52 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/2] compile: Add 'set compile-gcc'
  2015-04-27 15:47     ` Pedro Alves
@ 2015-04-27 17:54       ` Jan Kratochvil
  2015-04-27 19:55         ` Pedro Alves
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Kratochvil @ 2015-04-27 17:54 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Mon, 27 Apr 2015 17:47:42 +0200, Pedro Alves wrote:
> IIUC, gdb will always apply the same search as when this is set
> empty?  That is, the user can also set this to a regex.  So it seems to me
> that the documentation (manual and help) doesn't match the implementation?

That it can be also a regex is an API bug because I wanted to make a minimal
API change.  Rather than officially documenting such bug I find then better to
rather make a proper complex change to the API.  Given that you requested an
API rework anyway I will try to post the new API even with this change.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename
  2015-04-27 17:52       ` Jan Kratochvil
@ 2015-04-27 19:17         ` Pedro Alves
  2015-04-27 20:44           ` Jan Kratochvil
  2015-04-27 20:50           ` Jan Kratochvil
  0 siblings, 2 replies; 14+ messages in thread
From: Pedro Alves @ 2015-04-27 19:17 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches, Phil Muldoon

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/2] compile: Add 'set compile-gcc'
  2015-04-27 17:54       ` Jan Kratochvil
@ 2015-04-27 19:55         ` Pedro Alves
  0 siblings, 0 replies; 14+ messages in thread
From: Pedro Alves @ 2015-04-27 19:55 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches, Phil Muldoon

On 04/27/2015 06:54 PM, Jan Kratochvil wrote:
> On Mon, 27 Apr 2015 17:47:42 +0200, Pedro Alves wrote:
>> IIUC, gdb will always apply the same search as when this is set
>> empty?  That is, the user can also set this to a regex.  So it seems to me
>> that the documentation (manual and help) doesn't match the implementation?
> 
> That it can be also a regex is an API bug because I wanted to make a minimal
> API change.  Rather than officially documenting such bug I find then better to
> rather make a proper complex change to the API.  Given that you requested an
> API rework anyway I will try to post the new API even with this change.

This overload had given me lots of pause, and trying to think it through
(it wasn't clear what the intention was), it seemed to me that it kind of made at
least some sense to allow specifying a different regex, but details of the search
algorithms are foggy to me.  It probably really doesn't make sense to overload.
Given we now clearly understand how to add new methods and it isn't that complex,
and we're already bumping the API, yes, let's please avoid an overload hack
when we don't need it, avoiding such confusions.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename
  2015-04-27 19:17         ` Pedro Alves
@ 2015-04-27 20:44           ` Jan Kratochvil
  2015-04-27 20:50           ` Jan Kratochvil
  1 sibling, 0 replies; 14+ messages in thread
From: Jan Kratochvil @ 2015-04-27 20:44 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename
  2015-04-27 19:17         ` Pedro Alves
  2015-04-27 20:44           ` Jan Kratochvil
@ 2015-04-27 20:50           ` Jan Kratochvil
  2015-04-27 21:49             ` Phil Muldoon
  1 sibling, 1 reply; 14+ messages in thread
From: Jan Kratochvil @ 2015-04-27 20:50 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename
  2015-04-27 20:50           ` Jan Kratochvil
@ 2015-04-27 21:49             ` Phil Muldoon
  0 siblings, 0 replies; 14+ messages in thread
From: Phil Muldoon @ 2015-04-27 21:49 UTC (permalink / raw)
  To: Jan Kratochvil, Pedro Alves; +Cc: gdb-patches

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-04-27 20:44 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-23 20:34 [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename Jan Kratochvil
2015-04-23 20:34 ` [PATCH v2 2/2] compile: Add 'set compile-gcc' Jan Kratochvil
2015-04-23 21:08   ` [PATCH v3 " Jan Kratochvil
2015-04-27 15:47     ` Pedro Alves
2015-04-27 17:54       ` Jan Kratochvil
2015-04-27 19:55         ` Pedro Alves
2015-04-27 15:31 ` [PATCH v2 1/2] compile: set debug compile: Display GCC driver filename Pedro Alves
2015-04-27 16:48   ` Jan Kratochvil
2015-04-27 17:19     ` Pedro Alves
2015-04-27 17:52       ` Jan Kratochvil
2015-04-27 19:17         ` Pedro Alves
2015-04-27 20:44           ` Jan Kratochvil
2015-04-27 20:50           ` Jan Kratochvil
2015-04-27 21:49             ` Phil Muldoon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).