[PATCH] jit : Generate debug info for variables

[PATCH] jit : Generate debug info for variables

[Petter Tomner]

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

Hi,

This is a patch to generate debug info for local variables as well as globals. 
With this, "ptype foo", "info variables", "info locals" etc works when debugging in GDB.

Finalizing of global variable declares are moved to after locations are handled and done
as Fortran, C, Go etc do it. Also, primitive types have their TYPE_NAME set for debug info
on types to work.

Below are the patch, and I attached a testcase. Since it requires GDB to run it might
not be suitable? Make check-jit runs fine on Debian x64.

Regards,


From d77e77104024c7ae9ce31b419dad1f0a5801fda7 Mon Sep 17 00:00:00 2001
From: Petter Tomner <tomner@kth.se>
Date: Mon, 30 Aug 2021 01:44:07 +0200
Subject: [PATCH 1/2] libgccjit: Generate debug info for variables

Finalize declares via available helpers after location is set. Set
TYPE_NAME of primitives and friends to "int" etc. Debug info is now
set properly for variables.

2021-08-31	Petter Tomner	<tomner@kth.se>

gcc/jit
	jit-playback.c
	jit-playback.h
gcc/testsuite/jit.dg/
	test-error-array-bounds.c: Array is not unsigned
---
 gcc/jit/jit-playback.c                        | 76 ++++++++++++++-----
 gcc/jit/jit-playback.h                        |  5 ++
 .../jit.dg/test-error-array-bounds.c          |  2 +-
 3 files changed, 65 insertions(+), 18 deletions(-)

diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index 79ac525e5df..74321b0946a 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -165,7 +165,8 @@ gt_ggc_mx ()
 
 /* Given an enum gcc_jit_types value, get a "tree" type.  */
 
-static tree
+tree
+playback::context::
 get_tree_node_for_type (enum gcc_jit_types type_)
 {
   switch (type_)
@@ -192,11 +193,7 @@ get_tree_node_for_type (enum gcc_jit_types type_)
       return short_unsigned_type_node;
 
     case GCC_JIT_TYPE_CONST_CHAR_PTR:
-      {
-	tree const_char = build_qualified_type (char_type_node,
-						TYPE_QUAL_CONST);
-	return build_pointer_type (const_char);
-      }
+      return m_const_char_ptr;
 
     case GCC_JIT_TYPE_INT:
       return integer_type_node;
@@ -579,10 +576,6 @@ playback::lvalue *
 playback::context::
 global_finalize_lvalue (tree inner)
 {
-  varpool_node::get_create (inner);
-
-  varpool_node::finalize_decl (inner);
-
   m_globals.safe_push (inner);
 
   return new lvalue (this, inner);
@@ -2952,9 +2945,7 @@ replay ()
 {
   JIT_LOG_SCOPE (get_logger ());
 
-  m_const_char_ptr
-    = build_pointer_type (build_qualified_type (char_type_node,
-						TYPE_QUAL_CONST));
+  init_types ();
 
   /* Replay the recorded events:  */
   timevar_push (TV_JIT_REPLAY);
@@ -2984,15 +2975,22 @@ replay ()
     {
       int i;
       function *func;
-
+      tree global;
       /* No GC can happen yet; process the cached source locations.  */
       handle_locations ();
 
+      /* Finalize globals. See how FORTRAN 95 does it in gfc_be_parse_file()
+         for a simple reference. */
+      FOR_EACH_VEC_ELT (m_globals, i, global)
+    rest_of_decl_compilation (global, true, true);
+
+      wrapup_global_declarations (m_globals.address(), m_globals.length());
+
       /* We've now created tree nodes for the stmts in the various blocks
-	 in each function, but we haven't built each function's single stmt
-	 list yet.  Do so now.  */
+	       in each function, but we haven't built each function's single stmt
+	       list yet.  Do so now.  */
       FOR_EACH_VEC_ELT (m_functions, i, func)
-	func->build_stmt_list ();
+	  func->build_stmt_list ();
 
       /* No GC can have happened yet.  */
 
@@ -3081,6 +3079,50 @@ location_comparator (const void *lhs, const void *rhs)
   return loc_lhs->get_column_num () - loc_rhs->get_column_num ();
 }
 
+/* Initialize the NAME_TYPE of the primitive types as well as some
+   others. */
+void
+playback::context::
+init_types ()
+{
+  /* See lto_init() in lto-lang.c or void visit (TypeBasic *t) in D's types.cc 
+     for reference. If TYPE_NAME is not set, debug info will not contain types */
+#define NAME_TYPE(t,n) \
+if (t) \
+  TYPE_NAME (t) = build_decl (UNKNOWN_LOCATION, TYPE_DECL, \
+                              get_identifier (n), t)
+
+  NAME_TYPE (integer_type_node, "int");
+  NAME_TYPE (char_type_node, "char");
+  NAME_TYPE (long_integer_type_node, "long int");
+  NAME_TYPE (unsigned_type_node, "unsigned int");
+  NAME_TYPE (long_unsigned_type_node, "long unsigned int");
+  NAME_TYPE (long_long_integer_type_node, "long long int");
+  NAME_TYPE (long_long_unsigned_type_node, "long long unsigned int");
+  NAME_TYPE (short_integer_type_node, "short int");
+  NAME_TYPE (short_unsigned_type_node, "short unsigned int");
+  if (signed_char_type_node != char_type_node)
+    NAME_TYPE (signed_char_type_node, "signed char");
+  if (unsigned_char_type_node != char_type_node)
+    NAME_TYPE (unsigned_char_type_node, "unsigned char");
+  NAME_TYPE (float_type_node, "float");
+  NAME_TYPE (double_type_node, "double");
+  NAME_TYPE (long_double_type_node, "long double");
+  NAME_TYPE (void_type_node, "void");
+  NAME_TYPE (boolean_type_node, "bool");
+  NAME_TYPE (complex_float_type_node, "complex float");
+  NAME_TYPE (complex_double_type_node, "complex double");
+  NAME_TYPE (complex_long_double_type_node, "complex long double");
+
+  m_const_char_ptr = build_pointer_type(
+    build_qualified_type (char_type_node, TYPE_QUAL_CONST));
+
+  NAME_TYPE (m_const_char_ptr, "char");
+  NAME_TYPE (size_type_node, "size_t");
+  NAME_TYPE (fileptr_type_node, "FILE");
+#undef NAME_TYPE
+}
+
 /* Our API allows locations to be created in arbitrary orders, but the
    linemap API requires locations to be created in ascending order
    as if we were tokenizing files.
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index 825a3e172e9..f670c9e81df 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -271,8 +271,13 @@ private:
   source_file *
   get_source_file (const char *filename);
 
+  tree
+  get_tree_node_for_type (enum gcc_jit_types type_);
+
   void handle_locations ();
 
+  void init_types ();
+
   const char * get_path_c_file () const;
   const char * get_path_s_file () const;
   const char * get_path_so_file () const;
diff --git a/gcc/testsuite/jit.dg/test-error-array-bounds.c b/gcc/testsuite/jit.dg/test-error-array-bounds.c
index cd9361fba1d..b6c0ee526d4 100644
--- a/gcc/testsuite/jit.dg/test-error-array-bounds.c
+++ b/gcc/testsuite/jit.dg/test-error-array-bounds.c
@@ -70,5 +70,5 @@ verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
   /* ...and that the message was captured by the API.  */
   CHECK_STRING_VALUE (gcc_jit_context_get_first_error (ctxt),
 		      "array subscript 10 is above array bounds of"
-		      " 'unsigned char[10]' [-Warray-bounds]");
+		      " 'char[10]' [-Warray-bounds]");
 }
-- 
2.20.1



[-- Attachment #2: 0002-Adding-unit-test-for-weak-linkage.patch --]
[-- Type: application/octet-stream, Size: 13006 bytes --]

From ba058772b93dcb78db5b04a46c8d3fbd9e319e80 Mon Sep 17 00:00:00 2001
From: Petter Tomner <tomner@kth.se>
Date: Sun, 8 Aug 2021 02:02:02 +0200
Subject: [PATCH 2/3] Adding unit test for weak linkage

---
 gcc/testsuite/jit.dg/all-non-failing-tests.h  |  12 +-
 gcc/testsuite/jit.dg/harness.h                |   5 +
 gcc/testsuite/jit.dg/jit.exp                  |  69 ++++++
 .../jit.dg/test-compile-weak-symbols.c        | 207 ++++++++++++++++++
 gcc/testsuite/jit.dg/verify-weak-linkage.c    |  21 ++
 5 files changed, 313 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/jit.dg/test-compile-weak-symbols.c
 create mode 100644 gcc/testsuite/jit.dg/verify-weak-linkage.c

diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 84ef54a0386..4da34905a3d 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -105,6 +105,13 @@
 #undef create_code
 #undef verify_code
 
+/* test-compile-weak-symbols.c */
+#define create_code create_code_weak_symbols
+#define verify_code verify_code_weak_symbols
+#include "test-compile-weak-symbols.c"
+#undef create_code
+#undef verify_code
+
 /* test-compound-assignment.c */
 #define create_code create_code_compound_assignment
 #define verify_code verify_code_compound_assignment
@@ -454,7 +461,10 @@ const struct testcase testcases[] = {
    verify_code_version},
   {"volatile",
    create_code_volatile,
-   verify_code_volatile}
+   verify_code_volatile},
+  {"weak_symbols",
+   create_code_weak_symbols,
+   verify_code_weak_symbols}
 };
 
 const int num_testcases = (sizeof (testcases) / sizeof (testcases[0]));
diff --git a/gcc/testsuite/jit.dg/harness.h b/gcc/testsuite/jit.dg/harness.h
index 6b59fb57aae..84c248303f4 100644
--- a/gcc/testsuite/jit.dg/harness.h
+++ b/gcc/testsuite/jit.dg/harness.h
@@ -331,6 +331,10 @@ dump_reproducer (gcc_jit_context *ctxt, const char *argv0)
   free (reproducer_name);
 }
 
+static void
+test_jit (const char *argv0, void *user_data);
+
+#ifndef TEST_PROVIDES_TEST_JIT
 /* Run one iteration of the test.  */
 static void
 test_jit (const char *argv0, void *user_data)
@@ -383,6 +387,7 @@ test_jit (const char *argv0, void *user_data)
   if (logfile)
     fclose (logfile);
 }
+#endif /* #ifndef TEST_PROVIDES_TEST_JIT */
 #endif /* #ifndef TEST_ESCHEWS_TEST_JIT */
 
 /* We want to prefix all unit test results with the test, but dejagnu.exp's
diff --git a/gcc/testsuite/jit.dg/jit.exp b/gcc/testsuite/jit.dg/jit.exp
index 9af87f9c6ad..f26970a41e6 100644
--- a/gcc/testsuite/jit.dg/jit.exp
+++ b/gcc/testsuite/jit.dg/jit.exp
@@ -692,6 +692,19 @@ proc jit-verify-output-file-was-created { args } {
     }
 }
 
+proc jit-verify-output-files-were-created { args } {
+    verbose "jit-verify-output-files-were-created: $args"
+
+    foreach output_filename $args {
+        # Verify that the expected files was written out
+        if { [file exists $output_filename] == 1} {
+        pass "$output_filename exists"
+        } else {
+        fail "$output_filename does not exist"
+        }
+    }
+}
+
 # Verify that the given file exists, and is executable.
 # Attempt to execute it, and verify that its stdout matches
 # the given regex.
@@ -840,6 +853,62 @@ proc jit-verify-object { args } {
     jit-run-executable ${executable_from_obj} ${dg-output-text}
 }
 
+# Compile 'sources' and link in 'objects' and execute the program 
+# and check that its output matches 'expected_output'.
+#
+# The path to the JIT test source folder is prepended to each
+# word in 'source'.
+#
+# For use with test-compile-weak-symbols.c testcase.
+proc jit-verify-sources-objects { expected_output sources objects } {
+    global srcdir
+    global subdir
+
+    verbose "jit-verify-objects: $expected_output $sources $objects"
+    set dg-output-text $expected_output
+
+    # prepend the path to the folder with the JIT tests
+    # to each source
+    set rel_path "$srcdir/$subdir/"
+    set srcs [lmap i $sources { string cat $rel_path $i }]
+    verbose "sources full path: ${srcs}"
+
+    upvar 2 name name
+    verbose "name: $name"
+
+    upvar 2 prog prog
+    verbose "prog: $prog"
+
+    # Name the linked executable as the first source
+    # with ".exe" appended.
+    set executable_from_obj [lindex $sources 0].exe
+    verbose "  executable_from_obj: ${executable_from_obj}"
+
+    # Invoke the driver to link the .o file to the .exe
+    set comp_output [gcc_target_compile \
+            "${srcs} ${objects}" \
+             ${executable_from_obj} \
+             "executable" \
+             "{}"]
+    if ![jit_check_compile \
+         "$name" \
+         "link of ${executable_from_obj}" \
+         ${executable_from_obj} \
+         $comp_output] then {
+      return
+    }
+
+    # Verify that the executable was created.
+    if { [file exists $executable_from_obj] == 1} {
+    pass "$executable_from_obj exists"
+    } else {
+    fail "$executable_from_obj does not exist"
+    }
+
+    # Run it and verify that the output matches the regex.
+    jit-run-executable ${executable_from_obj} ${dg-output-text}
+}
+
 # Assuming that a .so file has been written out named
 # OUTPUT_FILENAME, build a test executable to use it,
 # and try to run the result.
diff --git a/gcc/testsuite/jit.dg/test-compile-weak-symbols.c b/gcc/testsuite/jit.dg/test-compile-weak-symbols.c
new file mode 100644
index 00000000000..5a746850341
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-compile-weak-symbols.c
@@ -0,0 +1,207 @@
+/* Essentially the goal of the test is to create a two object files:
+
+   weaksymtest_foobar_weak.o:
+    int __attribute__((weak)) weaksymtest_foo() {return 1;}
+    int __attribute__((weak)) weaksymtest_bar[] = {1};
+
+   weaksymtest_foobar.o:   
+    int weaksymtest_foo() {return 2;}
+    int weaksymtest_bar[] = {2};
+
+   and then link them together and verify that the weak symbol is actually 
+   weak and not the one used after linking.
+
+   Since multiple outfiles are needed this test uses TEST_COMPILING_TO_FILE
+   in a slightly different way then the other tests. It provides its own
+   test_jit() via TEST_PROVIDES_TEST_JIT.
+   
+   This test is dependent on verify-weak-linkage.c which is a main function
+   with declarations of foo and bar and that outputs foo() and bar[0] to 
+   stdout. */
+
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "libgccjit.h"
+
+/* When included in all-non-failing-tests.h with COMBINED_TEST defined 
+   this testfile does not produce any output files. */
+#ifndef COMBINED_TEST
+#define TEST_COMPILING_TO_FILE
+#define TEST_PROVIDES_TEST_JIT
+#include "harness.h"
+#undef TEST_PROVIDES_TEST_JIT
+#else
+#include "harness.h"
+#endif
+
+extern void
+create_code_weak_or_normal (gcc_jit_context *ctxt, int create_weak);
+
+typedef int (*fooptr)(void);
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+  (void) ctxt; /* Not used */
+
+  fooptr foo = gcc_jit_result_get_code (result, "weaksymtest_foo");
+  int *bar = gcc_jit_result_get_global (result, "weaksymtest_bar");
+
+  /* Just do a simple test accessing the weak symbols from the
+     JIT context */
+  CHECK_VALUE (*bar, 1);
+  CHECK_VALUE (foo(), 1);
+}
+
+/* Do not provide test_jit() when included in all-non-failing-test.h */
+#ifndef COMBINED_TEST
+/* Runs one iteration of the test. */
+static void
+test_jit (const char *argv0, void *user_data)
+{
+  gcc_jit_context *ctxt_normal, *ctxt_weak;
+  FILE *logfile_normal, *logfile_weak;
+  char *argv0_weak, *argv0_normal;
+
+  (void) user_data; /* Not used */
+
+  argv0_weak = concat_strings (argv0, ".weak");
+  argv0_normal = concat_strings (argv0, ".normal");
+
+  unlink ("weaksymtest_foobar_weak.o");
+  unlink ("weaksymtest_foobar.o");
+  
+  ctxt_weak = gcc_jit_context_acquire ();
+  ctxt_normal = gcc_jit_context_acquire ();
+  if (!ctxt_normal || !ctxt_weak)
+    {
+      fail ("gcc_jit_context_acquire failed");
+      return;
+    }
+  
+  logfile_weak = set_up_logging (ctxt_weak, argv0_weak);
+  logfile_normal = set_up_logging (ctxt_normal, argv0_normal);
+
+  set_options (ctxt_normal, argv0_normal);
+  set_options (ctxt_weak, argv0_weak);
+
+  free (argv0_weak); argv0_weak = 0;
+  free (argv0_normal); argv0_normal = 0;
+
+  create_code_weak_or_normal (ctxt_normal, 0);
+  create_code_weak_or_normal (ctxt_weak, 1);
+
+  /* Note that argv0 is used instead of argv0_weak since 
+     jit-dg-test expects a certain name of the dumpfile. */
+  dump_reproducer (ctxt_weak, argv0);
+
+  gcc_jit_context_compile_to_file (ctxt_normal,
+           GCC_JIT_OUTPUT_KIND_OBJECT_FILE,
+           "weaksymtest_foobar_n.o");
+  gcc_jit_context_compile_to_file (ctxt_weak,
+           GCC_JIT_OUTPUT_KIND_OBJECT_FILE,
+           "weaksymtest_foobar_w.o");
+
+  /* ld treats weak symbols in shared libraries like
+     normal symbols unless env.var. LD_DYNAMIC_WEAK is set,
+     so dynamic libraries are a bit awkward to test in
+     a unit test like we do here. However it is easy 
+     to check manually that the symbols are weak with 
+     objdump etc. */
+#if 0
+  unlink ("weaksymtest_foobar_weak.so");
+  unlink ("weaksymtest_foobar.so");
+  gcc_jit_context_compile_to_file (ctxt_weak,
+                  GCC_JIT_OUTPUT_KIND_DYNAMIC_LIBRARY,
+                  "libweaksymtest_foobar_w.so");
+  gcc_jit_context_compile_to_file (ctxt_normal,
+                  GCC_JIT_OUTPUT_KIND_DYNAMIC_LIBRARY,
+                  "libweaksymtest_foobar_n.so");
+#endif
+
+  /* For checking wether the c-like dump looks ok */
+#if 0
+  unlink("libweaksymtest_foobar_w.c");
+  gcc_jit_context_dump_to_file (ctxt_weak, "libweaksymtest_foobar_w.c", 0);
+#endif
+
+  CHECK_NO_ERRORS (ctxt_normal);
+  CHECK_NO_ERRORS (ctxt_weak);
+
+  gcc_jit_result *result_weak = gcc_jit_context_compile (ctxt_weak);
+  CHECK_NON_NULL (result_weak);
+  /* Just does some simple access and value check of the weak symbols */
+  CHECK_NO_ERRORS (ctxt_weak);
+  verify_code (ctxt_weak, result_weak);
+
+  gcc_jit_result_release (result_weak);
+
+  gcc_jit_context_release (ctxt_normal);
+  gcc_jit_context_release (ctxt_weak);
+
+  if (logfile_normal)
+    fclose (logfile_normal);
+  if (logfile_weak)
+    fclose (logfile_weak);
+}
+#endif /* #ifndef COMBINED_TEST */
+
+extern void
+create_code_weak_or_normal (gcc_jit_context *ctxt, int create_weak)
+{
+  enum gcc_jit_function_kind fn_kind;
+  enum gcc_jit_global_kind var_kind;
+  int value;
+  int arr[1];
+
+  if (create_weak) 
+    {
+      fn_kind = GCC_JIT_FUNCTION_EXPORTED_WEAK;
+      var_kind = GCC_JIT_GLOBAL_EXPORTED_WEAK;
+      value = 1;
+    } 
+  else 
+    {
+      fn_kind = GCC_JIT_FUNCTION_EXPORTED;
+      var_kind = GCC_JIT_GLOBAL_EXPORTED;
+      value = 2;
+    }
+
+  /* int __attribute__((weak)) foo() { return 1;}
+     int foo() { return 2;} */
+  gcc_jit_type *int_type = 
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+  gcc_jit_function *foo = 
+    gcc_jit_context_new_function (ctxt, 0, fn_kind, int_type, 
+    "weaksymtest_foo", 0,0,0);
+  gcc_jit_block *block1 =
+    gcc_jit_function_new_block (foo, "block1");
+  gcc_jit_block_end_with_return (block1, 0, 
+    gcc_jit_context_new_rvalue_from_int (ctxt, int_type, value));
+  
+  /* int __attribute__((weak)) bar[] = {1};
+     int bar[] = {2}; */
+  gcc_jit_lvalue *bar = gcc_jit_context_new_global (ctxt, 0, var_kind, 
+    gcc_jit_context_new_array_type (ctxt, 0, int_type, 1), "weaksymtest_bar");
+  
+  arr[0] = value;
+  gcc_jit_global_set_initializer (bar, (const void *)arr, sizeof(int));
+}
+
+/* This function is used in all-non-failing-test.h and just creates the
+   weak context */
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+  (void) user_data;
+  create_code_weak_or_normal (ctxt, 1);
+}
+
+/* { dg-final { jit-verify-output-files-were-created "weaksymtest_foobar_w.o" "weaksymtest_foobar_n.o"} } */
+
+/* jit-verify-sources-objects compiles "verify-weak-linkage.c" with the object files
+   and checks that the stdout of the resulting executable matches "2\n2\n" which 
+   indicates that the weak symbols are not used. */
+
+/* { dg-final { jit-verify-sources-objects "2\\n2\\n" "verify-weak-linkage.c" "weaksymtest_foobar_w.o weaksymtest_foobar_n.o"} } */
diff --git a/gcc/testsuite/jit.dg/verify-weak-linkage.c b/gcc/testsuite/jit.dg/verify-weak-linkage.c
new file mode 100644
index 00000000000..c3ac1b9a50d
--- /dev/null
+++ b/gcc/testsuite/jit.dg/verify-weak-linkage.c
@@ -0,0 +1,21 @@
+/* For use with jit-verify-sources-objects
+   used by test-compile-weak-symbols.c
+
+   This file is linked to two object files . One with
+   weak symbols, one with normal. It prints the value 
+   of bar[0] and foo() and should print 2\n2\n if the
+   weak symbols are not linked in. */
+
+#include <stdio.h>
+
+extern int weaksymtest_bar[1];
+int weaksymtest_foo();
+
+int
+main (int argc, char **argv)
+{
+  printf ("%d\n", weaksymtest_foo());
+  printf ("%d\n", weaksymtest_bar[0]);
+  return 0;
+}
+
-- 
2.20.1

Sv: [PATCH] jit : Generate debug info for variables

[Petter Tomner]

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

Well I seemed to have attached the wrong testcase. Here is the proper one attached.

Regards,

-----Ursprungligt meddelande-----
Från: Petter Tomner 
Skickat: den 31 augusti 2021 02:14
Till: gcc-patches@gcc.gnu.org; jit@gcc.gnu.org
Ämne: [PATCH] jit : Generate debug info for variables

Hi,

This is a patch to generate debug info for local variables as well as globals. 
With this, "ptype foo", "info variables", "info locals" etc works when debugging in GDB.

Finalizing of global variable declares are moved to after locations are handled and done
as Fortran, C, Go etc do it. Also, primitive types have their TYPE_NAME set for debug info
on types to work.

Below are the patch, and I attached a testcase. Since it requires GDB to run it might
not be suitable? Make check-jit runs fine on Debian x64.

Regards,

[-- Attachment #2: 0002-libgccjit-Test-cases-for-debug-info.patch --]
[-- Type: application/octet-stream, Size: 4668 bytes --]

From 6a5d24cbe80429d19042e643bd4c23940cd185fa Mon Sep 17 00:00:00 2001
From: Petter Tomner <tomner@kth.se>
Date: Mon, 30 Aug 2021 01:45:11 +0200
Subject: [PATCH 2/2] libgccjit: Test cases for debug info

Assure that debug info is available for variables.

gcc/testsuite/jit.dg/
	jit.exp: Helper function
	test-debuginfo.c
---
 gcc/testsuite/jit.dg/jit.exp          | 23 +++++++++
 gcc/testsuite/jit.dg/test-debuginfo.c | 72 +++++++++++++++++++++++++++
 2 files changed, 95 insertions(+)
 create mode 100644 gcc/testsuite/jit.dg/test-debuginfo.c

diff --git a/gcc/testsuite/jit.dg/jit.exp b/gcc/testsuite/jit.dg/jit.exp
index 005ba01601a..36c574dbca1 100644
--- a/gcc/testsuite/jit.dg/jit.exp
+++ b/gcc/testsuite/jit.dg/jit.exp
@@ -377,6 +377,29 @@ proc dg-jit-set-exe-params { args } {
     }
 }
 
+proc jit-check-debug-info { obj_file cmds match } {
+    verbose "Checking debug info for $obj_file with match: $match"
+
+    if { [catch {exec gdb -v} fid] } {
+        verbose "No gdb seems to be in path. Can't check debug info. Reporting expected fail."
+        xfail "No gdb seems to be in path. Can't check debug info"
+        return
+    }
+
+    spawn gdb $obj_file
+
+    foreach cmd $cmds {
+        send $cmd
+    }
+    expect {
+        -re $match { pass OK }
+        default { fail FAIL }
+    }
+
+    # Quit gdb
+    send q
+}
+
 proc jit-dg-test { prog do_what extra_tool_flags } {
     verbose "within jit-dg-test..."
     verbose "  prog: $prog"
diff --git a/gcc/testsuite/jit.dg/test-debuginfo.c b/gcc/testsuite/jit.dg/test-debuginfo.c
new file mode 100644
index 00000000000..0af5779fdd1
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-debuginfo.c
@@ -0,0 +1,72 @@
+/* Essentially this test checks that debug info are generated for globals
+   locals and functions, including type info.  The comment bellow is used
+   as fake code (does not affect the test, use for manual debugging). */
+/*
+int a_global_for_test_debuginfo;
+int main (int argc, char **argv)
+{
+    int a_local_for_test_debuginfo = 2;
+    return a_global_for_test_debuginfo + a_local_for_test_debuginfo;
+}
+*/
+#include "libgccjit.h"
+
+/* We don't want set_options() in harness.h to set -O3 so our little local
+   is optimized away. */
+#define TEST_ESCHEWS_SET_OPTIONS
+static void set_options (gcc_jit_context *ctxt, const char *argv0)
+{
+    gcc_jit_context_set_bool_option(ctxt, GCC_JIT_BOOL_OPTION_DEBUGINFO, 1);
+}
+
+#define TEST_COMPILING_TO_FILE
+#define OUTPUT_KIND      GCC_JIT_OUTPUT_KIND_EXECUTABLE
+#define OUTPUT_FILENAME  "jit-debuginfo.o"
+#include "harness.h"
+
+#define LOC(row, col) gcc_jit_context_new_location(ctxt, "test-debuginfo.c", row, col)
+
+void
+create_code (gcc_jit_context *ctxt, void* p)
+{
+  gcc_jit_type *int_type = gcc_jit_context_get_type(ctxt, GCC_JIT_TYPE_INT);
+
+  gcc_jit_lvalue *bar = gcc_jit_context_new_global(ctxt, 
+    LOC(5,1), GCC_JIT_GLOBAL_EXPORTED, 
+    int_type, "a_global_for_test_debuginfo");
+
+  gcc_jit_param *argc_para = gcc_jit_context_new_param(ctxt, LOC(6,15), 
+    int_type, "argc");
+  gcc_jit_param *argv_para = gcc_jit_context_new_param(ctxt, LOC(6,28), 
+    gcc_jit_type_get_pointer(
+      gcc_jit_type_get_pointer(
+        gcc_jit_context_get_type(ctxt, GCC_JIT_TYPE_CHAR))),
+    "argc");
+
+  gcc_jit_param *params[] = {argc_para, argv_para};
+
+  gcc_jit_function *foo_fn = gcc_jit_context_new_function(ctxt, LOC(6,5), 
+    GCC_JIT_FUNCTION_EXPORTED, int_type, "main", 2, params, 0);
+  gcc_jit_block *start_block = gcc_jit_function_new_block(foo_fn, 
+    "start_block");
+
+  gcc_jit_lvalue *a = gcc_jit_function_new_local(foo_fn, LOC(8,5), 
+    int_type, "a_local_for_test_debuginfo");
+  gcc_jit_block_add_assignment(start_block, LOC(8,36), a, 
+    gcc_jit_context_new_rvalue_from_int(ctxt, int_type, 2));
+  gcc_jit_rvalue *add = gcc_jit_context_new_binary_op(ctxt, LOC(9,40), 
+    GCC_JIT_BINARY_OP_PLUS, int_type, 
+    gcc_jit_lvalue_as_rvalue(a), gcc_jit_lvalue_as_rvalue(bar));
+
+  gcc_jit_block_end_with_return(start_block, LOC(9,5), add);
+}
+
+#undef LOC
+
+/* jit-check-debug-info fires up gdb and checks that the variables have 
+   debug info */
+
+/*  { dg-final { jit-check-debug-info "jit-debuginfo.o" {"info variables\n"} "int\\s+a_global_for_test_debuginfo;" } } */
+/*  { dg-final { jit-check-debug-info "jit-debuginfo.o" {"pt main\n"} "int\\s*\\(\\s*int\\s*,\\s*char\\s*\\*\\*\\s*\\)"} } */
+/*  { dg-final { jit-check-debug-info "jit-debuginfo.o" {"start\n" "info locals\n"} "a_local_for_test_debuginfo"} } */
+/*  { dg-final { jit-check-debug-info "jit-debuginfo.o" {"start\n" "pt a_local_for_test_debuginfo\n"} "int"} } */
\ No newline at end of file
-- 
2.20.1

Re: [PATCH] jit : Generate debug info for variables

[David Malcolm]

On Tue, 2021-08-31 at 00:13 +0000, Petter Tomner via Gcc-patches wrote:
> Hi,
> 
> This is a patch to generate debug info for local variables as well as
> globals. 
> With this, "ptype foo", "info variables", "info locals" etc works
> when debugging in GDB.
> 
> Finalizing of global variable declares are moved to after locations
> are handled and done
> as Fortran, C, Go etc do it. Also, primitive types have their
> TYPE_NAME set for debug info
> on types to work.
> 
> Below are the patch, and I attached a testcase. Since it requires GDB
> to run it might
> not be suitable? Make check-jit runs fine on Debian x64.

Thanks for the patches.  Overall, looks good, but I have some review
nits...

Reviewing patch 1 in this email...

> From d77e77104024c7ae9ce31b419dad1f0a5801fda7 Mon Sep 17 00:00:00
> 2001
> From: Petter Tomner <tomner@kth.se>
> Date: Mon, 30 Aug 2021 01:44:07 +0200
> Subject: [PATCH 1/2] libgccjit: Generate debug info for variables
> 
> Finalize declares via available helpers after location is set. Set
> TYPE_NAME of primitives and friends to "int" etc. Debug info is now
> set properly for variables.
> 
> 2021-08-31      Petter Tomner   <tomner@kth.se>
> 
> gcc/jit
>         jit-playback.c
>         jit-playback.h
> gcc/testsuite/jit.dg/
>         test-error-array-bounds.c: Array is not unsigned

Can you write non-empty ChangeLog entries please.

[...snip...]

> --- a/gcc/jit/jit-playback.c
> +++ b/gcc/jit/jit-playback.c

[...snip...]

> @@ -2984,15 +2975,22 @@ replay ()
>      {
>        int i;
>        function *func;
> -
> +      tree global;
>        /* No GC can happen yet; process the cached source locations. 
> */
>        handle_locations ();
>  
> +      /* Finalize globals. See how FORTRAN 95 does it in
> gfc_be_parse_file()
> +         for a simple reference. */
> +      FOR_EACH_VEC_ELT (m_globals, i, global)
> +    rest_of_decl_compilation (global, true, true);
> +
> +      wrapup_global_declarations (m_globals.address(),
> m_globals.length());
> +
>        /* We've now created tree nodes for the stmts in the various
> blocks
> -        in each function, but we haven't built each function's
> single stmt
> -        list yet.  Do so now.  */
> +              in each function, but we haven't built each function's
> single stmt
> +              list yet.  Do so now.  */
>        FOR_EACH_VEC_ELT (m_functions, i, func)
> -       func->build_stmt_list ();
> +         func->build_stmt_list ();

Looks like some whitespace churn above; did your text editor
accidentally convert tabs to spaces?  I prefer to avoid changes that
touch lines without changing things, as it messes up e.g. "git blame".

In case you haven't discovered it yet, "git add -p" is very helpful for
just staging individual hunks within a changed file.

[...snip...]

...plus some comments about the testcase which I'll post in reply to
the other patch.


Dave

Re: Sv: [PATCH] jit : Generate debug info for variables

[David Malcolm]

On Tue, 2021-08-31 at 00:23 +0000, Petter Tomner via Gcc-patches wrote:
> Well I seemed to have attached the wrong testcase. Here is the proper
> one attached.
> 
> Regards,
> 
> -----Ursprungligt meddelande-----
> Från: Petter Tomner 
> Skickat: den 31 augusti 2021 02:14
> Till: gcc-patches@gcc.gnu.org; jit@gcc.gnu.org
> Ämne: [PATCH] jit : Generate debug info for variables
> 
> Hi,
> 
> This is a patch to generate debug info for local variables as well as
> globals. 
> With this, "ptype foo", "info variables", "info locals" etc works when
> debugging in GDB.
> 
> Finalizing of global variable declares are moved to after locations are
> handled and done
> as Fortran, C, Go etc do it. Also, primitive types have their TYPE_NAME
> set for debug info
> on types to work.
> 
> Below are the patch, and I attached a testcase. Since it requires GDB
> to run it might
> not be suitable? Make check-jit runs fine on Debian x64.
> 
> Regards,

> From 6a5d24cbe80429d19042e643bd4c23940cd185fa Mon Sep 17 00:00:00 2001
> From: Petter Tomner <tomner@kth.se>
> Date: Mon, 30 Aug 2021 01:45:11 +0200
> Subject: [PATCH 2/2] libgccjit: Test cases for debug info
> 
> Assure that debug info is available for variables.
> 
> gcc/testsuite/jit.dg/
> 	jit.exp: Helper function
> 	test-debuginfo.c

Again, please provided non-empty ChangeLog entries.

You can use contrib/gcc-changelog/git_check_commit.py to validate them.

I don't see "Signed-off-by" tags in the patches.  Have you either filed
a copyright assignment with the FSF, or can you please add the tags to
certify that you wrote the patches and can contribute them, see:
  https://gcc.gnu.org/contribute.html#legal
  https://gcc.gnu.org/dco.html

[...snip...]

> +proc jit-check-debug-info { obj_file cmds match } {
> +    verbose "Checking debug info for $obj_file with match: $match"
> +
> +    if { [catch {exec gdb -v} fid] } {
> +        verbose "No gdb seems to be in path. Can't check debug info.
Reporting expected fail."
> +        xfail "No gdb seems to be in path. Can't check debug info"

I think this should be "unsupported" rather than "xfail".

[...snip...]

> diff --git a/gcc/testsuite/jit.dg/test-debuginfo.c
b/gcc/testsuite/jit.dg/test-debuginfo.c
> new file mode 100644
> index 00000000000..0af5779fdd1
> --- /dev/null
> +++ b/gcc/testsuite/jit.dg/test-debuginfo.c
> @@ -0,0 +1,72 @@
> +/* Essentially this test checks that debug info are generated for
globals
> +   locals and functions, including type info.  The comment bellow is
used
> +   as fake code (does not affect the test, use for manual
debugging). */
> +/*
> +int a_global_for_test_debuginfo;
> +int main (int argc, char **argv)
> +{
> +    int a_local_for_test_debuginfo = 2;
> +    return a_global_for_test_debuginfo + a_local_for_test_debuginfo;
> +}
> +*/

This is OK, but maybe using gcc_jit_context_dump_to_file with
update_locations == 1 might be more sustainable in the long run?  See:
  https://gcc.gnu.org/onlinedocs/jit/topics/locations.html#faking-it

OK otherwise.

Thanks
Dave

Sv: Sv: [PATCH] jit : Generate debug info for variables

[Petter Tomner]

Thanks for the review. I will post a revision to the mailing list.

I have not filed a copyright assignment with the FSF, but if DCO is enough
I'll add it. I also read the mail about the testcase of this patch.

Regards, Petter

-----Ursprungligt meddelande-----
Från: David Malcolm <dmalcolm@redhat.com> 
Skickat: den 2 september 2021 17:21
Till: Petter Tomner <tomner@kth.se>; gcc-patches@gcc.gnu.org; jit@gcc.gnu.org
Ämne: Re: Sv: [PATCH] jit : Generate debug info for variables

On Tue, 2021-08-31 at 00:23 +0000, Petter Tomner via Gcc-patches wrote:
> Well I seemed to have attached the wrong testcase. Here is the proper 
> one attached.
> 
> Regards,
> 
> -----Ursprungligt meddelande-----
> Från: Petter Tomner
> Skickat: den 31 augusti 2021 02:14
> Till: gcc-patches@gcc.gnu.org; jit@gcc.gnu.org
> Ämne: [PATCH] jit : Generate debug info for variables
> 
> Hi,
> 
> This is a patch to generate debug info for local variables as well as 
> globals.
> With this, "ptype foo", "info variables", "info locals" etc works when 
> debugging in GDB.
> 
> Finalizing of global variable declares are moved to after locations 
> are handled and done as Fortran, C, Go etc do it. Also, primitive 
> types have their TYPE_NAME set for debug info on types to work.
> 
> Below are the patch, and I attached a testcase. Since it requires GDB 
> to run it might not be suitable? Make check-jit runs fine on Debian 
> x64.
> 
> Regards,

> From 6a5d24cbe80429d19042e643bd4c23940cd185fa Mon Sep 17 00:00:00 2001
> From: Petter Tomner <tomner@kth.se>
> Date: Mon, 30 Aug 2021 01:45:11 +0200
> Subject: [PATCH 2/2] libgccjit: Test cases for debug info
> 
> Assure that debug info is available for variables.
> 
> gcc/testsuite/jit.dg/
> 	jit.exp: Helper function
> 	test-debuginfo.c

Again, please provided non-empty ChangeLog entries.

You can use contrib/gcc-changelog/git_check_commit.py to validate them.

I don't see "Signed-off-by" tags in the patches.  Have you either filed a copyright assignment with the FSF, or can you please add the tags to certify that you wrote the patches and can contribute them, see:
  https://gcc.gnu.org/contribute.html#legal
  https://gcc.gnu.org/dco.html

[...snip...]

> +proc jit-check-debug-info { obj_file cmds match } {
> +    verbose "Checking debug info for $obj_file with match: $match"
> +
> +    if { [catch {exec gdb -v} fid] } {
> +        verbose "No gdb seems to be in path. Can't check debug info.
Reporting expected fail."
> +        xfail "No gdb seems to be in path. Can't check debug info"

I think this should be "unsupported" rather than "xfail".

[...snip...]

> diff --git a/gcc/testsuite/jit.dg/test-debuginfo.c
b/gcc/testsuite/jit.dg/test-debuginfo.c
> new file mode 100644
> index 00000000000..0af5779fdd1
> --- /dev/null
> +++ b/gcc/testsuite/jit.dg/test-debuginfo.c
> @@ -0,0 +1,72 @@
> +/* Essentially this test checks that debug info are generated for
globals
> +   locals and functions, including type info.  The comment bellow is
used
> +   as fake code (does not affect the test, use for manual
debugging). */
> +/*
> +int a_global_for_test_debuginfo;
> +int main (int argc, char **argv)
> +{
> +    int a_local_for_test_debuginfo = 2;
> +    return a_global_for_test_debuginfo + a_local_for_test_debuginfo; 
> +} */

This is OK, but maybe using gcc_jit_context_dump_to_file with update_locations == 1 might be more sustainable in the long run?  See:
  https://gcc.gnu.org/onlinedocs/jit/topics/locations.html#faking-it

OK otherwise.

Thanks
Dave