public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v3 1/9] Code cleanup: Make parts of print_command_1 public
  2015-04-11 19:43 [PATCH v3 0/9] compile: compile print&printf Jan Kratochvil
@ 2015-04-11 19:43 ` Jan Kratochvil
  2015-04-29 15:44   ` Pedro Alves
  2015-04-11 19:43 ` [PATCH v3 3/9] Code cleanup: compile: Constify some parameters Jan Kratochvil
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-04-11 19:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

The later 'compile print' command should share its behavior with the existing
'print' command.  Make the needed existing parts of print_command_1 public.

gdb/ChangeLog
2015-03-26  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* printcmd.c (print_command_parse_format, print_value): New functions
	from ...
	(print_command_1): ... here.  Call them.
	* valprint.h (struct format_data, print_command_parse_format)
	(print_value): New declarations.
---
 0 files changed

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index deb501a..d89e6df 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -939,6 +939,56 @@ validate_format (struct format_data fmt, const char *cmdname)
 	   fmt.format, cmdname);
 }
 
+/* Parse print command format string and update *EXPP, return it allocated,
+   caller has to call xfree for it.  Return NULL if no format string has been
+   found.  CMDNAME should name the current command.  */
+
+struct format_data *
+print_command_parse_format (const char **expp, const char *cmdname)
+{
+  struct format_data *fmtp;
+  const char *exp = *expp;
+  struct cleanup *cleanup;
+
+  if (exp == NULL || *exp != '/')
+    return NULL;
+  exp++;
+
+  fmtp = xmalloc (sizeof (*fmtp));
+  cleanup = make_cleanup (xfree, fmtp);
+  *fmtp = decode_format (&exp, last_format, 0);
+  validate_format (*fmtp, cmdname);
+  last_format = fmtp->format;
+
+  discard_cleanups (cleanup);
+  *expp = exp;
+  return fmtp;
+}
+
+
+/* Print VAL to console, including recording it to the history.  */
+
+void
+print_value (struct value *val, const struct format_data *fmtp)
+{
+  struct value_print_options opts;
+  int histindex = record_latest_value (val);
+
+  annotate_value_history_begin (histindex, value_type (val));
+
+  printf_filtered ("$%d = ", histindex);
+
+  annotate_value_history_value ();
+
+  get_formatted_print_options (&opts, (fmtp == NULL ? 0 : fmtp->format));
+  opts.raw = (fmtp == NULL ? 0 : fmtp->raw);
+
+  print_formatted (val, (fmtp == NULL ? 0 : fmtp->size), &opts, gdb_stdout);
+  printf_filtered ("\n");
+
+  annotate_value_history_end ();
+}
+
 /* Evaluate string EXP as an expression in the current language and
    print the resulting value.  EXP may contain a format specifier as the
    first argument ("/x myvar" for example, to print myvar in hex).  */
@@ -947,25 +997,9 @@ static void
 print_command_1 (const char *exp, int voidprint)
 {
   struct expression *expr;
-  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
-  char format = 0;
   struct value *val;
-  struct format_data fmt;
-
-  if (exp && *exp == '/')
-    {
-      exp++;
-      fmt = decode_format (&exp, last_format, 0);
-      validate_format (fmt, "print");
-      last_format = format = fmt.format;
-    }
-  else
-    {
-      fmt.count = 1;
-      fmt.format = 0;
-      fmt.size = 0;
-      fmt.raw = 0;
-    }
+  struct format_data *fmtp = print_command_parse_format (&exp, "print");
+  struct cleanup *old_chain = make_cleanup (xfree, fmtp);
 
   if (exp && *exp)
     {
@@ -978,24 +1012,7 @@ print_command_1 (const char *exp, int voidprint)
 
   if (voidprint || (val && value_type (val) &&
 		    TYPE_CODE (value_type (val)) != TYPE_CODE_VOID))
-    {
-      struct value_print_options opts;
-      int histindex = record_latest_value (val);
-
-      annotate_value_history_begin (histindex, value_type (val));
-
-      printf_filtered ("$%d = ", histindex);
-
-      annotate_value_history_value ();
-
-      get_formatted_print_options (&opts, format);
-      opts.raw = fmt.raw;
-
-      print_formatted (val, fmt.size, &opts, gdb_stdout);
-      printf_filtered ("\n");
-
-      annotate_value_history_end ();
-    }
+    print_value (val, fmtp);
 
   do_cleanups (old_chain);
 }
diff --git a/gdb/valprint.h b/gdb/valprint.h
index e3d0137..3ab531f 100644
--- a/gdb/valprint.h
+++ b/gdb/valprint.h
@@ -217,4 +217,9 @@ extern void output_command_const (const char *args, int from_tty);
 
 extern int val_print_scalar_type_p (struct type *type);
 
+struct format_data;
+extern struct format_data *print_command_parse_format (const char **expp,
+						       const char *cmdname);
+extern void print_value (struct value *val, const struct format_data *fmtp);
+
 #endif

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

* [PATCH v3 0/9] compile: compile print&printf
@ 2015-04-11 19:43 Jan Kratochvil
  2015-04-11 19:43 ` [PATCH v3 1/9] Code cleanup: Make parts of print_command_1 public Jan Kratochvil
                   ` (8 more replies)
  0 siblings, 9 replies; 38+ messages in thread
From: Jan Kratochvil @ 2015-04-11 19:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

Hi,

just a rebase as there were some conflicting changes on trunk.
Also included the doc updates.

GIT branch:
	https://github.com/tromey/gdb/tree/pmuldoon/compile/print


Jan

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

* [PATCH v3 2/9] compile: Distribute scope, add scope_data
  2015-04-11 19:43 [PATCH v3 0/9] compile: compile print&printf Jan Kratochvil
  2015-04-11 19:43 ` [PATCH v3 1/9] Code cleanup: Make parts of print_command_1 public Jan Kratochvil
  2015-04-11 19:43 ` [PATCH v3 3/9] Code cleanup: compile: Constify some parameters Jan Kratochvil
@ 2015-04-11 19:43 ` Jan Kratochvil
  2015-04-29 15:44   ` Pedro Alves
  2015-04-11 19:44 ` [PATCH v3 6/9] Code cleanup: compile: func_addr -> func_sym Jan Kratochvil
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-04-11 19:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

Provide a way to access current 'scope' during struct the do_module_cleanup
stage and associate more data with it.

It should be all sub-classed but AFAIK GDB does not require C++ compiler yet.

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

	* cli/cli-script.c (execute_control_command): Update
	eval_compile_command caller.
	* compile/compile-object-load.c (compile_object_load): Add parameters
	scope and scope_data.  Set them.
	* compile/compile-object-load.h (struct compile_module): Add fields
	scope and scope_data.
	(compile_object_load): Add parameters scope and scope_data.
	* compile/compile-object-run.c (struct do_module_cleanup): Add fields
	scope and scope_data.
	(compile_object_run): Propage the fields scope and scope_data.
	* compile/compile.c (compile_file_command, compile_code_command):
	Update eval_compile_command callers.
	(eval_compile_command): Add parameter scope_data.  Pass it plus scope.
	* compile/compile.h (eval_compile_command): Add parameter scope_data.
	* defs.h (struct command_line): Add field scope_data.
---
 0 files changed

diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 7a4ed78..4bd18a7 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -615,7 +615,8 @@ execute_control_command (struct command_line *cmd)
       }
 
     case compile_control:
-      eval_compile_command (cmd, NULL, cmd->control_u.compile.scope);
+      eval_compile_command (cmd, NULL, cmd->control_u.compile.scope,
+			    cmd->control_u.compile.scope_data);
       ret = simple_control;
       break;
 
diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index 8a7f232..70b2e2e 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -464,7 +464,8 @@ store_regs (struct type *regs_type, CORE_ADDR regs_base)
    function returns.  */
 
 struct compile_module *
-compile_object_load (const char *object_file, const char *source_file)
+compile_object_load (const char *object_file, const char *source_file,
+		     enum compile_i_scope_types scope, void *scope_data)
 {
   struct cleanup *cleanups, *cleanups_free_objfile;
   bfd *abfd;
@@ -587,5 +588,7 @@ compile_object_load (const char *object_file, const char *source_file)
   retval->source_file = xstrdup (source_file);
   retval->func_addr = func_addr;
   retval->regs_addr = regs_addr;
+  retval->scope = scope;
+  retval->scope_data = scope_data;
   return retval;
 }
diff --git a/gdb/compile/compile-object-load.h b/gdb/compile/compile-object-load.h
index bef575e..4f9493b 100644
--- a/gdb/compile/compile-object-load.h
+++ b/gdb/compile/compile-object-load.h
@@ -31,9 +31,17 @@ struct compile_module
   /* Inferior registers address or NULL if the inferior function does not
      require any.  */
   CORE_ADDR regs_addr;
+
+  /* The "scope" of this compilation.  */
+  enum compile_i_scope_types scope;
+
+  /* User data for SCOPE in use.  */
+  void *scope_data;
 };
 
 extern struct compile_module *compile_object_load (const char *object_file,
-						   const char *source_file);
+						   const char *source_file,
+					       enum compile_i_scope_types scope,
+						   void *scope_data);
 
 #endif /* GDB_COMPILE_OBJECT_LOAD_H */
diff --git a/gdb/compile/compile-object-run.c b/gdb/compile/compile-object-run.c
index 6738aad..bfd2473 100644
--- a/gdb/compile/compile-object-run.c
+++ b/gdb/compile/compile-object-run.c
@@ -36,6 +36,10 @@ struct do_module_cleanup
   /* .c file OBJFILE was built from.  It needs to be xfree-d.  */
   char *source_file;
 
+  /* Copy from struct compile_module.  */
+  enum compile_i_scope_types scope;
+  void *scope_data;
+
   /* objfile_name of our objfile.  */
   char objfile_name_string[1];
 };
@@ -96,6 +100,8 @@ compile_object_run (struct compile_module *module)
   data->executedp = &executed;
   data->source_file = xstrdup (module->source_file);
   strcpy (data->objfile_name_string, objfile_name_s);
+  data->scope = module->scope;
+  data->scope_data = module->scope_data;
 
   xfree (module->source_file);
   xfree (module);
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 90cfc36..3cbbc29 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -117,7 +117,7 @@ compile_file_command (char *arg, int from_tty)
   make_cleanup (xfree, arg);
   buffer = xstrprintf ("#include \"%s\"\n", arg);
   make_cleanup (xfree, buffer);
-  eval_compile_command (NULL, buffer, scope);
+  eval_compile_command (NULL, buffer, scope, NULL);
   do_cleanups (cleanup);
 }
 
@@ -150,7 +150,7 @@ compile_code_command (char *arg, int from_tty)
     }
 
   if (arg && *arg)
-      eval_compile_command (NULL, arg, scope);
+    eval_compile_command (NULL, arg, scope, NULL);
   else
     {
       struct command_line *l = get_command_line (compile_control, "");
@@ -557,7 +557,7 @@ compile_command (char *args, int from_tty)
 
 void
 eval_compile_command (struct command_line *cmd, char *cmd_string,
-		      enum compile_i_scope_types scope)
+		      enum compile_i_scope_types scope, void *scope_data)
 {
   char *object_file, *source_file;
 
@@ -571,7 +571,8 @@ eval_compile_command (struct command_line *cmd, char *cmd_string,
       make_cleanup (xfree, source_file);
       cleanup_unlink = make_cleanup (cleanup_unlink_file, object_file);
       make_cleanup (cleanup_unlink_file, source_file);
-      compile_module = compile_object_load (object_file, source_file);
+      compile_module = compile_object_load (object_file, source_file,
+					    scope, scope_data);
       discard_cleanups (cleanup_unlink);
       do_cleanups (cleanup_xfree);
       compile_object_run (compile_module);
diff --git a/gdb/compile/compile.h b/gdb/compile/compile.h
index 1e3f934..93c4786 100644
--- a/gdb/compile/compile.h
+++ b/gdb/compile/compile.h
@@ -29,7 +29,8 @@ struct dynamic_prop;
    never both.  */
 
 extern void eval_compile_command (struct command_line *cmd, char *cmd_string,
-				  enum compile_i_scope_types scope);
+				  enum compile_i_scope_types scope,
+				  void *scope_data);
 
 /* Compile a DWARF location expression to C, suitable for use by the
    compiler.
diff --git a/gdb/defs.h b/gdb/defs.h
index eda6741..436f09e 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -397,6 +397,7 @@ struct command_line
 	struct
 	  {
 	    enum compile_i_scope_types scope;
+	    void *scope_data;
 	  }
 	compile;
       }

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

* [PATCH v3 3/9] Code cleanup: compile: Constify some parameters
  2015-04-11 19:43 [PATCH v3 0/9] compile: compile print&printf Jan Kratochvil
  2015-04-11 19:43 ` [PATCH v3 1/9] Code cleanup: Make parts of print_command_1 public Jan Kratochvil
@ 2015-04-11 19:43 ` Jan Kratochvil
  2015-04-29 15:47   ` Pedro Alves
  2015-04-11 19:43 ` [PATCH v3 2/9] compile: Distribute scope, add scope_data Jan Kratochvil
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-04-11 19:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

gdb/ChangeLog
2015-03-26  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* compile/compile.c (compile_to_object): Make the cmd_string parameter
	const.  Use new variables for the const compatibility.
	(eval_compile_command): Make the cmd_string parameter const.
	* compile/compile.h (eval_compile_command): Make the cmd_string
	parameter const.
---
 0 files changed

diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 3cbbc29..621de66 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -415,11 +415,12 @@ print_callback (void *ignore, const char *message)
    freeing both strings.  */
 
 static char *
-compile_to_object (struct command_line *cmd, char *cmd_string,
+compile_to_object (struct command_line *cmd, const char *cmd_string,
 		   enum compile_i_scope_types scope,
 		   char **source_filep)
 {
   char *code;
+  const char *input;
   char *source_file, *object_file;
   struct compile_instance *compiler;
   struct cleanup *cleanup, *inner_cleanup;
@@ -459,6 +460,7 @@ compile_to_object (struct command_line *cmd, char *cmd_string,
     {
       struct ui_file *stream = mem_fileopen ();
       struct command_line *iter;
+      char *stream_buf;
 
       make_cleanup_ui_file_delete (stream);
       for (iter = cmd->body_list[0]; iter; iter = iter->next)
@@ -467,15 +469,16 @@ compile_to_object (struct command_line *cmd, char *cmd_string,
 	  fputs_unfiltered ("\n", stream);
 	}
 
-      code = ui_file_xstrdup (stream, NULL);
-      make_cleanup (xfree, code);
+      stream_buf = ui_file_xstrdup (stream, NULL);
+      make_cleanup (xfree, stream_buf);
+      input = stream_buf;
     }
   else if (cmd_string != NULL)
-    code = cmd_string;
+    input = cmd_string;
   else
     error (_("Neither a simple expression, or a multi-line specified."));
 
-  code = current_language->la_compute_program (compiler, code, gdbarch,
+  code = current_language->la_compute_program (compiler, input, gdbarch,
 					       expr_block, expr_pc);
   make_cleanup (xfree, code);
   if (compile_debug)
@@ -556,7 +559,7 @@ compile_command (char *args, int from_tty)
 /* See compile.h.  */
 
 void
-eval_compile_command (struct command_line *cmd, char *cmd_string,
+eval_compile_command (struct command_line *cmd, const char *cmd_string,
 		      enum compile_i_scope_types scope, void *scope_data)
 {
   char *object_file, *source_file;
diff --git a/gdb/compile/compile.h b/gdb/compile/compile.h
index 93c4786..a973167 100644
--- a/gdb/compile/compile.h
+++ b/gdb/compile/compile.h
@@ -28,7 +28,8 @@ struct dynamic_prop;
    expression command.  GDB returns either a CMD, or a CMD_STRING, but
    never both.  */
 
-extern void eval_compile_command (struct command_line *cmd, char *cmd_string,
+extern void eval_compile_command (struct command_line *cmd,
+				  const char *cmd_string,
 				  enum compile_i_scope_types scope,
 				  void *scope_data);
 

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

* [PATCH v3 6/9] Code cleanup: compile: func_addr -> func_sym
  2015-04-11 19:43 [PATCH v3 0/9] compile: compile print&printf Jan Kratochvil
                   ` (2 preceding siblings ...)
  2015-04-11 19:43 ` [PATCH v3 2/9] compile: Distribute scope, add scope_data Jan Kratochvil
@ 2015-04-11 19:44 ` Jan Kratochvil
  2015-04-29 15:52   ` Pedro Alves
  2015-04-11 19:44 ` [PATCH v3 9/9] compile: compile printf: gdbserver support Jan Kratochvil
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-04-11 19:44 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

Hi,

currently the code fetches _gdb_expr address/types at multiple places, guessing
its parameters at multiple places etc.

Fetch it once, verify it has expected type and then rely on it.

While the patch tries to clean up the code it is still horrible due to the
missing C++ sub-classing.


Jan


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

	* compile/compile-object-load.c (get_regs_type): Add parameter func_sym.
	Rely on its parameter count.
	(compile_object_load): Replace lookup_minimal_symbol_text by
	lookup_global_symbol_from_objfile.  Verify FUNC_SYM.  Set it in the
	return value.
	* compile/compile-object-load.h (struct compile_module): Replace
	func_addr by func_sym.
	* compile/compile-object-run.c: Include block.h.
	(compile_object_run): Reset module variable after it is freed.  Use
	FUNC_SYM instead of FUNC_ADDR.  Rely on it.
---
 0 files changed

diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index ed2cad4..3c07840 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -354,40 +354,19 @@ copy_sections (bfd *abfd, asection *sect, void *data)
   do_cleanups (cleanups);
 }
 
-/* Fetch the type of first parameter of GCC_FE_WRAPPER_FUNCTION.
-   Return NULL if GCC_FE_WRAPPER_FUNCTION has no parameters.
-   Throw an error otherwise.  */
+/* Fetch the type of first parameter of FUNC_SYM.
+   Return NULL if FUNC_SYM has no parameters.  Throw an error otherwise.  */
 
 static struct type *
-get_regs_type (struct objfile *objfile)
+get_regs_type (struct symbol *func_sym, struct objfile *objfile)
 {
-  struct symbol *func_sym;
-  struct type *func_type, *regsp_type, *regs_type;
-
-  func_sym = lookup_global_symbol_from_objfile (objfile,
-						GCC_FE_WRAPPER_FUNCTION,
-						VAR_DOMAIN);
-  if (func_sym == NULL)
-    error (_("Cannot find function \"%s\" in compiled module \"%s\"."),
-	   GCC_FE_WRAPPER_FUNCTION, objfile_name (objfile));
-
-  func_type = SYMBOL_TYPE (func_sym);
-  if (TYPE_CODE (func_type) != TYPE_CODE_FUNC)
-    error (_("Invalid type code %d of function \"%s\" in compiled "
-	     "module \"%s\"."),
-	   TYPE_CODE (func_type), GCC_FE_WRAPPER_FUNCTION,
-	   objfile_name (objfile));
+  struct type *func_type = SYMBOL_TYPE (func_sym);
+  struct type *regsp_type, *regs_type;
 
   /* No register parameter present.  */
   if (TYPE_NFIELDS (func_type) == 0)
     return NULL;
 
-  if (TYPE_NFIELDS (func_type) != 1)
-    error (_("Invalid %d parameters of function \"%s\" in compiled "
-	     "module \"%s\"."),
-	   TYPE_NFIELDS (func_type), GCC_FE_WRAPPER_FUNCTION,
-	   objfile_name (objfile));
-
   regsp_type = check_typedef (TYPE_FIELD_TYPE (func_type, 0));
   if (TYPE_CODE (regsp_type) != TYPE_CODE_PTR)
     error (_("Invalid type code %d of first parameter of function \"%s\" "
@@ -470,7 +449,9 @@ compile_object_load (const char *object_file, const char *source_file,
   struct cleanup *cleanups, *cleanups_free_objfile;
   bfd *abfd;
   struct setup_sections_data setup_sections_data;
-  CORE_ADDR addr, func_addr, regs_addr;
+  CORE_ADDR addr, regs_addr;
+  struct symbol *func_sym;
+  struct type *func_type;
   struct bound_minimal_symbol bmsym;
   long storage_needed;
   asymbol **symbol_table, **symp;
@@ -481,6 +462,8 @@ compile_object_load (const char *object_file, const char *source_file,
   struct type *regs_type;
   char *filename, **matching;
   struct objfile *objfile;
+  int expect_parameters;
+  struct type *expect_return_type;
 
   filename = tilde_expand (object_file);
   cleanups = make_cleanup (xfree, filename);
@@ -515,11 +498,41 @@ compile_object_load (const char *object_file, const char *source_file,
   objfile = symbol_file_add_from_bfd (abfd, filename, 0, NULL, 0, NULL);
   cleanups_free_objfile = make_cleanup_free_objfile (objfile);
 
-  bmsym = lookup_minimal_symbol_text (GCC_FE_WRAPPER_FUNCTION, objfile);
-  if (bmsym.minsym == NULL || MSYMBOL_TYPE (bmsym.minsym) == mst_file_text)
-    error (_("Could not find symbol \"%s\" of compiled module \"%s\"."),
-	   GCC_FE_WRAPPER_FUNCTION, filename);
-  func_addr = BMSYMBOL_VALUE_ADDRESS (bmsym);
+  func_sym = lookup_global_symbol_from_objfile (objfile,
+						GCC_FE_WRAPPER_FUNCTION,
+						VAR_DOMAIN);
+  if (func_sym == NULL)
+    error (_("Cannot find function \"%s\" in compiled module \"%s\"."),
+	   GCC_FE_WRAPPER_FUNCTION, objfile_name (objfile));
+  func_type = SYMBOL_TYPE (func_sym);
+  if (TYPE_CODE (func_type) != TYPE_CODE_FUNC)
+    error (_("Invalid type code %d of function \"%s\" in compiled "
+	     "module \"%s\"."),
+	   TYPE_CODE (func_type), GCC_FE_WRAPPER_FUNCTION,
+	   objfile_name (objfile));
+
+  switch (scope)
+    {
+    case COMPILE_I_SIMPLE_SCOPE:
+      expect_parameters = 1;
+      expect_return_type = builtin_type (target_gdbarch ())->builtin_void;
+      break;
+    case COMPILE_I_RAW_SCOPE:
+      expect_parameters = 0;
+      expect_return_type = builtin_type (target_gdbarch ())->builtin_void;
+      break;
+    default:
+      internal_error (__FILE__, __LINE__, _("invalid scope %d"), scope);
+    }
+  if (TYPE_NFIELDS (func_type) != expect_parameters)
+    error (_("Invalid %d parameters of function \"%s\" in compiled "
+	     "module \"%s\"."),
+	   TYPE_NFIELDS (func_type), GCC_FE_WRAPPER_FUNCTION,
+	   objfile_name (objfile));
+  if (!types_deeply_equal (expect_return_type, TYPE_TARGET_TYPE (func_type)))
+    error (_("Invalid return type of function \"%s\" in compiled "
+	    "module \"%s\"."),
+	  GCC_FE_WRAPPER_FUNCTION, objfile_name (objfile));
 
   /* The memory may be later needed
      by bfd_generic_get_relocated_section_contents
@@ -571,7 +584,7 @@ compile_object_load (const char *object_file, const char *source_file,
 
   bfd_map_over_sections (abfd, copy_sections, symbol_table);
 
-  regs_type = get_regs_type (objfile);
+  regs_type = get_regs_type (func_sym, objfile);
   if (regs_type == NULL)
     regs_addr = 0;
   else
@@ -590,7 +603,7 @@ compile_object_load (const char *object_file, const char *source_file,
   retval = xmalloc (sizeof (*retval));
   retval->objfile = objfile;
   retval->source_file = xstrdup (source_file);
-  retval->func_addr = func_addr;
+  retval->func_sym = func_sym;
   retval->regs_addr = regs_addr;
   retval->scope = scope;
   retval->scope_data = scope_data;
diff --git a/gdb/compile/compile-object-load.h b/gdb/compile/compile-object-load.h
index 4f9493b..94f77be 100644
--- a/gdb/compile/compile-object-load.h
+++ b/gdb/compile/compile-object-load.h
@@ -25,8 +25,8 @@ struct compile_module
   /* .c file OBJFILE was built from.  It needs to be xfree-d.  */
   char *source_file;
 
-  /* Inferior function address.  */
-  CORE_ADDR func_addr;
+  /* Inferior function GCC_FE_WRAPPER_FUNCTION.  */
+  struct symbol *func_sym;
 
   /* Inferior registers address or NULL if the inferior function does not
      require any.  */
diff --git a/gdb/compile/compile-object-run.c b/gdb/compile/compile-object-run.c
index bfd2473..c454bd1 100644
--- a/gdb/compile/compile-object-run.c
+++ b/gdb/compile/compile-object-run.c
@@ -24,6 +24,7 @@
 #include "objfiles.h"
 #include "compile-internal.h"
 #include "dummy-frame.h"
+#include "block.h"
 
 /* Helper for do_module_cleanup.  */
 
@@ -93,8 +94,9 @@ compile_object_run (struct compile_module *module)
   struct do_module_cleanup *data;
   const char *objfile_name_s = objfile_name (module->objfile);
   int dtor_found, executed = 0;
-  CORE_ADDR func_addr = module->func_addr;
+  struct symbol *func_sym = module->func_sym;
   CORE_ADDR regs_addr = module->regs_addr;
+  struct objfile *objfile = module->objfile;
 
   data = xmalloc (sizeof (*data) + strlen (objfile_name_s));
   data->executedp = &executed;
@@ -105,26 +107,35 @@ compile_object_run (struct compile_module *module)
 
   xfree (module->source_file);
   xfree (module);
+  module = NULL;
 
   TRY
     {
-      func_val = value_from_pointer
-		 (builtin_type (target_gdbarch ())->builtin_func_ptr,
-		  func_addr);
-
-      if (regs_addr == 0)
-	call_function_by_hand_dummy (func_val, 0, NULL,
-				     do_module_cleanup, data);
-      else
+      struct type *func_type = SYMBOL_TYPE (func_sym);
+      htab_t copied_types;
+      int current_arg = 0;
+      struct value **vargs;
+
+      /* OBJFILE may disappear while FUNC_TYPE still will be in use.  */
+      copied_types = create_copied_types_hash (objfile);
+      func_type = copy_type_recursive (objfile, func_type, copied_types);
+      htab_delete (copied_types);
+
+      gdb_assert (TYPE_CODE (func_type) == TYPE_CODE_FUNC);
+      func_val = value_from_pointer (lookup_pointer_type (func_type),
+				   BLOCK_START (SYMBOL_BLOCK_VALUE (func_sym)));
+
+      vargs = alloca (sizeof (*vargs) * TYPE_NFIELDS (func_type));
+      if (TYPE_NFIELDS (func_type) >= 1)
 	{
-	  struct value *arg_val;
-
-	  arg_val = value_from_pointer
-		    (builtin_type (target_gdbarch ())->builtin_func_ptr,
-		     regs_addr);
-	  call_function_by_hand_dummy (func_val, 1, &arg_val,
-				       do_module_cleanup, data);
+	  gdb_assert (regs_addr != 0);
+	  vargs[current_arg] = value_from_pointer
+			  (TYPE_FIELD_TYPE (func_type, current_arg), regs_addr);
+	  ++current_arg;
 	}
+      gdb_assert (current_arg == TYPE_NFIELDS (func_type));
+      call_function_by_hand_dummy (func_val, TYPE_NFIELDS (func_type), vargs,
+				   do_module_cleanup, data);
     }
   CATCH (ex, RETURN_MASK_ERROR)
     {

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

* [PATCH v3 4/9] compile: Support relocation to GNU-IFUNCs
  2015-04-11 19:43 [PATCH v3 0/9] compile: compile print&printf Jan Kratochvil
                   ` (5 preceding siblings ...)
  2015-04-11 19:44 ` [PATCH v3 8/9] compile: New compile printf Jan Kratochvil
@ 2015-04-11 19:44 ` Jan Kratochvil
  2015-04-29 15:48   ` Pedro Alves
  2015-04-11 19:44 ` [PATCH v3 5/9] compile: Use -Wall, not -w Jan Kratochvil
  2015-04-11 19:44 ` [PATCH v3 7/9] compile: New 'compile print' Jan Kratochvil
  8 siblings, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-04-11 19:44 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

The 'compile print' part disclosed that calling memcpy() may fail as memcpy()
from libc is GNU-IFUNC.

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

	* compile/compile-object-load.c (compile_object_load): Support
	mst_text_gnu_ifunc.
---
 0 files changed

diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index 70b2e2e..ed2cad4 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -555,6 +555,10 @@ compile_object_load (const char *object_file, const char *source_file,
 	case mst_text:
 	  sym->value = BMSYMBOL_VALUE_ADDRESS (bmsym);
 	  break;
+	case mst_text_gnu_ifunc:
+	  sym->value = gnu_ifunc_resolve_addr (target_gdbarch (),
+					       BMSYMBOL_VALUE_ADDRESS (bmsym));
+	  break;
 	default:
 	  warning (_("Could not find symbol \"%s\" "
 		     "for compiled module \"%s\"."),

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

* [PATCH v3 8/9] compile: New compile printf
  2015-04-11 19:43 [PATCH v3 0/9] compile: compile print&printf Jan Kratochvil
                   ` (4 preceding siblings ...)
  2015-04-11 19:44 ` [PATCH v3 9/9] compile: compile printf: gdbserver support Jan Kratochvil
@ 2015-04-11 19:44 ` Jan Kratochvil
  2015-04-29 15:54   ` Pedro Alves
  2015-04-11 19:44 ` [PATCH v3 4/9] compile: Support relocation to GNU-IFUNCs Jan Kratochvil
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-04-11 19:44 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

Hi,

command naming needs to follow what gets decided for 'compile print'.

This part sends the output to inferior stdout, only the next patch will
redirect it to GDB (so that for example it works for remote gdbserver).

It cannot work for core files as one cannot execute inferior code there.
There were some ideas such as compiling the entered sources into GCC
intermediate form (GIMPLE?) and interpret it by GDB on top of the core file.
That would be much more complicated, this implementation is made according to
Phil's specification.

Besides existing
	(gdb) set compile-args
there will be now also:
	(gdb) set compile-printf-args
Maybe it would be worth to start some set sub-category 'compile' such as:
	(gdb) set compile args
	(gdb) set compile printf-args
That would mean the whole process of deprecating 'set compile-args' etc.


Jan


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

	* compile/compile-c-support.c (add_code_header, add_code_footer)
	(c_compute_program): Add COMPILE_I_PRINTF_SCOPE.
	* compile/compile-object-load.c (get_regs_type): Verify 2nd parameter
	type.
	(compile_object_load): Add COMPILE_I_PRINTF_SCOPE.
	* compile/compile.c (compile_print_command): Rename to ...
	(compile_print): ... here.
	(compile_print_command, compile_printf_command, compile_printf_args)
	(compile_printf_args_argv, set_compile_printf_args)
	(show_compile_printf_args): New.
	(get_args): Add COMPILE_I_PRINTF_SCOPE support.
	(_initialize_compile): Install compile_printf_command,
	set_compile_printf_args and show_compile_printf_args.  Set default
	COMPILE_PRINTF_ARGS.
	* defs.h (enum compile_i_scope_types): Add COMPILE_I_PRINTF_SCOPE.

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

	* gdb.texinfo (Compiling and Injecting Code): Add compile printf.

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

	* gdb.compile/compile-print.exp (compile printf "0x%x\n", varint)
	(compile printf "0x%x\n"): New.

diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index 86b02cf..92c41b6 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -206,7 +206,17 @@ add_code_header (enum compile_i_scope_types type, struct ui_file *buf)
 			") {\n",
 			buf);
       break;
-
+    case COMPILE_I_PRINTF_SCOPE:
+      fputs_unfiltered ("#include <stdio.h>\n"
+			"void "
+			GCC_FE_WRAPPER_FUNCTION
+			" (struct "
+			COMPILE_I_SIMPLE_REGISTER_STRUCT_TAG
+			" *"
+			COMPILE_I_SIMPLE_REGISTER_ARG_NAME
+			") {\n",
+			buf);
+      break;
     case COMPILE_I_RAW_SCOPE:
       break;
     default:
@@ -226,6 +236,7 @@ add_code_footer (enum compile_i_scope_types type, struct ui_file *buf)
     case COMPILE_I_SIMPLE_SCOPE:
     case COMPILE_I_PRINT_ADDRESS_SCOPE:
     case COMPILE_I_PRINT_VALUE_SCOPE:
+    case COMPILE_I_PRINTF_SCOPE:
       fputs_unfiltered ("}\n", buf);
       break;
     case COMPILE_I_RAW_SCOPE:
@@ -390,7 +401,8 @@ c_compute_program (struct compile_instance *inst,

   if (inst->scope == COMPILE_I_SIMPLE_SCOPE
       || inst->scope == COMPILE_I_PRINT_ADDRESS_SCOPE
-      || inst->scope == COMPILE_I_PRINT_VALUE_SCOPE)
+      || inst->scope == COMPILE_I_PRINT_VALUE_SCOPE
+      || inst->scope == COMPILE_I_PRINTF_SCOPE)
     {
       ui_file_put (var_stream, ui_file_write_for_put, buf);
       fputs_unfiltered ("#pragma GCC user_expression\n", buf);
@@ -418,6 +430,11 @@ c_compute_program (struct compile_instance *inst,
 			  (inst->scope == COMPILE_I_PRINT_ADDRESS_SCOPE
 			   ? "&" : ""));
       break;
+    case COMPILE_I_PRINTF_SCOPE:
+      fprintf_unfiltered (buf,
+"printf (%s);\n"
+			  , input);
+      break;
     default:
       fputs_unfiltered (input, buf);
       break;
diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index e67e5ba..bc74590 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -470,7 +470,7 @@ static struct type *
 get_regs_type (struct symbol *func_sym, struct objfile *objfile)
 {
   struct type *func_type = SYMBOL_TYPE (func_sym);
-  struct type *regsp_type, *regs_type;
+  struct type *regsp_type, *regs_type, *void_type, *voidp_type;

   /* No register parameter present.  */
   if (TYPE_NFIELDS (func_type) == 0)
@@ -490,6 +490,18 @@ get_regs_type (struct symbol *func_sym, struct objfile *objfile)
 	   TYPE_CODE (regs_type), GCC_FE_WRAPPER_FUNCTION,
 	   objfile_name (objfile));

+  /* No register parameter present.  */
+  if (TYPE_NFIELDS (func_type) == 1)
+    return regs_type;
+
+  void_type = builtin_type (target_gdbarch ())->builtin_void;
+  voidp_type = lookup_pointer_type (void_type);
+  if (!types_deeply_equal (voidp_type, TYPE_FIELD_TYPE (func_type, 1)))
+    error (_("Invalid type of function \"%s\" parameter \"%s\" in compiled "
+	    "module \"%s\"."),
+	  GCC_FE_WRAPPER_FUNCTION, COMPILE_I_PRINT_OUT_ARG,
+	  objfile_name (objfile));
+
   return regs_type;
 }

@@ -637,6 +649,10 @@ compile_object_load (const char *object_file, const char *source_file,
       expect_parameters = 2;
       expect_return_type = builtin_type (target_gdbarch ())->builtin_void;
       break;
+    case COMPILE_I_PRINTF_SCOPE:
+      expect_parameters = 1;
+      expect_return_type = builtin_type (target_gdbarch ())->builtin_void;
+      break;
     default:
       internal_error (__FILE__, __LINE__, _("invalid scope %d"), scope);
     }
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index c4b8f2e..6bbe8df 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -174,17 +174,14 @@ compile_print_value (struct value *val, void *data_voidp)
   print_value (val, fmtp);
 }

-/* Handle the input from the 'compile print' command.  The "compile
-   print" command is used to evaluate and print an expression that may
-   contain calls to the GCC compiler.  The language expected in this
-   compile command is the language currently set in GDB.  */
+/* Implement 'compile print' and 'compile printf' command according
+   to SCOPE.  */

 static void
-compile_print_command (char *arg_param, int from_tty)
+compile_print (char *arg_param, enum compile_i_scope_types scope)
 {
   const char *arg = arg_param;
   struct cleanup *cleanup;
-  enum compile_i_scope_types scope = COMPILE_I_PRINT_ADDRESS_SCOPE;
   struct format_data *fmtp;

   cleanup = make_cleanup_restore_integer (&interpreter_async);
@@ -207,6 +204,25 @@ compile_print_command (char *arg_param, int from_tty)
   do_cleanups (cleanup);
 }

+/* Handle the input from the 'compile print' command.  The "compile
+   print" command is used to evaluate and print an expression that may
+   contain calls to the GCC compiler.  The language expected in this
+   compile command is the language currently set in GDB.  */
+
+static void
+compile_print_command (char *arg, int from_tty)
+{
+  compile_print (arg, COMPILE_I_PRINT_ADDRESS_SCOPE);
+}
+
+/* Handle the input from the 'compile printf' command.  */
+
+static void
+compile_printf_command (char *arg, int from_tty)
+{
+  compile_print (arg, COMPILE_I_PRINTF_SCOPE);
+}
+
 /* A cleanup function to remove a directory and all its contents.  */

 static void
@@ -308,6 +324,14 @@ static char *compile_args;
 static int compile_args_argc;
 static char **compile_args_argv;

+/* String for 'set compile-printf-args' and 'show compile-printf-args'.  */
+static char *compile_printf_args;
+
+/* Parsed form of COMPILE_PRINTF_ARGS.
+   COMPILE_PRINTF_ARGS_ARGV is NULL terminated.  */
+static int compile_printf_args_argc;
+static char **compile_printf_args_argv;
+
 /* Implement 'set compile-args'.  */

 static void
@@ -328,6 +352,27 @@ show_compile_args (struct ui_file *file, int from_tty,
 		    value);
 }

+/* Implement 'set compile-printf-args'.  */
+
+static void
+set_compile_printf_args (char *args, int from_tty, struct cmd_list_element *c)
+{
+  freeargv (compile_printf_args_argv);
+  build_argc_argv (compile_printf_args,
+		   &compile_printf_args_argc, &compile_printf_args_argv);
+}
+
+/* Implement 'show compile-printf-args'.  */
+
+static void
+show_compile_printf_args (struct ui_file *file, int from_tty,
+		   struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Compile printf command command-line arguments "
+			    "are \"%s\".\n"),
+		    value);
+}
+
 /* Append ARGC and ARGV (as parsed by build_argc_argv) to *ARGCP and *ARGVP.
    ARGCP+ARGVP can be zero+NULL and also ARGC+ARGV can be zero+NULL.  */

@@ -422,6 +467,10 @@ get_args (const struct compile_instance *compiler, struct gdbarch *gdbarch,
   freeargv (argv_compiler);

   append_args (argcp, argvp, compile_args_argc, compile_args_argv);
+
+  if (compiler->scope == COMPILE_I_PRINTF_SCOPE)
+    append_args (argcp, argvp,
+		 compile_printf_args_argc, compile_printf_args_argv);
 }

 /* A cleanup function to destroy a gdb_gcc_instance.  */
@@ -727,6 +776,24 @@ EXPR may be preceded with /FMT, where FMT is a format letter\n\
 but no count or size letter (see \"x\" command)."),
 	   &compile_command_list);

+  add_cmd ("printf", class_obscure, compile_printf_command,
+	   _("\
+Print formatted output with the compiler.\n\
+\n\
+Usage: compile printf \"printf format string\", arg1, arg2, arg3, ..., argn\n\
+\n\
+The arguments may be specified on the same line as the command, e.g.:\n\
+\n\
+    compile printf \"%d\\n\", i\n\
+\n\
+Alternatively, you can type the arguments interactively.\n\
+You can invoke this mode when no argument is given to the command\n\
+(i.e., \"compile printf\" is typed with nothing after it).  An\n\
+interactive prompt will be shown allowing you to enter multiple\n\
+lines of source code.  Type a line containing \"end\" to indicate\n\
+the end of the source code."),
+	   &compile_command_list);
+
   add_setshow_boolean_cmd ("compile", class_maintenance, &compile_debug, _("\
 Set compile command debugging."), _("\
 Show compile command debugging."), _("\
@@ -744,6 +811,15 @@ String quoting is parsed like in shell, for example:\n\
   -mno-align-double \"-I/dir with a space/include\""),
 			  set_compile_args, show_compile_args, &setlist, &showlist);

+  add_setshow_string_cmd ("compile-printf-args", class_support,
+			  &compile_printf_args,
+			  _("Set compile command GCC command-line arguments FIXME"),
+			  _("Show compile command GCC command-line arguments FIXME"),
+			  _("\
+Use options like -Werror which get added for 'compile printf' command."),
+			  set_compile_printf_args, show_compile_printf_args,
+			  &setlist, &showlist);
+
   /* Override flags possibly coming from DW_AT_producer.  */
   compile_args = xstrdup ("-O0 -gdwarf-4"
   /* We use -fPIE Otherwise GDB would need to reserve space large enough for
@@ -762,4 +838,7 @@ String quoting is parsed like in shell, for example:\n\
 			 " -fno-stack-protector"
   );
   set_compile_args (compile_args, 0, NULL);
+
+  compile_printf_args = xstrdup ("-Werror=format");
+  set_compile_printf_args (compile_printf_args, 0, NULL);
 }
diff --git a/gdb/defs.h b/gdb/defs.h
index 8da7d24..0bcb7f0 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -73,9 +73,16 @@ enum compile_i_scope_types

     /* A printable expression scope.  Wrap an expression into a scope
        suitable for the "compile print" command.  It uses the generic
-       function name "_gdb_expr". */
+       function name "_gdb_expr".  COMPILE_I_PRINT_ADDRESS_SCOPE variant
+       is the usual one, taking address of the object.
+       COMPILE_I_PRINT_VALUE_SCOPE is needed for arrays where the array
+       name already specifies its address.  */
     COMPILE_I_PRINT_ADDRESS_SCOPE,
     COMPILE_I_PRINT_VALUE_SCOPE,
+
+    /* Implement the "compile printf" command.  SCOPE_DATA contains
+       string containing the format string together with its arguments.  */
+    COMPILE_I_PRINTF_SCOPE,
   };

 /* Just in case they're not defined in stdio.h.  */
diff --git a/gdb/testsuite/gdb.compile/compile-print.exp b/gdb/testsuite/gdb.compile/compile-print.exp
index 92c6240..14fd489 100644
--- a/gdb/testsuite/gdb.compile/compile-print.exp
+++ b/gdb/testsuite/gdb.compile/compile-print.exp
@@ -56,3 +56,8 @@ gdb_test "compile print/x 256" " = 0x100"
 gdb_test {print $} " = 256"

 gdb_test "compile print varobject" { = {field = 1}}
+
+if ![is_remote target] {
+    gdb_test {compile printf "0x%x\n", varint} "\r\n0xa"
+    gdb_test {compile printf "0x%x\n"} "\r\nCompilation failed\\."
+}
---
 gdb/doc/gdb.texinfo |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index 86b02cf..92c41b6 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -206,7 +206,17 @@ add_code_header (enum compile_i_scope_types type, struct ui_file *buf)
 			") {\n",
 			buf);
       break;
-
+    case COMPILE_I_PRINTF_SCOPE:
+      fputs_unfiltered ("#include <stdio.h>\n"
+			"void "
+			GCC_FE_WRAPPER_FUNCTION
+			" (struct "
+			COMPILE_I_SIMPLE_REGISTER_STRUCT_TAG
+			" *"
+			COMPILE_I_SIMPLE_REGISTER_ARG_NAME
+			") {\n",
+			buf);
+      break;
     case COMPILE_I_RAW_SCOPE:
       break;
     default:
@@ -226,6 +236,7 @@ add_code_footer (enum compile_i_scope_types type, struct ui_file *buf)
     case COMPILE_I_SIMPLE_SCOPE:
     case COMPILE_I_PRINT_ADDRESS_SCOPE:
     case COMPILE_I_PRINT_VALUE_SCOPE:
+    case COMPILE_I_PRINTF_SCOPE:
       fputs_unfiltered ("}\n", buf);
       break;
     case COMPILE_I_RAW_SCOPE:
@@ -390,7 +401,8 @@ c_compute_program (struct compile_instance *inst,
 
   if (inst->scope == COMPILE_I_SIMPLE_SCOPE
       || inst->scope == COMPILE_I_PRINT_ADDRESS_SCOPE
-      || inst->scope == COMPILE_I_PRINT_VALUE_SCOPE)
+      || inst->scope == COMPILE_I_PRINT_VALUE_SCOPE
+      || inst->scope == COMPILE_I_PRINTF_SCOPE)
     {
       ui_file_put (var_stream, ui_file_write_for_put, buf);
       fputs_unfiltered ("#pragma GCC user_expression\n", buf);
@@ -418,6 +430,11 @@ c_compute_program (struct compile_instance *inst,
 			  (inst->scope == COMPILE_I_PRINT_ADDRESS_SCOPE
 			   ? "&" : ""));
       break;
+    case COMPILE_I_PRINTF_SCOPE:
+      fprintf_unfiltered (buf,
+"printf (%s);\n"
+			  , input);
+      break;
     default:
       fputs_unfiltered (input, buf);
       break;
diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index e67e5ba..bc74590 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -470,7 +470,7 @@ static struct type *
 get_regs_type (struct symbol *func_sym, struct objfile *objfile)
 {
   struct type *func_type = SYMBOL_TYPE (func_sym);
-  struct type *regsp_type, *regs_type;
+  struct type *regsp_type, *regs_type, *void_type, *voidp_type;
 
   /* No register parameter present.  */
   if (TYPE_NFIELDS (func_type) == 0)
@@ -490,6 +490,18 @@ get_regs_type (struct symbol *func_sym, struct objfile *objfile)
 	   TYPE_CODE (regs_type), GCC_FE_WRAPPER_FUNCTION,
 	   objfile_name (objfile));
 
+  /* No register parameter present.  */
+  if (TYPE_NFIELDS (func_type) == 1)
+    return regs_type;
+
+  void_type = builtin_type (target_gdbarch ())->builtin_void;
+  voidp_type = lookup_pointer_type (void_type);
+  if (!types_deeply_equal (voidp_type, TYPE_FIELD_TYPE (func_type, 1)))
+    error (_("Invalid type of function \"%s\" parameter \"%s\" in compiled "
+	    "module \"%s\"."),
+	  GCC_FE_WRAPPER_FUNCTION, COMPILE_I_PRINT_OUT_ARG,
+	  objfile_name (objfile));
+
   return regs_type;
 }
 
@@ -637,6 +649,10 @@ compile_object_load (const char *object_file, const char *source_file,
       expect_parameters = 2;
       expect_return_type = builtin_type (target_gdbarch ())->builtin_void;
       break;
+    case COMPILE_I_PRINTF_SCOPE:
+      expect_parameters = 1;
+      expect_return_type = builtin_type (target_gdbarch ())->builtin_void;
+      break;
     default:
       internal_error (__FILE__, __LINE__, _("invalid scope %d"), scope);
     }
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 424a25b..0e92e27 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -174,17 +174,14 @@ compile_print_value (struct value *val, void *data_voidp)
   print_value (val, fmtp);
 }
 
-/* Handle the input from the 'compile print' command.  The "compile
-   print" command is used to evaluate and print an expression that may
-   contain calls to the GCC compiler.  The language expected in this
-   compile command is the language currently set in GDB.  */
+/* Implement 'compile print' and 'compile printf' command according
+   to SCOPE.  */
 
 static void
-compile_print_command (char *arg_param, int from_tty)
+compile_print (char *arg_param, enum compile_i_scope_types scope)
 {
   const char *arg = arg_param;
   struct cleanup *cleanup;
-  enum compile_i_scope_types scope = COMPILE_I_PRINT_ADDRESS_SCOPE;
   struct format_data *fmtp;
 
   cleanup = make_cleanup_restore_integer (&interpreter_async);
@@ -207,6 +204,25 @@ compile_print_command (char *arg_param, int from_tty)
   do_cleanups (cleanup);
 }
 
+/* Handle the input from the 'compile print' command.  The "compile
+   print" command is used to evaluate and print an expression that may
+   contain calls to the GCC compiler.  The language expected in this
+   compile command is the language currently set in GDB.  */
+
+static void
+compile_print_command (char *arg, int from_tty)
+{
+  compile_print (arg, COMPILE_I_PRINT_ADDRESS_SCOPE);
+}
+
+/* Handle the input from the 'compile printf' command.  */
+
+static void
+compile_printf_command (char *arg, int from_tty)
+{
+  compile_print (arg, COMPILE_I_PRINTF_SCOPE);
+}
+
 /* A cleanup function to remove a directory and all its contents.  */
 
 static void
@@ -308,6 +324,14 @@ static char *compile_args;
 static int compile_args_argc;
 static char **compile_args_argv;
 
+/* String for 'set compile-printf-args' and 'show compile-printf-args'.  */
+static char *compile_printf_args;
+
+/* Parsed form of COMPILE_PRINTF_ARGS.
+   COMPILE_PRINTF_ARGS_ARGV is NULL terminated.  */
+static int compile_printf_args_argc;
+static char **compile_printf_args_argv;
+
 /* Implement 'set compile-args'.  */
 
 static void
@@ -328,6 +352,27 @@ show_compile_args (struct ui_file *file, int from_tty,
 		    value);
 }
 
+/* Implement 'set compile-printf-args'.  */
+
+static void
+set_compile_printf_args (char *args, int from_tty, struct cmd_list_element *c)
+{
+  freeargv (compile_printf_args_argv);
+  build_argc_argv (compile_printf_args,
+		   &compile_printf_args_argc, &compile_printf_args_argv);
+}
+
+/* Implement 'show compile-printf-args'.  */
+
+static void
+show_compile_printf_args (struct ui_file *file, int from_tty,
+		   struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Compile printf command command-line arguments "
+			    "are \"%s\".\n"),
+		    value);
+}
+
 /* Append ARGC and ARGV (as parsed by build_argc_argv) to *ARGCP and *ARGVP.
    ARGCP+ARGVP can be zero+NULL and also ARGC+ARGV can be zero+NULL.  */
 
@@ -422,6 +467,10 @@ get_args (const struct compile_instance *compiler, struct gdbarch *gdbarch,
   freeargv (argv_compiler);
 
   append_args (argcp, argvp, compile_args_argc, compile_args_argv);
+
+  if (compiler->scope == COMPILE_I_PRINTF_SCOPE)
+    append_args (argcp, argvp,
+		 compile_printf_args_argc, compile_printf_args_argv);
 }
 
 /* A cleanup function to destroy a gdb_gcc_instance.  */
@@ -723,6 +772,24 @@ EXPR may be preceded with /FMT, where FMT is a format letter\n\
 but no count or size letter (see \"x\" command)."),
 	   &compile_command_list);
 
+  add_cmd ("printf", class_obscure, compile_printf_command,
+	   _("\
+Print formatted output with the compiler.\n\
+\n\
+Usage: compile printf \"printf format string\", arg1, arg2, arg3, ..., argn\n\
+\n\
+The arguments may be specified on the same line as the command, e.g.:\n\
+\n\
+    compile printf \"%d\\n\", i\n\
+\n\
+Alternatively, you can type the arguments interactively.\n\
+You can invoke this mode when no argument is given to the command\n\
+(i.e., \"compile printf\" is typed with nothing after it).  An\n\
+interactive prompt will be shown allowing you to enter multiple\n\
+lines of source code.  Type a line containing \"end\" to indicate\n\
+the end of the source code."),
+	   &compile_command_list);
+
   add_setshow_boolean_cmd ("compile", class_maintenance, &compile_debug, _("\
 Set compile command debugging."), _("\
 Show compile command debugging."), _("\
@@ -740,6 +807,15 @@ String quoting is parsed like in shell, for example:\n\
   -mno-align-double \"-I/dir with a space/include\""),
 			  set_compile_args, show_compile_args, &setlist, &showlist);
 
+  add_setshow_string_cmd ("compile-printf-args", class_support,
+			  &compile_printf_args,
+			  _("Set compile command GCC command-line arguments FIXME"),
+			  _("Show compile command GCC command-line arguments FIXME"),
+			  _("\
+Use options like -Werror which get added for 'compile printf' command."),
+			  set_compile_printf_args, show_compile_printf_args,
+			  &setlist, &showlist);
+
   /* Override flags possibly coming from DW_AT_producer.  */
   compile_args = xstrdup ("-O0 -gdwarf-4"
   /* We use -fPIE Otherwise GDB would need to reserve space large enough for
@@ -758,4 +834,7 @@ String quoting is parsed like in shell, for example:\n\
 			 " -fno-stack-protector"
   );
   set_compile_args (compile_args, 0, NULL);
+
+  compile_printf_args = xstrdup ("-Werror=format");
+  set_compile_printf_args (compile_printf_args, 0, NULL);
 }
diff --git a/gdb/defs.h b/gdb/defs.h
index 1c6f118..a62740a 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -79,6 +79,10 @@ enum compile_i_scope_types
        name already specifies its address.  */
     COMPILE_I_PRINT_ADDRESS_SCOPE,
     COMPILE_I_PRINT_VALUE_SCOPE,
+
+    /* Implement the "compile printf" command.  SCOPE_DATA contains
+       string containing the format string together with its arguments.  */
+    COMPILE_I_PRINTF_SCOPE,
   };
 
 /* Just in case they're not defined in stdio.h.  */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 65357ae..6688674 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17235,6 +17235,20 @@ command without any text following the command.  This will start the
 multiple-line editor.
 @end table
 
+@table @code
+@item compile printf @var{template}, @var{expressions}@dots{}
+Compile and execute @code{printf} function call with the compiler
+language found as the current language in @value{GDBN}
+(@pxref{Languages}).
+
+@item compile printf
+@cindex reprint the last value
+Alternatively you can enter the template and expression (source code
+producing them) as multiple lines of text.  To enter this mode, invoke
+the @samp{compile printf} command without any text following the
+command.  This will start the multiple-line editor.
+@end table
+
 @noindent
 The process of compiling and injecting the code can be inspected using:
 
diff --git a/gdb/testsuite/gdb.compile/compile-print.exp b/gdb/testsuite/gdb.compile/compile-print.exp
index 92c6240..14fd489 100644
--- a/gdb/testsuite/gdb.compile/compile-print.exp
+++ b/gdb/testsuite/gdb.compile/compile-print.exp
@@ -56,3 +56,8 @@ gdb_test "compile print/x 256" " = 0x100"
 gdb_test {print $} " = 256"
 
 gdb_test "compile print varobject" { = {field = 1}}
+
+if ![is_remote target] {
+    gdb_test {compile printf "0x%x\n", varint} "\r\n0xa"
+    gdb_test {compile printf "0x%x\n"} "\r\nCompilation failed\\."
+}

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

* [PATCH v3 5/9] compile: Use -Wall, not -w
  2015-04-11 19:43 [PATCH v3 0/9] compile: compile print&printf Jan Kratochvil
                   ` (6 preceding siblings ...)
  2015-04-11 19:44 ` [PATCH v3 4/9] compile: Support relocation to GNU-IFUNCs Jan Kratochvil
@ 2015-04-11 19:44 ` Jan Kratochvil
  2015-04-29 15:49   ` Pedro Alves
  2015-04-11 19:44 ` [PATCH v3 7/9] compile: New 'compile print' Jan Kratochvil
  8 siblings, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-04-11 19:44 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

Hi,

for a reason unknown to me GDB was using -w instead of -Wall for 'compile code'.
The problem is later patch for 'compile printf' really needs some warnings to
be able to catch for example missing format string parameters:
	(gdb) compile printf "%d\n"
GCC does not seem to be able to cancel -w (there is nothing like -no-w).

Besides that I think even 'compile code' can benefit from -Wall.

That #ifndef hack in print_one_macro() is not nice but while GCC does not warn
for redefinitions like
	#define MACRO val
	#define MACRO val
together with the GCC build-in macros I haven't found any other way how to
prevent the macro-redefinition warnings (when -w is no longer in effect).

That new testsuite XFAIL is there as if one changes the struct definition to be
compliant with cv-qualifiers (to prevent the warnings):
struct struct_type {
-  struct struct_type *selffield;
+  volatile struct struct_type *selffield;
only then GCC/GDB will hit the crash, described in that GDB PR 18202.


Thanks,
Jan


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

	* compile/compile-c-support.c (print_one_macro): Use #ifndef.
	(generate_register_struct): Use __gdb_uintptr for TYPE_CODE_PTR.
	(c_compute_program): Call generate_register_struct after typedefs.
	* compile/compile-loc2c.c (push, pushf_register_address)
	(pushf_register): Cast to GCC_UINTPTR.
	(do_compile_dwarf_expr_to_c): Use unused attribute.  Add space after
	type.  Use GCC_UINTPTR instead of void *.  Remove excessive cast.
	(compile_dwarf_expr_to_c): Use GCC_UINTPTR instead of void *.
	* compile/compile.c (_initialize_compile): Enable warnings for
	COMPILE_ARGS.

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

	* gdb.compile/compile-ops.exp: Cast param to void.
	* gdb.compile/compile.exp: Complete type for _gdb_expr.
	(compile code struct_object.selffield = &struct_object): Add xfail.
---
 0 files changed

diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index 1711cda..48a17e2 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -131,7 +131,9 @@ print_one_macro (const char *name, const struct macro_definition *macro,
   if (line == 0)
     return;
 
-  fprintf_filtered (file, "#define %s", name);
+  /* None of -Wno-builtin-macro-redefined, #undef first
+     or plain #define of the same value would avoid a warning.  */
+  fprintf_filtered (file, "#ifndef %s\n# define %s", name, name);
 
   if (macro->kind == macro_function_like)
     {
@@ -147,7 +149,7 @@ print_one_macro (const char *name, const struct macro_definition *macro,
       fputs_filtered (")", file);
     }
 
-  fprintf_filtered (file, " %s\n", macro->replacement);
+  fprintf_filtered (file, " %s\n#endif\n", macro->replacement);
 }
 
 /* Write macro definitions at PC to FILE.  */
@@ -252,7 +254,7 @@ generate_register_struct (struct ui_file *stream, struct gdbarch *gdbarch,
 	    switch (TYPE_CODE (regtype))
 	      {
 	      case TYPE_CODE_PTR:
-		fprintf_filtered (stream, "void *%s", regname);
+		fprintf_filtered (stream, "__gdb_uintptr %s", regname);
 		break;
 
 	      case TYPE_CODE_INT:
@@ -340,8 +342,6 @@ c_compute_program (struct compile_instance *inst,
 							  expr_block, expr_pc);
       make_cleanup (xfree, registers_used);
 
-      generate_register_struct (buf, gdbarch, registers_used);
-
       fputs_unfiltered ("typedef unsigned int"
 			" __attribute__ ((__mode__(__pointer__)))"
 			" __gdb_uintptr;\n",
@@ -363,6 +363,8 @@ c_compute_program (struct compile_instance *inst,
 			      " __gdb_int_%s;\n",
 			      mode, mode);
 	}
+
+      generate_register_struct (buf, gdbarch, registers_used);
     }
 
   add_code_header (inst->scope, buf);
diff --git a/gdb/compile/compile-loc2c.c b/gdb/compile/compile-loc2c.c
index 6a3615d..6f53814 100644
--- a/gdb/compile/compile-loc2c.c
+++ b/gdb/compile/compile-loc2c.c
@@ -436,7 +436,8 @@ compute_stack_depth (enum bfd_endian byte_order, unsigned int addr_size,
 static void
 push (int indent, struct ui_file *stream, ULONGEST l)
 {
-  fprintfi_filtered (indent, stream, "__gdb_stack[++__gdb_tos] = %s;\n",
+  fprintfi_filtered (indent, stream,
+		     "__gdb_stack[++__gdb_tos] = (" GCC_UINTPTR ") %s;\n",
 		     hex_string (l));
 }
 
@@ -520,7 +521,8 @@ pushf_register_address (int indent, struct ui_file *stream,
   struct cleanup *cleanups = make_cleanup (xfree, regname);
 
   registers_used[regnum] = 1;
-  pushf (indent, stream, "&" COMPILE_I_SIMPLE_REGISTER_ARG_NAME	 "->%s",
+  pushf (indent, stream,
+	 "(" GCC_UINTPTR ") &" COMPILE_I_SIMPLE_REGISTER_ARG_NAME "->%s",
 	 regname);
 
   do_cleanups (cleanups);
@@ -544,7 +546,8 @@ pushf_register (int indent, struct ui_file *stream,
     pushf (indent, stream, COMPILE_I_SIMPLE_REGISTER_ARG_NAME "->%s",
 	   regname);
   else
-    pushf (indent, stream, COMPILE_I_SIMPLE_REGISTER_ARG_NAME "->%s + %s",
+    pushf (indent, stream,
+	   COMPILE_I_SIMPLE_REGISTER_ARG_NAME "->%s + (" GCC_UINTPTR ") %s",
 	   regname, hex_string (offset));
 
   do_cleanups (cleanups);
@@ -605,7 +608,8 @@ do_compile_dwarf_expr_to_c (int indent, struct ui_file *stream,
 
   ++scope;
 
-  fprintfi_filtered (indent, stream, "%s%s;\n", type_name, result_name);
+  fprintfi_filtered (indent, stream, "__attribute__ ((unused)) %s %s;\n",
+		     type_name, result_name);
   fprintfi_filtered (indent, stream, "{\n");
   indent += 2;
 
@@ -899,7 +903,7 @@ do_compile_dwarf_expr_to_c (int indent, struct ui_file *stream,
 		       (long) (op_ptr - base));
 
 	    do_compile_dwarf_expr_to_c (indent, stream,
-					"void *", fb_name,
+					GCC_UINTPTR, fb_name,
 					sym, pc,
 					arch, registers_used, addr_size,
 					datastart, datastart + datalen,
@@ -1080,7 +1084,7 @@ do_compile_dwarf_expr_to_c (int indent, struct ui_file *stream,
 			   "__cfa_%ld", (long) (op_ptr - base));
 
 		do_compile_dwarf_expr_to_c (indent, stream,
-					    "void *", cfa_name,
+					    GCC_UINTPTR, cfa_name,
 					    sym, pc, arch, registers_used,
 					    addr_size,
 					    cfa_start, cfa_end,
@@ -1117,8 +1121,8 @@ do_compile_dwarf_expr_to_c (int indent, struct ui_file *stream,
 	}
     }
 
-  fprintfi_filtered (indent, stream, "%s = (%s) __gdb_stack[__gdb_tos];\n",
-		     result_name, type_name);
+  fprintfi_filtered (indent, stream, "%s = __gdb_stack[__gdb_tos];\n",
+		     result_name);
   fprintfi_filtered (indent - 2, stream, "}\n");
 
   do_cleanups (cleanup);
@@ -1134,7 +1138,7 @@ compile_dwarf_expr_to_c (struct ui_file *stream, const char *result_name,
 			 const gdb_byte *op_ptr, const gdb_byte *op_end,
 			 struct dwarf2_per_cu_data *per_cu)
 {
-  do_compile_dwarf_expr_to_c (2, stream, "void *", result_name, sym, pc,
+  do_compile_dwarf_expr_to_c (2, stream, GCC_UINTPTR, result_name, sym, pc,
 			      arch, registers_used, addr_size, op_ptr, op_end,
 			      NULL, per_cu);
 }
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 621de66..ab19a5d 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -680,8 +680,11 @@ String quoting is parsed like in shell, for example:\n\
      absolute target address.
      -fPIC is not used at is would require from GDB to generate .got.  */
 			 " -fPIE"
-  /* We don't want warnings.  */
-			 " -w"
+  /* We want warnings, except for some commonly happening for GDB commands.  */
+			 " -Wall "
+			 " -Wno-implicit-function-declaration"
+			 " -Wno-unused-but-set-variable"
+			 " -Wno-unused-variable"
   /* Override CU's possible -fstack-protector-strong.  */
 			 " -fno-stack-protector"
   );
diff --git a/gdb/testsuite/gdb.compile/compile-ops.exp b/gdb/testsuite/gdb.compile/compile-ops.exp
index 26882bd..0ef3c8d 100644
--- a/gdb/testsuite/gdb.compile/compile-ops.exp
+++ b/gdb/testsuite/gdb.compile/compile-ops.exp
@@ -416,7 +416,7 @@ if {[skip_compile_feature_tests]} {
 }
 
 # If we have a bug, this will hang.
-gdb_test_no_output "compile code param"
+gdb_test_no_output "compile code (void) param"
 
 # We can't access optimized-out variables, but their presence should
 # not affect compilations that don't refer to them.
diff --git a/gdb/testsuite/gdb.compile/compile.exp b/gdb/testsuite/gdb.compile/compile.exp
index dc09770..abe5e9b 100644
--- a/gdb/testsuite/gdb.compile/compile.exp
+++ b/gdb/testsuite/gdb.compile/compile.exp
@@ -71,13 +71,13 @@ gdb_test_no_output "compile -- f = 10" \
 gdb_test "compile f = 10;" ".*= 10;: No such file.*" \
     "Test abbreviations and code collision"
 
-gdb_test_no_output "compile -r -- _gdb_expr(){int i = 5;}" \
+gdb_test_no_output "compile -r -- void _gdb_expr(){int i = 5;}" \
     "Test delimiter with -r"
 
-gdb_test_no_output "compile -raw -- _gdb_expr(){int i = 5;}" \
+gdb_test_no_output "compile -raw -- void _gdb_expr(){int i = 5;}" \
     "Test delimiter with -raw"
 
-gdb_test "compile -- -r  _gdb_expr(){int i = 5;}" \
+gdb_test "compile -- -r  void _gdb_expr(){int i = 5;}" \
     ".* error: 'r' undeclared \\(first use in this function\\).*" \
     "Test delimiter with -r after it"
 
@@ -189,7 +189,12 @@ gdb_test "p localvar" " = 1"
 # Test setting fields and also many different types.
 #
 
-gdb_test_no_output "compile code struct_object.selffield = &struct_object"
+set test "compile code struct_object.selffield = &struct_object"
+gdb_test_multiple $test $test {
+    -re "gdb command line:1:25: warning: assignment discards 'volatile' qualifier from pointer target type \\\[-Wdiscarded-qualifiers\\\]\r\n$gdb_prompt $" {
+	xfail "$test (PR compile/18202)"
+    }
+}
 gdb_test "print struct_object.selffield == &struct_object" " = 1"
 
 gdb_test_no_output "compile code struct_object.charfield = 1"
@@ -261,7 +266,7 @@ gdb_test "print 'compile.c'::globalshadow" " = 77000" \
 
 # Test GOT vs. resolving jit function pointers.
 
-gdb_test_no_output "compile -raw -- int func(){return 21;} _gdb_expr(){int (*funcp)()=func; if (funcp()!=21) abort();}" \
+gdb_test_no_output "compile -raw -- int func(){return 21;} void _gdb_expr(){ void abort (void); int (*funcp)()=func; if (funcp()!=21) abort(); }" \
     "pointer to jit function"
 
 #

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

* [PATCH v3 9/9] compile: compile printf: gdbserver support
  2015-04-11 19:43 [PATCH v3 0/9] compile: compile print&printf Jan Kratochvil
                   ` (3 preceding siblings ...)
  2015-04-11 19:44 ` [PATCH v3 6/9] Code cleanup: compile: func_addr -> func_sym Jan Kratochvil
@ 2015-04-11 19:44 ` Jan Kratochvil
  2015-04-26  9:33   ` Jan Kratochvil
  2015-04-29 16:12   ` Pedro Alves
  2015-04-11 19:44 ` [PATCH v3 8/9] compile: New compile printf Jan Kratochvil
                   ` (3 subsequent siblings)
  8 siblings, 2 replies; 38+ messages in thread
From: Jan Kratochvil @ 2015-04-11 19:44 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

Hi,

former patch injects plain:
	printf (...);
This patch injects gdbserver-compatible:
	f = open_memstream (&s, ...);
	fprintf (f, ...);
	fclose (f);
	return s;

Jan


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

	* compile/compile-c-support.c (add_code_header, add_code_footer)
	(c_compute_program): Use open_memstream, fprintf and fclose.
	* compile/compile-object-load.c (compile_object_load): Set expected
	char * for COMPILE_I_PRINTF_SCOPE.
	* compile/compile-object-run.c: Include gdbcore.h.
	(free_inferior_memory): New function.
	(compile_object_run): Support COMPILE_I_PRINTF_SCOPE's return value.

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

	* gdb.texinfo (Compiling and Injecting Code): Mention for compile
	printf open_memstream and fprintf.

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

	* gdb.compile/compile-print.exp (compile printf "0x%x\n", varint)
	(compile printf "0x%x\n"): Remove !is_remote conditional.

diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index 92c41b6..3b9bb52 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -208,13 +208,21 @@ add_code_header (enum compile_i_scope_types type, struct ui_file *buf)
       break;
     case COMPILE_I_PRINTF_SCOPE:
       fputs_unfiltered ("#include <stdio.h>\n"
-			"void "
+			"char *"
 			GCC_FE_WRAPPER_FUNCTION
 			" (struct "
 			COMPILE_I_SIMPLE_REGISTER_STRUCT_TAG
 			" *"
 			COMPILE_I_SIMPLE_REGISTER_ARG_NAME
-			") {\n",
+			") {\n"
+			"\tchar *__gdb_retval;\n"
+			"\tsize_t __gdb_retval_size;\n"
+			"\tFILE *__gdb_outf;\n"
+			"\n"
+			"\t__gdb_outf = open_memstream (&__gdb_retval, "
+						       "&__gdb_retval_size);\n"
+			"\tif (__gdb_outf == NULL)\n"
+			"\t\treturn NULL;\n",
 			buf);
       break;
     case COMPILE_I_RAW_SCOPE:
@@ -233,10 +241,15 @@ add_code_footer (enum compile_i_scope_types type, struct ui_file *buf)
 {
   switch (type)
     {
+    case COMPILE_I_PRINTF_SCOPE:
+      fputs_unfiltered ("\tif (fclose (__gdb_outf) != 0)\n"
+			"\t\treturn NULL;\n"
+			"\treturn __gdb_retval;\n",
+			buf);
+      // FALLTHRU
     case COMPILE_I_SIMPLE_SCOPE:
     case COMPILE_I_PRINT_ADDRESS_SCOPE:
     case COMPILE_I_PRINT_VALUE_SCOPE:
-    case COMPILE_I_PRINTF_SCOPE:
       fputs_unfiltered ("}\n", buf);
       break;
     case COMPILE_I_RAW_SCOPE:
@@ -432,7 +445,7 @@ c_compute_program (struct compile_instance *inst,
       break;
     case COMPILE_I_PRINTF_SCOPE:
       fprintf_unfiltered (buf,
-"printf (%s);\n"
+"fprintf (__gdb_outf, %s);\n"
 			  , input);
       break;
     default:
diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index bc74590..d7c13c9 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -651,7 +651,8 @@ compile_object_load (const char *object_file, const char *source_file,
       break;
     case COMPILE_I_PRINTF_SCOPE:
       expect_parameters = 1;
-      expect_return_type = builtin_type (target_gdbarch ())->builtin_void;
+      expect_return_type = lookup_pointer_type
+			       (builtin_type (target_gdbarch ())->builtin_char);
       break;
     default:
       internal_error (__FILE__, __LINE__, _("invalid scope %d"), scope);
diff --git a/gdb/compile/compile-object-run.c b/gdb/compile/compile-object-run.c
index 216cbf9..ed56410 100644
--- a/gdb/compile/compile-object-run.c
+++ b/gdb/compile/compile-object-run.c
@@ -27,6 +27,7 @@
 #include "block.h"
 #include "valprint.h"
 #include "compile.h"
+#include "gdbcore.h"

 /* Helper for do_module_cleanup.  */

@@ -99,6 +100,18 @@ do_module_cleanup (void *arg)
   xfree (data);
 }

+static void
+free_inferior_memory (CORE_ADDR addr)
+{
+  struct objfile *objf;
+  struct value *func_val = find_function_in_inferior ("free", &objf);
+  struct gdbarch *gdbarch = get_objfile_arch (objf);
+  struct type *addr_type = builtin_type (gdbarch)->builtin_data_ptr;
+  struct value *addr_val = value_from_pointer (addr_type, addr);
+
+  call_function_by_hand (func_val, 1, &addr_val);
+}
+
 /* Perform inferior call of MODULE.  This function may throw an error.
    This function may leave files referenced by MODULE on disk until
    the inferior call dummy frame is discarded.  This function may throw errors.
@@ -117,6 +130,7 @@ compile_object_run (struct compile_module *module)
   struct symbol *func_sym = module->func_sym;
   CORE_ADDR regs_addr = module->regs_addr;
   struct objfile *objfile = module->objfile;
+  enum compile_i_scope_types scope = module->scope;

   data = xmalloc (sizeof (*data) + strlen (objfile_name_s));
   data->executedp = &executed;
@@ -136,7 +150,7 @@ compile_object_run (struct compile_module *module)
       struct type *func_type = SYMBOL_TYPE (func_sym);
       htab_t copied_types;
       int current_arg = 0;
-      struct value **vargs;
+      struct value **vargs, *func_return_value;

       /* OBJFILE may disappear while FUNC_TYPE still will be in use.  */
       copied_types = create_copied_types_hash (objfile);
@@ -163,8 +177,42 @@ compile_object_run (struct compile_module *module)
 	  ++current_arg;
 	}
       gdb_assert (current_arg == TYPE_NFIELDS (func_type));
-      call_function_by_hand_dummy (func_val, TYPE_NFIELDS (func_type), vargs,
-				   do_module_cleanup, data);
+      func_return_value = call_function_by_hand_dummy (func_val,
+						       TYPE_NFIELDS (func_type),
+						       vargs,
+						       do_module_cleanup, data);
+
+      // DATA can be already freed now.
+      data = NULL;
+
+      if (scope == COMPILE_I_PRINTF_SCOPE)
+	{
+	  struct value_print_options opts;
+	  gdb_byte *buffer = NULL;
+	  struct cleanup *old_chain;
+	  int errcode, bytes_read;
+	  struct type *retval_type = value_type (func_return_value);
+	  struct type *char_type;
+
+	  gdb_assert (TYPE_CODE (retval_type) == TYPE_CODE_PTR);
+	  char_type = TYPE_TARGET_TYPE (retval_type);
+	  gdb_assert (TYPE_CODE (char_type) == TYPE_CODE_INT);
+
+	  get_user_print_options (&opts);
+	  errcode = read_string (value_as_address (func_return_value), -1,
+				 TYPE_LENGTH (char_type), opts.print_max,
+				 gdbarch_byte_order (target_gdbarch ()),
+				 &buffer, &bytes_read);
+	  old_chain = make_cleanup (xfree, buffer);
+	  if (errcode != 0)
+	    memory_error (errcode, value_as_address (func_return_value));
+
+	  while (bytes_read-- > 0)
+	    putchar_filtered (*buffer++);
+	  do_cleanups (old_chain);
+
+	  free_inferior_memory (value_as_address (func_return_value));
+	}
     }
   CATCH (ex, RETURN_MASK_ERROR)
     {
diff --git a/gdb/testsuite/gdb.compile/compile-print.exp b/gdb/testsuite/gdb.compile/compile-print.exp
index 14fd489..43b9f24 100644
--- a/gdb/testsuite/gdb.compile/compile-print.exp
+++ b/gdb/testsuite/gdb.compile/compile-print.exp
@@ -57,7 +57,5 @@ gdb_test {print $} " = 256"

 gdb_test "compile print varobject" { = {field = 1}}

-if ![is_remote target] {
-    gdb_test {compile printf "0x%x\n", varint} "\r\n0xa"
-    gdb_test {compile printf "0x%x\n"} "\r\nCompilation failed\\."
-}
+gdb_test {compile printf "0x%x\n", varint} "\r\n0xa"
+gdb_test {compile printf "0x%x\n"} "\r\nCompilation failed\\."
---
 gdb/doc/gdb.texinfo |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index 92c41b6..3b9bb52 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -208,13 +208,21 @@ add_code_header (enum compile_i_scope_types type, struct ui_file *buf)
       break;
     case COMPILE_I_PRINTF_SCOPE:
       fputs_unfiltered ("#include <stdio.h>\n"
-			"void "
+			"char *"
 			GCC_FE_WRAPPER_FUNCTION
 			" (struct "
 			COMPILE_I_SIMPLE_REGISTER_STRUCT_TAG
 			" *"
 			COMPILE_I_SIMPLE_REGISTER_ARG_NAME
-			") {\n",
+			") {\n"
+			"\tchar *__gdb_retval;\n"
+			"\tsize_t __gdb_retval_size;\n"
+			"\tFILE *__gdb_outf;\n"
+			"\n"
+			"\t__gdb_outf = open_memstream (&__gdb_retval, "
+						       "&__gdb_retval_size);\n"
+			"\tif (__gdb_outf == NULL)\n"
+			"\t\treturn NULL;\n",
 			buf);
       break;
     case COMPILE_I_RAW_SCOPE:
@@ -233,10 +241,15 @@ add_code_footer (enum compile_i_scope_types type, struct ui_file *buf)
 {
   switch (type)
     {
+    case COMPILE_I_PRINTF_SCOPE:
+      fputs_unfiltered ("\tif (fclose (__gdb_outf) != 0)\n"
+			"\t\treturn NULL;\n"
+			"\treturn __gdb_retval;\n",
+			buf);
+      // FALLTHRU
     case COMPILE_I_SIMPLE_SCOPE:
     case COMPILE_I_PRINT_ADDRESS_SCOPE:
     case COMPILE_I_PRINT_VALUE_SCOPE:
-    case COMPILE_I_PRINTF_SCOPE:
       fputs_unfiltered ("}\n", buf);
       break;
     case COMPILE_I_RAW_SCOPE:
@@ -432,7 +445,7 @@ c_compute_program (struct compile_instance *inst,
       break;
     case COMPILE_I_PRINTF_SCOPE:
       fprintf_unfiltered (buf,
-"printf (%s);\n"
+"fprintf (__gdb_outf, %s);\n"
 			  , input);
       break;
     default:
diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index bc74590..d7c13c9 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -651,7 +651,8 @@ compile_object_load (const char *object_file, const char *source_file,
       break;
     case COMPILE_I_PRINTF_SCOPE:
       expect_parameters = 1;
-      expect_return_type = builtin_type (target_gdbarch ())->builtin_void;
+      expect_return_type = lookup_pointer_type
+			       (builtin_type (target_gdbarch ())->builtin_char);
       break;
     default:
       internal_error (__FILE__, __LINE__, _("invalid scope %d"), scope);
diff --git a/gdb/compile/compile-object-run.c b/gdb/compile/compile-object-run.c
index 216cbf9..3606d42 100644
--- a/gdb/compile/compile-object-run.c
+++ b/gdb/compile/compile-object-run.c
@@ -27,6 +27,7 @@
 #include "block.h"
 #include "valprint.h"
 #include "compile.h"
+#include "gdbcore.h"
 
 /* Helper for do_module_cleanup.  */
 
@@ -99,6 +100,20 @@ do_module_cleanup (void *arg)
   xfree (data);
 }
 
+/* Call inferior function "free" for ADDR.  */
+
+static void
+free_inferior_memory (CORE_ADDR addr)
+{
+  struct objfile *objf;
+  struct value *func_val = find_function_in_inferior ("free", &objf);
+  struct gdbarch *gdbarch = get_objfile_arch (objf);
+  struct type *addr_type = builtin_type (gdbarch)->builtin_data_ptr;
+  struct value *addr_val = value_from_pointer (addr_type, addr);
+
+  call_function_by_hand (func_val, 1, &addr_val);
+}
+
 /* Perform inferior call of MODULE.  This function may throw an error.
    This function may leave files referenced by MODULE on disk until
    the inferior call dummy frame is discarded.  This function may throw errors.
@@ -117,6 +132,7 @@ compile_object_run (struct compile_module *module)
   struct symbol *func_sym = module->func_sym;
   CORE_ADDR regs_addr = module->regs_addr;
   struct objfile *objfile = module->objfile;
+  enum compile_i_scope_types scope = module->scope;
 
   data = xmalloc (sizeof (*data) + strlen (objfile_name_s));
   data->executedp = &executed;
@@ -136,7 +152,7 @@ compile_object_run (struct compile_module *module)
       struct type *func_type = SYMBOL_TYPE (func_sym);
       htab_t copied_types;
       int current_arg = 0;
-      struct value **vargs;
+      struct value **vargs, *func_return_value;
 
       /* OBJFILE may disappear while FUNC_TYPE still will be in use.  */
       copied_types = create_copied_types_hash (objfile);
@@ -163,8 +179,42 @@ compile_object_run (struct compile_module *module)
 	  ++current_arg;
 	}
       gdb_assert (current_arg == TYPE_NFIELDS (func_type));
-      call_function_by_hand_dummy (func_val, TYPE_NFIELDS (func_type), vargs,
-				   do_module_cleanup, data);
+      func_return_value = call_function_by_hand_dummy (func_val,
+						       TYPE_NFIELDS (func_type),
+						       vargs,
+						       do_module_cleanup, data);
+
+      // DATA can be already freed now.
+      data = NULL;
+
+      if (scope == COMPILE_I_PRINTF_SCOPE)
+	{
+	  struct value_print_options opts;
+	  gdb_byte *buffer = NULL;
+	  struct cleanup *old_chain;
+	  int errcode, bytes_read;
+	  struct type *retval_type = value_type (func_return_value);
+	  struct type *char_type;
+
+	  gdb_assert (TYPE_CODE (retval_type) == TYPE_CODE_PTR);
+	  char_type = TYPE_TARGET_TYPE (retval_type);
+	  gdb_assert (TYPE_CODE (char_type) == TYPE_CODE_INT);
+
+	  get_user_print_options (&opts);
+	  errcode = read_string (value_as_address (func_return_value), -1,
+				 TYPE_LENGTH (char_type), opts.print_max,
+				 gdbarch_byte_order (target_gdbarch ()),
+				 &buffer, &bytes_read);
+	  old_chain = make_cleanup (xfree, buffer);
+	  if (errcode != 0)
+	    memory_error (errcode, value_as_address (func_return_value));
+
+	  while (bytes_read-- > 0)
+	    putchar_filtered (*buffer++);
+	  do_cleanups (old_chain);
+
+	  free_inferior_memory (value_as_address (func_return_value));
+	}
     }
   CATCH (ex, RETURN_MASK_ERROR)
     {
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 6688674..fd7ff05 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17239,7 +17239,9 @@ multiple-line editor.
 @item compile printf @var{template}, @var{expressions}@dots{}
 Compile and execute @code{printf} function call with the compiler
 language found as the current language in @value{GDBN}
-(@pxref{Languages}).
+(@pxref{Languages}).  The value is printed by @value{GDBN} and not the
+inferior, inferior does not execute specifically the function
+@code{printf}.
 
 @item compile printf
 @cindex reprint the last value
diff --git a/gdb/testsuite/gdb.compile/compile-print.exp b/gdb/testsuite/gdb.compile/compile-print.exp
index 14fd489..43b9f24 100644
--- a/gdb/testsuite/gdb.compile/compile-print.exp
+++ b/gdb/testsuite/gdb.compile/compile-print.exp
@@ -57,7 +57,5 @@ gdb_test {print $} " = 256"
 
 gdb_test "compile print varobject" { = {field = 1}}
 
-if ![is_remote target] {
-    gdb_test {compile printf "0x%x\n", varint} "\r\n0xa"
-    gdb_test {compile printf "0x%x\n"} "\r\nCompilation failed\\."
-}
+gdb_test {compile printf "0x%x\n", varint} "\r\n0xa"
+gdb_test {compile printf "0x%x\n"} "\r\nCompilation failed\\."

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

* [PATCH v3 7/9] compile: New 'compile print'
  2015-04-11 19:43 [PATCH v3 0/9] compile: compile print&printf Jan Kratochvil
                   ` (7 preceding siblings ...)
  2015-04-11 19:44 ` [PATCH v3 5/9] compile: Use -Wall, not -w Jan Kratochvil
@ 2015-04-11 19:44 ` Jan Kratochvil
  2015-04-29 15:53   ` Pedro Alves
  8 siblings, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-04-11 19:44 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

It is planned the existing GDB command 'print' will be able to evaluate its
expressions using the compiler.  There will be some option to choose between
the existing GDB evaluation and the compiler evaluation.  But as an
intermediate step this patch provides the expression printing feature as a new
command.

I can imagine it could be also called 'maintenance compile print' as in the
future one should be able to use its functionality by the normal 'print'
command.

There was a discussion with Eli about the command name:
	https://sourceware.org/ml/gdb-patches/2015-03/msg00880.html
As there were no other comments yet I haven't renamed it yet, before there is
some confirmation about settlement on the final name.

Support for the GDB '@' operator to create arrays has been submitted for GCC:
	[gcc patch] libcc1: '@' GDB array operator
	https://gcc.gnu.org/ml/gcc-patches/2015-03/msg01451.html


gdb/ChangeLog
2015-03-26  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Phil Muldoon  <pmuldoon@redhat.com>

	* NEWS (Changes since GDB 7.9): Add compile print.
	* compile/compile-c-support.c (add_code_header, add_code_footer)
	(c_compute_program): Add COMPILE_I_PRINT_ADDRESS_SCOPE and
	COMPILE_I_PRINT_VALUE_SCOPE.
	* compile/compile-internal.h (COMPILE_I_PRINT_OUT_ARG_TYPE)
	(COMPILE_I_PRINT_OUT_ARG, COMPILE_I_EXPR_VAL, COMPILE_I_EXPR_PTR_TYPE):
	New.
	* compile/compile-object-load.c: Include block.h.
	(get_out_value_type): New function.
	(compile_object_load): Handle COMPILE_I_PRINT_ADDRESS_SCOPE and
	COMPILE_I_PRINT_VALUE_SCOPE.  Set compile_module's OUT_VALUE_ADDR and
	OUT_VALUE_TYPE.
	* compile/compile-object-load.h (struct compile_module): Add fields
	out_value_addr and out_value_type.
	* compile/compile-object-run.c: Include valprint.h and compile.h.
	(struct do_module_cleanup): Add fields out_value_addr and
	out_value_type.
	(do_module_cleanup): Handle COMPILE_I_PRINT_ADDRESS_SCOPE and
	COMPILE_I_PRINT_VALUE_SCOPE.
	(compile_object_run): Propagate out_value_addr and out_value_type.
	Pass OUT_VALUE_ADDR.
	* compile/compile.c: Include valprint.h.
	(compile_print_value, compile_print_command): New functions.
	(eval_compile_command): Handle failed COMPILE_I_PRINT_ADDRESS_SCOPE.
	(_initialize_compile): Update compile code help text.  Install
	compile_print_command.
	* compile/compile.h (compile_print_value): New prototype.
	* defs.h (enum compile_i_scope_types): Add
	COMPILE_I_PRINT_ADDRESS_SCOPE and COMPILE_I_PRINT_VALUE_SCOPE.

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

	* gdb.texinfo (Compiling and Injecting Code): Add compile print.

gdb/testsuite/ChangeLog
2015-03-26  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.compile/compile-print.c: New file.
	* gdb.compile/compile-print.exp: New file.
---
 gdb/doc/gdb.texinfo |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index c24195e..8b1aad1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -53,6 +53,9 @@ record btrace bts
 record bts
   Start branch trace recording using Branch Trace Store (BTS) format.
 
+compile print
+  Evaluate expression by using the compiler and print result.
+
 * New options
 
 set max-completions
diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index 48a17e2..86b02cf 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -190,6 +190,23 @@ add_code_header (enum compile_i_scope_types type, struct ui_file *buf)
 			") {\n",
 			buf);
       break;
+    case COMPILE_I_PRINT_ADDRESS_SCOPE:
+    case COMPILE_I_PRINT_VALUE_SCOPE:
+      fputs_unfiltered ("#include <string.h>\n"
+			"void "
+			GCC_FE_WRAPPER_FUNCTION
+			" (struct "
+			COMPILE_I_SIMPLE_REGISTER_STRUCT_TAG
+			" *"
+			COMPILE_I_SIMPLE_REGISTER_ARG_NAME
+			", "
+			COMPILE_I_PRINT_OUT_ARG_TYPE
+			" "
+			COMPILE_I_PRINT_OUT_ARG
+			") {\n",
+			buf);
+      break;
+
     case COMPILE_I_RAW_SCOPE:
       break;
     default:
@@ -207,6 +224,8 @@ add_code_footer (enum compile_i_scope_types type, struct ui_file *buf)
   switch (type)
     {
     case COMPILE_I_SIMPLE_SCOPE:
+    case COMPILE_I_PRINT_ADDRESS_SCOPE:
+    case COMPILE_I_PRINT_VALUE_SCOPE:
       fputs_unfiltered ("}\n", buf);
       break;
     case COMPILE_I_RAW_SCOPE:
@@ -369,7 +388,9 @@ c_compute_program (struct compile_instance *inst,
 
   add_code_header (inst->scope, buf);
 
-  if (inst->scope == COMPILE_I_SIMPLE_SCOPE)
+  if (inst->scope == COMPILE_I_SIMPLE_SCOPE
+      || inst->scope == COMPILE_I_PRINT_ADDRESS_SCOPE
+      || inst->scope == COMPILE_I_PRINT_VALUE_SCOPE)
     {
       ui_file_put (var_stream, ui_file_write_for_put, buf);
       fputs_unfiltered ("#pragma GCC user_expression\n", buf);
@@ -383,7 +404,25 @@ c_compute_program (struct compile_instance *inst,
     fputs_unfiltered ("{\n", buf);
 
   fputs_unfiltered ("#line 1 \"gdb command line\"\n", buf);
-  fputs_unfiltered (input, buf);
+
+  switch (inst->scope)
+    {
+    case COMPILE_I_PRINT_ADDRESS_SCOPE:
+    case COMPILE_I_PRINT_VALUE_SCOPE:
+      fprintf_unfiltered (buf,
+"__auto_type " COMPILE_I_EXPR_VAL " = %s;\n"
+"typeof (%s) *" COMPILE_I_EXPR_PTR_TYPE ";\n"
+"memcpy (" COMPILE_I_PRINT_OUT_ARG ", %s" COMPILE_I_EXPR_VAL ",\n"
+	 "sizeof (*" COMPILE_I_EXPR_PTR_TYPE "));\n"
+			  , input, input,
+			  (inst->scope == COMPILE_I_PRINT_ADDRESS_SCOPE
+			   ? "&" : ""));
+      break;
+    default:
+      fputs_unfiltered (input, buf);
+      break;
+    }
+
   fputs_unfiltered ("\n", buf);
 
   /* For larger user expressions the automatic semicolons may be
diff --git a/gdb/compile/compile-internal.h b/gdb/compile/compile-internal.h
index c369d46..b1a5a88 100644
--- a/gdb/compile/compile-internal.h
+++ b/gdb/compile/compile-internal.h
@@ -84,6 +84,10 @@ struct compile_c_instance
 #define COMPILE_I_SIMPLE_REGISTER_STRUCT_TAG "__gdb_regs"
 #define COMPILE_I_SIMPLE_REGISTER_ARG_NAME "__regs"
 #define COMPILE_I_SIMPLE_REGISTER_DUMMY "_dummy"
+#define COMPILE_I_PRINT_OUT_ARG_TYPE "void *"
+#define COMPILE_I_PRINT_OUT_ARG "__gdb_out_param"
+#define COMPILE_I_EXPR_VAL "__gdb_expr_val"
+#define COMPILE_I_EXPR_PTR_TYPE "__gdb_expr_ptr_type"
 
 /* Call gdbarch_register_name (GDBARCH, REGNUM) and convert its result
    to a form suitable for the compiler source.  The register names
diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index 3c07840..e67e5ba 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -29,6 +29,7 @@
 #include "regcache.h"
 #include "inferior.h"
 #include "compile.h"
+#include "block.h"
 #include "arch-utils.h"
 
 /* Helper data for setup_sections.  */
@@ -354,6 +355,114 @@ copy_sections (bfd *abfd, asection *sect, void *data)
   do_cleanups (cleanups);
 }
 
+/* Fetch the type of COMPILE_I_EXPR_PTR_TYPE and COMPILE_I_EXPR_VAL
+   symbols in OBJFILE so we can calculate how much memory to allocate
+   for the out parameter.  This avoids needing a malloc in the generated
+   code.  Throw an error if anything fails.
+   Set *OUT_VALUE_TAKE_ADDRESSP depending whether inferior code should
+   copy COMPILE_I_EXPR_VAL or its address - this depends on __auto_type
+   array-to-pointer type conversion of COMPILE_I_EXPR_VAL, as detected
+   by COMPILE_I_EXPR_PTR_TYPE preserving the array type.  */
+
+static struct type *
+get_out_value_type (struct symbol *func_sym, struct objfile *objfile,
+		    enum compile_i_scope_types scope)
+{
+  struct symbol *gdb_ptr_type_sym, *gdb_val_sym;
+  struct type *gdb_ptr_type, *gdb_type_from_ptr, *gdb_type, *retval;
+  const struct block *block;
+  const struct blockvector *bv;
+  int nblocks = 0;
+  int block_loop = 0;
+
+  bv = SYMTAB_BLOCKVECTOR (func_sym->owner.symtab);
+  nblocks = BLOCKVECTOR_NBLOCKS (bv);
+
+  gdb_ptr_type_sym = NULL;
+  for (block_loop = 0; block_loop < nblocks; block_loop++)
+    {
+      struct symbol *function;
+      const struct block *function_block;
+
+      block = BLOCKVECTOR_BLOCK (bv, block_loop);
+      if (BLOCK_FUNCTION (block) != NULL)
+	continue;
+      gdb_val_sym = block_lookup_symbol (block, COMPILE_I_EXPR_VAL, VAR_DOMAIN);
+      if (gdb_val_sym == NULL)
+	continue;
+
+      function_block = block;
+      while (function_block != BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK)
+	     && function_block != BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK))
+	{
+	  function_block = BLOCK_SUPERBLOCK (function_block);
+	  function = BLOCK_FUNCTION (function_block);
+	  if (function != NULL)
+	    break;
+	}
+      if (function != NULL
+	  && (BLOCK_SUPERBLOCK (function_block)
+	      == BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK))
+	  && (strcmp (SYMBOL_LINKAGE_NAME (function), GCC_FE_WRAPPER_FUNCTION)
+	      == 0))
+	break;
+    }
+  if (block_loop == nblocks)
+    error (_("No \"%s\" symbol found"), COMPILE_I_EXPR_PTR_TYPE);
+
+  gdb_type = SYMBOL_TYPE (gdb_val_sym);
+  CHECK_TYPEDEF (gdb_type);
+
+  gdb_ptr_type_sym = block_lookup_symbol (block, COMPILE_I_EXPR_PTR_TYPE,
+					  VAR_DOMAIN);
+  if (gdb_ptr_type_sym == NULL)
+    error (_("No \"%s\" symbol found"), COMPILE_I_EXPR_PTR_TYPE);
+  gdb_ptr_type = SYMBOL_TYPE (gdb_ptr_type_sym);
+  CHECK_TYPEDEF (gdb_ptr_type);
+  if (TYPE_CODE (gdb_ptr_type) != TYPE_CODE_PTR)
+    error (_("Type of \"%s\" is not a pointer"), COMPILE_I_EXPR_PTR_TYPE);
+  gdb_type_from_ptr = TYPE_TARGET_TYPE (gdb_ptr_type);
+
+  if (types_deeply_equal (gdb_type, gdb_type_from_ptr))
+    {
+      if (scope != COMPILE_I_PRINT_ADDRESS_SCOPE)
+	error (_("Expected address scope in compiled module \"%s\"."),
+	       objfile_name (objfile));
+      return gdb_type;
+    }
+
+  if (TYPE_CODE (gdb_type) != TYPE_CODE_PTR)
+    error (_("Invalid type code %d of symbol \"%s\" "
+	     "in compiled module \"%s\"."),
+	   TYPE_CODE (gdb_type_from_ptr), COMPILE_I_EXPR_VAL,
+	   objfile_name (objfile));
+  
+  switch (TYPE_CODE (gdb_type_from_ptr))
+    {
+    case TYPE_CODE_ARRAY:
+      retval = gdb_type_from_ptr;
+      gdb_type_from_ptr = TYPE_TARGET_TYPE (gdb_type_from_ptr);
+      break;
+    case TYPE_CODE_FUNC:
+      retval = gdb_type_from_ptr;
+      break;
+    default:
+      error (_("Invalid type code %d of symbol \"%s\" "
+	       "in compiled module \"%s\"."),
+	     TYPE_CODE (gdb_type_from_ptr), COMPILE_I_EXPR_PTR_TYPE,
+	     objfile_name (objfile));
+    }
+  if (!types_deeply_equal (gdb_type_from_ptr,
+			   TYPE_TARGET_TYPE (gdb_type)))
+    error (_("Referenced types do not match for symbols \"%s\" and \"%s\" "
+	     "in compiled module \"%s\"."),
+	   COMPILE_I_EXPR_PTR_TYPE, COMPILE_I_EXPR_VAL,
+	   objfile_name (objfile));
+  if (scope == COMPILE_I_PRINT_ADDRESS_SCOPE)
+    return NULL;
+  return retval;
+}
+
 /* Fetch the type of first parameter of FUNC_SYM.
    Return NULL if FUNC_SYM has no parameters.  Throw an error otherwise.  */
 
@@ -440,7 +549,9 @@ store_regs (struct type *regs_type, CORE_ADDR regs_base)
    Caller must fully dispose the return value by calling compile_object_run.
    SOURCE_FILE's copy is stored into the returned object.
    Caller should free both OBJECT_FILE and SOURCE_FILE immediatelly after this
-   function returns.  */
+   function returns.
+   Function returns NULL only for COMPILE_I_PRINT_ADDRESS_SCOPE when
+   COMPILE_I_PRINT_VALUE_SCOPE should have been used instead.  */
 
 struct compile_module *
 compile_object_load (const char *object_file, const char *source_file,
@@ -449,7 +560,7 @@ compile_object_load (const char *object_file, const char *source_file,
   struct cleanup *cleanups, *cleanups_free_objfile;
   bfd *abfd;
   struct setup_sections_data setup_sections_data;
-  CORE_ADDR addr, regs_addr;
+  CORE_ADDR addr, regs_addr, out_value_addr = 0;
   struct symbol *func_sym;
   struct type *func_type;
   struct bound_minimal_symbol bmsym;
@@ -459,7 +570,7 @@ compile_object_load (const char *object_file, const char *source_file,
   struct type *dptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
   unsigned dptr_type_len = TYPE_LENGTH (dptr_type);
   struct compile_module *retval;
-  struct type *regs_type;
+  struct type *regs_type, *out_value_type = NULL;
   char *filename, **matching;
   struct objfile *objfile;
   int expect_parameters;
@@ -521,6 +632,11 @@ compile_object_load (const char *object_file, const char *source_file,
       expect_parameters = 0;
       expect_return_type = builtin_type (target_gdbarch ())->builtin_void;
       break;
+    case COMPILE_I_PRINT_ADDRESS_SCOPE:
+    case COMPILE_I_PRINT_VALUE_SCOPE:
+      expect_parameters = 2;
+      expect_return_type = builtin_type (target_gdbarch ())->builtin_void;
+      break;
     default:
       internal_error (__FILE__, __LINE__, _("invalid scope %d"), scope);
     }
@@ -597,6 +713,23 @@ compile_object_load (const char *object_file, const char *source_file,
       store_regs (regs_type, regs_addr);
     }
 
+  if (scope == COMPILE_I_PRINT_ADDRESS_SCOPE
+      || scope == COMPILE_I_PRINT_VALUE_SCOPE)
+    {
+      out_value_type = get_out_value_type (func_sym, objfile, scope);
+      if (out_value_type == NULL)
+	{
+	  do_cleanups (cleanups);
+	  return NULL;
+	}
+      check_typedef (out_value_type);
+      out_value_addr = gdbarch_infcall_mmap (target_gdbarch (),
+					     TYPE_LENGTH (out_value_type),
+					     (GDB_MMAP_PROT_READ
+					      | GDB_MMAP_PROT_WRITE));
+      gdb_assert (out_value_addr != 0);
+    }
+
   discard_cleanups (cleanups_free_objfile);
   do_cleanups (cleanups);
 
@@ -607,5 +740,7 @@ compile_object_load (const char *object_file, const char *source_file,
   retval->regs_addr = regs_addr;
   retval->scope = scope;
   retval->scope_data = scope_data;
+  retval->out_value_addr = out_value_addr;
+  retval->out_value_type = out_value_type;
   return retval;
 }
diff --git a/gdb/compile/compile-object-load.h b/gdb/compile/compile-object-load.h
index 94f77be..ac72a6a 100644
--- a/gdb/compile/compile-object-load.h
+++ b/gdb/compile/compile-object-load.h
@@ -37,6 +37,14 @@ struct compile_module
 
   /* User data for SCOPE in use.  */
   void *scope_data;
+
+  /* Inferior parameter out value address or NULL if the inferior function does not
+     have one.  */
+  CORE_ADDR out_value_addr;
+
+  /* Inferior parameter out value type or NULL if the inferior function does not
+     have one.  */
+  struct type *out_value_type;
 };
 
 extern struct compile_module *compile_object_load (const char *object_file,
diff --git a/gdb/compile/compile-object-run.c b/gdb/compile/compile-object-run.c
index c454bd1..216cbf9 100644
--- a/gdb/compile/compile-object-run.c
+++ b/gdb/compile/compile-object-run.c
@@ -25,6 +25,8 @@
 #include "compile-internal.h"
 #include "dummy-frame.h"
 #include "block.h"
+#include "valprint.h"
+#include "compile.h"
 
 /* Helper for do_module_cleanup.  */
 
@@ -41,6 +43,10 @@ struct do_module_cleanup
   enum compile_i_scope_types scope;
   void *scope_data;
 
+  /* Copy from struct compile_module.  */
+  CORE_ADDR out_value_addr;
+  struct type *out_value_type;
+
   /* objfile_name of our objfile.  */
   char objfile_name_string[1];
 };
@@ -56,7 +62,21 @@ do_module_cleanup (void *arg)
   struct objfile *objfile;
 
   if (data->executedp != NULL)
-    *data->executedp = 1;
+    {
+      *data->executedp = 1;
+
+      /* This code cannot be in compile_object_run as OUT_VALUE_TYPE
+	 no longer exists there.  */
+      if (data->scope == COMPILE_I_PRINT_ADDRESS_SCOPE
+	  || data->scope == COMPILE_I_PRINT_VALUE_SCOPE)
+	{
+	  struct value *addr_value;
+	  struct type *ptr_type = lookup_pointer_type (data->out_value_type);
+
+	  addr_value = value_from_pointer (ptr_type, data->out_value_addr);
+	  compile_print_value (value_ind (addr_value), data->scope_data);
+	}
+    }
 
   ALL_OBJFILES (objfile)
     if ((objfile->flags & OBJF_USERLOADED) == 0
@@ -104,6 +124,8 @@ compile_object_run (struct compile_module *module)
   strcpy (data->objfile_name_string, objfile_name_s);
   data->scope = module->scope;
   data->scope_data = module->scope_data;
+  data->out_value_addr = module->out_value_addr;
+  data->out_value_type = module->out_value_type;
 
   xfree (module->source_file);
   xfree (module);
@@ -133,6 +155,13 @@ compile_object_run (struct compile_module *module)
 			  (TYPE_FIELD_TYPE (func_type, current_arg), regs_addr);
 	  ++current_arg;
 	}
+      if (TYPE_NFIELDS (func_type) >= 2)
+	{
+	  gdb_assert (data->out_value_addr != 0);
+	  vargs[current_arg] = value_from_pointer
+	       (TYPE_FIELD_TYPE (func_type, current_arg), data->out_value_addr);
+	  ++current_arg;
+	}
       gdb_assert (current_arg == TYPE_NFIELDS (func_type));
       call_function_by_hand_dummy (func_val, TYPE_NFIELDS (func_type), vargs,
 				   do_module_cleanup, data);
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index ab19a5d..424a25b 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -38,6 +38,7 @@
 #include "target.h"
 #include "osabi.h"
 #include "gdb_wait.h"
+#include "valprint.h"
 
 \f
 
@@ -163,6 +164,49 @@ compile_code_command (char *arg, int from_tty)
   do_cleanups (cleanup);
 }
 
+/* Callback for compile_print_command.  */
+
+void
+compile_print_value (struct value *val, void *data_voidp)
+{
+  const struct format_data *fmtp = data_voidp;
+
+  print_value (val, fmtp);
+}
+
+/* Handle the input from the 'compile print' command.  The "compile
+   print" command is used to evaluate and print an expression that may
+   contain calls to the GCC compiler.  The language expected in this
+   compile command is the language currently set in GDB.  */
+
+static void
+compile_print_command (char *arg_param, int from_tty)
+{
+  const char *arg = arg_param;
+  struct cleanup *cleanup;
+  enum compile_i_scope_types scope = COMPILE_I_PRINT_ADDRESS_SCOPE;
+  struct format_data *fmtp;
+
+  cleanup = make_cleanup_restore_integer (&interpreter_async);
+  interpreter_async = 0;
+
+  fmtp = print_command_parse_format (&arg, "compile print");
+
+  if (arg && *arg)
+    eval_compile_command (NULL, arg, scope, fmtp);
+  else
+    {
+      struct command_line *l = get_command_line (compile_control, "");
+
+      make_cleanup_free_command_lines (&l);
+      l->control_u.compile.scope = scope;
+      l->control_u.compile.scope_data = fmtp;
+      execute_control_command_untraced (l);
+    }
+
+  do_cleanups (cleanup);
+}
+
 /* A cleanup function to remove a directory and all its contents.  */
 
 static void
@@ -576,6 +620,14 @@ eval_compile_command (struct command_line *cmd, const char *cmd_string,
       make_cleanup (cleanup_unlink_file, source_file);
       compile_module = compile_object_load (object_file, source_file,
 					    scope, scope_data);
+      if (compile_module == NULL)
+	{
+	  gdb_assert (scope == COMPILE_I_PRINT_ADDRESS_SCOPE);
+	  do_cleanups (cleanup_xfree);
+	  eval_compile_command (cmd, cmd_string,
+				COMPILE_I_PRINT_VALUE_SCOPE, scope_data);
+	  return;
+	}
       discard_cleanups (cleanup_unlink);
       do_cleanups (cleanup_xfree);
       compile_object_run (compile_module);
@@ -637,12 +689,10 @@ The source code may be specified as a simple one line expression, e.g.:\n\
 \n\
     compile code printf(\"Hello world\\n\");\n\
 \n\
-Alternatively, you can type the source code interactively.\n\
-You can invoke this mode when no argument is given to the command\n\
-(i.e.,\"compile code\" is typed with nothing after it).  An\n\
-interactive prompt will be shown allowing you to enter multiple\n\
-lines of source code.  Type a line containing \"end\" to indicate\n\
-the end of the source code."),
+Alternatively, you can type a multiline expression by invoking\n\
+this command with no argument.  GDB will then prompt for the\n\
+expression interactively; type a line containing \"end\" to\n\
+indicate the end of the expression."),
 	   &compile_command_list);
 
   c = add_cmd ("file", class_obscure, compile_file_command,
@@ -654,6 +704,25 @@ Usage: compile file [-r|-raw] [filename]\n\
 	       &compile_command_list);
   set_cmd_completer (c, filename_completer);
 
+  add_cmd ("print", class_obscure, compile_print_command,
+	   _("\
+Evaluate EXPR by using the compiler and print result.\n\
+\n\
+Usage: compile print[/FMT] [EXPR]\n\
+\n\
+The expression may be specified on the same line as the command, e.g.:\n\
+\n\
+    compile print i\n\
+\n\
+Alternatively, you can type a multiline expression by invoking\n\
+this command with no argument.  GDB will then prompt for the\n\
+expression interactively; type a line containing \"end\" to\n\
+indicate the end of the expression.\n\
+\n\
+EXPR may be preceded with /FMT, where FMT is a format letter\n\
+but no count or size letter (see \"x\" command)."),
+	   &compile_command_list);
+
   add_setshow_boolean_cmd ("compile", class_maintenance, &compile_debug, _("\
 Set compile command debugging."), _("\
 Show compile command debugging."), _("\
diff --git a/gdb/compile/compile.h b/gdb/compile/compile.h
index a973167..6e108c7 100644
--- a/gdb/compile/compile.h
+++ b/gdb/compile/compile.h
@@ -101,4 +101,6 @@ extern void compile_dwarf_bounds_to_c (struct ui_file *stream,
 				       const gdb_byte *op_end,
 				       struct dwarf2_per_cu_data *per_cu);
 
+extern void compile_print_value (struct value *val, void *data_voidp);
+
 #endif /* GDB_COMPILE_H */
diff --git a/gdb/defs.h b/gdb/defs.h
index 436f09e..1c6f118 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -70,6 +70,15 @@ enum compile_i_scope_types
     /* Do not wrap the expression,
        it has to provide function "_gdb_expr" on its own.  */
     COMPILE_I_RAW_SCOPE,
+
+    /* A printable expression scope.  Wrap an expression into a scope
+       suitable for the "compile print" command.  It uses the generic
+       function name "_gdb_expr".  COMPILE_I_PRINT_ADDRESS_SCOPE variant
+       is the usual one, taking address of the object.
+       COMPILE_I_PRINT_VALUE_SCOPE is needed for arrays where the array
+       name already specifies its address.  */
+    COMPILE_I_PRINT_ADDRESS_SCOPE,
+    COMPILE_I_PRINT_VALUE_SCOPE,
   };
 
 /* Just in case they're not defined in stdio.h.  */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 6255c14..65357ae 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17216,6 +17216,25 @@ compile file /home/user/example.c
 @end smallexample
 @end table
 
+@table @code
+@item compile print @var{expr}
+@itemx compile print /@var{f} @var{expr}
+Compile and execute @var{expr} with the compiler language found as the
+current language in @value{GDBN} (@pxref{Languages}).  By default the
+value of @var{expr} is printed in a format appropriate to its data type;
+you can choose a different format by specifying @samp{/@var{f}}, where
+@var{f} is a letter specifying the format; see @ref{Output Formats,,Output
+Formats}.
+
+@item compile print
+@itemx compile print /@var{f}
+@cindex reprint the last value
+Alternatively you can enter the expression (source code producing it) as
+multiple lines of text.  To enter this mode, invoke the @samp{compile print}
+command without any text following the command.  This will start the
+multiple-line editor.
+@end table
+
 @noindent
 The process of compiling and injecting the code can be inspected using:
 
diff --git a/gdb/testsuite/gdb.compile/compile-print.c b/gdb/testsuite/gdb.compile/compile-print.c
new file mode 100644
index 0000000..e83b620
--- /dev/null
+++ b/gdb/testsuite/gdb.compile/compile-print.c
@@ -0,0 +1,32 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+
+int varint = 10;
+int vararray[] = { 1, 2, 3, 4, 5 };
+int *vararrayp = vararray;
+struct object
+{
+  int field;
+} varobject = { 1 };
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.compile/compile-print.exp b/gdb/testsuite/gdb.compile/compile-print.exp
new file mode 100644
index 0000000..92c6240
--- /dev/null
+++ b/gdb/testsuite/gdb.compile/compile-print.exp
@@ -0,0 +1,58 @@
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+if { [prepare_for_testing ${testfile}.exp "$testfile"] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+if {[skip_compile_feature_tests]} {
+    untested "compile command not supported (could not find libcc1 shared library?)"
+    return -1
+}
+
+gdb_test "compile print varint" " = 10"
+gdb_test "compile print vararray" " = \\{1, 2, 3, 4, 5\\}"
+gdb_test "compile print main" " = \\{int \\(void\\)\\} 0x\[0-9a-f\]+"
+
+set test "compile print *vararray@3"
+gdb_test_multiple $test $test {
+    -re " = \\{1, 2, 3\\}\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re ": error: stray '@' in program\r\n.*\r\n$gdb_prompt $" {
+	xfail "$test (gcc does not support '@')"
+    }
+}
+
+set test "compile print *vararrayp@3"
+gdb_test_multiple $test $test {
+    -re " = \\{1, 2, 3\\}\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re ": error: stray '@' in program\r\n.*\r\n$gdb_prompt $" {
+	xfail "$test (gcc does not support '@')"
+    }
+}
+
+gdb_test "compile print/x 256" " = 0x100"
+gdb_test {print $} " = 256"
+
+gdb_test "compile print varobject" { = {field = 1}}

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

* Re: [PATCH v3 9/9] compile: compile printf: gdbserver support
  2015-04-11 19:44 ` [PATCH v3 9/9] compile: compile printf: gdbserver support Jan Kratochvil
@ 2015-04-26  9:33   ` Jan Kratochvil
  2015-04-29 18:19     ` Pedro Alves
  2015-04-29 16:12   ` Pedro Alves
  1 sibling, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-04-26  9:33 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

On Sat, 11 Apr 2015 21:44:37 +0200, Jan Kratochvil wrote:
> former patch injects plain:
> 	printf (...);
> This patch injects gdbserver-compatible:
> 	f = open_memstream (&s, ...);
> 	fprintf (f, ...);
> 	fclose (f);
> 	return s;

I have realized this print+printf patchset introduces calling inferior
implicit malloc() + explicit free() (by free_inferior_memory) which the
original 'compile code' series avoided (using gdbarch_infcall_mmap() instead).
The goal was not to crash the inferior futher with print commands when
analyzing corrupted inferior memory lists.

I somehow expected that printf()/fprintf() are so heavyweight they will call
malloc() on their own so this mmap goal is no longer achievable for printf.
But I have found now glibc in most real world cases uses just alloca().

The problem is even calling fmemopen() instead of open_memstream() still
implicitly calls malloc() - for fmemopen_cookie_t and for FILE.

The only idea I have is to redirect by a breakpoint glibc's implicit calls to
malloc() into GDB's allocator by inferior mmap.  But that seems a bit ugly.

So currently keeping it as a known bug.


Jan

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

* Re: [PATCH v3 1/9] Code cleanup: Make parts of print_command_1 public
  2015-04-11 19:43 ` [PATCH v3 1/9] Code cleanup: Make parts of print_command_1 public Jan Kratochvil
@ 2015-04-29 15:44   ` Pedro Alves
  2015-04-30  0:24     ` Jan Kratochvil
  0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2015-04-29 15:44 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

On 04/11/2015 08:43 PM, Jan Kratochvil wrote:

> +/* Parse print command format string and update *EXPP, return it allocated,
> +   caller has to call xfree for it.  Return NULL if no format string has been
> +   found.  CMDNAME should name the current command.  */
> +
> +struct format_data *
> +print_command_parse_format (const char **expp, const char *cmdname)
> +{

I read the series, and AFAICS, this will be used by
compile_print_command in patch #7.   But then AFAICS, compile_print_command
leaks fmtp.

I think this all ends up simpler if it follows the pattern that
the current code already follows.  That is, instead of having
print_command_parse_format "struct format_data" on the heap,
make the function take an output format parameter:

void
print_command_parse_format (const char **expp, const char *cmdname,
                            struct format_data *fmt)
{


> +  struct format_data *fmtp;
> +  const char *exp = *expp;
> +  struct cleanup *cleanup;
> +
> +  if (exp == NULL || *exp != '/')
> +    return NULL;

and in this case set the defaults in the passed in fmt, like the
original code does:

> -  if (exp && *exp == '/')
> -    {
...
> -    }
> -  else
> -    {
> -      fmt.count = 1;
> -      fmt.format = 0;
> -      fmt.size = 0;
> -      fmt.raw = 0;
> -    }

(the else branch)

> +  exp++;
> +
> +  fmtp = xmalloc (sizeof (*fmtp));
> +  cleanup = make_cleanup (xfree, fmtp);
> +  *fmtp = decode_format (&exp, last_format, 0);
> +  validate_format (*fmtp, cmdname);
> +  last_format = fmtp->format;
> +
> +  discard_cleanups (cleanup);

... and then no need for the xmalloc and cleanups...

> +  *expp = exp;
> +  return fmtp;
> +}
> +
> +
> +/* Print VAL to console, including recording it to the history.  */
> +
> +void
> +print_value (struct value *val, const struct format_data *fmtp)
> +{
> +  struct value_print_options opts;
> +  int histindex = record_latest_value (val);
> +
> +  annotate_value_history_begin (histindex, value_type (val));
> +
> +  printf_filtered ("$%d = ", histindex);
> +
> +  annotate_value_history_value ();
> +
> +  get_formatted_print_options (&opts, (fmtp == NULL ? 0 : fmtp->format));
> +  opts.raw = (fmtp == NULL ? 0 : fmtp->raw);
> +
> +  print_formatted (val, (fmtp == NULL ? 0 : fmtp->size), &opts, gdb_stdout);

... and no need for the NULL checks here...

> +  printf_filtered ("\n");
> +
> +  annotate_value_history_end ();
> +}
> +
>  /* Evaluate string EXP as an expression in the current language and
>     print the resulting value.  EXP may contain a format specifier as the
>     first argument ("/x myvar" for example, to print myvar in hex).  */
> @@ -947,25 +997,9 @@ static void
>  print_command_1 (const char *exp, int voidprint)
>  {
>    struct expression *expr;
> -  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
> -  char format = 0;
>    struct value *val;
> -  struct format_data fmt;

... and this 'fmt' variable stays on the stack ...

> -
> -  if (exp && *exp == '/')
> -    {
> -      exp++;
> -      fmt = decode_format (&exp, last_format, 0);
> -      validate_format (fmt, "print");
> -      last_format = format = fmt.format;
> -    }
> -  else
> -    {
> -      fmt.count = 1;
> -      fmt.format = 0;
> -      fmt.size = 0;
> -      fmt.raw = 0;
> -    }
> +  struct format_data *fmtp = print_command_parse_format (&exp, "print");
> +  struct cleanup *old_chain = make_cleanup (xfree, fmtp);

... and no need for this cleanup either.

>  
>    if (exp && *exp)
>      {
> @@ -978,24 +1012,7 @@ print_command_1 (const char *exp, int voidprint)
>  
>    if (voidprint || (val && value_type (val) &&
>  		    TYPE_CODE (value_type (val)) != TYPE_CODE_VOID))
> -    {
> -      struct value_print_options opts;
> -      int histindex = record_latest_value (val);
> -
> -      annotate_value_history_begin (histindex, value_type (val));
> -
> -      printf_filtered ("$%d = ", histindex);
> -
> -      annotate_value_history_value ();
> -
> -      get_formatted_print_options (&opts, format);
> -      opts.raw = fmt.raw;
> -
> -      print_formatted (val, fmt.size, &opts, gdb_stdout);
> -      printf_filtered ("\n");
> -
> -      annotate_value_history_end ();
> -    }
> +    print_value (val, fmtp);
>  
>    do_cleanups (old_chain);
>  }
> diff --git a/gdb/valprint.h b/gdb/valprint.h
> index e3d0137..3ab531f 100644
> --- a/gdb/valprint.h
> +++ b/gdb/valprint.h
> @@ -217,4 +217,9 @@ extern void output_command_const (const char *args, int from_tty);
>  
>  extern int val_print_scalar_type_p (struct type *type);
>  
> +struct format_data;
> +extern struct format_data *print_command_parse_format (const char **expp,
> +						       const char *cmdname);
> +extern void print_value (struct value *val, const struct format_data *fmtp);
> +
>  #endif
> 

Thanks,
Pedro Alves

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

* Re: [PATCH v3 2/9] compile: Distribute scope, add scope_data
  2015-04-11 19:43 ` [PATCH v3 2/9] compile: Distribute scope, add scope_data Jan Kratochvil
@ 2015-04-29 15:44   ` Pedro Alves
  0 siblings, 0 replies; 38+ messages in thread
From: Pedro Alves @ 2015-04-29 15:44 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

On 04/11/2015 08:43 PM, Jan Kratochvil wrote:
> Provide a way to access current 'scope' during struct the do_module_cleanup

"struct" ?

> stage and associate more data with it.
> 

> It should be all sub-classed but AFAIK GDB does not require C++ compiler yet.
> 

Right, it does not.

> gdb/ChangeLog
> 2015-04-06  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* cli/cli-script.c (execute_control_command): Update
> 	eval_compile_command caller.
> 	* compile/compile-object-load.c (compile_object_load): Add parameters
> 	scope and scope_data.  Set them.
> 	* compile/compile-object-load.h (struct compile_module): Add fields
> 	scope and scope_data.
> 	(compile_object_load): Add parameters scope and scope_data.
> 	* compile/compile-object-run.c (struct do_module_cleanup): Add fields
> 	scope and scope_data.
> 	(compile_object_run): Propage the fields scope and scope_data.

Propagate.

> 	* compile/compile.c (compile_file_command, compile_code_command):
> 	Update eval_compile_command callers.
> 	(eval_compile_command): Add parameter scope_data.  Pass it plus scope.
> 	* compile/compile.h (eval_compile_command): Add parameter scope_data.
> 	* defs.h (struct command_line): Add field 


>  
>  extern struct compile_module *compile_object_load (const char *object_file,
> -						   const char *source_file);
> +						   const char *source_file,
> +					       enum compile_i_scope_types scope,
> +						   void *scope_data);

If the line overflows, then break before the (, and indent with two spaces.
See e.g., extension.h:

extern const struct extension_language_defn *get_ext_lang_defn
  (enum extension_language lang);

Otherwise OK.

Thanks,
Pedro Alves

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

* Re: [PATCH v3 3/9] Code cleanup: compile: Constify some parameters
  2015-04-11 19:43 ` [PATCH v3 3/9] Code cleanup: compile: Constify some parameters Jan Kratochvil
@ 2015-04-29 15:47   ` Pedro Alves
  2015-05-06 18:58     ` [commit] " Jan Kratochvil
  0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2015-04-29 15:47 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

On 04/11/2015 08:43 PM, Jan Kratochvil wrote:
> gdb/ChangeLog
> 2015-03-26  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* compile/compile.c (compile_to_object): Make the cmd_string parameter
> 	const.  Use new variables for the const compatibility.
> 	(eval_compile_command): Make the cmd_string parameter const.
> 	* compile/compile.h (eval_compile_command): Make the cmd_string
> 	parameter const.

OK.

Thanks,
Pedro Alves

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

* Re: [PATCH v3 4/9] compile: Support relocation to GNU-IFUNCs
  2015-04-11 19:44 ` [PATCH v3 4/9] compile: Support relocation to GNU-IFUNCs Jan Kratochvil
@ 2015-04-29 15:48   ` Pedro Alves
  2015-05-06 19:00     ` [commit] " Jan Kratochvil
  0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2015-04-29 15:48 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

On 04/11/2015 08:43 PM, Jan Kratochvil wrote:
> The 'compile print' part disclosed that calling memcpy() may fail as memcpy()
> from libc is GNU-IFUNC.
> 
> gdb/ChangeLog
> 2015-04-06  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* compile/compile-object-load.c (compile_object_load): Support
> 	mst_text_gnu_ifunc.

OK.

Thanks,
Pedro Alves

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

* Re: [PATCH v3 5/9] compile: Use -Wall, not -w
  2015-04-11 19:44 ` [PATCH v3 5/9] compile: Use -Wall, not -w Jan Kratochvil
@ 2015-04-29 15:49   ` Pedro Alves
  2015-05-03 14:05     ` Jan Kratochvil
  0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2015-04-29 15:49 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

On 04/11/2015 08:44 PM, Jan Kratochvil wrote:
> Hi,
> 
> for a reason unknown to me GDB was using -w instead of -Wall for 'compile code'.
> The problem is later patch for 'compile printf' really needs some warnings to
> be able to catch for example missing format string parameters:
> 	(gdb) compile printf "%d\n"
> GCC does not seem to be able to cancel -w (there is nothing like -no-w).
> 
> Besides that I think even 'compile code' can benefit from -Wall.
> 
> That #ifndef hack in print_one_macro() is not nice but while GCC does not warn
> for redefinitions like
> 	#define MACRO val
> 	#define MACRO val
> together with the GCC build-in macros I haven't found any other way how to
> prevent the macro-redefinition warnings (when -w is no longer in effect).

I think GCC also knows how to suppress such warnings if the redefinitions
are in system includes.  So I guess GCC already has the smarts
to suppress those. '#pragma GCC system_header' might be close, though it may
be ignored if not done on a header.

Note we have #pragma GCC user_expression, which is a pragma handled by
the GCC plugin:

 static void
 plugin_init_extra_pragmas (void *, void *)
 {
   c_register_pragma ("GCC", "user_expression", plugin_pragma_user_expression);
 }

So if we need to, we can easily add another gdb-specific pragma that
enables whatever mode in gcc we need, and wrap the macros with that.

OTOH, if it's the inferior's version of the macro that is always wanted,
then #ifndef should be fine.  If it's gdb's version that is wanted though,
then that could be handled by an #undef before the #define.

But then again, I'm not exactly sure on what you mean by build-in
macros here.  Can you give an example?

>  
> -gdb_test_no_output "compile code struct_object.selffield = &struct_object"
> +set test "compile code struct_object.selffield = &struct_object"
> +gdb_test_multiple $test $test {
> +    -re "gdb command line:1:25: warning: assignment discards 'volatile' qualifier from pointer target type \\\[-Wdiscarded-qualifiers\\\]\r\n$gdb_prompt $" {
> +	xfail "$test (PR compile/18202)"
> +    }
> +}

Please leave a PASS path in place.  I think this would work:

gdb_test_multiple $test $test {
    -re "^$test\r\n$gdb_prompt $ $" {
	pass "$test"
    }
    -re "gdb command line:1:25: warning: assignment discards 'volatile' qualifier from pointer target type \\\[-Wdiscarded-qualifiers\\\]\r\n$gdb_prompt $" {
	xfail "$test (PR compile/18202)"
    }
}

Other than resolving/clarifying the #ifdef issue, this looks
good to me.

Thanks,
Pedro Alves

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

* Re: [PATCH v3 6/9] Code cleanup: compile: func_addr -> func_sym
  2015-04-11 19:44 ` [PATCH v3 6/9] Code cleanup: compile: func_addr -> func_sym Jan Kratochvil
@ 2015-04-29 15:52   ` Pedro Alves
  0 siblings, 0 replies; 38+ messages in thread
From: Pedro Alves @ 2015-04-29 15:52 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

On 04/11/2015 08:44 PM, Jan Kratochvil wrote:
> Hi,
> 
> currently the code fetches _gdb_expr address/types at multiple places, guessing
> its parameters at multiple places etc.
> 
> Fetch it once, verify it has expected type and then rely on it.
> 

OK.

Thanks,
Pedro Alves

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

* Re: [PATCH v3 7/9] compile: New 'compile print'
  2015-04-11 19:44 ` [PATCH v3 7/9] compile: New 'compile print' Jan Kratochvil
@ 2015-04-29 15:53   ` Pedro Alves
  2015-05-03 14:06     ` Jan Kratochvil
  0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2015-04-29 15:53 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

On 04/11/2015 08:44 PM, Jan Kratochvil wrote:

> +    case COMPILE_I_PRINT_ADDRESS_SCOPE:
> +    case COMPILE_I_PRINT_VALUE_SCOPE:
> +      fputs_unfiltered ("#include <string.h>\n"

OOC, why do we need the include?

> +			"void "
> +			GCC_FE_WRAPPER_FUNCTION
> +			" (struct "
> +			COMPILE_I_SIMPLE_REGISTER_STRUCT_TAG
> +			" *"
> +			COMPILE_I_SIMPLE_REGISTER_ARG_NAME
> +			", "
> +			COMPILE_I_PRINT_OUT_ARG_TYPE
> +			" "
> +			COMPILE_I_PRINT_OUT_ARG
> +			") {\n",
> +			buf);
> +      break;
> +
>      case COMPILE_I_RAW_SCOPE:
>        break;
>      default:

> @@ -354,6 +355,114 @@ copy_sections (bfd *abfd, asection *sect, void *data)
>    do_cleanups (cleanups);
>  }
>  
> +/* Fetch the type of COMPILE_I_EXPR_PTR_TYPE and COMPILE_I_EXPR_VAL
> +   symbols in OBJFILE so we can calculate how much memory to allocate
> +   for the out parameter.  This avoids needing a malloc in the generated
> +   code.  Throw an error if anything fails.
> +   Set *OUT_VALUE_TAKE_ADDRESSP depending whether inferior code should
> +   copy COMPILE_I_EXPR_VAL or its address - this depends on __auto_type
> +   array-to-pointer type conversion of COMPILE_I_EXPR_VAL, as detected
> +   by COMPILE_I_EXPR_PTR_TYPE preserving the array type.  */

This comment seems a bit stale.  At least, I don't see an
OUT_VALUE_TAKE_ADDRESSP parameter.

> +
> +static struct type *
> +get_out_value_type (struct symbol *func_sym, struct objfile *objfile,
> +		    enum compile_i_scope_types scope)
> +{
> +  struct symbol *gdb_ptr_type_sym, *gdb_val_sym;
> +  struct type *gdb_ptr_type, *gdb_type_from_ptr, *gdb_type, *retval;
> +  const struct block *block;
> +  const struct blockvector *bv;
> +  int nblocks = 0;
> +  int block_loop = 0;

Otherwise this looks reasonable to me.

Thanks,
Pedro Alves

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

* Re: [PATCH v3 8/9] compile: New compile printf
  2015-04-11 19:44 ` [PATCH v3 8/9] compile: New compile printf Jan Kratochvil
@ 2015-04-29 15:54   ` Pedro Alves
  2015-05-03 14:06     ` Jan Kratochvil
  0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2015-04-29 15:54 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

On 04/11/2015 08:44 PM, Jan Kratochvil wrote:
> Hi,
> 
> command naming needs to follow what gets decided for 'compile print'.
> 
> This part sends the output to inferior stdout, only the next patch will
> redirect it to GDB (so that for example it works for remote gdbserver).
> 
> It cannot work for core files as one cannot execute inferior code there.
> There were some ideas such as compiling the entered sources into GCC
> intermediate form (GIMPLE?) and interpret it by GDB on top of the core file.

Yeah, though that's a general idea for "compile print" as well, not
just printf.  I'd love to see us get there, but these new commands
are useful on their own as interim steps too.  The usefulness of "compile printf"
specifically isn't as immediately clear though.  I think the manual
should say something about why you want to use "compile printf" over
the alternatives.  (Edit: Ah, I see that's in the next patch.)

The main advantage is that after the next patch, the output always
appears in gdb's console, while "compile code printf" works just like
  (gdb) print printf (...)
meaning, in the "compile the output should go to the inferior's stdout.

Or is there another advantage I missed, perhaps?

> That would be much more complicated, this implementation is made according to
> Phil's specification.
> 
> Besides existing
> 	(gdb) set compile-args
> there will be now also:
> 	(gdb) set compile-printf-args
> Maybe it would be worth to start some set sub-category 'compile' such as:
> 	(gdb) set compile args
> 	(gdb) set compile printf-args

That'd be fine with me.

But can give an example of why you'd want to set "set compile-printf-args"
differently to "set compile-args" ?

> That would mean the whole process of deprecating 'set compile-args' etc.


> +  add_setshow_string_cmd ("compile-printf-args", class_support,
> +			  &compile_printf_args,
> +			  _("Set compile command GCC command-line arguments FIXME"),
> +			  _("Show compile command GCC command-line arguments FIXME"),

Some FIXMEs here.

Overall looks reasonable.

Thanks,
Pedro Alves

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

* Re: [PATCH v3 9/9] compile: compile printf: gdbserver support
  2015-04-11 19:44 ` [PATCH v3 9/9] compile: compile printf: gdbserver support Jan Kratochvil
  2015-04-26  9:33   ` Jan Kratochvil
@ 2015-04-29 16:12   ` Pedro Alves
  1 sibling, 0 replies; 38+ messages in thread
From: Pedro Alves @ 2015-04-29 16:12 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

On 04/11/2015 08:44 PM, Jan Kratochvil wrote:
> Hi,
> 
> former patch injects plain:
> 	printf (...);
> This patch injects gdbserver-compatible:
> 	f = open_memstream (&s, ...);
> 	fprintf (f, ...);
> 	fclose (f);
> 	return s;

(A more expanded explanation would have helped here.  The first time I
skimmed this, I didn't really understand what this meant.)

So the issue here is that calling "printf" in the inferior ends up
with output sent to the inferior's stdout.  If gdbserver is running on a
separate terminal (or machine), then the output of "compile printf" without
this patch goes to the inferior's terminal, unlike using "(gdb) printf ...".

That's not just an issue for gdbserver, actually.  Even with the native target,
using "compile printf" without this patch against an inferior that gdb attached
to, with "attach PID" (a process that was already running on a separate
terminal), or if you use the "set inferior-tty" option, you get the exact
same problem.

> @@ -233,10 +241,15 @@ add_code_footer (enum compile_i_scope_types type, struct ui_file *buf)
>  {
>    switch (type)
>      {
> +    case COMPILE_I_PRINTF_SCOPE:
> +      fputs_unfiltered ("\tif (fclose (__gdb_outf) != 0)\n"
> +			"\t\treturn NULL;\n"
> +			"\treturn __gdb_retval;\n",
> +			buf);
> +      // FALLTHRU

Please use /* */ comments, here and elsewhere.

(replying to the other mail)

Thanks,
Pedro Alves

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

* Re: [PATCH v3 9/9] compile: compile printf: gdbserver support
  2015-04-26  9:33   ` Jan Kratochvil
@ 2015-04-29 18:19     ` Pedro Alves
  2015-05-03 14:06       ` Jan Kratochvil
  0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2015-04-29 18:19 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

On 04/26/2015 10:33 AM, Jan Kratochvil wrote:
> On Sat, 11 Apr 2015 21:44:37 +0200, Jan Kratochvil wrote:
>> former patch injects plain:
>> 	printf (...);
>> This patch injects gdbserver-compatible:
>> 	f = open_memstream (&s, ...);
>> 	fprintf (f, ...);
>> 	fclose (f);
>> 	return s;
> 
> I have realized this print+printf patchset introduces calling inferior
> implicit malloc() + explicit free() (by free_inferior_memory) which the
> original 'compile code' series avoided (using gdbarch_infcall_mmap() instead).
> The goal was not to crash the inferior futher with print commands when
> analyzing corrupted inferior memory lists.

Right.  The "compile code" infrastructure should restrict itself
to async-signal-safe functions for its internal mechanisms for that reason.
Of course, if the expression the user injects runs non-async-signal-safe
at the wrong time, the user gets what she asked for.

> 
> I somehow expected that printf()/fprintf() are so heavyweight they will call
> malloc() on their own so this mmap goal is no longer achievable for printf.
> But I have found now glibc in most real world cases uses just alloca().
> 
> The problem is even calling fmemopen() instead of open_memstream() still
> implicitly calls malloc() - for fmemopen_cookie_t and for FILE.
> 
> The only idea I have is to redirect by a breakpoint glibc's implicit calls to
> malloc() into GDB's allocator by inferior mmap.  But that seems a bit ugly.

Using mmap along with snprintf would be safer, but given that snprintf is
not async-signal-safe in general either, it's fine with me to leave this
as you have it.

I think the manual should say that the command internally may call
functions that are not async-signal-safe though.

> So currently keeping it as a known bug.

Otherwise looks good to me.

Thanks,
Pedro Alves

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

* Re: [PATCH v3 1/9] Code cleanup: Make parts of print_command_1 public
  2015-04-29 15:44   ` Pedro Alves
@ 2015-04-30  0:24     ` Jan Kratochvil
  0 siblings, 0 replies; 38+ messages in thread
From: Jan Kratochvil @ 2015-04-30  0:24 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Wed, 29 Apr 2015 17:43:48 +0200, Pedro Alves wrote:
> I read the series, and AFAICS, this will be used by
> compile_print_command in patch #7.   But then AFAICS, compile_print_command
> leaks fmtp.

Yes, you are right it gets leaked in the end.


> I think this all ends up simpler if it follows the pattern that
> the current code already follows.

It was reworked because struct format_data * becomes a part of struct
do_module_cleanup which can be processed independently in the future.
So the forgotten xfree was inteded+forgotten to be in do_module_cleanup.

OTOH this COMPILE_I_PRINT_ADDRESS_SCOPE and COMPILE_I_PRINT_VALUE_SCOPE
processing in do_module_cleanup is suppressed if the caller no longer exists
(the 'compile print' command already finished).  That means that you are right
do_module_cleanup can depend on a local variable of the compile_print_command
caller.

I have to mention with sub-classing and C++ automatic memory management there
would be nothing left to solve.


Jan

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

* Re: [PATCH v3 5/9] compile: Use -Wall, not -w
  2015-04-29 15:49   ` Pedro Alves
@ 2015-05-03 14:05     ` Jan Kratochvil
  2015-05-06 10:21       ` Pedro Alves
  0 siblings, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-05-03 14:05 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Wed, 29 Apr 2015 17:47:08 +0200, Pedro Alves wrote:
> On 04/11/2015 08:44 PM, Jan Kratochvil wrote:
> I think GCC also knows how to suppress such warnings if the redefinitions
> are in system includes.  So I guess GCC already has the smarts
> to suppress those. '#pragma GCC system_header' might be close, though it may
> be ignored if not done on a header.

Another problem is that (currently) it would not be possible to turn it off in
the same file.  Therefore the user expression without not get any warnings
which would nullify the goal of this patch.


> OTOH, if it's the inferior's version of the macro that is always wanted,
> then #ifndef should be fine.  If it's gdb's version that is wanted though,
> then that could be handled by an #undef before the #define.

Only some GCC's internal macros are affected and I guess it is better to leave
them alone, therefore #ifndef is better (contrary to forced re-#define).


> But then again, I'm not exactly sure on what you mean by build-in
> macros here.  Can you give an example?

Without that #ifndef/#endif one gets:
	compile -r -- void _gdb_expr(){int i = 5;}^M
	/tmp/gdbobj-xpU1yB/out4.c:4:0: warning: "__FILE__" redefined [-Wbuiltin-macro-redefined]^M
	/tmp/gdbobj-xpU1yB/out4.c:5:0: warning: "__LINE__" redefined^M
	/tmp/gdbobj-xpU1yB/out4.c:6:0: warning: "__STDC_IEC_559_COMPLEX__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:46:0: note: this is the location of the previous definition^M
	/tmp/gdbobj-xpU1yB/out4.c:7:0: warning: "__STDC_IEC_559__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:38:0: note: this is the location of the previous definition^M
	/tmp/gdbobj-xpU1yB/out4.c:8:0: warning: "__STDC_ISO_10646__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:54:0: note: this is the location of the previous definition^M
	/tmp/gdbobj-xpU1yB/out4.c:9:0: warning: "__STDC_NO_THREADS__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:57:0: note: this is the location of the previous definition^M
	(gdb) FAIL: gdb.compile/compile.exp: Test delimiter with -r

Without that #ifndef/#endif and with -Wno-builtin-macro-redefined one
expectedly gets mostly the same:
	compile -r -- void _gdb_expr(){int i = 5;}^M
	/tmp/gdbobj-sJlZmG/out4.c:5:0: warning: "__LINE__" redefined^M
	/tmp/gdbobj-sJlZmG/out4.c:6:0: warning: "__STDC_IEC_559_COMPLEX__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:46:0: note: this is the location of the previous definition^M
	/tmp/gdbobj-sJlZmG/out4.c:7:0: warning: "__STDC_IEC_559__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:38:0: note: this is the location of the previous definition^M
	/tmp/gdbobj-sJlZmG/out4.c:8:0: warning: "__STDC_ISO_10646__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:54:0: note: this is the location of the previous definition^M
	/tmp/gdbobj-sJlZmG/out4.c:9:0: warning: "__STDC_NO_THREADS__" redefined^M
	In file included from <command-line>:0:0:^M
	/usr/include/stdc-predef.h:57:0: note: this is the location of the previous definition^M
	(gdb) FAIL: gdb.compile/compile.exp: Test delimiter with -r

Using #undef before the #define one gets:
	compile -r -- void _gdb_expr(){int i = 5;}^M
	/tmp/gdbobj-vCA0XG/out4.c:7:0: warning: undefining "__FILE__" [-Wbuiltin-macro-redefined]^M
	/tmp/gdbobj-vCA0XG/out4.c:9:8: warning: undefining "__LINE__"^M
	/tmp/gdbobj-vCA0XG/out4.c:11:8: warning: undefining "__STDC_IEC_559_COMPLEX__"^M
	/tmp/gdbobj-vCA0XG/out4.c:13:8: warning: undefining "__STDC_IEC_559__"^M
	/tmp/gdbobj-vCA0XG/out4.c:15:8: warning: undefining "__STDC_ISO_10646__"^M
	/tmp/gdbobj-vCA0XG/out4.c:17:8: warning: undefining "__STDC_NO_THREADS__"^M
	(gdb) FAIL: gdb.compile/compile.exp: Test delimiter with -r

In the end the #ifndef+#define is not such a problem I think.


> Please leave a PASS path in place.  I think this would work:

Yes, almost:
> 
> gdb_test_multiple $test $test {
>     -re "^$test\r\n$gdb_prompt $ $" {
      -re "^$test\r\n$gdb_prompt $" {
> 	pass "$test"
>     }


Thanks,
Jan

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

* Re: [PATCH v3 7/9] compile: New 'compile print'
  2015-04-29 15:53   ` Pedro Alves
@ 2015-05-03 14:06     ` Jan Kratochvil
  2015-05-06 10:22       ` Pedro Alves
  0 siblings, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-05-03 14:06 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Wed, 29 Apr 2015 17:49:50 +0200, Pedro Alves wrote:
> On 04/11/2015 08:44 PM, Jan Kratochvil wrote:
> 
> > +    case COMPILE_I_PRINT_ADDRESS_SCOPE:
> > +    case COMPILE_I_PRINT_VALUE_SCOPE:
> > +      fputs_unfiltered ("#include <string.h>\n"
> 
> OOC, why do we need the include?

Added:
+      /* <string.h> is needed for a memcpy call below.  */


> > @@ -354,6 +355,114 @@ copy_sections (bfd *abfd, asection *sect, void *data)
> >    do_cleanups (cleanups);
> >  }
> >  
> > +/* Fetch the type of COMPILE_I_EXPR_PTR_TYPE and COMPILE_I_EXPR_VAL
> > +   symbols in OBJFILE so we can calculate how much memory to allocate
> > +   for the out parameter.  This avoids needing a malloc in the generated
> > +   code.  Throw an error if anything fails.
> > +   Set *OUT_VALUE_TAKE_ADDRESSP depending whether inferior code should
> > +   copy COMPILE_I_EXPR_VAL or its address - this depends on __auto_type
> > +   array-to-pointer type conversion of COMPILE_I_EXPR_VAL, as detected
> > +   by COMPILE_I_EXPR_PTR_TYPE preserving the array type.  */
> 
> This comment seems a bit stale.  At least, I don't see an
> OUT_VALUE_TAKE_ADDRESSP parameter.

OK, yes, updated.

   Function returns NULL only for COMPILE_I_PRINT_ADDRESS_SCOPE when
   COMPILE_I_PRINT_VALUE_SCOPE should have been used instead.  
   This depends on __auto_type array-to-pointer type conversion of
   COMPILE_I_EXPR_VAL, as detected by COMPILE_I_EXPR_PTR_TYPE preserving 
   the array type.  */


Thanks,
Jan

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

* Re: [PATCH v3 8/9] compile: New compile printf
  2015-04-29 15:54   ` Pedro Alves
@ 2015-05-03 14:06     ` Jan Kratochvil
  2015-05-06 10:22       ` Pedro Alves
  0 siblings, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-05-03 14:06 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Wed, 29 Apr 2015 17:52:09 +0200, Pedro Alves wrote:
> The usefulness of "compile printf"
> specifically isn't as immediately clear though.  I think the manual
> should say something about why you want to use "compile printf" over
> the alternatives.  (Edit: Ah, I see that's in the next patch.)

I do not know, I have never used the existing GDB printf command myself.
GDB Manual could describe what the existing GDB printf command is good for.
IMO in the cases where one needs the printf command one already has to use
some extension language (such as Python) which can do that on its own.
This patch was created upon request by Phil.


> The main advantage is that after the next patch, the output always
> appears in gdb's console, while "compile code printf" works just like
>   (gdb) print printf (...)
> meaning, in the "compile the output should go to the inferior's stdout.
> 
> Or is there another advantage I missed, perhaps?

This patch is just to split it to two mails for review.  I do not think it
makes sense on its own, it messes up debugging output with inferior output.


> But can give an example of why you'd want to set "set compile-printf-args"
> differently to "set compile-args" ?

I do not know exactly myself but currently there is already:
	+  compile_printf_args = xstrdup ("-Werror=format");

so one may need to modify that for whatever reason.  I do not think there
should be non-overridable GCC options.


> Some FIXMEs here.

Fixed:
                          _("Set compile printf command "
                            "GCC command-line arguments"),
                          _("Show compile printf command "
                            "GCC command-line arguments"),


Thanks,
Jan

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

* Re: [PATCH v3 9/9] compile: compile printf: gdbserver support
  2015-04-29 18:19     ` Pedro Alves
@ 2015-05-03 14:06       ` Jan Kratochvil
  2015-05-06 10:22         ` Pedro Alves
  0 siblings, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-05-03 14:06 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Wed, 29 Apr 2015 17:54:11 +0200, Pedro Alves wrote:
> On 04/26/2015 10:33 AM, Jan Kratochvil wrote:
> > The only idea I have is to redirect by a breakpoint glibc's implicit calls to
> > malloc() into GDB's allocator by inferior mmap.  But that seems a bit ugly.
> 
> Using mmap along with snprintf would be safer, but given that snprintf is
> not async-signal-safe in general either, it's fine with me to leave this
> as you have it.

OK, snprintf into mmap()ped buffer looks easier.  While snprintf is not
async-signal-safe in general IMO it is async-signal-safe for most format
strings given in almost always uses alloca() instead of malloc().
Or do you realize other problems it may have?

Still I am sure fine to check it in as is if it is approved this way.


> I think the manual should say that the command internally may call
> functions that are not async-signal-safe though.

Done.


Thanks,
Jan

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

* Re: [PATCH v3 5/9] compile: Use -Wall, not -w
  2015-05-03 14:05     ` Jan Kratochvil
@ 2015-05-06 10:21       ` Pedro Alves
  0 siblings, 0 replies; 38+ messages in thread
From: Pedro Alves @ 2015-05-06 10:21 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches, Phil Muldoon

On 05/03/2015 03:05 PM, Jan Kratochvil wrote:

> In the end the #ifndef+#define is not such a problem I think.

OK, let's keep that route then.

>> Please leave a PASS path in place.  I think this would work:
> 
> Yes, almost:
>>
>> gdb_test_multiple $test $test {
>>     -re "^$test\r\n$gdb_prompt $ $" {
>       -re "^$test\r\n$gdb_prompt $" {
>> 	pass "$test"
>>     }

Yeah, that double $ was a typo.

Thanks,
Pedro Alves

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

* Re: [PATCH v3 7/9] compile: New 'compile print'
  2015-05-03 14:06     ` Jan Kratochvil
@ 2015-05-06 10:22       ` Pedro Alves
  2015-05-06 12:23         ` Jan Kratochvil
  0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2015-05-06 10:22 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches, Phil Muldoon

On 05/03/2015 03:05 PM, Jan Kratochvil wrote:
> On Wed, 29 Apr 2015 17:49:50 +0200, Pedro Alves wrote:
>> On 04/11/2015 08:44 PM, Jan Kratochvil wrote:
>>
>>> +    case COMPILE_I_PRINT_ADDRESS_SCOPE:
>>> +    case COMPILE_I_PRINT_VALUE_SCOPE:
>>> +      fputs_unfiltered ("#include <string.h>\n"
>>
>> OOC, why do we need the include?
> 
> Added:
> +      /* <string.h> is needed for a memcpy call below.  */
> 

Thanks.

>>>  
>>> +/* Fetch the type of COMPILE_I_EXPR_PTR_TYPE and COMPILE_I_EXPR_VAL
>>> +   symbols in OBJFILE so we can calculate how much memory to allocate
>>> +   for the out parameter.  This avoids needing a malloc in the generated
>>> +   code.  Throw an error if anything fails.
>>> +   Set *OUT_VALUE_TAKE_ADDRESSP depending whether inferior code should
>>> +   copy COMPILE_I_EXPR_VAL or its address - this depends on __auto_type
>>> +   array-to-pointer type conversion of COMPILE_I_EXPR_VAL, as detected
>>> +   by COMPILE_I_EXPR_PTR_TYPE preserving the array type.  */
>>
>> This comment seems a bit stale.  At least, I don't see an
>> OUT_VALUE_TAKE_ADDRESSP parameter.
> 
> OK, yes, updated.
> 
>    Function returns NULL only for COMPILE_I_PRINT_ADDRESS_SCOPE when
>    COMPILE_I_PRINT_VALUE_SCOPE should have been used instead.  

What does "should have been used instead" mean?  Is that a bug in the
caller?

>    This depends on __auto_type array-to-pointer type conversion of
>    COMPILE_I_EXPR_VAL, as detected by COMPILE_I_EXPR_PTR_TYPE preserving 
>    the array type.  */

Thanks,
Pedro Alves

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

* Re: [PATCH v3 9/9] compile: compile printf: gdbserver support
  2015-05-03 14:06       ` Jan Kratochvil
@ 2015-05-06 10:22         ` Pedro Alves
  0 siblings, 0 replies; 38+ messages in thread
From: Pedro Alves @ 2015-05-06 10:22 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches, Phil Muldoon

On 05/03/2015 03:06 PM, Jan Kratochvil wrote:
> On Wed, 29 Apr 2015 17:54:11 +0200, Pedro Alves wrote:
>> On 04/26/2015 10:33 AM, Jan Kratochvil wrote:
>>> The only idea I have is to redirect by a breakpoint glibc's implicit calls to
>>> malloc() into GDB's allocator by inferior mmap.  But that seems a bit ugly.
>>
>> Using mmap along with snprintf would be safer, but given that snprintf is
>> not async-signal-safe in general either, it's fine with me to leave this
>> as you have it.
> 
> OK, snprintf into mmap()ped buffer looks easier.  While snprintf is not
> async-signal-safe in general IMO it is async-signal-safe for most format
> strings given in almost always uses alloca() instead of malloc().

Right.

> Or do you realize other problems it may have?

Nope.

Thanks,
Pedro Alves

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

* Re: [PATCH v3 8/9] compile: New compile printf
  2015-05-03 14:06     ` Jan Kratochvil
@ 2015-05-06 10:22       ` Pedro Alves
  2015-05-06 11:30         ` Jan Kratochvil
  0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2015-05-06 10:22 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches, Phil Muldoon

On 05/03/2015 03:06 PM, Jan Kratochvil wrote:
> On Wed, 29 Apr 2015 17:52:09 +0200, Pedro Alves wrote:
>> The usefulness of "compile printf"
>> specifically isn't as immediately clear though.  I think the manual
>> should say something about why you want to use "compile printf" over
>> the alternatives.  (Edit: Ah, I see that's in the next patch.)
> 
> I do not know, I have never used the existing GDB printf command myself.
> GDB Manual could describe what the existing GDB printf command is good for.

It's useful for formatted output in user-defined commands, without
using extension languages.  But indeed it's not clear what
"compile printf" in its current form is really useful for.

> IMO in the cases where one needs the printf command one already has to use
> some extension language (such as Python) which can do that on its own.

(The printf command predates extensions languages.)

> This patch was created upon request by Phil.

I'm wondering / trying to understand why we're making
"compile printf" do the printf-ing in the inferior.  It seems we may
be making our lives harder for possibly no good reason.

Consider where we'll ideally be in the future:

 #0 - by default, the compiler outputs intermediate IR which is
      interpreted by gdb, instead of injecting and calling code
      in the inferior.

 #1 - all expression evaluation goes through the compiler.

 #2 - the user no longer needs to know to use "compile "
      prefixed commands.  Instead, "print" etc. just use the compiler
      when possible, or when the user asks for it with some option.

With these in mind, maybe a better direction is to make

  (gdb) compile printf "%s, %d", expr1, expr2

instead evaluate expr1 and expr2 using the "compile print"
mechanism to get the values of expr1 and expr2, and
then do the printf formatting all on the gdb side, just
like "(gdb) printf".  Basically, in the current
sequence for "printf":

  printf_command -> ui_printf -> parse_to_comma_and_eval

Make parse_to_comma_and_eval eval using the compiler.

This avoids all the complication related to calling printf
in the inferior, which would necessarily behave differently
with #0 above (gdb's printf vs inferior's printf).

> 
>> The main advantage is that after the next patch, the output always
>> appears in gdb's console, while "compile code printf" works just like
>>   (gdb) print printf (...)
>> meaning, in the "compile the output should go to the inferior's stdout.
>>
>> Or is there another advantage I missed, perhaps?
> 
> This patch is just to split it to two mails for review. 

I understand that.  But what I was asking is (after the series is wholly
pushed), what is the advantage of "(gdb) compile printf"
over "(gdb) compile print printf (...)" and "(gdb) call printf (...)".

> I do not think it
> makes sense on its own, it messes up debugging output with inferior output.
> 
> 
>> But can give an example of why you'd want to set "set compile-printf-args"
>> differently to "set compile-args" ?
> 
> I do not know exactly myself but currently there is already:
> 	+  compile_printf_args = xstrdup ("-Werror=format");
> 
> so one may need to modify that for whatever reason.  I do not think there
> should be non-overridable GCC options.

Agreed on the latter, but the question really is: why do we need
"set compile-printf-args" instead of using "set compile-args" for
all expression evaluation through the compiler?
Shouldn't "-Werror=format" be in "set compile-args" too?
The fewer knobs the user must learn the better, and I'd like to
avoid ending up with a bunch of "set compile-foo-args" knobs
if possible.

Thanks,
Pedro Alves

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

* Re: [PATCH v3 8/9] compile: New compile printf
  2015-05-06 10:22       ` Pedro Alves
@ 2015-05-06 11:30         ` Jan Kratochvil
  2015-05-06 11:47           ` Pedro Alves
  0 siblings, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-05-06 11:30 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Wed, 06 May 2015 12:22:41 +0200, Pedro Alves wrote:
> I understand that.  But what I was asking is (after the series is wholly
> pushed), what is the advantage of "(gdb) compile printf"
> over "(gdb) compile print printf (...)" and "(gdb) call printf (...)".

This patch, that is
	[PATCH v3 8/9] compile: New compile printf
without the part
	[PATCH v3 9/9] compile: compile printf: gdbserver support
is really just that
	(gdb) compile print printf (...)
and the patch is also therefore very simple.

According to Phil - roughly, not citing - such 'compile printf' was simple
enough to code to make it worth such a feature, despite it has many
shortcomings.


> Agreed on the latter, but the question really is: why do we need
> "set compile-printf-args" instead of using "set compile-args" for
> all expression evaluation through the compiler?
> Shouldn't "-Werror=format" be in "set compile-args" too?

Why not, this is a matter of opinion.  IMO cc itself should have -Werror by
default as otherwise by default it is willing to knowingly produce crashing
programs.  The only safe warnings are -Wunused* ones and maybe few others.
So again, this patch tries to make minimal changes to what is the current
established wrong standard.


Jan

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

* Re: [PATCH v3 8/9] compile: New compile printf
  2015-05-06 11:30         ` Jan Kratochvil
@ 2015-05-06 11:47           ` Pedro Alves
  0 siblings, 0 replies; 38+ messages in thread
From: Pedro Alves @ 2015-05-06 11:47 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches, Phil Muldoon

On 05/06/2015 12:29 PM, Jan Kratochvil wrote:
> On Wed, 06 May 2015 12:22:41 +0200, Pedro Alves wrote:
>> I understand that.  But what I was asking is (after the series is wholly
>> pushed), what is the advantage of "(gdb) compile printf"
>> over "(gdb) compile print printf (...)" and "(gdb) call printf (...)".
> 
> This patch, that is
> 	[PATCH v3 8/9] compile: New compile printf
> without the part
> 	[PATCH v3 9/9] compile: compile printf: gdbserver support
> is really just that
> 	(gdb) compile print printf (...)
> and the patch is also therefore very simple.
> 
> According to Phil - roughly, not citing - such 'compile printf' was simple
> enough to code to make it worth such a feature, despite it has many
> shortcomings.

OK, since we don't have a real use case of calling the
inferior's printf (other than because it's seemingly simple), I'd
rather we avoid it and do as outlined in the previous email.

>> Agreed on the latter, but the question really is: why do we need
>> "set compile-printf-args" instead of using "set compile-args" for
>> all expression evaluation through the compiler?
>> Shouldn't "-Werror=format" be in "set compile-args" too?
> 
> Why not, this is a matter of opinion.  IMO cc itself should have -Werror by
> default as otherwise by default it is willing to knowingly produce crashing
> programs.  The only safe warnings are -Wunused* ones and maybe few others.
> So again, this patch tries to make minimal changes to what is the current
> established wrong standard.

The minimal change then is putting -Werror=format in "set compile-args".
Sounds like you're OK with that; let's do it.

Thanks,
Pedro Alves

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

* Re: [PATCH v3 7/9] compile: New 'compile print'
  2015-05-06 10:22       ` Pedro Alves
@ 2015-05-06 12:23         ` Jan Kratochvil
  2015-05-06 14:11           ` Pedro Alves
  0 siblings, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-05-06 12:23 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Wed, 06 May 2015 12:22:01 +0200, Pedro Alves wrote:
> On 05/03/2015 03:05 PM, Jan Kratochvil wrote:
> >    Function returns NULL only for COMPILE_I_PRINT_ADDRESS_SCOPE when
> >    COMPILE_I_PRINT_VALUE_SCOPE should have been used instead.  
> 
> What does "should have been used instead" mean?  Is that a bug in the
> caller?

Currently GDB has to decide whether it should compile with GCC
	memcpy (buffer, &variable, ...)
or
	memcpy (buffer, variable, ...)
depending on whether 'variable' is scalar (first) or an array (second).

Currently GDB can figure it out only from DWARF, after compiling it first.

This is all a hack how to get it working for live targets without implementing
the compiler IR (intermediate representation) interpreter in GDB (making
'compile' commands compatible with core files). So far AFAIK C++ live
functionality has been a top priority, not the IR interpreter.  This
COMPILE_I_PRINT_ADDRESS_SCOPE-or-COMPILE_I_PRINT_VALUE_SCOPE conditional would
be some simple runtime conditional in the IR interpreter instead.

If an implementation on top of IR interpreter is required for these patches
then this whole patch series should be dropped and we need to start to work on
the IR interpreter instead.


Jan

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

* Re: [PATCH v3 7/9] compile: New 'compile print'
  2015-05-06 12:23         ` Jan Kratochvil
@ 2015-05-06 14:11           ` Pedro Alves
  2015-05-06 19:18             ` Jan Kratochvil
  0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2015-05-06 14:11 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches, Phil Muldoon

On 05/06/2015 01:23 PM, Jan Kratochvil wrote:
> On Wed, 06 May 2015 12:22:01 +0200, Pedro Alves wrote:
>> On 05/03/2015 03:05 PM, Jan Kratochvil wrote:
>>>    Function returns NULL only for COMPILE_I_PRINT_ADDRESS_SCOPE when
>>>    COMPILE_I_PRINT_VALUE_SCOPE should have been used instead.  
>>
>> What does "should have been used instead" mean?  Is that a bug in the
>> caller?
> 
> Currently GDB has to decide whether it should compile with GCC
> 	memcpy (buffer, &variable, ...)
> or
> 	memcpy (buffer, variable, ...)
> depending on whether 'variable' is scalar (first) or an array (second).
> 
> Currently GDB can figure it out only from DWARF, after compiling it first.
> 
> This is all a hack how to get it working for live targets without implementing
> the compiler IR (intermediate representation) interpreter in GDB (making
> 'compile' commands compatible with core files). So far AFAIK C++ live
> functionality has been a top priority, not the IR interpreter.  This
> COMPILE_I_PRINT_ADDRESS_SCOPE-or-COMPILE_I_PRINT_VALUE_SCOPE conditional would
> be some simple runtime conditional in the IR interpreter instead.
> 
> If an implementation on top of IR interpreter is required for these patches
> then this whole patch series should be dropped and we need to start to work on
> the IR interpreter instead.

No, IR interpreter is certainly not a requirement.

But I think the comment that explains the current implementation should
be clear.  As is, it's just that I still don't understand what you mean by:

>    Function returns NULL only for COMPILE_I_PRINT_ADDRESS_SCOPE when
>    COMPILE_I_PRINT_VALUE_SCOPE should have been used instead.

because reading this one wonders: "OK, if COMPILE_I_PRINT_VALUE_SCOPE should
have been used, why wasn't it used then?  Is that a bug in the caller?"

Thanks,
Pedro Alves

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

* [commit] [PATCH v3 3/9] Code cleanup: compile: Constify some parameters
  2015-04-29 15:47   ` Pedro Alves
@ 2015-05-06 18:58     ` Jan Kratochvil
  0 siblings, 0 replies; 38+ messages in thread
From: Jan Kratochvil @ 2015-05-06 18:58 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Wed, 29 Apr 2015 17:44:12 +0200, Pedro Alves wrote:
> On 04/11/2015 08:43 PM, Jan Kratochvil wrote:
> > gdb/ChangeLog
> > 2015-03-26  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	* compile/compile.c (compile_to_object): Make the cmd_string parameter
> > 	const.  Use new variables for the const compatibility.
> > 	(eval_compile_command): Make the cmd_string parameter const.
> > 	* compile/compile.h (eval_compile_command): Make the cmd_string
> > 	parameter const.
> 
> OK.

Checked in out of the series:
	851c90917fff745b94e29c1fec4f3d00ca36f598


Jan

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

* [commit] [PATCH v3 4/9] compile: Support relocation to GNU-IFUNCs
  2015-04-29 15:48   ` Pedro Alves
@ 2015-05-06 19:00     ` Jan Kratochvil
  0 siblings, 0 replies; 38+ messages in thread
From: Jan Kratochvil @ 2015-05-06 19:00 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Wed, 29 Apr 2015 17:44:26 +0200, Pedro Alves wrote:
> On 04/11/2015 08:43 PM, Jan Kratochvil wrote:
> > The 'compile print' part disclosed that calling memcpy() may fail as memcpy()
> > from libc is GNU-IFUNC.
> > 
> > gdb/ChangeLog
> > 2015-04-06  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	* compile/compile-object-load.c (compile_object_load): Support
> > 	mst_text_gnu_ifunc.
> 
> OK.

Checked in out of the series:
	e26efa4066a5076f59427c927cc51c52b6b92f35


Jan

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

* Re: [PATCH v3 7/9] compile: New 'compile print'
  2015-05-06 14:11           ` Pedro Alves
@ 2015-05-06 19:18             ` Jan Kratochvil
  2015-05-15 16:35               ` Pedro Alves
  0 siblings, 1 reply; 38+ messages in thread
From: Jan Kratochvil @ 2015-05-06 19:18 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Wed, 06 May 2015 16:10:59 +0200, Pedro Alves wrote:
> On 05/06/2015 01:23 PM, Jan Kratochvil wrote:
> > This is all a hack how to get it working for live targets without implementing
> > the compiler IR (intermediate representation) interpreter in GDB (making
> > 'compile' commands compatible with core files). So far AFAIK C++ live
> > functionality has been a top priority, not the IR interpreter.  This
> > COMPILE_I_PRINT_ADDRESS_SCOPE-or-COMPILE_I_PRINT_VALUE_SCOPE conditional would
> > be some simple runtime conditional in the IR interpreter instead.
> > 
> > If an implementation on top of IR interpreter is required for these patches
> > then this whole patch series should be dropped and we need to start to work on
> > the IR interpreter instead.
> 
> No, IR interpreter is certainly not a requirement.

IIUC in this case the hack has an acceptable scale (contrary to that printf).


> But I think the comment that explains the current implementation should
> be clear.  As is, it's just that I still don't understand what you mean by:
> 
> >    Function returns NULL only for COMPILE_I_PRINT_ADDRESS_SCOPE when
> >    COMPILE_I_PRINT_VALUE_SCOPE should have been used instead.
> 
> because reading this one wonders: "OK, if COMPILE_I_PRINT_VALUE_SCOPE should
> have been used, why wasn't it used then?  Is that a bug in the caller?"

I have put there now:
   GDB first tries to compile the code with COMPILE_I_PRINT_ADDRESS_SCOPE.
   If it finds user tries to print an array type this function returns
   NULL.  Caller will then regenerate the code with
   COMPILE_I_PRINT_VALUE_SCOPE, recompiles it again and finally runs it.
   This is because __auto_type array-to-pointer type conversion of
   COMPILE_I_EXPR_VAL which gets detected by COMPILE_I_EXPR_PTR_TYPE
   preserving the array type.  */

Plus at:
enum compile_i_scope_types
       name already specifies its address.  See get_out_value_type.  */
    COMPILE_I_PRINT_ADDRESS_SCOPE,
    COMPILE_I_PRINT_VALUE_SCOPE, 


Jan

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

* Re: [PATCH v3 7/9] compile: New 'compile print'
  2015-05-06 19:18             ` Jan Kratochvil
@ 2015-05-15 16:35               ` Pedro Alves
  0 siblings, 0 replies; 38+ messages in thread
From: Pedro Alves @ 2015-05-15 16:35 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches, Phil Muldoon

On 05/06/2015 08:18 PM, Jan Kratochvil wrote:
> On Wed, 06 May 2015 16:10:59 +0200, Pedro Alves wrote:
>> On 05/06/2015 01:23 PM, Jan Kratochvil wrote:
>>> This is all a hack how to get it working for live targets without implementing
>>> the compiler IR (intermediate representation) interpreter in GDB (making
>>> 'compile' commands compatible with core files). So far AFAIK C++ live
>>> functionality has been a top priority, not the IR interpreter.  This
>>> COMPILE_I_PRINT_ADDRESS_SCOPE-or-COMPILE_I_PRINT_VALUE_SCOPE conditional would
>>> be some simple runtime conditional in the IR interpreter instead.
>>>
>>> If an implementation on top of IR interpreter is required for these patches
>>> then this whole patch series should be dropped and we need to start to work on
>>> the IR interpreter instead.
>>
>> No, IR interpreter is certainly not a requirement.
> 
> IIUC in this case the hack has an acceptable scale (contrary to that printf).
> 
> 
>> But I think the comment that explains the current implementation should
>> be clear.  As is, it's just that I still don't understand what you mean by:
>>
>>>    Function returns NULL only for COMPILE_I_PRINT_ADDRESS_SCOPE when
>>>    COMPILE_I_PRINT_VALUE_SCOPE should have been used instead.
>>
>> because reading this one wonders: "OK, if COMPILE_I_PRINT_VALUE_SCOPE should
>> have been used, why wasn't it used then?  Is that a bug in the caller?"
> 
> I have put there now:
>    GDB first tries to compile the code with COMPILE_I_PRINT_ADDRESS_SCOPE.
>    If it finds user tries to print an array type this function returns
>    NULL.  Caller will then regenerate the code with
>    COMPILE_I_PRINT_VALUE_SCOPE, recompiles it again and finally runs it.
>    This is because __auto_type array-to-pointer type conversion of
>    COMPILE_I_EXPR_VAL which gets detected by COMPILE_I_EXPR_PTR_TYPE
>    preserving the array type.  */
> 

Aaaaaaaaah.  I see now.  Thanks, that's _so_ much better.

Yes, this hack is fine with me.  We have bigger fish to fry.

Let me go take a look at the new series you posted.

-- 
Pedro Alves

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

end of thread, other threads:[~2015-05-15 16:35 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-11 19:43 [PATCH v3 0/9] compile: compile print&printf Jan Kratochvil
2015-04-11 19:43 ` [PATCH v3 1/9] Code cleanup: Make parts of print_command_1 public Jan Kratochvil
2015-04-29 15:44   ` Pedro Alves
2015-04-30  0:24     ` Jan Kratochvil
2015-04-11 19:43 ` [PATCH v3 3/9] Code cleanup: compile: Constify some parameters Jan Kratochvil
2015-04-29 15:47   ` Pedro Alves
2015-05-06 18:58     ` [commit] " Jan Kratochvil
2015-04-11 19:43 ` [PATCH v3 2/9] compile: Distribute scope, add scope_data Jan Kratochvil
2015-04-29 15:44   ` Pedro Alves
2015-04-11 19:44 ` [PATCH v3 6/9] Code cleanup: compile: func_addr -> func_sym Jan Kratochvil
2015-04-29 15:52   ` Pedro Alves
2015-04-11 19:44 ` [PATCH v3 9/9] compile: compile printf: gdbserver support Jan Kratochvil
2015-04-26  9:33   ` Jan Kratochvil
2015-04-29 18:19     ` Pedro Alves
2015-05-03 14:06       ` Jan Kratochvil
2015-05-06 10:22         ` Pedro Alves
2015-04-29 16:12   ` Pedro Alves
2015-04-11 19:44 ` [PATCH v3 8/9] compile: New compile printf Jan Kratochvil
2015-04-29 15:54   ` Pedro Alves
2015-05-03 14:06     ` Jan Kratochvil
2015-05-06 10:22       ` Pedro Alves
2015-05-06 11:30         ` Jan Kratochvil
2015-05-06 11:47           ` Pedro Alves
2015-04-11 19:44 ` [PATCH v3 4/9] compile: Support relocation to GNU-IFUNCs Jan Kratochvil
2015-04-29 15:48   ` Pedro Alves
2015-05-06 19:00     ` [commit] " Jan Kratochvil
2015-04-11 19:44 ` [PATCH v3 5/9] compile: Use -Wall, not -w Jan Kratochvil
2015-04-29 15:49   ` Pedro Alves
2015-05-03 14:05     ` Jan Kratochvil
2015-05-06 10:21       ` Pedro Alves
2015-04-11 19:44 ` [PATCH v3 7/9] compile: New 'compile print' Jan Kratochvil
2015-04-29 15:53   ` Pedro Alves
2015-05-03 14:06     ` Jan Kratochvil
2015-05-06 10:22       ` Pedro Alves
2015-05-06 12:23         ` Jan Kratochvil
2015-05-06 14:11           ` Pedro Alves
2015-05-06 19:18             ` Jan Kratochvil
2015-05-15 16:35               ` Pedro Alves

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