public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v5 1/7] compile: Add one debug message
  2015-05-13 20:16 [PATCH v5 0/7] compile: compile print (&unmap) Jan Kratochvil
  2015-05-13 20:16 ` [PATCH v5 2/7] Code cleanup: Make parts of print_command_1 public Jan Kratochvil
@ 2015-05-13 20:16 ` Jan Kratochvil
  2015-05-15 16:49   ` Pedro Alves
  2015-05-13 20:16 ` [PATCH v5 4/7] compile: Use -Wall, not -w Jan Kratochvil
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-13 20:16 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

Hi,

trivial, there is an inter-patch dependency.


Jan

2015-04-28  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* compile/compile-object-load.c (compile_object_load): Add
	COMPILE_DEBUG message.
---
 gdb/compile/compile-object-load.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index fe23448..745d787 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -580,6 +580,12 @@ compile_object_load (const char *object_file, const char *source_file)
 					TYPE_LENGTH (regs_type),
 					GDB_MMAP_PROT_READ);
       gdb_assert (regs_addr != 0);
+      if (compile_debug)
+	fprintf_unfiltered (gdb_stdout,
+			    "allocated %s bytes at %s for registers\n",
+			    paddress (target_gdbarch (),
+				      TYPE_LENGTH (regs_type)),
+			    paddress (target_gdbarch (), regs_addr));
       store_regs (regs_type, regs_addr);
     }
 

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

* [PATCH v5 4/7] compile: Use -Wall, not -w
  2015-05-13 20:16 [PATCH v5 0/7] compile: compile print (&unmap) Jan Kratochvil
  2015-05-13 20:16 ` [PATCH v5 2/7] Code cleanup: Make parts of print_command_1 public Jan Kratochvil
  2015-05-13 20:16 ` [PATCH v5 1/7] compile: Add one debug message Jan Kratochvil
@ 2015-05-13 20:16 ` Jan Kratochvil
  2015-05-15 16:49   ` Pedro Alves
  2015-05-13 20:16 ` [PATCH v5 3/7] compile: Distribute scope, add scope_data Jan Kratochvil
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-13 20:16 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.
---
 gdb/compile/compile-c-support.c           |   12 +++++++-----
 gdb/compile/compile-loc2c.c               |   22 +++++++++++++---------
 gdb/compile/compile.c                     |    7 +++++--
 gdb/testsuite/gdb.compile/compile-ops.exp |    2 +-
 gdb/testsuite/gdb.compile/compile.exp     |   18 +++++++++++++-----
 5 files changed, 39 insertions(+), 22 deletions(-)

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..07276bd 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,15 @@ 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 "^$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)"
+    }
+}
 gdb_test "print struct_object.selffield == &struct_object" " = 1"
 
 gdb_test_no_output "compile code struct_object.charfield = 1"
@@ -261,7 +269,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] 25+ messages in thread

* [PATCH v5 2/7] Code cleanup: Make parts of print_command_1 public
  2015-05-13 20:16 [PATCH v5 0/7] compile: compile print (&unmap) Jan Kratochvil
@ 2015-05-13 20:16 ` Jan Kratochvil
  2015-05-16 12:27   ` [commit] " Jan Kratochvil
  2015-05-13 20:16 ` [PATCH v5 1/7] compile: Add one debug message Jan Kratochvil
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-13 20:16 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-05-03  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* printcmd.c (struct format_data): Move it to valprint.h.
	(print_command_parse_format, print_value): New functions from ...
	(print_command_1): ... here.  Call them.
	* valprint.h (struct format_data): Move it here from printcmd.c.
	(print_command_parse_format, print_value): New declarations.
---
 gdb/printcmd.c |   97 +++++++++++++++++++++++++++++++-------------------------
 gdb/valprint.h |   15 +++++++++
 2 files changed, 68 insertions(+), 44 deletions(-)

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index deb501a..0ae402f 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -52,17 +52,6 @@
 #include "tui/tui.h"		/* For tui_active et al.   */
 #endif
 
-struct format_data
-  {
-    int count;
-    char format;
-    char size;
-
-    /* True if the value should be printed raw -- that is, bypassing
-       python-based formatters.  */
-    unsigned char raw;
-  };
-
 /* Last specified output format.  */
 
 static char last_format = 0;
@@ -939,6 +928,57 @@ validate_format (struct format_data fmt, const char *cmdname)
 	   fmt.format, cmdname);
 }
 
+/* Parse print command format string into *FMTP and update *EXPP.
+   CMDNAME should name the current command.  */
+
+void
+print_command_parse_format (const char **expp, const char *cmdname,
+			    struct format_data *fmtp)
+{
+  const char *exp = *expp;
+
+  if (exp && *exp == '/')
+    {
+      exp++;
+      *fmtp = decode_format (&exp, last_format, 0);
+      validate_format (*fmtp, cmdname);
+      last_format = fmtp->format;
+    }
+  else
+    {
+      fmtp->count = 1;
+      fmtp->format = 0;
+      fmtp->size = 0;
+      fmtp->raw = 0;
+    }
+
+  *expp = exp;
+}
+
+/* Print VAL to console according to *FMTP, 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->format);
+  opts.raw = fmtp->raw;
+
+  print_formatted (val, 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).  */
@@ -948,24 +988,10 @@ 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;
-    }
+  print_command_parse_format (&exp, "print", &fmt);
 
   if (exp && *exp)
     {
@@ -978,24 +1004,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, &fmt);
 
   do_cleanups (old_chain);
 }
diff --git a/gdb/valprint.h b/gdb/valprint.h
index e3d0137..ed4964f 100644
--- a/gdb/valprint.h
+++ b/gdb/valprint.h
@@ -217,4 +217,19 @@ extern void output_command_const (const char *args, int from_tty);
 
 extern int val_print_scalar_type_p (struct type *type);
 
+struct format_data
+  {
+    int count;
+    char format;
+    char size;
+
+    /* True if the value should be printed raw -- that is, bypassing
+       python-based formatters.  */
+    unsigned char raw;
+  };
+
+extern void print_command_parse_format (const char **expp, const char *cmdname,
+					struct format_data *fmtp);
+extern void print_value (struct value *val, const struct format_data *fmtp);
+
 #endif

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

* [PATCH v5 0/7] compile: compile print (&unmap)
@ 2015-05-13 20:16 Jan Kratochvil
  2015-05-13 20:16 ` [PATCH v5 2/7] Code cleanup: Make parts of print_command_1 public Jan Kratochvil
                   ` (6 more replies)
  0 siblings, 7 replies; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-13 20:16 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

Hi Pedro,

from the past review mails it looks to me the series is now approved as is but
the approval was not explicit anywhere there.

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


Jan

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

* [PATCH v5 3/7] compile: Distribute scope, add scope_data
  2015-05-13 20:16 [PATCH v5 0/7] compile: compile print (&unmap) Jan Kratochvil
                   ` (2 preceding siblings ...)
  2015-05-13 20:16 ` [PATCH v5 4/7] compile: Use -Wall, not -w Jan Kratochvil
@ 2015-05-13 20:16 ` Jan Kratochvil
  2015-05-15 16:49   ` Pedro Alves
  2015-05-13 20:17 ` [PATCH v5 6/7] compile: New 'compile print' Jan Kratochvil
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-13 20:16 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

Provide a way to access current 'scope' during 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): Propagate 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.
---
 gdb/cli/cli-script.c              |    3 ++-
 gdb/compile/compile-object-load.c |    5 ++++-
 gdb/compile/compile-object-load.h |   11 +++++++++--
 gdb/compile/compile-object-run.c  |    6 ++++++
 gdb/compile/compile.c             |    9 +++++----
 gdb/compile/compile.h             |    3 ++-
 gdb/defs.h                        |    1 +
 7 files changed, 29 insertions(+), 9 deletions(-)

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 745d787..fd440be 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;
@@ -597,5 +598,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..311fb09 100644
--- a/gdb/compile/compile-object-load.h
+++ b/gdb/compile/compile-object-load.h
@@ -31,9 +31,16 @@ 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);
+extern struct compile_module *compile_object_load
+  (const char *object_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 422693b..c194705 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 5ede27b..621de66 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, "");
@@ -560,7 +560,7 @@ compile_command (char *args, int from_tty)
 
 void
 eval_compile_command (struct command_line *cmd, const char *cmd_string,
-		      enum compile_i_scope_types scope)
+		      enum compile_i_scope_types scope, void *scope_data)
 {
   char *object_file, *source_file;
 
@@ -574,7 +574,8 @@ eval_compile_command (struct command_line *cmd, const 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 ccb1361..a973167 100644
--- a/gdb/compile/compile.h
+++ b/gdb/compile/compile.h
@@ -30,7 +30,8 @@ struct dynamic_prop;
 
 extern void eval_compile_command (struct command_line *cmd,
 				  const 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 29bb8c4..98cf4d9 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] 25+ messages in thread

* [PATCH v5 7/7] RFC only: compile: Use also inferior munmap
  2015-05-13 20:16 [PATCH v5 0/7] compile: compile print (&unmap) Jan Kratochvil
                   ` (5 preceding siblings ...)
  2015-05-13 20:17 ` [PATCH v5 5/7] Code cleanup: compile: func_addr -> func_sym Jan Kratochvil
@ 2015-05-13 20:17 ` Jan Kratochvil
  6 siblings, 0 replies; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-13 20:17 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

Hi,

currently inferior memory is allocated by inferior mmap() but it is never
deallocated; despite the injected objfile incl. its symbols is freed.  This was
intentional so that one can do for example:
inferior:
	char *str = "foo";
GDB:
	(gdb) compile code str = "bar";

I believe later patches will be needed to introduce full control over keeping
vs. discarding the injected module as being discussed in:
	compile: objfiles lifetime UI
	https://sourceware.org/ml/gdb/2015-04/msg00051.html
	Message-ID: <20150429135735.GA16974@host1.jankratochvil.net>
	https://sourceware.org/ml/gdb/2015-05/msg00007.html
This patch at least introduces code which will be needed for the part/cases of
really freeing all the resources of an injected module.

It is "RFC only" as given the patch as is it regresses GDB functionality.


Jan


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

	* arch-utils.c (default_infcall_munmap): New.
	* arch-utils.h (default_infcall_munmap): New declaration.
	* compile/compile-object-load.c (struct munmap_list, munmap_list_add)
	(munmap_list_free, munmap_listp_free_cleanup): New.
	(struct setup_sections_data): Add field munmap_list_headp.
	(setup_sections): Call munmap_list_add.
	(compile_object_load): New variable munmap_list_head, initialize
	setup_sections_data.munmap_list_headp, return munmap_list_head.
	* compile/compile-object-load.h (struct munmap_list): New declaration.
	(struct compile_module): Add field munmap_list_head.
	(munmap_list_free): New declaration.
	* compile/compile-object-run.c (struct do_module_cleanup): Add field
	munmap_list_head.
	(do_module_cleanup): Call munmap_list_free.
	(compile_object_run): Pass munmap_list_head to do_module_cleanup.
	* gdbarch.c: Regenerate.
	* gdbarch.h: Regenerate.
	* gdbarch.sh (infcall_munmap): New.
	* linux-tdep.c (linux_infcall_munmap): New.
	(linux_init_abi): Install it.

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

	* gdb.compile/compile.exp (keep jit in memory): Rename to ...
	(do not keep jit in memory): ... this.
	(expect 5): Change it to ...
	(expect no 5): ... this.
---
 gdb/arch-utils.c                      |    6 +++
 gdb/arch-utils.h                      |    1 
 gdb/compile/compile-object-load.c     |   71 +++++++++++++++++++++++++++++++++
 gdb/compile/compile-object-load.h     |    6 +++
 gdb/compile/compile-object-run.c      |    6 +++
 gdb/gdbarch.c                         |   23 +++++++++++
 gdb/gdbarch.h                         |    7 +++
 gdb/gdbarch.sh                        |    4 ++
 gdb/linux-tdep.c                      |   30 ++++++++++++++
 gdb/testsuite/gdb.compile/compile.exp |    4 +-
 10 files changed, 155 insertions(+), 3 deletions(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index e1c8ab0..362c71b 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -864,6 +864,12 @@ default_infcall_mmap (CORE_ADDR size, unsigned prot)
   error (_("This target does not support inferior memory allocation by mmap."));
 }
 
+void
+default_infcall_munmap (CORE_ADDR addr, CORE_ADDR size)
+{
+  /* Memory reserved by inferior mmap is kept leaked.  */
+}
+
 /* -mcmodel=large is used so that no GOT (Global Offset Table) is needed to be
    created in inferior memory by GDB (normally it is set by ld.so).  */
 
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index ed3bec9..f40e076 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -200,6 +200,7 @@ extern void default_skip_permanent_breakpoint (struct regcache *regcache);
 #define GDB_MMAP_PROT_EXEC	0x4	/* Page can be executed.  */
 
 extern CORE_ADDR default_infcall_mmap (CORE_ADDR size, unsigned prot);
+extern void default_infcall_munmap (CORE_ADDR addr, CORE_ADDR size);
 extern char *default_gcc_target_options (struct gdbarch *gdbarch);
 extern const char *default_gnu_triplet_regexp (struct gdbarch *gdbarch);
 
diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index ad940b2..50d02bf 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -32,6 +32,58 @@
 #include "block.h"
 #include "arch-utils.h"
 
+/* Track inferior memory reserved by inferior mmap.  */
+
+struct munmap_list
+{
+  struct munmap_list *next;
+  CORE_ADDR addr, size;
+};
+
+/* Add inferior mmap memory range ADDR..ADDR+SIZE (exclusive) to list
+   HEADP.  *HEADP needs to be initialized to NULL.  */
+
+static void
+munmap_list_add (struct munmap_list **headp, CORE_ADDR addr, CORE_ADDR size)
+{
+  struct munmap_list *head_new = xmalloc (sizeof (*head_new));
+
+  head_new->next = *headp;
+  *headp = head_new;
+  head_new->addr = addr;
+  head_new->size = size;
+}
+
+/* Free list of inferior mmap memory ranges HEAD.  HEAD is the first
+   element of the list, it can be NULL.  After calling this function
+   HEAD pointer is invalid and the possible list needs to be
+   reinitialized by caller to NULL.  */
+
+void
+munmap_list_free (struct munmap_list *head)
+{
+  while (head)
+    {
+      struct munmap_list *todo = head;
+
+      head = todo->next;
+      gdbarch_infcall_munmap (target_gdbarch (), todo->addr, todo->size);
+      xfree (todo);
+    }
+}
+
+/* Stub for munmap_list_free suitable for make_cleanup.  Contrary to
+   munmap_list_free this function's parameter is a pointer to the first
+   list element pointer.  */
+
+static void
+munmap_listp_free_cleanup (void *headp_voidp)
+{
+  struct munmap_list **headp = headp_voidp;
+
+  munmap_list_free (*headp);
+}
+
 /* Helper data for setup_sections.  */
 
 struct setup_sections_data
@@ -48,6 +100,10 @@ struct setup_sections_data
   /* Maximum of alignments of all sections matching LAST_PROT.
      This value is always at least 1.  This value is always a power of 2.  */
   CORE_ADDR last_max_alignment;
+
+  /* List of inferior mmap ranges where setup_sections should add its
+     next range.  */
+  struct munmap_list **munmap_list_headp;
 };
 
 /* Place all ABFD sections next to each other obeying all constraints.  */
@@ -97,6 +153,7 @@ setup_sections (bfd *abfd, asection *sect, void *data_voidp)
 	{
 	  addr = gdbarch_infcall_mmap (target_gdbarch (), data->last_size,
 				       data->last_prot);
+	  munmap_list_add (data->munmap_list_headp, addr, data->last_size);
 	  if (compile_debug)
 	    fprintf_unfiltered (gdb_stdout,
 				"allocated %s bytes at %s prot %u\n",
@@ -578,6 +635,7 @@ compile_object_load (const char *object_file, const char *source_file,
   struct objfile *objfile;
   int expect_parameters;
   struct type *expect_return_type;
+  struct munmap_list *munmap_list_head = NULL;
 
   filename = tilde_expand (object_file);
   cleanups = make_cleanup (xfree, filename);
@@ -599,6 +657,8 @@ compile_object_load (const char *object_file, const char *source_file,
   setup_sections_data.last_section_first = abfd->sections;
   setup_sections_data.last_prot = -1;
   setup_sections_data.last_max_alignment = 1;
+  setup_sections_data.munmap_list_headp = &munmap_list_head;
+  make_cleanup (munmap_listp_free_cleanup, &munmap_list_head);
   bfd_map_over_sections (abfd, setup_sections, &setup_sections_data);
   setup_sections (abfd, NULL, &setup_sections_data);
 
@@ -713,6 +773,7 @@ compile_object_load (const char *object_file, const char *source_file,
 					TYPE_LENGTH (regs_type),
 					GDB_MMAP_PROT_READ);
       gdb_assert (regs_addr != 0);
+      munmap_list_add (&munmap_list_head, regs_addr, TYPE_LENGTH (regs_type));
       if (compile_debug)
 	fprintf_unfiltered (gdb_stdout,
 			    "allocated %s bytes at %s for registers\n",
@@ -737,6 +798,8 @@ compile_object_load (const char *object_file, const char *source_file,
 					     (GDB_MMAP_PROT_READ
 					      | GDB_MMAP_PROT_WRITE));
       gdb_assert (out_value_addr != 0);
+      munmap_list_add (&munmap_list_head, out_value_addr,
+		       TYPE_LENGTH (out_value_type));
       if (compile_debug)
 	fprintf_unfiltered (gdb_stdout,
 			    "allocated %s bytes at %s for printed value\n",
@@ -746,7 +809,6 @@ compile_object_load (const char *object_file, const char *source_file,
     }
 
   discard_cleanups (cleanups_free_objfile);
-  do_cleanups (cleanups);
 
   retval = xmalloc (sizeof (*retval));
   retval->objfile = objfile;
@@ -757,5 +819,12 @@ compile_object_load (const char *object_file, const char *source_file,
   retval->scope_data = scope_data;
   retval->out_value_addr = out_value_addr;
   retval->out_value_type = out_value_type;
+
+  /* CLEANUPS will free MUNMAP_LIST_HEAD.  */
+  retval->munmap_list_head = munmap_list_head;
+  munmap_list_head = NULL;
+
+  do_cleanups (cleanups);
+
   return retval;
 }
diff --git a/gdb/compile/compile-object-load.h b/gdb/compile/compile-object-load.h
index 3e0e45b..61d8563 100644
--- a/gdb/compile/compile-object-load.h
+++ b/gdb/compile/compile-object-load.h
@@ -17,6 +17,8 @@
 #ifndef GDB_COMPILE_OBJECT_LOAD_H
 #define GDB_COMPILE_OBJECT_LOAD_H
 
+struct munmap_list;
+
 struct compile_module
 {
   /* objfile for the compiled module.  */
@@ -45,10 +47,14 @@ struct compile_module
   /* Inferior parameter out value type or NULL if the inferior function does not
      have one.  */
   struct type *out_value_type;
+
+  /* Track inferior memory reserved by inferior mmap.  */
+  struct munmap_list *munmap_list_head;
 };
 
 extern struct compile_module *compile_object_load
   (const char *object_file, const char *source_file,
    enum compile_i_scope_types scope, void *scope_data);
+extern void munmap_list_free (struct munmap_list *head);
 
 #endif /* GDB_COMPILE_OBJECT_LOAD_H */
diff --git a/gdb/compile/compile-object-run.c b/gdb/compile/compile-object-run.c
index 9e11c00..d5cc892 100644
--- a/gdb/compile/compile-object-run.c
+++ b/gdb/compile/compile-object-run.c
@@ -47,6 +47,9 @@ struct do_module_cleanup
   CORE_ADDR out_value_addr;
   struct type *out_value_type;
 
+  /* Copy from struct compile_module.  */
+  struct munmap_list *munmap_list_head;
+
   /* objfile_name of our objfile.  */
   char objfile_name_string[1];
 };
@@ -96,6 +99,8 @@ do_module_cleanup (void *arg, int registers_valid)
   unlink (data->source_file);
   xfree (data->source_file);
 
+  munmap_list_free (data->munmap_list_head);
+
   /* Delete the .o file.  */
   unlink (data->objfile_name_string);
   xfree (data);
@@ -128,6 +133,7 @@ compile_object_run (struct compile_module *module)
   data->scope_data = module->scope_data;
   data->out_value_addr = module->out_value_addr;
   data->out_value_type = module->out_value_type;
+  data->munmap_list_head = module->munmap_list_head;
 
   xfree (module->source_file);
   xfree (module);
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 97874c9..aa2789e 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -326,6 +326,7 @@ struct gdbarch
   gdbarch_auxv_parse_ftype *auxv_parse;
   gdbarch_vsyscall_range_ftype *vsyscall_range;
   gdbarch_infcall_mmap_ftype *infcall_mmap;
+  gdbarch_infcall_munmap_ftype *infcall_munmap;
   gdbarch_gcc_target_options_ftype *gcc_target_options;
   gdbarch_gnu_triplet_regexp_ftype *gnu_triplet_regexp;
 };
@@ -426,6 +427,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->insn_is_jump = default_insn_is_jump;
   gdbarch->vsyscall_range = default_vsyscall_range;
   gdbarch->infcall_mmap = default_infcall_mmap;
+  gdbarch->infcall_munmap = default_infcall_munmap;
   gdbarch->gcc_target_options = default_gcc_target_options;
   gdbarch->gnu_triplet_regexp = default_gnu_triplet_regexp;
   /* gdbarch_alloc() */
@@ -658,6 +660,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of auxv_parse, has predicate.  */
   /* Skip verify of vsyscall_range, invalid_p == 0 */
   /* Skip verify of infcall_mmap, invalid_p == 0 */
+  /* Skip verify of infcall_munmap, invalid_p == 0 */
   /* Skip verify of gcc_target_options, invalid_p == 0 */
   /* Skip verify of gnu_triplet_regexp, invalid_p == 0 */
   buf = ui_file_xstrdup (log, &length);
@@ -1029,6 +1032,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: infcall_mmap = <%s>\n",
                       host_address_to_string (gdbarch->infcall_mmap));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: infcall_munmap = <%s>\n",
+                      host_address_to_string (gdbarch->infcall_munmap));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: gdbarch_info_proc_p() = %d\n",
                       gdbarch_info_proc_p (gdbarch));
   fprintf_unfiltered (file,
@@ -4673,6 +4679,23 @@ set_gdbarch_infcall_mmap (struct gdbarch *gdbarch,
   gdbarch->infcall_mmap = infcall_mmap;
 }
 
+void
+gdbarch_infcall_munmap (struct gdbarch *gdbarch, CORE_ADDR addr, CORE_ADDR size)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->infcall_munmap != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_infcall_munmap called\n");
+  gdbarch->infcall_munmap (addr, size);
+}
+
+void
+set_gdbarch_infcall_munmap (struct gdbarch *gdbarch,
+                            gdbarch_infcall_munmap_ftype infcall_munmap)
+{
+  gdbarch->infcall_munmap = infcall_munmap;
+}
+
 char *
 gdbarch_gcc_target_options (struct gdbarch *gdbarch)
 {
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index c94c19c..59882df 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1440,6 +1440,13 @@ typedef CORE_ADDR (gdbarch_infcall_mmap_ftype) (CORE_ADDR size, unsigned prot);
 extern CORE_ADDR gdbarch_infcall_mmap (struct gdbarch *gdbarch, CORE_ADDR size, unsigned prot);
 extern void set_gdbarch_infcall_mmap (struct gdbarch *gdbarch, gdbarch_infcall_mmap_ftype *infcall_mmap);
 
+/* Deallocate SIZE bytes of memory at ADDR in inferior from gdbarch_infcall_mmap.
+   Print a warning if it is not possible. */
+
+typedef void (gdbarch_infcall_munmap_ftype) (CORE_ADDR addr, CORE_ADDR size);
+extern void gdbarch_infcall_munmap (struct gdbarch *gdbarch, CORE_ADDR addr, CORE_ADDR size);
+extern void set_gdbarch_infcall_munmap (struct gdbarch *gdbarch, gdbarch_infcall_munmap_ftype *infcall_munmap);
+
 /* Return string (caller has to use xfree for it) with options for GCC
    to produce code for this target, typically "-m64", "-m32" or "-m31".
    These options are put before CU's DW_AT_producer compilation options so that
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 0f303a4..f719ec1 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1097,6 +1097,10 @@ m:int:vsyscall_range:struct mem_range *range:range::default_vsyscall_range::0
 # Throw an error if it is not possible.  Returned address is always valid.
 f:CORE_ADDR:infcall_mmap:CORE_ADDR size, unsigned prot:size, prot::default_infcall_mmap::0
 
+# Deallocate SIZE bytes of memory at ADDR in inferior from gdbarch_infcall_mmap.
+# Print a warning if it is not possible.
+f:void:infcall_munmap:CORE_ADDR addr, CORE_ADDR size:addr, size::default_infcall_munmap::0
+
 # Return string (caller has to use xfree for it) with options for GCC
 # to produce code for this target, typically "-m64", "-m32" or "-m31".
 # These options are put before CU's DW_AT_producer compilation options so that
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 9d75b66..8cb91c9 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -2348,6 +2348,35 @@ linux_infcall_mmap (CORE_ADDR size, unsigned prot)
   return retval;
 }
 
+/* See gdbarch.sh 'infcall_munmap'.  */
+
+static void
+linux_infcall_munmap (CORE_ADDR addr, CORE_ADDR size)
+{
+  struct objfile *objf;
+  struct value *munmap_val = find_function_in_inferior ("munmap", &objf);
+  struct value *retval_val;
+  struct gdbarch *gdbarch = get_objfile_arch (objf);
+  LONGEST retval;
+  enum
+    {
+      ARG_ADDR, ARG_LENGTH, ARG_LAST
+    };
+  struct value *arg[ARG_LAST];
+
+  arg[ARG_ADDR] = value_from_pointer (builtin_type (gdbarch)->builtin_data_ptr,
+				      addr);
+  /* Assuming sizeof (unsigned long) == sizeof (size_t).  */
+  arg[ARG_LENGTH] = value_from_ulongest
+		    (builtin_type (gdbarch)->builtin_unsigned_long, size);
+  retval_val = call_function_by_hand (munmap_val, ARG_LAST, arg);
+  retval = value_as_long (retval_val);
+  if (retval != 0)
+    warning (_("Failed inferior munmap call at %s for %s bytes, "
+	       "errno is changed."),
+	     hex_string (addr), pulongest (size));
+}
+
 /* See linux-tdep.h.  */
 
 CORE_ADDR
@@ -2409,6 +2438,7 @@ linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 				    linux_gdb_signal_to_target);
   set_gdbarch_vsyscall_range (gdbarch, linux_vsyscall_range);
   set_gdbarch_infcall_mmap (gdbarch, linux_infcall_mmap);
+  set_gdbarch_infcall_munmap (gdbarch, linux_infcall_munmap);
 }
 
 /* Provide a prototype to silence -Wmissing-prototypes.  */
diff --git a/gdb/testsuite/gdb.compile/compile.exp b/gdb/testsuite/gdb.compile/compile.exp
index 07276bd..dd46a5f 100644
--- a/gdb/testsuite/gdb.compile/compile.exp
+++ b/gdb/testsuite/gdb.compile/compile.exp
@@ -131,8 +131,8 @@ gdb_test_no_output "compile code globalvar = static_local"
 gdb_test "p globalvar" " = 77000" "check static_local"
 
 gdb_test_no_output "compile code static int staticvar = 5; intptr = &staticvar" \
-    "keep jit in memory"
-gdb_test "p *intptr" " = 5" "expect 5"
+    "do not keep jit in memory"
+gdb_test "p *intptr" "Cannot access memory at address 0x\[0-9a-f\]+" "expect no 5"
 
 gdb_test "compile code func_doesnotexist ();" "warning: Could not find symbol \"func_doesnotexist\" for .*"
 

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

* [PATCH v5 5/7] Code cleanup: compile: func_addr -> func_sym
  2015-05-13 20:16 [PATCH v5 0/7] compile: compile print (&unmap) Jan Kratochvil
                   ` (4 preceding siblings ...)
  2015-05-13 20:17 ` [PATCH v5 6/7] compile: New 'compile print' Jan Kratochvil
@ 2015-05-13 20:17 ` Jan Kratochvil
  2015-05-15 16:51   ` Pedro Alves
  2015-05-13 20:17 ` [PATCH v5 7/7] RFC only: compile: Use also inferior munmap Jan Kratochvil
  6 siblings, 1 reply; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-13 20:17 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.
---
 gdb/compile/compile-object-load.c |   81 +++++++++++++++++++++----------------
 gdb/compile/compile-object-load.h |    4 +-
 gdb/compile/compile-object-run.c  |   43 ++++++++++++--------
 3 files changed, 76 insertions(+), 52 deletions(-)

diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index fd440be..f04b43c 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
@@ -596,7 +609,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 311fb09..f5e887d 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 c194705..15d3130 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] 25+ messages in thread

* [PATCH v5 6/7] compile: New 'compile print'
  2015-05-13 20:16 [PATCH v5 0/7] compile: compile print (&unmap) Jan Kratochvil
                   ` (3 preceding siblings ...)
  2015-05-13 20:16 ` [PATCH v5 3/7] compile: Distribute scope, add scope_data Jan Kratochvil
@ 2015-05-13 20:17 ` Jan Kratochvil
  2015-05-15 17:04   ` Pedro Alves
  2015-05-13 20:17 ` [PATCH v5 5/7] Code cleanup: compile: func_addr -> func_sym Jan Kratochvil
  2015-05-13 20:17 ` [PATCH v5 7/7] RFC only: compile: Use also inferior munmap Jan Kratochvil
  6 siblings, 1 reply; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-13 20:17 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/NEWS                                    |    3 +
 gdb/compile/compile-c-support.c             |   44 ++++++++
 gdb/compile/compile-internal.h              |    4 +
 gdb/compile/compile-object-load.c           |  150 ++++++++++++++++++++++++++-
 gdb/compile/compile-object-load.h           |    8 +
 gdb/compile/compile-object-run.c            |   33 ++++++
 gdb/compile/compile.c                       |   83 ++++++++++++++-
 gdb/compile/compile.h                       |    2 
 gdb/defs.h                                  |    9 ++
 gdb/doc/gdb.texinfo                         |   19 +++
 gdb/testsuite/gdb.compile/compile-print.c   |   32 ++++++
 gdb/testsuite/gdb.compile/compile-print.exp |   58 ++++++++++
 12 files changed, 433 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.compile/compile-print.c
 create mode 100644 gdb/testsuite/gdb.compile/compile-print.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 7778830..d27beaa 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -75,6 +75,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..39f06c6 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -190,6 +190,24 @@ 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:
+      /* <string.h> is needed for a memcpy call below.  */
+      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 +225,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 +389,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 +405,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 f04b43c..ad940b2 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,117 @@ 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.
+   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.  */
+
+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 +552,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 +563,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 +573,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 +635,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);
     }
@@ -603,6 +722,29 @@ 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);
+      if (compile_debug)
+	fprintf_unfiltered (gdb_stdout,
+			    "allocated %s bytes at %s for printed value\n",
+			    paddress (target_gdbarch (),
+				      TYPE_LENGTH (out_value_type)),
+			    paddress (target_gdbarch (), out_value_addr));
+    }
+
   discard_cleanups (cleanups_free_objfile);
   do_cleanups (cleanups);
 
@@ -613,5 +755,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 f5e887d..3e0e45b 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
diff --git a/gdb/compile/compile-object-run.c b/gdb/compile/compile-object-run.c
index 15d3130..9e11c00 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,23 @@ do_module_cleanup (void *arg, int registers_valid)
   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);
+
+	  /* SCOPE_DATA would be stale unlesse EXECUTEDP != NULL.  */
+	  compile_print_value (value_ind (addr_value), data->scope_data);
+	}
+    }
 
   ALL_OBJFILES (objfile)
     if ((objfile->flags & OBJF_USERLOADED) == 0
@@ -104,6 +126,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 +157,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..fbecf8c 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,51 @@ 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 fmt;
+
+  cleanup = make_cleanup_restore_integer (&interpreter_async);
+  interpreter_async = 0;
+
+  /* Passing &FMT as SCOPE_DATA is safe as do_module_cleanup will not
+     touch the stale pointer if compile_object_run has already quit.  */
+  print_command_parse_format (&arg, "compile print", &fmt);
+
+  if (arg && *arg)
+    eval_compile_command (NULL, arg, scope, &fmt);
+  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 = &fmt;
+      execute_control_command_untraced (l);
+    }
+
+  do_cleanups (cleanup);
+}
+
 /* A cleanup function to remove a directory and all its contents.  */
 
 static void
@@ -576,6 +622,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 +691,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 +706,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 98cf4d9..44702ea 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.  See get_out_value_type.  */
+    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 cb9cd35..1665372 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17221,6 +17221,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] 25+ messages in thread

* Re: [PATCH v5 3/7] compile: Distribute scope, add scope_data
  2015-05-13 20:16 ` [PATCH v5 3/7] compile: Distribute scope, add scope_data Jan Kratochvil
@ 2015-05-15 16:49   ` Pedro Alves
  2015-05-16 12:39     ` [commit] " Jan Kratochvil
  0 siblings, 1 reply; 25+ messages in thread
From: Pedro Alves @ 2015-05-15 16:49 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

On 05/13/2015 09:16 PM, Jan Kratochvil wrote:
> Provide a way to access current 'scope' during 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.

Yes, it does not require C++ yet.  (You know now, so no need to have that
in the commit log.)

OK.

Thanks,
Pedro Alves

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

* Re: [PATCH v5 1/7] compile: Add one debug message
  2015-05-13 20:16 ` [PATCH v5 1/7] compile: Add one debug message Jan Kratochvil
@ 2015-05-15 16:49   ` Pedro Alves
  2015-05-16 12:19     ` [commit] " Jan Kratochvil
  0 siblings, 1 reply; 25+ messages in thread
From: Pedro Alves @ 2015-05-15 16:49 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

On 05/13/2015 09:16 PM, Jan Kratochvil wrote:

> diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
> index fe23448..745d787 100644
> --- a/gdb/compile/compile-object-load.c
> +++ b/gdb/compile/compile-object-load.c
> @@ -580,6 +580,12 @@ compile_object_load (const char *object_file, const char *source_file)
>  					TYPE_LENGTH (regs_type),
>  					GDB_MMAP_PROT_READ);
>        gdb_assert (regs_addr != 0);
> +      if (compile_debug)
> +	fprintf_unfiltered (gdb_stdout,
> +			    "allocated %s bytes at %s for registers\n",
> +			    paddress (target_gdbarch (),
> +				      TYPE_LENGTH (regs_type)),
> +			    paddress (target_gdbarch (), regs_addr));
>        store_regs (regs_type, regs_addr);
>      }
>  

Please send debug output to gdb_stdlog.  OK with that change.

Thanks,
Pedro Alves

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

* Re: [PATCH v5 4/7] compile: Use -Wall, not -w
  2015-05-13 20:16 ` [PATCH v5 4/7] compile: Use -Wall, not -w Jan Kratochvil
@ 2015-05-15 16:49   ` Pedro Alves
  2015-05-16 12:43     ` [commit] " Jan Kratochvil
  0 siblings, 1 reply; 25+ messages in thread
From: Pedro Alves @ 2015-05-15 16:49 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

On 05/13/2015 09:16 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).

It'd be good if the commit logs were updated reflecting previous
discussions.  For instance, I'd update this like:

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

That #ifndef change in print_one_macro() is needed otherwise we get
macro-redefinition warnings for the GCC built-in macros (as -w is no
longer in effect).  For example, without the #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
        ...

It makes more sense to pick the inferior's version of the macros, hence
#ifndef instead of #undef.

(...)
~~~

OK.

Thanks,
Pedro Alves

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

* Re: [PATCH v5 5/7] Code cleanup: compile: func_addr -> func_sym
  2015-05-13 20:17 ` [PATCH v5 5/7] Code cleanup: compile: func_addr -> func_sym Jan Kratochvil
@ 2015-05-15 16:51   ` Pedro Alves
  2015-05-16 12:45     ` [commit] " Jan Kratochvil
  0 siblings, 1 reply; 25+ messages in thread
From: Pedro Alves @ 2015-05-15 16:51 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

On 05/13/2015 09:16 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.
> 
> While the patch tries to clean up the code it is still horrible due to the
> missing C++ sub-classing.
> 

OK.

Thanks,
Pedro Alves

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

* Re: [PATCH v5 6/7] compile: New 'compile print'
  2015-05-13 20:17 ` [PATCH v5 6/7] compile: New 'compile print' Jan Kratochvil
@ 2015-05-15 17:04   ` Pedro Alves
  2015-05-16 12:58     ` [commit] " Jan Kratochvil
  0 siblings, 1 reply; 25+ messages in thread
From: Pedro Alves @ 2015-05-15 17:04 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches; +Cc: Phil Muldoon

A couple very minor nits below, and this is good to go.

On 05/13/2015 09:17 PM, Jan Kratochvil wrote:

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

AFAIC, retval is always gdb_type_from_ptr, and could be
moved out of the switch.

> +      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));
> +    }

> diff --git a/gdb/compile/compile-object-load.h b/gdb/compile/compile-object-load.h
> index f5e887d..3e0e45b 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.  */

Line too long, I think.  Also, "NULL if the inferior function does not
have one" looks like a copy/paste, given this is not a pointer
variable.   I'd swap around the fields, and write:

  /* Inferior parameter out value type or NULL if the inferior function does not
     have one.  */
  struct type *out_value_type;

  /* If the inferior function has an out value, this is its address.  */
  CORE_ADDR out_value_addr;

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

Thanks,
Pedro Alves

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

* [commit] [PATCH v5 1/7] compile: Add one debug message
  2015-05-15 16:49   ` Pedro Alves
@ 2015-05-16 12:19     ` Jan Kratochvil
  2015-05-16 13:09       ` [patch] compile: gdb_stdout -> gdb_stdlog [compile: Add one debug message] Jan Kratochvil
  0 siblings, 1 reply; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-16 12:19 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Fri, 15 May 2015 18:49:15 +0200, Pedro Alves wrote:
> On 05/13/2015 09:16 PM, Jan Kratochvil wrote:
> 
> > diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
> > index fe23448..745d787 100644
> > --- a/gdb/compile/compile-object-load.c
> > +++ b/gdb/compile/compile-object-load.c
> > @@ -580,6 +580,12 @@ compile_object_load (const char *object_file, const char *source_file)
> >  					TYPE_LENGTH (regs_type),
> >  					GDB_MMAP_PROT_READ);
> >        gdb_assert (regs_addr != 0);
> > +      if (compile_debug)
> > +	fprintf_unfiltered (gdb_stdout,
> > +			    "allocated %s bytes at %s for registers\n",
> > +			    paddress (target_gdbarch (),
> > +				      TYPE_LENGTH (regs_type)),
> > +			    paddress (target_gdbarch (), regs_addr));
> >        store_regs (regs_type, regs_addr);
> >      }
> >  
> 
> Please send debug output to gdb_stdlog.  OK with that change.

OK but gdb/compile/ is using now only gdb_stdout; the error above is due to
a copy-paste.  So I will send a follow-up patch to change all the other
gdb/compile/ gdb_stdout strings to gdb_stdlog.

Checked in:
	b6de3f9642c58439c31690255c3a4326728da88d


Jan

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

* [commit] [PATCH v5 2/7] Code cleanup: Make parts of print_command_1 public
  2015-05-13 20:16 ` [PATCH v5 2/7] Code cleanup: Make parts of print_command_1 public Jan Kratochvil
@ 2015-05-16 12:27   ` Jan Kratochvil
  0 siblings, 0 replies; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-16 12:27 UTC (permalink / raw)
  To: gdb-patches; +Cc: Phil Muldoon

On Wed, 13 May 2015 22:16:33 +0200, Jan Kratochvil wrote:
> gdb/ChangeLog
> 2015-05-03  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* printcmd.c (struct format_data): Move it to valprint.h.
> 	(print_command_parse_format, print_value): New functions from ...
> 	(print_command_1): ... here.  Call them.
> 	* valprint.h (struct format_data): Move it here from printcmd.c.
> 	(print_command_parse_format, print_value): New declarations.

Checked in:
	1c88ceb1bedc81dbfd1d076e4a49bbf533b4e238


Jan

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

* [commit] [PATCH v5 3/7] compile: Distribute scope, add scope_data
  2015-05-15 16:49   ` Pedro Alves
@ 2015-05-16 12:39     ` Jan Kratochvil
  0 siblings, 0 replies; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-16 12:39 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Fri, 15 May 2015 18:49:07 +0200, Pedro Alves wrote:
> On 05/13/2015 09:16 PM, Jan Kratochvil wrote:
> > It should be all sub-classed but AFAIK GDB does not require C++ compiler yet.
> 
> Yes, it does not require C++ yet.  
> 
> (You know now, so no need to have that in the commit log.)

That is not a valid reason why to delete that line but for too many reasons
I do not think that matters to discuss it more.


> OK.

Checked in:
	5c65b58a58a4c77b1ec38b4e31549aaa090b4845


Jan

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

* [commit] [PATCH v5 4/7] compile: Use -Wall, not -w
  2015-05-15 16:49   ` Pedro Alves
@ 2015-05-16 12:43     ` Jan Kratochvil
  0 siblings, 0 replies; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-16 12:43 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Fri, 15 May 2015 18:48:55 +0200, Pedro Alves wrote:
> It'd be good if the commit logs were updated reflecting previous
> discussions.  For instance, I'd update this like:

Done.

Checked in:
	3a9558c494e9b461f752ce26382701d4446f0958


Jan

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

* [commit] [PATCH v5 5/7] Code cleanup: compile: func_addr -> func_sym
  2015-05-15 16:51   ` Pedro Alves
@ 2015-05-16 12:45     ` Jan Kratochvil
  0 siblings, 0 replies; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-16 12:45 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Fri, 15 May 2015 18:51:38 +0200, Pedro Alves wrote:
> OK.

Checked in:
	83d3415ef530c41af7e1ae98a7add97adb0cf5e0


Jan

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

* [commit] [PATCH v5 6/7] compile: New 'compile print'
  2015-05-15 17:04   ` Pedro Alves
@ 2015-05-16 12:58     ` Jan Kratochvil
  2015-05-16 13:39       ` [commit#2] " Jan Kratochvil
  2015-05-16 14:25       ` [commit] " Patrick Palka
  0 siblings, 2 replies; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-16 12:58 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Fri, 15 May 2015 19:04:03 +0200, Pedro Alves wrote:
> A couple very minor nits below, and this is good to go.
> 
> On 05/13/2015 09:17 PM, Jan Kratochvil wrote:
> 
> > +  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;
> 
> AFAIC, retval is always gdb_type_from_ptr, and could be
> moved out of the switch.

It was written this way as it has semantical logic - how the type is
determined depends on TYPE_CODE_* of that object.  Just accidentally in this
case it is the same.  I do not do performance micro-optimizations which
compiler will do anyway.  But I have changed it.


> > +  /* Inferior parameter out value address or NULL if the inferior function does not
> > +     have one.  */
> 
> Line too long, I think.

Yes, fixed.  (Not discussing the 72 vs. 78 or 80 or which width of comments
rule.)


> Also, "NULL if the inferior function does not have one" looks like
> a copy/paste, given this is not a pointer variable.

It is "inferior NULL", so I find that "NULL" valid there.


> I'd swap around the fields, and write:
> 
>   /* Inferior parameter out value type or NULL if the inferior function does not
>      have one.  */
>   struct type *out_value_type;
> 
>   /* If the inferior function has an out value, this is its address.  */
>   CORE_ADDR out_value_addr;

Done although I have added line "Otherwise it is zero." there:

+  /* Inferior parameter out value type or NULL if the inferior function does not
+     have one.  */
+  struct type *out_value_type;
+
+  /* If the inferior function has an out value, this is its address.
+     Otherwise it is zero.  */
+  CORE_ADDR out_value_addr;


Without that line 'out_value_addr' would be unspecified if out_value_type is
NULL which does not correspond to the current code.


Checked in:
	36de76f9cc2eea0bd5f1b7ce74ef60e1aa0b27c2


Jan

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

* [patch] compile: gdb_stdout -> gdb_stdlog  [compile: Add one debug message]
  2015-05-16 12:19     ` [commit] " Jan Kratochvil
@ 2015-05-16 13:09       ` Jan Kratochvil
  2015-05-19 11:47         ` Pedro Alves
  0 siblings, 1 reply; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-16 13:09 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

[-- Attachment #1: Type: text/plain, Size: 283 bytes --]

On Sat, 16 May 2015 14:19:46 +0200, Jan Kratochvil wrote:
> OK but gdb/compile/ is using now only gdb_stdout; the error above is due to
> a copy-paste.  So I will send a follow-up patch to change all the other
> gdb/compile/ gdb_stdout strings to gdb_stdlog.

OK for check-in?


Jan

[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 6327 bytes --]

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

	* compile/compile-c-symbols.c (convert_symbol_sym, gcc_convert_symbol)
	(gcc_symbol_address): Change gdb_stdout to gdb_stdlog.
	* compile/compile-object-load.c (setup_sections, compile_object_load):
	Likewise.
	* compile/compile.c (compile_to_object): Likewise.

diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c
index 15efeff..455114c 100644
--- a/gdb/compile/compile-c-symbols.c
+++ b/gdb/compile/compile-c-symbols.c
@@ -338,7 +338,7 @@ convert_symbol_sym (struct compile_c_instance *context, const char *identifier,
 	  && block_found != block_static_block (block_found))
 	{
 	  if (compile_debug)
-	    fprintf_unfiltered (gdb_stdout,
+	    fprintf_unfiltered (gdb_stdlog,
 				"gcc_convert_symbol \"%s\": global symbol\n",
 				identifier);
 	  convert_one_symbol (context, global_sym, 1, 0);
@@ -346,7 +346,7 @@ convert_symbol_sym (struct compile_c_instance *context, const char *identifier,
     }
 
   if (compile_debug)
-    fprintf_unfiltered (gdb_stdout,
+    fprintf_unfiltered (gdb_stdlog,
 			"gcc_convert_symbol \"%s\": local symbol\n",
 			identifier);
   convert_one_symbol (context, sym, 0, is_local_symbol);
@@ -473,7 +473,7 @@ gcc_convert_symbol (void *datum,
   END_CATCH
 
   if (compile_debug && !found)
-    fprintf_unfiltered (gdb_stdout,
+    fprintf_unfiltered (gdb_stdlog,
 			"gcc_convert_symbol \"%s\": lookup_symbol failed\n",
 			identifier);
   return;
@@ -500,7 +500,7 @@ gcc_symbol_address (void *datum, struct gcc_c_context *gcc_context,
       if (sym != NULL && SYMBOL_CLASS (sym) == LOC_BLOCK)
 	{
 	  if (compile_debug)
-	    fprintf_unfiltered (gdb_stdout,
+	    fprintf_unfiltered (gdb_stdlog,
 				"gcc_symbol_address \"%s\": full symbol\n",
 				identifier);
 	  result = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
@@ -516,7 +516,7 @@ gcc_symbol_address (void *datum, struct gcc_c_context *gcc_context,
 	  if (msym.minsym != NULL)
 	    {
 	      if (compile_debug)
-		fprintf_unfiltered (gdb_stdout,
+		fprintf_unfiltered (gdb_stdlog,
 				    "gcc_symbol_address \"%s\": minimal "
 				    "symbol\n",
 				    identifier);
@@ -535,7 +535,7 @@ gcc_symbol_address (void *datum, struct gcc_c_context *gcc_context,
   END_CATCH
 
   if (compile_debug && !found)
-    fprintf_unfiltered (gdb_stdout,
+    fprintf_unfiltered (gdb_stdlog,
 			"gcc_symbol_address \"%s\": failed\n",
 			identifier);
   return result;
diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index 3273e47..31f4bde 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -132,7 +132,7 @@ setup_sections (bfd *abfd, asection *sect, void *data_voidp)
 	prot |= GDB_MMAP_PROT_EXEC;
 
       if (compile_debug)
-	fprintf_unfiltered (gdb_stdout,
+	fprintf_unfiltered (gdb_stdlog,
 			    "module \"%s\" section \"%s\" size %s prot %u\n",
 			    bfd_get_filename (abfd),
 			    bfd_get_section_name (abfd, sect),
@@ -155,7 +155,7 @@ setup_sections (bfd *abfd, asection *sect, void *data_voidp)
 				       data->last_prot);
 	  munmap_list_add (data->munmap_list_headp, addr, data->last_size);
 	  if (compile_debug)
-	    fprintf_unfiltered (gdb_stdout,
+	    fprintf_unfiltered (gdb_stdlog,
 				"allocated %s bytes at %s prot %u\n",
 				paddress (target_gdbarch (), data->last_size),
 				paddress (target_gdbarch (), addr),
@@ -728,7 +728,7 @@ compile_object_load (const char *object_file, const char *source_file,
       if (sym->flags != 0)
 	continue;
       if (compile_debug)
-	fprintf_unfiltered (gdb_stdout,
+	fprintf_unfiltered (gdb_stdlog,
 			    "lookup undefined ELF symbol \"%s\"\n",
 			    sym->name);
       sym->flags = BSF_GLOBAL;
@@ -773,7 +773,7 @@ compile_object_load (const char *object_file, const char *source_file,
       gdb_assert (regs_addr != 0);
       munmap_list_add (&munmap_list_head, regs_addr, TYPE_LENGTH (regs_type));
       if (compile_debug)
-	fprintf_unfiltered (gdb_stdout,
+	fprintf_unfiltered (gdb_stdlog,
 			    "allocated %s bytes at %s for registers\n",
 			    paddress (target_gdbarch (),
 				      TYPE_LENGTH (regs_type)),
@@ -799,7 +799,7 @@ compile_object_load (const char *object_file, const char *source_file,
       munmap_list_add (&munmap_list_head, out_value_addr,
 		       TYPE_LENGTH (out_value_type));
       if (compile_debug)
-	fprintf_unfiltered (gdb_stdout,
+	fprintf_unfiltered (gdb_stdlog,
 			    "allocated %s bytes at %s for printed value\n",
 			    paddress (target_gdbarch (),
 				      TYPE_LENGTH (out_value_type)),
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index fbecf8c..499c530 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -528,7 +528,7 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
 					       expr_block, expr_pc);
   make_cleanup (xfree, code);
   if (compile_debug)
-    fprintf_unfiltered (gdb_stdout, "debug output:\n\n%s", code);
+    fprintf_unfiltered (gdb_stdlog, "debug output:\n\n%s", code);
 
   os_rx = osabi_triplet_regexp (gdbarch_osabi (gdbarch));
   arch_rx = gdbarch_gnu_triplet_regexp (gdbarch);
@@ -553,9 +553,9 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
     {
       int argi;
 
-      fprintf_unfiltered (gdb_stdout, "Passing %d compiler options:\n", argc);
+      fprintf_unfiltered (gdb_stdlog, "Passing %d compiler options:\n", argc);
       for (argi = 0; argi < argc; argi++)
-	fprintf_unfiltered (gdb_stdout, "Compiler option %d: <%s>\n",
+	fprintf_unfiltered (gdb_stdlog, "Compiler option %d: <%s>\n",
 			    argi, argv[argi]);
     }
 
@@ -572,7 +572,7 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
   fclose (src);
 
   if (compile_debug)
-    fprintf_unfiltered (gdb_stdout, "source file produced: %s\n\n",
+    fprintf_unfiltered (gdb_stdlog, "source file produced: %s\n\n",
 			source_file);
 
   /* Call the compiler and start the compilation process.  */
@@ -583,7 +583,7 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
     error (_("Compilation failed."));
 
   if (compile_debug)
-    fprintf_unfiltered (gdb_stdout, "object file produced: %s\n\n",
+    fprintf_unfiltered (gdb_stdlog, "object file produced: %s\n\n",
 			object_file);
 
   discard_cleanups (inner_cleanup);

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

* [commit#2] [PATCH v5 6/7] compile: New 'compile print'
  2015-05-16 12:58     ` [commit] " Jan Kratochvil
@ 2015-05-16 13:39       ` Jan Kratochvil
  2015-05-16 14:25       ` [commit] " Patrick Palka
  1 sibling, 0 replies; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-16 13:39 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]

On Sat, 16 May 2015 14:58:14 +0200, Jan Kratochvil wrote:
> On Fri, 15 May 2015 19:04:03 +0200, Pedro Alves wrote:
> > On 05/13/2015 09:17 PM, Jan Kratochvil wrote:
> > > +  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;
> > 
> > AFAIC, retval is always gdb_type_from_ptr, and could be
> > moved out of the switch.
> 
> It was written this way as it has semantical logic - how the type is
> determined depends on TYPE_CODE_* of that object.  Just accidentally in this
> case it is the same.  I do not do performance micro-optimizations which
> compiler will do anyway.  But I have changed it.

I see I made a thinko in that optimization, therefore checked-in this fix-up:
	4d18dfad9edf822df205edc2c1fe3fe9f1e467b8


Jan

[-- Attachment #2: Type: message/rfc822, Size: 2046 bytes --]

From: Jan Kratochvil <jan.kratochvil@redhat.com>
Subject: [PATCH] compile: Fix detected inferior type
Date: Sat, 16 May 2015 15:36:33 +0200

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

	* compile/compile-object-load.c (get_out_value_type): Fix returned type.
---
 gdb/ChangeLog                     | 4 ++++
 gdb/compile/compile-object-load.c | 5 +++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3868599..e5de834 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,4 +1,8 @@
 2015-05-16  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* compile/compile-object-load.c (get_out_value_type): Fix returned type.
+
+2015-05-16  Jan Kratochvil  <jan.kratochvil@redhat.com>
 	    Phil Muldoon  <pmuldoon@redhat.com>
 
 	* NEWS (Changes since GDB 7.9): Add compile print.
diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index ed5ef88..e2d8f2f 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -372,7 +372,7 @@ 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;
+  struct type *gdb_ptr_type, *gdb_type_from_ptr, *gdb_type, *retval;
   const struct block *block;
   const struct blockvector *bv;
   int nblocks = 0;
@@ -440,6 +440,7 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile,
 	   TYPE_CODE (gdb_type_from_ptr), COMPILE_I_EXPR_VAL,
 	   objfile_name (objfile));
   
+  retval = gdb_type_from_ptr;
   switch (TYPE_CODE (gdb_type_from_ptr))
     {
     case TYPE_CODE_ARRAY:
@@ -461,7 +462,7 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile,
 	   objfile_name (objfile));
   if (scope == COMPILE_I_PRINT_ADDRESS_SCOPE)
     return NULL;
-  return gdb_type_from_ptr;
+  return retval;
 }
 
 /* Fetch the type of first parameter of FUNC_SYM.
-- 
2.1.0

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

* Re: [commit] [PATCH v5 6/7] compile: New 'compile print'
  2015-05-16 12:58     ` [commit] " Jan Kratochvil
  2015-05-16 13:39       ` [commit#2] " Jan Kratochvil
@ 2015-05-16 14:25       ` Patrick Palka
  2015-05-16 14:39         ` [obv] " Jan Kratochvil
  1 sibling, 1 reply; 25+ messages in thread
From: Patrick Palka @ 2015-05-16 14:25 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Pedro Alves, gdb-patches, Phil Muldoon

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1402 bytes --]

GCC 4.9 emits -Wmaybe-uninitialized warnings for the function get_out_value_type():


/home/patrick/code/binutils-gdb/gdb/compile/compile-object-load.c: In function Β‘compile_object_loadΒ’:
/home/patrick/code/binutils-gdb/gdb/compile/compile-object-load.c:419:20: error: Β‘blockΒ’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
    gdb_ptr_type_sym = block_lookup_symbol (block, COMPILE_I_EXPR_PTR_TYPE,
                     ^
/home/patrick/code/binutils-gdb/gdb/compile/compile-object-load.c:376:23: note: Β‘blockΒ’ was declared here
    const struct block *block;
                        ^
/home/patrick/code/binutils-gdb/gdb/compile/compile-object-load.c:416:12: error: Β‘gdb_val_symΒ’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
    gdb_type = SYMBOL_TYPE (gdb_val_sym);
             ^
/home/patrick/code/binutils-gdb/gdb/compile/compile-object-load.c:374:37: note: Β‘gdb_val_symΒ’ was declared here
    struct symbol *gdb_ptr_type_sym, *gdb_val_sym;
                                      ^
/home/patrick/code/binutils-gdb/gdb/compile/compile-object-load.c:406:10: error: Β‘functionΒ’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
        if (function != NULL
           ^
/home/patrick/code/binutils-gdb/gdb/compile/compile-object-load.c:387:22: note: Β‘functionΒ’ was declared here
        struct symbol *function;

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

* [obv] [commit] [PATCH v5 6/7] compile: New 'compile print'
  2015-05-16 14:25       ` [commit] " Patrick Palka
@ 2015-05-16 14:39         ` Jan Kratochvil
  0 siblings, 0 replies; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-16 14:39 UTC (permalink / raw)
  To: Patrick Palka; +Cc: Pedro Alves, gdb-patches, Phil Muldoon

[-- Attachment #1: Type: text/plain, Size: 209 bytes --]

On Sat, 16 May 2015 16:24:53 +0200, Patrick Palka wrote:
> GCC 4.9 emits -Wmaybe-uninitialized warnings for the function get_out_value_type():

Checked in as obvious, I did not test it with -O2.


Thanks,
Jan

[-- Attachment #2: Type: message/rfc822, Size: 2158 bytes --]

From: Jan Kratochvil <jan.kratochvil@redhat.com>
Subject: [PATCH] compile: Fix uninitialized variable compiler warnings
Date: Sat, 16 May 2015 16:36:44 +0200

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

	* compile/compile-object-load.c (get_out_value_type): Fix uninitialized
	variable compiler warnings.
---
 gdb/ChangeLog                     | 5 +++++
 gdb/compile/compile-object-load.c | 9 ++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e5de834..500eae1 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2015-05-16  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
+	* compile/compile-object-load.c (get_out_value_type): Fix uninitialized
+	variable compiler warnings.
+
+2015-05-16  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
 	* compile/compile-object-load.c (get_out_value_type): Fix returned type.
 
 2015-05-16  Jan Kratochvil  <jan.kratochvil@redhat.com>
diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index e2d8f2f..67bc69a 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -371,9 +371,12 @@ 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 symbol *gdb_ptr_type_sym;
+  /* Initialize it just to avoid a GCC false warning.  */
+  struct symbol *gdb_val_sym = NULL;
   struct type *gdb_ptr_type, *gdb_type_from_ptr, *gdb_type, *retval;
-  const struct block *block;
+  /* Initialize it just to avoid a GCC false warning.  */
+  const struct block *block = NULL;
   const struct blockvector *bv;
   int nblocks = 0;
   int block_loop = 0;
@@ -384,7 +387,7 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile,
   gdb_ptr_type_sym = NULL;
   for (block_loop = 0; block_loop < nblocks; block_loop++)
     {
-      struct symbol *function;
+      struct symbol *function = NULL;
       const struct block *function_block;
 
       block = BLOCKVECTOR_BLOCK (bv, block_loop);
-- 
2.1.0

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

* Re: [patch] compile: gdb_stdout -> gdb_stdlog  [compile: Add one debug message]
  2015-05-16 13:09       ` [patch] compile: gdb_stdout -> gdb_stdlog [compile: Add one debug message] Jan Kratochvil
@ 2015-05-19 11:47         ` Pedro Alves
  2015-05-19 12:28           ` [commit] " Jan Kratochvil
  0 siblings, 1 reply; 25+ messages in thread
From: Pedro Alves @ 2015-05-19 11:47 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches, Phil Muldoon

On 05/16/2015 02:09 PM, Jan Kratochvil wrote:
> On Sat, 16 May 2015 14:19:46 +0200, Jan Kratochvil wrote:
>> OK but gdb/compile/ is using now only gdb_stdout; the error above is due to
>> a copy-paste.  So I will send a follow-up patch to change all the other
>> gdb/compile/ gdb_stdout strings to gdb_stdlog.
> 
> OK for check-in?

OK.

Thanks,
Pedro Alves

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

* [commit] [patch] compile: gdb_stdout -> gdb_stdlog  [compile: Add one debug message]
  2015-05-19 11:47         ` Pedro Alves
@ 2015-05-19 12:28           ` Jan Kratochvil
  0 siblings, 0 replies; 25+ messages in thread
From: Jan Kratochvil @ 2015-05-19 12:28 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Phil Muldoon

On Tue, 19 May 2015 13:47:12 +0200, Pedro Alves wrote:
> On 05/16/2015 02:09 PM, Jan Kratochvil wrote:
> > On Sat, 16 May 2015 14:19:46 +0200, Jan Kratochvil wrote:
> >> OK but gdb/compile/ is using now only gdb_stdout; the error above is due to
> >> a copy-paste.  So I will send a follow-up patch to change all the other
> >> gdb/compile/ gdb_stdout strings to gdb_stdlog.
> > 
> > OK for check-in?
> 
> OK.

Checked in:
	a40635885c50f14782d80251a8966bf4dd271f76


Jan

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

end of thread, other threads:[~2015-05-19 12:28 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-13 20:16 [PATCH v5 0/7] compile: compile print (&unmap) Jan Kratochvil
2015-05-13 20:16 ` [PATCH v5 2/7] Code cleanup: Make parts of print_command_1 public Jan Kratochvil
2015-05-16 12:27   ` [commit] " Jan Kratochvil
2015-05-13 20:16 ` [PATCH v5 1/7] compile: Add one debug message Jan Kratochvil
2015-05-15 16:49   ` Pedro Alves
2015-05-16 12:19     ` [commit] " Jan Kratochvil
2015-05-16 13:09       ` [patch] compile: gdb_stdout -> gdb_stdlog [compile: Add one debug message] Jan Kratochvil
2015-05-19 11:47         ` Pedro Alves
2015-05-19 12:28           ` [commit] " Jan Kratochvil
2015-05-13 20:16 ` [PATCH v5 4/7] compile: Use -Wall, not -w Jan Kratochvil
2015-05-15 16:49   ` Pedro Alves
2015-05-16 12:43     ` [commit] " Jan Kratochvil
2015-05-13 20:16 ` [PATCH v5 3/7] compile: Distribute scope, add scope_data Jan Kratochvil
2015-05-15 16:49   ` Pedro Alves
2015-05-16 12:39     ` [commit] " Jan Kratochvil
2015-05-13 20:17 ` [PATCH v5 6/7] compile: New 'compile print' Jan Kratochvil
2015-05-15 17:04   ` Pedro Alves
2015-05-16 12:58     ` [commit] " Jan Kratochvil
2015-05-16 13:39       ` [commit#2] " Jan Kratochvil
2015-05-16 14:25       ` [commit] " Patrick Palka
2015-05-16 14:39         ` [obv] " Jan Kratochvil
2015-05-13 20:17 ` [PATCH v5 5/7] Code cleanup: compile: func_addr -> func_sym Jan Kratochvil
2015-05-15 16:51   ` Pedro Alves
2015-05-16 12:45     ` [commit] " Jan Kratochvil
2015-05-13 20:17 ` [PATCH v5 7/7] RFC only: compile: Use also inferior munmap Jan Kratochvil

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