public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH,V4 0/2] CTF: bug fixes and new features.
@ 2021-04-27  1:00 Weimin Pan
  2021-04-27  1:00 ` [PATCH,V4 1/2] CTF: fix incorrect function return type Weimin Pan
  0 siblings, 1 reply; 16+ messages in thread
From: Weimin Pan @ 2021-04-27  1:00 UTC (permalink / raw)
  To: gdb-patches

[Changes from V3:
 - Use ctf_type_name_raw to get type name which will live as long as its 
   ctf file remains open.]
[Changes from V2:
 - Don't strdup name that's returned from libctf's ctf_type_aname_raw.
 - Simplify struct ctf_per_tu_data.]
[Changes from V1:
 - Created a test directory gdb.ctf for CTF tests.
 - Replaced XNEW with std::vector.
 - Added ChangeLog and/or testsuite/ChangeLog for each patch.]

    This patch series contains bug fixes and new features for the CTF 
    (Compact Ansi-C Type Format) support in gdb.
    Two submissions on which this gdb work depends were posted earlier:

     * On the gcc mailing list - Support for the CTF and BTF debug format:
       https://gcc.gnu.org/pipermail/gcc-patches/2021-March/565998.html
     * On the binutils mailing list - adding libctf which creates, updates,
       reads, and manipulates the CTF data.

    For more information, please refer to the CTF specification:
    http://www.esperi.org.uk/~oranix/ctf/ctf-spec.pdf

Weimin Pan (2):
  CTF: fix incorrect function return type
  CTF: multi-CU and archive support

 gdb/ChangeLog                             |   6 +
 gdb/ctfread.c                             | 368 +++++++++++++++++-------------
 gdb/testsuite/ChangeLog                   |   5 +
 gdb/testsuite/gdb.ctf/cross-tu-cyclic-1.c |  18 ++
 gdb/testsuite/gdb.ctf/cross-tu-cyclic-2.c |  16 ++
 gdb/testsuite/gdb.ctf/cross-tu-cyclic-3.c |   3 +
 gdb/testsuite/gdb.ctf/cross-tu-cyclic-4.c |   4 +
 gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp |  43 ++++
 gdb/testsuite/gdb.ctf/ctf-a.c             |  32 +++
 gdb/testsuite/gdb.ctf/ctf-a.h             |  22 ++
 gdb/testsuite/gdb.ctf/ctf-b.c             |  25 ++
 gdb/testsuite/gdb.ctf/ctf-b.h             |  22 ++
 gdb/testsuite/gdb.ctf/ctf-c.c             |  25 ++
 gdb/testsuite/gdb.ctf/ctf-c.h             |  21 ++
 gdb/testsuite/gdb.ctf/funcreturn.exp      | 190 +++++++++++++++
 gdb/testsuite/gdb.ctf/multi.exp           |  42 ++++
 gdb/testsuite/gdb.ctf/whatis.c            | 339 +++++++++++++++++++++++++++
 17 files changed, 1027 insertions(+), 154 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ctf/cross-tu-cyclic-1.c
 create mode 100644 gdb/testsuite/gdb.ctf/cross-tu-cyclic-2.c
 create mode 100644 gdb/testsuite/gdb.ctf/cross-tu-cyclic-3.c
 create mode 100644 gdb/testsuite/gdb.ctf/cross-tu-cyclic-4.c
 create mode 100644 gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
 create mode 100644 gdb/testsuite/gdb.ctf/ctf-a.c
 create mode 100644 gdb/testsuite/gdb.ctf/ctf-a.h
 create mode 100644 gdb/testsuite/gdb.ctf/ctf-b.c
 create mode 100644 gdb/testsuite/gdb.ctf/ctf-b.h
 create mode 100644 gdb/testsuite/gdb.ctf/ctf-c.c
 create mode 100644 gdb/testsuite/gdb.ctf/ctf-c.h
 create mode 100644 gdb/testsuite/gdb.ctf/funcreturn.exp
 create mode 100644 gdb/testsuite/gdb.ctf/multi.exp
 create mode 100644 gdb/testsuite/gdb.ctf/whatis.c

-- 
1.8.3.1


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

* [PATCH,V4 1/2] CTF: fix incorrect function return type
  2021-04-27  1:00 [PATCH,V4 0/2] CTF: bug fixes and new features Weimin Pan
@ 2021-04-27  1:00 ` Weimin Pan
  2021-04-27  1:00   ` [PATCH,V4 2/2] CTF: multi-CU and archive support Weimin Pan
  2021-05-07 20:07   ` [PATCH,V4 1/2] CTF: fix incorrect function return type Tom Tromey
  0 siblings, 2 replies; 16+ messages in thread
From: Weimin Pan @ 2021-04-27  1:00 UTC (permalink / raw)
  To: gdb-patches

The problems can be illustrated, with any program, below:

(gdb) print main
$1 = {main} 0x0

The return type was incorrectly set in read_func_kind_type, with
the name of the function, which leads c_type_print_base_1 to print
it. In addition, the address of a new function needs to be set with
that info in its minimal symtab entry, when the new function is added.

After the fix:

(gdb) print main
$1 = {int ()} 0x4004b7 <main>

A new test, gdb.ctf/funcreturn.exp, is added to the testsuite.
---
 gdb/ChangeLog                        |   6 +
 gdb/ctfread.c                        |  57 +++---
 gdb/testsuite/ChangeLog              |   5 +
 gdb/testsuite/gdb.ctf/funcreturn.exp | 190 ++++++++++++++++++++
 gdb/testsuite/gdb.ctf/whatis.c       | 339 +++++++++++++++++++++++++++++++++++
 5 files changed, 566 insertions(+), 31 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ctf/funcreturn.exp
 create mode 100644 gdb/testsuite/gdb.ctf/whatis.c

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e817ba6..8abea2d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2021-04-16  Weimin Pan  <weimin.pan@oracle.com>
+
+	* ctfread.c (new_symbol): Set function address.
+	(read_func_kind_type): Remove incorrect type name setting.
+	Use ctf_type_name_raw to get type name throughout file.
+
 2021-04-07  Weimin Pan  <weimin.pan@oracle.com>
 
 	* ctfread.c (fetch_tid_type): New function, use throughout file.
diff --git a/gdb/ctfread.c b/gdb/ctfread.c
index e2b65b6..97355bf 100644
--- a/gdb/ctfread.c
+++ b/gdb/ctfread.c
@@ -462,14 +462,14 @@ struct ctf_tid_and_type
   ctf_dict_t *fp = ccp->fp;
   struct symbol *sym = nullptr;
 
-  gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
+  const char *name = ctf_type_name_raw (fp, tid);
   if (name != nullptr)
     {
       sym = new (&objfile->objfile_obstack) symbol;
       OBJSTAT (objfile, n_syms++);
 
       sym->set_language (language_c, &objfile->objfile_obstack);
-      sym->compute_and_set_names (name.get (), true, objfile->per_bfd);
+      sym->compute_and_set_names (name, true, objfile->per_bfd);
       SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
       SYMBOL_ACLASS_INDEX (sym) = LOC_OPTIMIZED_OUT;
 
@@ -487,6 +487,7 @@ struct ctf_tid_and_type
 	    break;
 	  case CTF_K_FUNCTION:
 	    SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
+	    set_symbol_address (objfile, sym, sym->linkage_name ());
 	    break;
 	  case CTF_K_CONST:
 	    if (SYMBOL_TYPE (sym)->code () == TYPE_CODE_VOID)
@@ -525,7 +526,7 @@ struct ctf_tid_and_type
   ctf_dict_t *fp = ccp->fp;
   ctf_encoding_t cet;
   struct type *type = nullptr;
-  char *name;
+  const char *name;
   uint32_t kind;
 
   if (ctf_type_encoding (fp, tid, &cet))
@@ -535,16 +536,15 @@ struct ctf_tid_and_type
       return nullptr;
     }
 
-  gdb::unique_xmalloc_ptr<char> copied_name (ctf_type_aname_raw (fp, tid));
-  if (copied_name == nullptr || strlen (copied_name.get ()) == 0)
-    {
+  name = ctf_type_name_raw (fp, tid);
+  if (name == nullptr || strlen (name) == 0)
+  {
       name = ctf_type_aname (fp, tid);
       if (name == nullptr)
 	complaint (_("ctf_type_aname read_base_type failed - %s"),
 		   ctf_errmsg (ctf_errno (fp)));
-    }
-  else
-    name = obstack_strdup (&of->objfile_obstack, copied_name.get ());
+  }
+
 
   kind = ctf_type_kind (fp, tid);
   if (kind == CTF_K_INTEGER)
@@ -623,9 +623,9 @@ struct ctf_tid_and_type
 
   type = alloc_type (of);
 
-  gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
-  if (name != nullptr && strlen (name.get ()) != 0)
-    type->set_name (obstack_strdup (&of->objfile_obstack, name.get ()));
+  const char *name = ctf_type_name_raw (fp, tid);
+  if (name != nullptr && strlen (name) != 0)
+    type->set_name (name);
 
   kind = ctf_type_kind (fp, tid);
   if (kind == CTF_K_UNION)
@@ -682,10 +682,6 @@ struct ctf_tid_and_type
 
   type = alloc_type (of);
 
-  gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
-  if (name != nullptr && strlen (name.get ()) != 0)
-    type->set_name (obstack_strdup (&of->objfile_obstack, name.get ()));
-
   type->set_code (TYPE_CODE_FUNC);
   ctf_func_type_info (fp, tid, &cfi);
   rettype = fetch_tid_type (ccp, cfi.ctc_return);
@@ -734,9 +730,9 @@ struct ctf_tid_and_type
 
   type = alloc_type (of);
 
-  gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
-  if (name != nullptr && strlen (name.get ()) != 0)
-    type->set_name (obstack_strdup (&of->objfile_obstack, name.get ()));
+  const char *name = ctf_type_name_raw (fp, tid);
+  if (name != nullptr && strlen (name) != 0)
+    type->set_name (name);
 
   type->set_code (TYPE_CODE_ENUM);
   TYPE_LENGTH (type) = ctf_type_size (fp, tid);
@@ -972,9 +968,9 @@ struct ctf_tid_and_type
 
   type = alloc_type (of);
 
-  gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
-  if (name != NULL && strlen (name.get()) != 0)
-    type->set_name (obstack_strdup (&of->objfile_obstack, name.get ()));
+  const char *name = ctf_type_name_raw (fp, tid);
+  if (name != nullptr && strlen (name) != 0)
+    type->set_name (name);
 
   kind = ctf_type_kind_forwarded (fp, tid);
   if (kind == CTF_K_UNION)
@@ -1017,9 +1013,9 @@ struct ctf_tid_and_type
 	break;
       case CTF_K_TYPEDEF:
 	{
-	  gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
+	  const char *name = ctf_type_name_raw (fp, tid);
 	  btid = ctf_type_reference (fp, tid);
-	  type = read_typedef_type (ccp, tid, btid, name.get ());
+	  type = read_typedef_type (ccp, tid, btid, name);
 	}
 	break;
       case CTF_K_VOLATILE:
@@ -1219,8 +1215,7 @@ struct ctf_tid_and_type
   if (ctf_func_args (ccp->fp, idx, argc, argv) == CTF_ERR)
     return nullptr;
 
-  gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (ccp->fp, idx));
-  if (name == nullptr)
+  if (ctf_type_name_raw (ccp->fp, idx) == nullptr)
     return nullptr;
 
   tid = ctf_lookup_by_symbol (ccp->fp, idx);
@@ -1444,7 +1439,6 @@ struct ctf_tid_and_type
   short section = -1;
 
   ccp = (struct ctf_context *) arg;
-  gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (ccp->fp, tid));
 
   domain_enum domain = UNDEF_DOMAIN;
   enum address_class aclass = LOC_UNDEF;
@@ -1486,10 +1480,11 @@ struct ctf_tid_and_type
 	return 0;
     }
 
-  if (name == nullptr || strlen (name.get ()) == 0)
+  const char *name = ctf_type_name_raw (ccp->fp, tid);
+  if (name == nullptr || strlen (name) == 0)
     return 0;
 
-  ccp->pst->add_psymbol (name.get (), true,
+  ccp->pst->add_psymbol (name, true,
 			 domain, aclass, section,
 			 psymbol_placement::GLOBAL,
 			 0, language_c, ccp->partial_symtabs, ccp->of);
@@ -1545,7 +1540,7 @@ struct ctf_tid_and_type
 	else
 	  continue;
 	}
-      gdb::unique_xmalloc_ptr<char> tname (ctf_type_aname_raw (cfp, tid));
+      const char *tname = ctf_type_name_raw (cfp, tid);
       uint32_t kind = ctf_type_kind (cfp, tid);
       address_class aclass;
       domain_enum tdomain;
@@ -1568,7 +1563,7 @@ struct ctf_tid_and_type
       else
 	aclass = LOC_TYPEDEF;
 
-      pst->add_psymbol (tname.get (), true,
+      pst->add_psymbol (tname, true,
 			tdomain, aclass, -1,
 			psymbol_placement::STATIC,
 			0, language_c, partial_symtabs, of);
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 53333f9..3d800c2 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2021-04-16  Weimin Pan  <weimin.pan@oracle.com>
+
+	* gdb.ctf/cross-tu-cyclic.exp: New file.
+	* gdb.ctf/multi.exp: New file.
+
 2021-04-07  Weimin Pan  <weimin.pan@oracle.com>
 
 	* gdb.base/ctf-ptype.c: Add struct link containing a forward
diff --git a/gdb/testsuite/gdb.ctf/funcreturn.exp b/gdb/testsuite/gdb.ctf/funcreturn.exp
new file mode 100644
index 0000000..874160e
--- /dev/null
+++ b/gdb/testsuite/gdb.ctf/funcreturn.exp
@@ -0,0 +1,190 @@
+# Copyright 2021 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/>.
+
+if [skip_ctf_tests] {
+    unsupported "no CTF debug format support, or CTF disabled in GDB"
+    return 0
+}
+
+if [target_info exists no_long_long] {
+    set exec_opts [list debug additional_flags=-DNO_LONG_LONG]
+} else {
+    set exec_opts [list debug]
+}
+
+standard_testfile whatis.c
+
+# Using `-gt` generates full-fledged CTF debug information.
+set opts "additional_flags=-gt -Wl,--export-dynamic"
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile] [list $opts nowarnings]] } {
+    return 0
+}
+
+# Create and source the file that provides information about the compiler
+# used to compile the test case.
+if [get_compiler_info] {
+    return -1
+}
+
+# test print command with functions return type
+set void "(void|)"
+gdb_test "print v_char_func" \
+    "$decimal = \{char \\(\\)\} 0x\[0-9a-z\]+ <v_char_func>.*" \
+    "print char function"
+
+gdb_test "print v_signed_char_func" \
+    "$decimal = \{signed char \\(\\)\} 0x\[0-9a-z\]+ <v_signed_char_func>.*" \
+    "print signed char function"
+
+gdb_test "print v_unsigned_char_func" \
+    "$decimal = \{unsigned char \\(\\)\} 0x\[0-9a-z\]+ <v_unsigned_char_func>.*" \
+    "print unsigned char function"
+
+gdb_test "print v_short_func" \
+    "$decimal = \{short \\(\\)\} 0x\[0-9a-z\]+ <v_short_func>.*" \
+    "print short function"
+
+gdb_test "print v_signed_short_func" \
+    "$decimal = \{signed short|short \\(\\)\} 0x\[0-9a-z\]+ <v_signed_short_func>.*" \
+    "print signed short function"
+
+gdb_test "print v_unsigned_short_func" \
+    "$decimal = \{unsigned short \\(\\)\} 0x\[0-9a-z\]+ <v_unsigned_short_func>.*" \
+    "print unsigned short function"
+
+gdb_test "print v_int_func" \
+    "$decimal = \{int \\(\\)\} 0x\[0-9a-z\]+ <v_int_func>.*" \
+    "print int function"
+
+gdb_test "print v_signed_int_func" \
+    "$decimal = \{signed int|int \\(\\)\} 0x\[0-9a-z\]+ <v_signed_int_func>.*" \
+    "print signed int function"
+
+gdb_test "print v_unsigned_int_func" \
+    "$decimal = \{unsigned int \\(\\)\} 0x\[0-9a-z\]+ <v_unsigned_int_func>.*" \
+    "print unsigned int function"
+
+gdb_test "print v_long_func" \
+    "$decimal = \{long \\(\\)\} 0x\[0-9a-z\]+ <v_long_func>.*" \
+    "print long function"
+
+gdb_test "print v_signed_long_func" \
+    "$decimal = \{signed long|long \\(\\)\} 0x\[0-9a-z\]+ <v_signed_long_func>.*" \
+    "print signed long function"
+
+gdb_test "print v_unsigned_long_func" \
+    "$decimal = \{unsigned long|long \\(\\)\} 0x\[0-9a-z\]+ <v_unsigned_long_func>.*" \
+    "print unsigned long function"
+
+if ![target_info exists no_long_long] {
+    gdb_test "print v_long_long_func" \
+	    "$decimal = \{long long \\(\\)\} 0x\[0-9a-z\]+ <v_long_long_func>.*" \
+		"print long long function"
+
+    gdb_test "print v_signed_long_long_func" \
+	    "$decimal = \{long long \\(\\)\} 0x\[0-9a-z\]+ <v_signed_long_long_func>.*" \
+		"print signed long long function"
+
+    gdb_test "print v_unsigned_long_long_func" \
+	    "$decimal = \{unsigned long long \\(\\)\} 0x\[0-9a-z\]+ <v_unsigned_long_long_func>.*" \
+		"print unsigned long long function"
+}
+
+# Sun /bin/cc calls this a function returning double.
+if {!$gcc_compiled} then {setup_xfail "*-sun-sunos4*"}
+	gdb_test "print v_float_func" \
+	    "$decimal = \{float \\(\\)\} 0x\[0-9a-z\]+.*" \
+	    "print float function"
+
+	gdb_test "print v_double_func" \
+	    "$decimal = \{double \\(\\)\} 0x\[0-9a-z\]+.*" \
+	    "print double function" \
+}
+
+# test whatis command with functions return type
+gdb_test "whatis v_char_func" \
+    "type = (signed |unsigned |)char \\($void\\)" \
+    "whatis char function"
+
+gdb_test "whatis v_signed_char_func" \
+    "type = (signed |unsigned |)char \\($void\\)" \
+    "whatis signed char function"
+
+gdb_test "whatis v_unsigned_char_func" \
+    "type = unsigned char \\($void\\)"	\
+    "whatis unsigned char function"
+
+gdb_test "whatis v_short_func" \
+    "type = short (int |)\\($void\\)" \
+    "whatis short function"
+
+gdb_test "whatis v_signed_short_func" \
+    "type = (signed |)short (int |)\\($void\\)" \
+    "whatis signed short function"
+
+gdb_test "whatis v_unsigned_short_func" \
+    "type = (unsigned short|short unsigned int) \\($void\\)" \
+    "whatis unsigned short function"
+
+gdb_test "whatis v_int_func" \
+    "type = int \\($void\\)" \
+    "whatis int function"
+
+gdb_test "whatis v_signed_int_func" \
+    "type = (signed |)int \\($void\\)" \
+    "whatis signed int function"
+
+gdb_test "whatis v_unsigned_int_func" \
+    "type = unsigned int \\($void\\)" \
+    "whatis unsigned int function"
+
+gdb_test "whatis v_long_func" \
+    "type = (long|int|long int) \\($void\\)" \
+    "whatis long function"
+
+gdb_test "whatis v_signed_long_func" \
+    "type = (signed |)(int|long|long int) \\($void\\)" \
+    "whatis signed long function"
+
+gdb_test "whatis v_unsigned_long_func" \
+    "type = (unsigned (int|long|long int)|long unsigned int) \\($void\\)" \
+    "whatis unsigned long function"
+
+if ![target_info exists no_long_long] {
+    gdb_test "whatis v_long_long_func" \
+	"type = long long(| int) \\($void\\)" \
+	"whatis long long function"
+
+    gdb_test "whatis v_signed_long_long_func" \
+	"type = (signed |)long long(| int) \\($void\\)" \
+	"whatis signed long long function"
+
+    gdb_test "whatis v_unsigned_long_long_func" \
+	"type = (unsigned long long(| int)|long long unsigned int) \\($void\\)" \
+	"whatis unsigned long long function"
+}
+
+# Sun /bin/cc calls this a function returning double.
+if {!$gcc_compiled} then {setup_xfail "*-sun-sunos4*"}
+	gdb_test "whatis v_float_func" \
+	    "type = float \\($void\\)" \
+	    "whatis float function"
+
+	gdb_test "whatis v_double_func" \
+	    "type = double \\($void\\)" \
+	    "whatis double function" \
+}
diff --git a/gdb/testsuite/gdb.ctf/whatis.c b/gdb/testsuite/gdb.ctf/whatis.c
new file mode 100644
index 0000000..aec899d
--- /dev/null
+++ b/gdb/testsuite/gdb.ctf/whatis.c
@@ -0,0 +1,339 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 1992-2021 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/>.  */
+
+/*
+ *	Test file with lots of different types, for testing the
+ *	"whatis" command.
+ */
+
+/*
+ *	First the basic C types.
+ */
+
+char		v_char;
+signed char	v_signed_char;
+unsigned char	v_unsigned_char;
+
+short		v_short;
+signed short	v_signed_short;
+unsigned short	v_unsigned_short;
+
+int		v_int;
+signed int	v_signed_int;
+unsigned int	v_unsigned_int;
+
+long		v_long;
+signed long	v_signed_long;
+unsigned long	v_unsigned_long;
+
+#ifndef NO_LONG_LONG
+long long		v_long_long;
+signed long long	v_signed_long_long;
+unsigned long long	v_unsigned_long_long;
+#endif
+
+float		v_float;
+double		v_double;
+
+/*
+ *	Now some derived types, which are arrays, functions-returning,
+ *	pointers, structures, unions, and enumerations.
+ */
+
+/**** arrays *******/
+
+char		v_char_array[2];
+signed char	v_signed_char_array[2];
+unsigned char	v_unsigned_char_array[2];
+
+short		v_short_array[2];
+signed short	v_signed_short_array[2];
+unsigned short	v_unsigned_short_array[2];
+
+int		v_int_array[2];
+signed int	v_signed_int_array[2];
+unsigned int	v_unsigned_int_array[2];
+
+long		v_long_array[2];
+signed long	v_signed_long_array[2];
+unsigned long	v_unsigned_long_array[2];
+
+#ifndef NO_LONG_LONG
+long long		v_long_long_array[2];
+signed long long	v_signed_long_long_array[2];
+unsigned long long	v_unsigned_long_long_array[2];
+#endif
+
+float		v_float_array[2];
+double		v_double_array[2];
+
+/**** pointers *******/
+
+/* Make sure they still print as pointer to foo even there is a typedef
+   for that type.  Test this not just for char *, which might be
+   a special case kludge in GDB (Unix system include files like to define
+   caddr_t), but for a variety of types.  */
+typedef char *char_addr;
+char_addr a_char_addr;
+typedef unsigned short *ushort_addr;
+ushort_addr a_ushort_addr;
+typedef signed long *slong_addr;
+slong_addr a_slong_addr;
+#ifndef NO_LONG_LONG
+typedef signed long long *slong_long_addr;
+slong_long_addr a_slong_long_addr;
+#endif
+
+char		*v_char_pointer;
+signed char	*v_signed_char_pointer;
+unsigned char	*v_unsigned_char_pointer;
+
+short		*v_short_pointer;
+signed short	*v_signed_short_pointer;
+unsigned short	*v_unsigned_short_pointer;
+
+int		*v_int_pointer;
+signed int	*v_signed_int_pointer;
+unsigned int	*v_unsigned_int_pointer;
+
+long		*v_long_pointer;
+signed long	*v_signed_long_pointer;
+unsigned long	*v_unsigned_long_pointer;
+
+#ifndef NO_LONG_LONG
+long long		*v_long_long_pointer;
+signed long long	*v_signed_long_long_pointer;
+unsigned long long	*v_unsigned_long_long_pointer;
+#endif
+
+float		*v_float_pointer;
+double		*v_double_pointer;
+
+/**** structs *******/
+
+struct t_struct {
+    char	v_char_member;
+    short	v_short_member;
+    int		v_int_member;
+    long	v_long_member;
+#ifndef NO_LONG_LONG
+    long long	v_long_long_member;
+#endif
+    float	v_float_member;
+    double	v_double_member;
+} v_struct1, *v_struct_ptr1;
+
+struct {
+    char	v_char_member;
+    short	v_short_member;
+    int		v_int_member;
+    long	v_long_member;
+#ifndef NO_LONG_LONG
+    long long	v_long_long_member;
+#endif
+    float	v_float_member;
+    double	v_double_member;
+} v_struct2, *v_struct_ptr2;
+
+/**** unions *******/
+
+union t_union {
+    char	v_char_member;
+    short	v_short_member;
+    int		v_int_member;
+    long	v_long_member;
+#ifndef NO_LONG_LONG
+    long long	v_long_long_member;
+#endif
+    float	v_float_member;
+    double	v_double_member;
+} v_union, *v_union_ptr;
+
+union {
+    char	v_char_member;
+    short	v_short_member;
+    int		v_int_member;
+    long	v_long_member;
+#ifndef NO_LONG_LONG
+    long long	v_long_long_member;
+#endif
+    float	v_float_member;
+    double	v_double_member;
+} v_union2, *v_union_ptr2;
+
+/*** Functions returning type ********/
+
+char		v_char_func () { return(0); }
+signed char	v_signed_char_func () { return (0); }
+unsigned char	v_unsigned_char_func () { return (0); }
+
+short		v_short_func () { return (0); }
+signed short	v_signed_short_func () { return (0); }
+unsigned short	v_unsigned_short_func () { return (0); }
+
+int		v_int_func () { return (0); }
+signed int	v_signed_int_func () { return (0); }
+unsigned int	v_unsigned_int_func () { return (0); }
+
+long		v_long_func () { return (0); }
+signed long	v_signed_long_func () { return (0); }
+unsigned long	v_unsigned_long_func () { return (0); }
+
+#ifndef NO_LONG_LONG
+long long		v_long_long_func () { return (0); }
+signed long long	v_signed_long_long_func () { return (0); }
+unsigned long long	v_unsigned_long_long_func () { return (0); }
+#endif
+
+float		v_float_func () { return (0.0); }
+double		v_double_func () { return (0.0); }
+
+/**** Some misc more complicated things *******/
+
+struct link {
+	struct link *next;
+#ifdef __STDC__
+	struct link *(*linkfunc) (struct link *self, int flags);
+#else
+	struct link *(*linkfunc) ();
+#endif
+	struct t_struct stuff[1][2][3];
+} *s_link;
+
+union tu_link {
+	struct link *next;
+#ifdef __STDC__
+	struct link *(*linkfunc) (struct link *self, int flags);
+#else
+	struct link *(*linkfunc) ();
+#endif
+	struct t_struct stuff[1][2][3];
+} u_link;
+
+struct outer_struct {
+	int outer_int;
+	struct inner_struct {
+		int inner_int;
+		long inner_long;
+	}inner_struct_instance;
+	union inner_union {
+		int inner_union_int;
+		long inner_union_long;
+	}inner_union_instance;
+	long outer_long;
+} nested_su;
+
+/**** Enumerations *******/
+
+enum colors {red, green, blue} color;
+enum cars {chevy, ford, porsche} clunker;
+
+/***********/
+
+int main ()
+{
+  /* Some linkers (e.g. on AIX) remove unreferenced variables,
+     so make sure to reference them. */
+  v_char = 0;
+  v_signed_char = 1;
+  v_unsigned_char = 2;
+
+  v_short = 3;
+  v_signed_short = 4;
+  v_unsigned_short = 5;    
+
+  v_int = 6;
+  v_signed_int = 7;
+  v_unsigned_int = 8;    
+
+  v_long = 9;
+  v_signed_long = 10;
+  v_unsigned_long = 11;    
+
+#ifndef NO_LONG_LONG
+  v_long_long = 12;
+  v_signed_long_long = 13;
+  v_unsigned_long_long = 14;
+#endif
+
+  v_float = 100.0;
+  v_double = 200.0;
+
+
+  v_char_array[0] = v_char;
+  v_signed_char_array[0] = v_signed_char;
+  v_unsigned_char_array[0] = v_unsigned_char;
+
+  v_short_array[0] = v_short;
+  v_signed_short_array[0] = v_signed_short;
+  v_unsigned_short_array[0] = v_unsigned_short;
+
+  v_int_array[0] = v_int;
+  v_signed_int_array[0] = v_signed_int;
+  v_unsigned_int_array[0] = v_unsigned_int;
+
+  v_long_array[0] = v_long;
+  v_signed_long_array[0] = v_signed_long;
+  v_unsigned_long_array[0] = v_unsigned_long;
+
+#ifndef NO_LONG_LONG
+  v_long_long_array[0] = v_long_long;
+  v_signed_long_long_array[0] = v_signed_long_long;
+  v_unsigned_long_long_array[0] = v_unsigned_long_long;
+#endif
+
+  v_float_array[0] = v_float;
+  v_double_array[0] = v_double;
+
+  v_char_pointer = &v_char;
+  v_signed_char_pointer = &v_signed_char;
+  v_unsigned_char_pointer = &v_unsigned_char;
+
+  v_short_pointer = &v_short;
+  v_signed_short_pointer = &v_signed_short;
+  v_unsigned_short_pointer = &v_unsigned_short;
+
+  v_int_pointer = &v_int;
+  v_signed_int_pointer = &v_signed_int;
+  v_unsigned_int_pointer = &v_unsigned_int;
+
+  v_long_pointer = &v_long;
+  v_signed_long_pointer = &v_signed_long;
+  v_unsigned_long_pointer = &v_unsigned_long;
+
+#ifndef NO_LONG_LONG
+  v_long_long_pointer = &v_long_long;
+  v_signed_long_long_pointer = &v_signed_long_long;
+  v_unsigned_long_long_pointer = &v_unsigned_long_long;
+#endif
+
+  v_float_pointer = &v_float;
+  v_double_pointer = &v_double;
+
+  color = red;
+  clunker = porsche;
+
+  u_link.next = s_link;
+
+  v_union2.v_short_member = v_union.v_short_member;
+
+  v_struct1.v_char_member = 0;
+  v_struct2.v_char_member = 0;
+
+  nested_su.outer_int = 0;
+  return 0;
+}
-- 
1.8.3.1


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

* [PATCH,V4 2/2] CTF: multi-CU and archive support
  2021-04-27  1:00 ` [PATCH,V4 1/2] CTF: fix incorrect function return type Weimin Pan
@ 2021-04-27  1:00   ` Weimin Pan
  2021-05-07 20:29     ` Tom Tromey
  2021-05-08 20:13     ` Tom Tromey
  2021-05-07 20:07   ` [PATCH,V4 1/2] CTF: fix incorrect function return type Tom Tromey
  1 sibling, 2 replies; 16+ messages in thread
From: Weimin Pan @ 2021-04-27  1:00 UTC (permalink / raw)
  To: gdb-patches

Now gdb is capable of debugging executable, which consists of multiple 
compilation units, with CTF. An executable could potentially have one
or more archives, which, in CTF context, contain conflicting types.   
When comparing to DWARF2, there is a major difference in the type sections, 

all changes were made in ctfread.c where elfctf_build_psymtabs and 
scan_partial_symbols, with the new ctf_per_tu_data struct, were modify to 
handle archives which were treated as dependencies in gdb, via the ctf 
archive iterator and its callback build_ctf_archive_member, and were then 
expanded with expand_dependencies when psymtabs were expanded. 

Also changes were made to handle CTF's data object section and function
info section which now share the same format for their contents - an array 
of type IDs. New functions ctf_psymtab_add_stt_entries, which is called by 
ctf_psymtab_add_stt_obj and ctf_psymtab_add_stt_func, and add_stt_entries, 
which is called by add_stt_obj and add_stt_func when setting up psymtabs 
and full symtab, respectively.

---
 gdb/ctfread.c                             | 317 ++++++++++++++++++------------
 gdb/testsuite/gdb.ctf/cross-tu-cyclic-1.c |  18 ++
 gdb/testsuite/gdb.ctf/cross-tu-cyclic-2.c |  16 ++
 gdb/testsuite/gdb.ctf/cross-tu-cyclic-3.c |   3 +
 gdb/testsuite/gdb.ctf/cross-tu-cyclic-4.c |   4 +
 gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp |  43 ++++
 gdb/testsuite/gdb.ctf/ctf-a.c             |  32 +++
 gdb/testsuite/gdb.ctf/ctf-a.h             |  22 +++
 gdb/testsuite/gdb.ctf/ctf-b.c             |  25 +++
 gdb/testsuite/gdb.ctf/ctf-b.h             |  22 +++
 gdb/testsuite/gdb.ctf/ctf-c.c             |  25 +++
 gdb/testsuite/gdb.ctf/ctf-c.h             |  21 ++
 gdb/testsuite/gdb.ctf/multi.exp           |  42 ++++
 13 files changed, 464 insertions(+), 126 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ctf/cross-tu-cyclic-1.c
 create mode 100644 gdb/testsuite/gdb.ctf/cross-tu-cyclic-2.c
 create mode 100644 gdb/testsuite/gdb.ctf/cross-tu-cyclic-3.c
 create mode 100644 gdb/testsuite/gdb.ctf/cross-tu-cyclic-4.c
 create mode 100644 gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
 create mode 100644 gdb/testsuite/gdb.ctf/ctf-a.c
 create mode 100644 gdb/testsuite/gdb.ctf/ctf-a.h
 create mode 100644 gdb/testsuite/gdb.ctf/ctf-b.c
 create mode 100644 gdb/testsuite/gdb.ctf/ctf-b.h
 create mode 100644 gdb/testsuite/gdb.ctf/ctf-c.c
 create mode 100644 gdb/testsuite/gdb.ctf/ctf-c.h
 create mode 100644 gdb/testsuite/gdb.ctf/multi.exp

diff --git a/gdb/ctfread.c b/gdb/ctfread.c
index 97355bf..284cf91 100644
--- a/gdb/ctfread.c
+++ b/gdb/ctfread.c
@@ -117,6 +117,7 @@ struct ctf_context
   struct objfile *of;
   psymtab_storage *partial_symtabs;
   partial_symtab *pst;
+  ctf_archive_t *arc;
   struct buildsym_compunit *builder;
 };
 
@@ -166,6 +167,18 @@ struct ctf_field_info
   std::vector<struct decl_field> nested_types_list;
 };
 
+/* Persistent data held for a translation unit.  */
+
+struct ctf_per_tu_data
+{
+  ctf_dict_t *fp;
+  struct objfile *of;
+  ctf_archive_t *arc;
+  ctf_psymtab *parent_psymtab;
+  psymtab_storage *pss;
+  psymbol_functions *psf;
+  std::vector <struct partial_symtab *> imported_symtabs;
+};
 
 /* Local function prototypes */
 
@@ -245,10 +258,8 @@ struct ctf_tid_and_type
   ids.tid = tid;
   ids.type = typ;
   slot = (struct ctf_tid_and_type **) htab_find_slot (htab, &ids, INSERT);
-  if (*slot)
-    complaint (_("An internal GDB problem: ctf_ id_t %ld type already set"),
-	       (tid));
-  *slot = XOBNEW (&of->objfile_obstack, struct ctf_tid_and_type);
+  if (*slot == nullptr)
+    *slot = XOBNEW (&of->objfile_obstack, struct ctf_tid_and_type);
   **slot = ids;
   return typ;
 }
@@ -1144,7 +1155,8 @@ struct ctf_tid_and_type
 	if (type)
 	  {
 	    sym = new_symbol (ccp, type, id);
-	    sym->compute_and_set_names (name, false, ccp->of->per_bfd);
+	    if (sym)
+	      sym->compute_and_set_names (name, false, ccp->of->per_bfd);
 	  }
 	break;
       case CTF_K_STRUCT:
@@ -1174,80 +1186,48 @@ struct ctf_tid_and_type
   return 0;
 }
 
-/* Add an ELF STT_OBJ symbol with index IDX to the symbol table.  */
+/* Add entries in either data objects or function info section, controlled
+   by FUNCTIONS.  */
 
-static struct symbol *
-add_stt_obj (struct ctf_context *ccp, unsigned long idx)
+static void
+add_stt_entries (struct ctf_context *ccp, int functions)
 {
-  struct symbol *sym;
-  struct type *type;
+  ctf_next_t *i = nullptr;
+  const char *tname;
   ctf_id_t tid;
+  struct symbol *sym = nullptr;
+  struct type *type;
 
-  if ((tid = ctf_lookup_by_symbol (ccp->fp, idx)) == CTF_ERR)
-    return nullptr;
-
-  type = fetch_tid_type (ccp, tid);
-  if (type == nullptr)
-    return nullptr;
-
-  sym = new_symbol (ccp, type, tid);
-
-  return sym;
+  while ((tid = ctf_symbol_next (ccp->fp, &i, &tname, functions)) != CTF_ERR)
+    {
+      type = get_tid_type (ccp->of, tid);
+      if (type == nullptr)
+	continue;
+      sym = new (&ccp->of->objfile_obstack) symbol;
+      OBJSTAT (ccp->of, n_syms++);
+      SYMBOL_TYPE (sym) = type;
+      SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
+      SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
+      sym->compute_and_set_names (tname, false, ccp->of->per_bfd);
+      add_symbol_to_list (sym, ccp->builder->get_global_symbols ());
+      set_symbol_address (ccp->of, sym, tname);
+    }
 }
 
-/* Add an ELF STT_FUNC symbol with index IDX to the symbol table.  */
+/* Add entries in data objects section.  */
 
-static struct symbol *
-add_stt_func (struct ctf_context *ccp, unsigned long idx)
+static void
+add_stt_obj (struct ctf_context *ccp)
 {
-  struct type *ftype, *atyp, *rettyp;
-  struct symbol *sym;
-  ctf_funcinfo_t finfo;
-  ctf_id_t argv[32];
-  uint32_t argc;
-  ctf_id_t tid;
-  struct type *void_type = objfile_type (ccp->of)->builtin_void;
-
-  if (ctf_func_info (ccp->fp, idx, &finfo) == CTF_ERR)
-    return nullptr;
-
-  argc = finfo.ctc_argc;
-  if (ctf_func_args (ccp->fp, idx, argc, argv) == CTF_ERR)
-    return nullptr;
-
-  if (ctf_type_name_raw (ccp->fp, idx) == nullptr)
-    return nullptr;
-
-  tid = ctf_lookup_by_symbol (ccp->fp, idx);
-  ftype = fetch_tid_type (ccp, tid);
-  if ((finfo.ctc_flags & CTF_FUNC_VARARG) != 0)
-    ftype->set_has_varargs (true);
-  ftype->set_num_fields (argc);
-
-  /* If argc is 0, it has a "void" type.  */
-  if (argc != 0)
-    ftype->set_fields
-      ((struct field *) TYPE_ZALLOC (ftype, argc * sizeof (struct field)));
-
-  /* TYPE_FIELD_TYPE must never be NULL.  Fill it with void_type, if failed
-     to find the argument type.  */
-  for (int iparam = 0; iparam < argc; iparam++)
-    {
-      atyp = fetch_tid_type (ccp, argv[iparam]);
-      if (atyp)
-	ftype->field (iparam).set_type (atyp);
-      else
-	ftype->field (iparam).set_type (void_type);
-    }
+  add_stt_entries (ccp, 0);
+}
 
-  sym = new_symbol (ccp, ftype, tid);
-  rettyp = fetch_tid_type (ccp, finfo.ctc_return);
-  if (rettyp != nullptr)
-    SYMBOL_TYPE (sym) = rettyp;
-  else
-    SYMBOL_TYPE (sym) = void_type;
+/* Add entries in function info section.  */
 
-  return sym;
+static void
+add_stt_func (struct ctf_context *ccp)
+{
+  add_stt_entries (ccp, 1);
 }
 
 /* Get text segment base for OBJFILE, TSIZE contains the segment size.  */
@@ -1270,12 +1250,20 @@ struct ctf_tid_and_type
 		  struct objfile *of, CORE_ADDR text_offset)
 {
   struct ctf_context *ccp;
+  ctf_psymtab *dp;
 
   ccp = pst->context;
   ccp->builder = new buildsym_compunit
 		       (of, of->original_name, nullptr,
 		       language_c, text_offset);
   ccp->builder->record_debugformat ("ctf");
+
+  for (int i = 0; i < pst->number_of_dependencies; i++)
+    {
+      dp = (ctf_psymtab *)pst->dependencies[i];
+      dp->context->builder = ccp->builder;
+      dp->context->of = of;
+    }
 }
 
 /* Finish reading symbol/type definitions in CTF format.
@@ -1317,17 +1305,77 @@ struct ctf_tid_and_type
 	       ctf_errmsg (ctf_errno (ccp->fp)));
 }
 
+/* Add entries in either data objects or function info section, controlled
+   by FUNCTIONS, to psymtab.  */
+
+static void
+ctf_psymtab_add_stt_entries (ctf_dict_t *cfp, ctf_psymtab *pst,
+			     struct objfile *of, int functions)
+{
+  ctf_next_t *i = nullptr;
+  ctf_id_t tid;
+  const char *tname;
+
+  while ((tid = ctf_symbol_next (cfp, &i, &tname, functions)) != CTF_ERR)
+    {
+      uint32_t kind = ctf_type_kind (cfp, tid);
+      address_class aclass;
+      domain_enum tdomain;
+      switch (kind)
+	{
+	  case CTF_K_STRUCT:
+	  case CTF_K_UNION:
+	  case CTF_K_ENUM:
+	    tdomain = STRUCT_DOMAIN;
+	    break;
+	  default:
+	    tdomain = VAR_DOMAIN;
+	    break;
+	}
+
+      if (kind == CTF_K_FUNCTION)
+	aclass = LOC_STATIC;
+      else if (kind == CTF_K_CONST)
+	aclass = LOC_CONST;
+      else
+	aclass = LOC_TYPEDEF;
+
+      pst->add_psymbol (tname, true,
+			tdomain, aclass, -1,
+			psymbol_placement::GLOBAL,
+			0, language_c, pst->context->partial_symtabs, of);
+    }
+}
+
+/* Add entries in data objects section to psymtab.  */
+
+static void
+ctf_psymtab_add_stt_obj (ctf_dict_t *cfp, ctf_psymtab *pst,
+			 struct objfile *of)
+{
+  ctf_psymtab_add_stt_entries (cfp, pst, of, 0);
+}
+
+/* Add entries in function info section to psymtab.  */
+
+static void
+ctf_psymtab_add_stt_func (ctf_dict_t *cfp, ctf_psymtab *pst,
+			  struct objfile *of)
+{
+  ctf_psymtab_add_stt_entries (cfp, pst, of, 1);
+}
+
 /* Read in full symbols for PST, and anything it depends on.  */
 
 void
 ctf_psymtab::expand_psymtab (struct objfile *objfile)
 {
-  struct symbol *sym;
   struct ctf_context *ccp;
 
   gdb_assert (!readin);
 
   ccp = context;
+  expand_dependencies (objfile);
 
   /* Iterate over entries in data types section.  */
   if (ctf_type_iter (ccp->fp, ctf_add_type_cb, ccp) == CTF_ERR)
@@ -1341,22 +1389,10 @@ struct ctf_tid_and_type
 	       ctf_errmsg (ctf_errno (ccp->fp)));
 
   /* Add entries in data objects and function info sections.  */
-  for (unsigned long i = 0; ; i++)
-    {
-      sym = add_stt_obj (ccp, i);
-      if (sym == nullptr)
-	{
-	  if (ctf_errno (ccp->fp) == EINVAL
-	      || ctf_errno (ccp->fp) == ECTF_NOSYMTAB)
-	    break;
-	  sym = add_stt_func (ccp, i);
-	}
-      if (sym == nullptr)
-	continue;
-
-      set_symbol_address (ccp->of, sym, sym->linkage_name ());
-    }
+  add_stt_obj (ccp);
+  add_stt_func (ccp);
 
+  ctf_dict_close (ccp->fp);
   readin = true;
 }
 
@@ -1409,6 +1445,7 @@ struct ctf_tid_and_type
 
 static ctf_psymtab *
 create_partial_symtab (const char *name,
+		       ctf_archive_t *arc,
 		       ctf_dict_t *cfp,
 		       psymtab_storage *partial_symtabs,
 		       struct objfile *objfile)
@@ -1419,11 +1456,12 @@ struct ctf_tid_and_type
   pst = new ctf_psymtab (name, partial_symtabs, objfile->per_bfd, 0);
 
   ccx = XOBNEW (&objfile->objfile_obstack, struct ctf_context);
+  ccx->arc = arc;
   ccx->fp = cfp;
+  ctf_ref (cfp);
   ccx->of = objfile;
   ccx->partial_symtabs = partial_symtabs;
   ccx->pst = pst;
-  ccx->builder = nullptr;
   pst->context = ccx;
 
   return pst;
@@ -1509,13 +1547,15 @@ struct ctf_tid_and_type
 /* Setup partial_symtab's describing each source file for which
    debugging information is available.  */
 
-static void
+static ctf_psymtab *
 scan_partial_symbols (ctf_dict_t *cfp, psymtab_storage *partial_symtabs,
-		      struct objfile *of)
+		      struct ctf_per_tu_data *tup)
 {
+  struct objfile *of = tup->of;
   bfd *abfd = of->obfd;
   const char *name = bfd_get_filename (abfd);
-  ctf_psymtab *pst = create_partial_symtab (name, cfp, partial_symtabs, of);
+  ctf_psymtab *pst = create_partial_symtab (name, tup->arc, cfp,
+			partial_symtabs, of);
 
   struct ctf_context *ccx = pst->context;
 
@@ -1530,46 +1570,46 @@ struct ctf_tid_and_type
   /* Scan CTF object and function sections which correspond to each
      STT_FUNC or STT_OBJECT entry in the symbol table,
      pick up what init_symtab has done.  */
-  for (unsigned long idx = 0; ; idx++)
-    {
-      ctf_id_t tid;
-      if ((tid = ctf_lookup_by_symbol (cfp, idx)) == CTF_ERR)
-	{
-	if (ctf_errno (cfp) == EINVAL || ctf_errno (cfp) == ECTF_NOSYMTAB)
-	  break;	// Done, reach end of the section.
-	else
-	  continue;
-	}
-      const char *tname = ctf_type_name_raw (cfp, tid);
-      uint32_t kind = ctf_type_kind (cfp, tid);
-      address_class aclass;
-      domain_enum tdomain;
-      switch (kind)
-	{
-	  case CTF_K_STRUCT:
-	  case CTF_K_UNION:
-	  case CTF_K_ENUM:
-	    tdomain = STRUCT_DOMAIN;
-	    break;
-	  default:
-	    tdomain = VAR_DOMAIN;
-	    break;
-	}
+  ctf_psymtab_add_stt_obj (cfp, pst, of);
+  ctf_psymtab_add_stt_func (cfp, pst, of);
 
-      if (kind == CTF_K_FUNCTION)
-	aclass = LOC_STATIC;
-      else if (kind == CTF_K_CONST)
-	aclass = LOC_CONST;
-      else
-	aclass = LOC_TYPEDEF;
+  pst->end ();
 
-      pst->add_psymbol (tname, true,
-			tdomain, aclass, -1,
-			psymbol_placement::STATIC,
-			0, language_c, partial_symtabs, of);
+  return pst;
+}
+
+/* Callback to build the psymtab for archive member NAME.  */
+
+static int
+build_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
+{
+  struct ctf_per_tu_data *tup = (struct ctf_per_tu_data *) arg;
+  ctf_dict_t *parent = tup->fp;
+  ctf_psymtab *pst;
+  int isparent = 0;
+
+  if (strcmp (name, ".ctf") != 0)
+    {
+      ctf_import (ctf, parent);
     }
+  else
+    isparent = 1;
 
-  pst->end ();
+  if (info_verbose)
+    {
+      printf_filtered (_("Scanning archive member %s..."), name);
+      gdb_flush (gdb_stdout);
+    }
+
+  psymtab_storage *pss = tup->psf->get_partial_symtabs ().get ();
+  pst = scan_partial_symbols (ctf, pss, tup);
+
+  if (isparent)
+    tup->parent_psymtab = pst;
+  else
+    tup->imported_symtabs.push_back (pst);
+
+  return 0;
 }
 
 /* Read CTF debugging information from a BFD section.  This is
@@ -1579,6 +1619,7 @@ struct ctf_tid_and_type
 void
 elfctf_build_psymtabs (struct objfile *of)
 {
+  struct ctf_per_tu_data pcu;
   bfd *abfd = of->obfd;
   int err;
 
@@ -1593,10 +1634,34 @@ struct ctf_tid_and_type
 	   bfd_get_filename (abfd), ctf_errmsg (err));
   ctf_dict_key.emplace (of, fp);
 
+  pcu.fp = fp;
+  pcu.of = of;
+  pcu.arc = arc;
+
   psymbol_functions *psf = new psymbol_functions ();
   psymtab_storage *partial_symtabs = psf->get_partial_symtabs ().get ();
   of->qf.emplace_front (psf);
-  scan_partial_symbols (fp, partial_symtabs, of);
+  pcu.psf = psf;
+
+  if (ctf_archive_iter (arc, build_ctf_archive_member, &pcu) < 0)
+    error (_("ctf_archive_iter failed in input file %s: - %s"),
+	   bfd_get_filename (abfd), ctf_errmsg (err));
+
+  int arch_cnt = pcu.imported_symtabs.size ();
+  struct partial_symtab *parent_pst = pcu.parent_psymtab;
+
+  if (parent_pst && arch_cnt)
+    {
+      /* Fill in the 'dependencies'.  */
+      parent_pst->number_of_dependencies = arch_cnt;
+      parent_pst->dependencies
+	= partial_symtabs->allocate_dependencies (arch_cnt);
+      for (int i = 0; i < arch_cnt; ++i)
+	{
+	  parent_pst->dependencies[i]
+	    = pcu.imported_symtabs.at (i);
+	}
+    }
 }
 
 #else
diff --git a/gdb/testsuite/gdb.ctf/cross-tu-cyclic-1.c b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-1.c
new file mode 100644
index 0000000..fe52b9e
--- /dev/null
+++ b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-1.c
@@ -0,0 +1,18 @@
+struct A;
+struct B
+{
+  int foo;
+  struct A *bar;
+};
+
+struct A
+{
+  long a;
+  struct B *foo;
+};
+
+static struct A *foo __attribute__((used));
+
+int main()
+{
+}
diff --git a/gdb/testsuite/gdb.ctf/cross-tu-cyclic-2.c b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-2.c
new file mode 100644
index 0000000..aa2d177
--- /dev/null
+++ b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-2.c
@@ -0,0 +1,16 @@
+struct B;
+struct A
+{
+  long a;
+  struct B *foo;
+  struct C *bar;
+};
+
+struct C
+{
+  struct B *foo;
+  int b;
+};
+
+static struct C *foo __attribute__((used));
+static struct A *bar __attribute__((used));
diff --git a/gdb/testsuite/gdb.ctf/cross-tu-cyclic-3.c b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-3.c
new file mode 100644
index 0000000..19947e8
--- /dev/null
+++ b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-3.c
@@ -0,0 +1,3 @@
+struct A { struct B *foo; };
+static struct A *a __attribute__((__used__));
+static struct A *conflicty __attribute__((__used__));
diff --git a/gdb/testsuite/gdb.ctf/cross-tu-cyclic-4.c b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-4.c
new file mode 100644
index 0000000..6e0c957
--- /dev/null
+++ b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-4.c
@@ -0,0 +1,4 @@
+struct A { struct B *foo; };
+struct B { struct B *next; };
+static struct A *a __attribute__((__used__));
+static struct B *conflicty __attribute__((__used__));
diff --git a/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp b/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
new file mode 100644
index 0000000..734892d
--- /dev/null
+++ b/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
@@ -0,0 +1,43 @@
+# Copyright 2021 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/>.
+
+# This file is a subset of ptype.exp written by Rob Savoye. (rob@cygnus.com)
+
+if [skip_ctf_tests] {
+    unsupported "no CTF debug format support, or CTF disabled in GDB"
+    return 0
+}
+
+standard_testfile cross-tu-cyclic-1.c  cross-tu-cyclic-2.c \
+	cross-tu-cyclic-3.c  cross-tu-cyclic-4.c
+
+# Using `-gt` generates full-fledged CTF debug information.
+set opts "additional_flags=-gt -Wl,--export-dynamic"
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $srcfile2 $srcfile3 $srcfile4] \
+	  [list $opts nowarnings]] } {
+    return 0
+}
+
+# Create and source the file that provides information about the compiler
+# used to compile the test case.
+if [get_compiler_info] {
+    return -1
+}
+
+# Same thing with struct and union.
+gdb_test "ptype struct A" "type = struct A \{\[\r\n\]+\[ \t\]+long a;\[\r\n\]+\[ \t\]+struct B \\*foo;\[\r\n\]+\}.*" "ptype structure A"
+gdb_test "ptype struct B" "type = struct B \{\[\r\n\]+\[ \t\]+int foo;\[\r\n\]+\[ \t\]+struct A \\*bar;\[\r\n\]+\}.*" "ptype structure B"
+gdb_test "ptype struct C" "type = struct C \{\[\r\n\]+\[ \t\]+struct B \\*foo;\[\r\n\]+\[ \t\]+int b;\[\r\n\]+\}.*" "ptype structure C"
diff --git a/gdb/testsuite/gdb.ctf/ctf-a.c b/gdb/testsuite/gdb.ctf/ctf-a.c
new file mode 100644
index 0000000..9aa2a8f
--- /dev/null
+++ b/gdb/testsuite/gdb.ctf/ctf-a.c
@@ -0,0 +1,32 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2021 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 "ctf-a.h"
+
+static struct A a __attribute__((used));
+
+extern struct C *foo ();
+extern int bar ();
+
+int main ()
+{
+  struct C *cp;
+  cp = foo ();
+  if (cp)
+    return bar ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.ctf/ctf-a.h b/gdb/testsuite/gdb.ctf/ctf-a.h
new file mode 100644
index 0000000..297d740
--- /dev/null
+++ b/gdb/testsuite/gdb.ctf/ctf-a.h
@@ -0,0 +1,22 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2021 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/>.  */
+
+struct A {
+  struct B *b;
+  struct A *next;
+};
+
diff --git a/gdb/testsuite/gdb.ctf/ctf-b.c b/gdb/testsuite/gdb.ctf/ctf-b.c
new file mode 100644
index 0000000..c3a8ce5
--- /dev/null
+++ b/gdb/testsuite/gdb.ctf/ctf-b.c
@@ -0,0 +1,25 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2021 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 "ctf-b.h"
+
+static struct B b __attribute__((used));
+
+int bar ()
+{
+  return b.wombat;
+}
diff --git a/gdb/testsuite/gdb.ctf/ctf-b.h b/gdb/testsuite/gdb.ctf/ctf-b.h
new file mode 100644
index 0000000..9dbdd7d
--- /dev/null
+++ b/gdb/testsuite/gdb.ctf/ctf-b.h
@@ -0,0 +1,22 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2021 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/>.  */
+
+struct B {
+  struct C *c;
+  int wombat;
+};
+
diff --git a/gdb/testsuite/gdb.ctf/ctf-c.c b/gdb/testsuite/gdb.ctf/ctf-c.c
new file mode 100644
index 0000000..b4051b3
--- /dev/null
+++ b/gdb/testsuite/gdb.ctf/ctf-c.c
@@ -0,0 +1,25 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2021 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 "ctf-c.h"
+
+static struct C c __attribute__((used));
+
+struct C * foo ()
+{
+  return &c;
+}
diff --git a/gdb/testsuite/gdb.ctf/ctf-c.h b/gdb/testsuite/gdb.ctf/ctf-c.h
new file mode 100644
index 0000000..fb18157
--- /dev/null
+++ b/gdb/testsuite/gdb.ctf/ctf-c.h
@@ -0,0 +1,21 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2021 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/>.  */
+
+struct C {
+  struct A *a;
+  int b;
+};
diff --git a/gdb/testsuite/gdb.ctf/multi.exp b/gdb/testsuite/gdb.ctf/multi.exp
new file mode 100644
index 0000000..973115e
--- /dev/null
+++ b/gdb/testsuite/gdb.ctf/multi.exp
@@ -0,0 +1,42 @@
+# Copyright 2021 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/>.
+
+# This file is a subset of ptype.exp written by Rob Savoye. (rob@cygnus.com)
+
+if [skip_ctf_tests] {
+    unsupported "no CTF debug format support, or CTF disabled in GDB"
+    return 0
+}
+
+standard_testfile ctf-a.c ctf-b.c ctf-c.c
+
+# Using `-gt` generates full-fledged CTF debug information.
+set opts "additional_flags=-gt -Wl,--export-dynamic"
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $srcfile2 $srcfile3] \
+	  [list $opts nowarnings]] } {
+    return 0
+}
+
+# Create and source the file that provides information about the compiler
+# used to compile the test case.
+if [get_compiler_info] {
+    return -1
+}
+
+# Same thing with struct and union.
+gdb_test "ptype struct A" "type = struct A \{\[\r\n\]+\[ \t\]+struct B \\*b;\[\r\n\]+\[ \t\]+struct A \\*next;\[\r\n\]+\}.*" "ptype structure A"
+gdb_test "ptype struct B" "type = struct B \{\[\r\n\]+\[ \t\]+struct C \\*c;\[\r\n\]+\[ \t\]+int \\wombat;\[\r\n\]+\}.*" "ptype structure B"
+gdb_test "ptype struct C" "type = struct C \{\[\r\n\]+\[ \t\]+struct A \\*a;\[\r\n\]+\[ \t\]+int b;\[\r\n\]+\}.*" "ptype structure C"
-- 
1.8.3.1


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

* Re: [PATCH,V4 1/2] CTF: fix incorrect function return type
  2021-04-27  1:00 ` [PATCH,V4 1/2] CTF: fix incorrect function return type Weimin Pan
  2021-04-27  1:00   ` [PATCH,V4 2/2] CTF: multi-CU and archive support Weimin Pan
@ 2021-05-07 20:07   ` Tom Tromey
  2021-05-10  2:40     ` Wei-min Pan
  2021-05-16 22:31     ` Wei-min Pan
  1 sibling, 2 replies; 16+ messages in thread
From: Tom Tromey @ 2021-05-07 20:07 UTC (permalink / raw)
  To: Weimin Pan via Gdb-patches

>>>>> ">" == Weimin Pan via Gdb-patches <gdb-patches@sourceware.org> writes:

>> The problems can be illustrated, with any program, below:
>> (gdb) print main
>> $1 = {main} 0x0

Hi.  Sorry about the delay in my re-review.
It's fine (and expected) to send patch pings to the list.
It helps keep us honest I guess.

>> -      sym->compute_and_set_names (name.get (), true, objfile->per_bfd);
>> +      sym->compute_and_set_names (name, true, objfile->per_bfd);

If the lifetime of 'name' is ok, then you don't need to copy it here --
so you can pass 'false' as the second parameter.  See the comment in
symtab.c just before general_symbol_info::compute_and_set_names.

>> +  name = ctf_type_name_raw (fp, tid);
>> +  if (name == nullptr || strlen (name) == 0)
>> +  {

The braces (and the code they surround) look under-indented to me.

>> -  ccp->pst->add_psymbol (name.get (), true,
>> +  ccp->pst->add_psymbol (name, true,

Same here.

>> -      pst->add_psymbol (tname.get (), true,
>> +      pst->add_psymbol (tname, true,

And here.

If you fix these up, and it still works, it is fine to push.
If it doesn't work then that would tend to put some doubt on the other
parts of the patch that don't copy names.

thanks,
Tom

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

* Re: [PATCH,V4 2/2] CTF: multi-CU and archive support
  2021-04-27  1:00   ` [PATCH,V4 2/2] CTF: multi-CU and archive support Weimin Pan
@ 2021-05-07 20:29     ` Tom Tromey
  2021-05-10  3:11       ` Wei-min Pan
  2021-05-08 20:13     ` Tom Tromey
  1 sibling, 1 reply; 16+ messages in thread
From: Tom Tromey @ 2021-05-07 20:29 UTC (permalink / raw)
  To: Weimin Pan via Gdb-patches

>>>>> ">" == Weimin Pan via Gdb-patches <gdb-patches@sourceware.org> writes:

>> Now gdb is capable of debugging executable, which consists of multiple 
>> compilation units, with CTF. An executable could potentially have one
>> or more archives, which, in CTF context, contain conflicting types.   
>> When comparing to DWARF2, there is a major difference in the type sections, 

I didn't understand the last sentence here.

I'm afraid there's a lot about this patch I don't really understand.
This doesn't mean anything about the patch, it's more my ignorance of
CTF.  But, I'd like to at least have some idea before approving it.

>> +/* Persistent data held for a translation unit.  */
>> +
>> +struct ctf_per_tu_data

I don't think "persistent" is the right term here, because one of these
is created in elfctf_build_psymtabs and then discarded when the function
returns.

>> +  for (int i = 0; i < pst->number_of_dependencies; i++)
>> +    {
>> +      dp = (ctf_psymtab *)pst->dependencies[i];

Should be a space after the cast.

>> +      pst->add_psymbol (tname, true,
>> +			tdomain, aclass, -1,
>> +			psymbol_placement::GLOBAL,
>> +			0, language_c, pst->context->partial_symtabs, of);

I don't know what ctf_symbol_next does, but I wonder if 'tname' truly
needs to be copied here, or whether it's possible to change true->false
in the call.

>> +  ctf_dict_close (ccp->fp);

I didn't understand the placement of this addition.
I expected it to be paired with some kind of dict_open call, but I
didn't see that.  Could you explain it?

>> +/* Callback to build the psymtab for archive member NAME.  */
>> +
>> +static int
>> +build_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
>> +{
>> +  struct ctf_per_tu_data *tup = (struct ctf_per_tu_data *) arg;
>> +  ctf_dict_t *parent = tup->fp;
>> +  ctf_psymtab *pst;
>> +  int isparent = 0;
>> +
>> +  if (strcmp (name, ".ctf") != 0)
>> +    {
>> +      ctf_import (ctf, parent);
>>      }

Remove the brace here.

I don't know what the ".ctf" thing means here, but I guess it's some
special marker?  Could you explain the layout here a little?

>> +  if (isparent)
>> +    tup->parent_psymtab = pst;
>> +  else
>> +    tup->imported_symtabs.push_back (pst);

Like, how many psymtabs might be marked as a parent, and how many would
be pushed as dependencies?

Maybe it would be helpful to know how the new test cases would look
here.

thanks,
Tom

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

* Re: [PATCH,V4 2/2] CTF: multi-CU and archive support
  2021-04-27  1:00   ` [PATCH,V4 2/2] CTF: multi-CU and archive support Weimin Pan
  2021-05-07 20:29     ` Tom Tromey
@ 2021-05-08 20:13     ` Tom Tromey
  2021-05-10  3:12       ` Wei-min Pan
  1 sibling, 1 reply; 16+ messages in thread
From: Tom Tromey @ 2021-05-08 20:13 UTC (permalink / raw)
  To: Weimin Pan via Gdb-patches

>> +      for (int i = 0; i < arch_cnt; ++i)
>> +	{
>> +	  parent_pst->dependencies[i]
>> +	    = pcu.imported_symtabs.at (i);
>> +	}

I forgot to mention -- don't use .at(), just use [] instead.
This loop could probably just be a memcpy or a std::copy, though an
explicit loop is also fine.

Tom

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

* Re: [PATCH,V4 1/2] CTF: fix incorrect function return type
  2021-05-07 20:07   ` [PATCH,V4 1/2] CTF: fix incorrect function return type Tom Tromey
@ 2021-05-10  2:40     ` Wei-min Pan
  2021-05-10 14:00       ` Tom Tromey
  2021-05-16 22:31     ` Wei-min Pan
  1 sibling, 1 reply; 16+ messages in thread
From: Wei-min Pan @ 2021-05-10  2:40 UTC (permalink / raw)
  To: Tom Tromey, Weimin Pan via Gdb-patches


On 5/7/2021 1:07 PM, Tom Tromey wrote:
>>>>>> ">" == Weimin Pan via Gdb-patches <gdb-patches@sourceware.org> writes:
>>> The problems can be illustrated, with any program, below:
>>> (gdb) print main
>>> $1 = {main} 0x0
> Hi.  Sorry about the delay in my re-review.
> It's fine (and expected) to send patch pings to the list.
> It helps keep us honest I guess.
Sorry about that. Shall follow the protocol by pinging the patch going 
forward.

>
>>> -      sym->compute_and_set_names (name.get (), true, objfile->per_bfd);
>>> +      sym->compute_and_set_names (name, true, objfile->per_bfd);
> If the lifetime of 'name' is ok, then you don't need to copy it here --
> so you can pass 'false' as the second parameter.  See the comment in
> symtab.c just before general_symbol_info::compute_and_set_names.

Thanks - passing 'false' as the second parameter to compute_and_set_names is
the right thing to do. All CTF tests are still passing with the change.

>>> +  name = ctf_type_name_raw (fp, tid);
>>> +  if (name == nullptr || strlen (name) == 0)
>>> +  {
> The braces (and the code they surround) look under-indented to me.

Corrected.

>>> -  ccp->pst->add_psymbol (name.get (), true,
>>> +  ccp->pst->add_psymbol (name, true,
> Same here.

Sorry, the only place I can find is here and it looks indented correctly?

-  if (name == nullptr || strlen (name.get ()) == 0)
+  const char *name = ctf_type_name_raw (ccp->fp, tid);
+  if (name == nullptr || strlen (name) == 0)
      return 0;

-  ccp->pst->add_psymbol (name.get (), true,
+  ccp->pst->add_psymbol (name, true,

>
>>> -      pst->add_psymbol (tname.get (), true,
>>> +      pst->add_psymbol (tname, true,
> And here.

Also the indentation looks correct?

        else
         aclass = LOC_TYPEDEF;

-      pst->add_psymbol (tname.get (), true,
+      pst->add_psymbol (tname, true,

>
> If you fix these up, and it still works, it is fine to push.
> If it doesn't work then that would tend to put some doubt on the other
> parts of the patch that don't copy names.

OK, thank you.

>
> thanks,
> Tom

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

* Re: [PATCH,V4 2/2] CTF: multi-CU and archive support
  2021-05-07 20:29     ` Tom Tromey
@ 2021-05-10  3:11       ` Wei-min Pan
  2021-05-10 21:27         ` Tom Tromey
  0 siblings, 1 reply; 16+ messages in thread
From: Wei-min Pan @ 2021-05-10  3:11 UTC (permalink / raw)
  To: Tom Tromey, Weimin Pan via Gdb-patches


On 5/7/2021 1:29 PM, Tom Tromey wrote:
>>>>>> ">" == Weimin Pan via Gdb-patches <gdb-patches@sourceware.org> writes:
>>> Now gdb is capable of debugging executable, which consists of multiple
>>> compilation units, with CTF. An executable could potentially have one
>>> or more archives, which, in CTF context, contain conflicting types.
>>> When comparing to DWARF2, there is a major difference in the type sections,
> I didn't understand the last sentence here.
>
> I'm afraid there's a lot about this patch I don't really understand.
> This doesn't mean anything about the patch, it's more my ignorance of
> CTF.  But, I'd like to at least have some idea before approving it.

Hi Tom,

Thank you for taking the time. Please see below for some explanation 
about CTF
and let me know if you need more info.

>>> +/* Persistent data held for a translation unit.  */
>>> +
>>> +struct ctf_per_tu_data
> I don't think "persistent" is the right term here, because one of these
> is created in elfctf_build_psymtabs and then discarded when the function
> returns.

Makes sense, will drop it.

>
>>> +  for (int i = 0; i < pst->number_of_dependencies; i++)
>>> +    {
>>> +      dp = (ctf_psymtab *)pst->dependencies[i];
> Should be a space after the cast.

Done.

>
>>> +      pst->add_psymbol (tname, true,
>>> +			tdomain, aclass, -1,
>>> +			psymbol_placement::GLOBAL,
>>> +			0, language_c, pst->context->partial_symtabs, of);
> I don't know what ctf_symbol_next does, but I wonder if 'tname' truly
> needs to be copied here, or whether it's possible to change true->false
> in the call.

I think you're right. Will check with Nick Alcock who is maintaining 
libctf to confirm.

>
>>> +  ctf_dict_close (ccp->fp);
> I didn't understand the placement of this addition.
> I expected it to be paired with some kind of dict_open call, but I
> didn't see that.  Could you explain it?

This file closing call is incorrect and will be removed. The proper 
place for
closing should be done in ctf_fp_info's destructor, which corresponds to
ctf_dict_open in elfctf_build_psymtabs.

>>> +/* Callback to build the psymtab for archive member NAME.  */
>>> +
>>> +static int
>>> +build_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
>>> +{
>>> +  struct ctf_per_tu_data *tup = (struct ctf_per_tu_data *) arg;
>>> +  ctf_dict_t *parent = tup->fp;
>>> +  ctf_psymtab *pst;
>>> +  int isparent = 0;
>>> +
>>> +  if (strcmp (name, ".ctf") != 0)
>>> +    {
>>> +      ctf_import (ctf, parent);
>>>       }
> Remove the brace here.

OK.

>
> I don't know what the ".ctf" thing means here, but I guess it's some
> special marker?  Could you explain the layout here a little?
>
>>> +  if (isparent)
>>> +    tup->parent_psymtab = pst;
>>> +  else
>>> +    tup->imported_symtabs.push_back (pst);
> Like, how many psymtabs might be marked as a parent, and how many would
> be pushed as dependencies?
>
> Maybe it would be helpful to know how the new test cases would look
> here.

A CTF archive is a CU which contains all CTF sections. The difference
is the "Types" section in an archive contains conflicting types only.
It's been treated like a dependency and gdb traverses each archive,
via ctf_archive_iter, to extract its CTF info and build its internal
partial/full symtabs. And expand_dependencies is called to expand
the archives when its parent is expanded.

Here is an example in which "struct A" is being defined multiple times:

% cat t.c
struct A;
struct B
{
   int foo;
   struct A *bar;
};

struct A
{
   long a;
   struct B *foo;
};
static struct A *foo __attribute__((used));

int main() { }

% cat t1.c
struct B;
struct A
{
   long a;
   struct B *foo1;
   struct C *bar1;
};
static struct A *bar __attribute__((used));

% cat t.2
struct A { struct B *foo2; };
static struct A *a __attribute__((__used__));
static struct A *conflicty __attribute__((__used__));

% gcc -gt -Wl,--export-dynamic *.c
% objdump --ctf=.ctf a.out
a.out:     file format elf64-x86-64

Contents of CTF section .ctf:

   Header:
     Magic number: dff2
     Version: 4 (CTF_VERSION_3)
     Flags: 0xe (CTF_F_NEWFUNCINFO, CTF_F_IDXSORTED, CTF_F_DYNSTR)
     Function info section:      0x0 -- 0xb (0xc bytes)
     Type section:       0xc -- 0x9f (0x94 bytes)
     String section:     0xa0 -- 0xbc (0x1d bytes)

   Labels:

   Data objects:

   Function objects:
     main -> int (*) (...)

   Variables:

   Types:
      1: long int [0x0:0x40] (size 0x8)
         [0x0] (ID 0x1) (kind 1) long int:64 (aligned at 0x8, format 
0x1, offset:bits 0x0:0x40)
      2: struct B (size 0x10)
         [0x0] (ID 0x2) (kind 6) struct B (aligned at 0x4)
             [0x0] (ID 0x3) (kind 1) int foo:32 (aligned at 0x4, format 
0x1, offset:bits 0x0:0x20)
             [0x40] (ID 0x5) (kind 3) struct A * bar (aligned at 0x8)
      3: int [0x0:0x20] (size 0x4)
         [0x0] (ID 0x3) (kind 1) int:32 (aligned at 0x4, format 0x1, 
offset:bits 0x0:0x20)
      4: struct A (size 0x6)
         [0x0] (ID 0x4) (kind 9) struct A (aligned at 0x6)
      5: struct A * (size 0x8) -> 4: struct A (size 0x6)
         [0x0] (ID 0x5) (kind 3) struct A * (aligned at 0x8)
      6: struct B * (size 0x8) -> 2: struct B (size 0x10)
         [0x0] (ID 0x6) (kind 3) struct B * (aligned at 0x8)
      7: int (*) (...) (size 0x0)
         [0x0] (ID 0x7) (kind 5) int (*) (...) (aligned at 0x8)
      8: struct C (size 0x6)
         [0x0] (ID 0x8) (kind 9) struct C (aligned at 0x6)
      9: struct C * (size 0x8) -> 8: struct C (size 0x6)
         [0x0] (ID 0x9) (kind 3) struct C * (aligned at 0x8)

   Strings:
     0:
     1: A
     3: B
     5: C
     7: bar
     b: foo
     f: int
     13: long int

CTF archive member: /tests/ctf/cross/t.c:

   Header:
     Magic number: dff2
     Version: 4 (CTF_VERSION_3)
     Flags: 0xe (CTF_F_NEWFUNCINFO, CTF_F_IDXSORTED, CTF_F_DYNSTR)
     Parent name: .ctf
     Compilation unit name: /tests/ctf/cross/t.c
     Type section:       0x0 -- 0x23 (0x24 bytes)
     String section:     0x24 -- 0x58 (0x35 bytes)

   Labels:

   Data objects:

   Function objects:

   Variables:

   Types:
      80000001: struct A (size 0x10)
         [0x0] (ID 0x80000001) (kind 6) struct A (aligned at 0x8)
             [0x0] (ID 0x1) (kind 1) long int a:64 (aligned at 0x8, 
format 0x1, offset:bits 0x0:0x40)
             [0x40] (ID 0x6) (kind 3) struct B * foo (aligned at 0x8)

   Strings:
     0:
     1: .ctf
     6: /tests/ctf/cross/t.c
     2c: A
     2e: a
     30: foo

CTF archive member: /tests/ctf/cross/t1.c:

   Header:
     Magic number: dff2
     Version: 4 (CTF_VERSION_3)
     Flags: 0xe (CTF_F_NEWFUNCINFO, CTF_F_IDXSORTED, CTF_F_DYNSTR)
     Parent name: .ctf
     Compilation unit name: /tests/ctf/cross/t1.c
     Type section:       0x0 -- 0x2f (0x30 bytes)
     String section:     0x30 -- 0x6b (0x3c bytes)

   Labels:

   Data objects:

   Function objects:

   Variables:

   Types:
      80000001: struct A (size 0x18)
         [0x0] (ID 0x80000001) (kind 6) struct A (aligned at 0x8)
             [0x0] (ID 0x1) (kind 1) long int a:64 (aligned at 0x8, 
format 0x1, offset:bits 0x0:0x40)
             [0x40] (ID 0x6) (kind 3) struct B * foo1 (aligned at 0x8)
             [0x80] (ID 0x9) (kind 3) struct C * bar1 (aligned at 0x8)

   Strings:
     0:
     1: .ctf
     6: /tests/ctf/cross/t1.c
     2d: A
     2f: a
     31: bar1
     36: foo1

CTF archive member: /tests/ctf/cross/t2.c:

   Header:
     Magic number: dff2
     Version: 4 (CTF_VERSION_3)
     Flags: 0xe (CTF_F_NEWFUNCINFO, CTF_F_IDXSORTED, CTF_F_DYNSTR)
     Parent name: .ctf
     Compilation unit name: /tests/ctf/cross/t2.c
     Type section:       0x0 -- 0x17 (0x18 bytes)
     String section:     0x18 -- 0x4c (0x35 bytes)

   Labels:

   Data objects:

   Function objects:

   Variables:

   Types:
      80000001: struct A (size 0x8)
         [0x0] (ID 0x80000001) (kind 6) struct A (aligned at 0x8)
             [0x0] (ID 0x6) (kind 3) struct B * foo2 (aligned at 0x8)

   Strings:
     0:
     1: .ctf
     6: /tests/ctf/cross/t2.c
     2d: A
     2f: foo2

Where each archive has the CU as its name and ".ctf" as its parent's name
(emitted by the linker.) In terms of psymtabs, there is only one parent
which can have any number of archives.

Thanks again.
>
> thanks,
> Tom

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

* Re: [PATCH,V4 2/2] CTF: multi-CU and archive support
  2021-05-08 20:13     ` Tom Tromey
@ 2021-05-10  3:12       ` Wei-min Pan
  0 siblings, 0 replies; 16+ messages in thread
From: Wei-min Pan @ 2021-05-10  3:12 UTC (permalink / raw)
  To: Tom Tromey, Weimin Pan via Gdb-patches


On 5/8/2021 1:13 PM, Tom Tromey wrote:
>>> +      for (int i = 0; i < arch_cnt; ++i)
>>> +	{
>>> +	  parent_pst->dependencies[i]
>>> +	    = pcu.imported_symtabs.at (i);
>>> +	}
> I forgot to mention -- don't use .at(), just use [] instead.
> This loop could probably just be a memcpy or a std::copy, though an
> explicit loop is also fine.

OK, thanks.

>
> Tom

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

* Re: [PATCH,V4 1/2] CTF: fix incorrect function return type
  2021-05-10  2:40     ` Wei-min Pan
@ 2021-05-10 14:00       ` Tom Tromey
  0 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2021-05-10 14:00 UTC (permalink / raw)
  To: Wei-min Pan via Gdb-patches; +Cc: Tom Tromey, Wei-min Pan

>>>> -  ccp->pst->add_psymbol (name.get (), true,
>>>> +  ccp->pst->add_psymbol (name, true,
>> Same here.

> Sorry, the only place I can find is here and it looks indented correctly?

My apologies - I was unclear here, I meant that the "true" could be
"false" here as well.

>>>> -      pst->add_psymbol (tname.get (), true,
>>>> +      pst->add_psymbol (tname, true,
>> And here.

> Also the indentation looks correct?

This was also the true/false thing, to avoid name copying.

Tom

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

* Re: [PATCH,V4 2/2] CTF: multi-CU and archive support
  2021-05-10  3:11       ` Wei-min Pan
@ 2021-05-10 21:27         ` Tom Tromey
  2021-05-11  0:33           ` Weimin Pan
  2021-05-11 18:58           ` Weimin Pan
  0 siblings, 2 replies; 16+ messages in thread
From: Tom Tromey @ 2021-05-10 21:27 UTC (permalink / raw)
  To: Wei-min Pan; +Cc: Tom Tromey, Weimin Pan via Gdb-patches

>>>>> ">" == Wei-min Pan <weimin.pan@oracle.com> writes:

>> Thank you for taking the time. Please see below for some explanation
>> about CTF and let me know if you need more info.

Thank you.

>> Where each archive has the CU as its name and ".ctf" as its parent's name
>> (emitted by the linker.) In terms of psymtabs, there is only one parent
>> which can have any number of archives.

I think this will probably yield over-expansion in gdb.  What I mean is
that if any symbol is found in CTF, all types and symbols will be
expanded.  Normally this isn't what you want, though -- you only really
need or want to expand the psymtab that has the type (or whatever) that
was asked for.

Normally what you want is for a single psymtab to correspond to a single
source file.  In gdb the dependencies are used for #includes, which I
guess comes from how stabs were originally implemented.

Tom

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

* Re: [PATCH,V4 2/2] CTF: multi-CU and archive support
  2021-05-10 21:27         ` Tom Tromey
@ 2021-05-11  0:33           ` Weimin Pan
  2021-05-11 18:58           ` Weimin Pan
  1 sibling, 0 replies; 16+ messages in thread
From: Weimin Pan @ 2021-05-11  0:33 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Weimin Pan via Gdb-patches


On 5/10/2021 2:27 PM, Tom Tromey wrote:
>>>>>> ">" == Wei-min Pan <weimin.pan@oracle.com> writes:
>>> Thank you for taking the time. Please see below for some explanation
>>> about CTF and let me know if you need more info.
> Thank you.
>
>>> Where each archive has the CU as its name and ".ctf" as its parent's name
>>> (emitted by the linker.) In terms of psymtabs, there is only one parent
>>> which can have any number of archives.
> I think this will probably yield over-expansion in gdb.  What I mean is
> that if any symbol is found in CTF, all types and symbols will be
> expanded.  Normally this isn't what you want, though -- you only really
> need or want to expand the psymtab that has the type (or whatever) that
> was asked for.
>
> Normally what you want is for a single psymtab to correspond to a single
> source file.  In gdb the dependencies are used for #includes, which I
> guess comes from how stabs were originally implemented.

Yes, that's what happens with the CTF layout. While there is some
benefit of generating psymtabs, it's quite limited as you pointed out.
And I doubt that even getting rid of emitting psymtabs altogether
will not help much. Do you have any suggestions?

Thanks.

>
> Tom

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

* Re: [PATCH,V4 2/2] CTF: multi-CU and archive support
  2021-05-10 21:27         ` Tom Tromey
  2021-05-11  0:33           ` Weimin Pan
@ 2021-05-11 18:58           ` Weimin Pan
  2021-05-13 17:21             ` Tom Tromey
  1 sibling, 1 reply; 16+ messages in thread
From: Weimin Pan @ 2021-05-11 18:58 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Weimin Pan via Gdb-patches


On 5/10/2021 2:27 PM, Tom Tromey wrote:
>>>>>> ">" == Wei-min Pan <weimin.pan@oracle.com> writes:
>>> Thank you for taking the time. Please see below for some explanation
>>> about CTF and let me know if you need more info.
> Thank you.
>
>>> Where each archive has the CU as its name and ".ctf" as its parent's name
>>> (emitted by the linker.) In terms of psymtabs, there is only one parent
>>> which can have any number of archives.
> I think this will probably yield over-expansion in gdb.  What I mean is
> that if any symbol is found in CTF, all types and symbols will be
> expanded.  Normally this isn't what you want, though -- you only really
> need or want to expand the psymtab that has the type (or whatever) that
> was asked for.
>
> Normally what you want is for a single psymtab to correspond to a single
> source file.  In gdb the dependencies are used for #includes, which I
> guess comes from how stabs were originally implemented.

Or we can treat CTF archives as CUs, as opposed to dependencies.

>
> Tom

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

* Re: [PATCH,V4 2/2] CTF: multi-CU and archive support
  2021-05-11 18:58           ` Weimin Pan
@ 2021-05-13 17:21             ` Tom Tromey
  2021-05-13 21:40               ` Wei-min Pan
  0 siblings, 1 reply; 16+ messages in thread
From: Tom Tromey @ 2021-05-13 17:21 UTC (permalink / raw)
  To: Weimin Pan; +Cc: Tom Tromey, Weimin Pan via Gdb-patches

>> Normally what you want is for a single psymtab to correspond to a single
>> source file.  In gdb the dependencies are used for #includes, which I
>> guess comes from how stabs were originally implemented.

> Or we can treat CTF archives as CUs, as opposed to dependencies.

Yeah, I tend to think that would be better.

Tom

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

* Re: [PATCH,V4 2/2] CTF: multi-CU and archive support
  2021-05-13 17:21             ` Tom Tromey
@ 2021-05-13 21:40               ` Wei-min Pan
  0 siblings, 0 replies; 16+ messages in thread
From: Wei-min Pan @ 2021-05-13 21:40 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Weimin Pan via Gdb-patches


On 5/13/2021 10:21 AM, Tom Tromey wrote:
>>> Normally what you want is for a single psymtab to correspond to a single
>>> source file.  In gdb the dependencies are used for #includes, which I
>>> guess comes from how stabs were originally implemented.
>> Or we can treat CTF archives as CUs, as opposed to dependencies.
> Yeah, I tend to think that would be better.

OK, will make the change. Thanks.

> Tom

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

* Re: [PATCH,V4 1/2] CTF: fix incorrect function return type
  2021-05-07 20:07   ` [PATCH,V4 1/2] CTF: fix incorrect function return type Tom Tromey
  2021-05-10  2:40     ` Wei-min Pan
@ 2021-05-16 22:31     ` Wei-min Pan
  1 sibling, 0 replies; 16+ messages in thread
From: Wei-min Pan @ 2021-05-16 22:31 UTC (permalink / raw)
  To: Tom Tromey, Weimin Pan via Gdb-patches


On 5/7/2021 1:07 PM, Tom Tromey wrote:
>>>>>> ">" == Weimin Pan via Gdb-patches <gdb-patches@sourceware.org> writes:
>>> The problems can be illustrated, with any program, below:
>>> (gdb) print main
>>> $1 = {main} 0x0
> Hi.  Sorry about the delay in my re-review.
> It's fine (and expected) to send patch pings to the list.
> It helps keep us honest I guess.
>
>>> -      sym->compute_and_set_names (name.get (), true, objfile->per_bfd);
>>> +      sym->compute_and_set_names (name, true, objfile->per_bfd);
> If the lifetime of 'name' is ok, then you don't need to copy it here --
> so you can pass 'false' as the second parameter.  See the comment in
> symtab.c just before general_symbol_info::compute_and_set_names.
>
>>> +  name = ctf_type_name_raw (fp, tid);
>>> +  if (name == nullptr || strlen (name) == 0)
>>> +  {
> The braces (and the code they surround) look under-indented to me.
>
>>> -  ccp->pst->add_psymbol (name.get (), true,
>>> +  ccp->pst->add_psymbol (name, true,
> Same here.
>
>>> -      pst->add_psymbol (tname.get (), true,
>>> +      pst->add_psymbol (tname, true,
> And here.
>
> If you fix these up, and it still works, it is fine to push.
> If it doesn't work then that would tend to put some doubt on the other
> parts of the patch that don't copy names.

After making all the changes you suggested, plus changing 'true' to 
'false' in the
following two places to avoid name copying.

 > >> -  ccp->pst->add_psymbol (name.get (), true,
 > >> +  ccp->pst->add_psymbol (name, true,
 >
 > Same here.
 >
 > >> -      pst->add_psymbol (tname.get (), true,
 > >> +      pst->add_psymbol (tname, true,
 >
 > And here.

The CTF tests are passing. Just pushed.

Thanks.

>
> thanks,
> Tom

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

end of thread, other threads:[~2021-05-16 22:31 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-27  1:00 [PATCH,V4 0/2] CTF: bug fixes and new features Weimin Pan
2021-04-27  1:00 ` [PATCH,V4 1/2] CTF: fix incorrect function return type Weimin Pan
2021-04-27  1:00   ` [PATCH,V4 2/2] CTF: multi-CU and archive support Weimin Pan
2021-05-07 20:29     ` Tom Tromey
2021-05-10  3:11       ` Wei-min Pan
2021-05-10 21:27         ` Tom Tromey
2021-05-11  0:33           ` Weimin Pan
2021-05-11 18:58           ` Weimin Pan
2021-05-13 17:21             ` Tom Tromey
2021-05-13 21:40               ` Wei-min Pan
2021-05-08 20:13     ` Tom Tromey
2021-05-10  3:12       ` Wei-min Pan
2021-05-07 20:07   ` [PATCH,V4 1/2] CTF: fix incorrect function return type Tom Tromey
2021-05-10  2:40     ` Wei-min Pan
2021-05-10 14:00       ` Tom Tromey
2021-05-16 22:31     ` Wei-min Pan

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