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

[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 debug format:
       https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564092.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 (5):
  CTF: fix incorrect function return type
  CTF: set up debug info for function arguments
  CTF: handle forward reference type
  CTF: add all members of an enum type to psymtab
  CTF: multi-CU and archive support

 gdb/ChangeLog                             |  35 +++
 gdb/ctfread.c                             | 499 +++++++++++++++++++++---------
 gdb/testsuite/ChangeLog                   |  19 ++
 gdb/testsuite/gdb.base/ctf-ptype.c        |  12 +
 gdb/testsuite/gdb.base/ctf-ptype.exp      |  21 +-
 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 ++++++++++++++++++++
 19 files changed, 1236 insertions(+), 152 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] 21+ messages in thread

* [PATCH,V2 1/5] CTF: fix incorrect function return type
  2021-03-02  1:53 [PATCH,V2 0/5] CTF: bug fixes and new features Weimin Pan
@ 2021-03-02  1:53 ` Weimin Pan
  2021-03-02  1:53   ` [PATCH,V2 2/5] CTF: set up debug info for function arguments Weimin Pan
  2021-03-03 20:37   ` [PATCH,V2 1/5] CTF: fix incorrect function return type Tom Tromey
  0 siblings, 2 replies; 21+ messages in thread
From: Weimin Pan @ 2021-03-02  1:53 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                        |   5 +
 gdb/ctfread.c                        |   5 +-
 gdb/testsuite/ChangeLog              |   4 +
 gdb/testsuite/gdb.ctf/funcreturn.exp | 190 ++++++++++++++++++++
 gdb/testsuite/gdb.ctf/whatis.c       | 339 +++++++++++++++++++++++++++++++++++
 5 files changed, 539 insertions(+), 4 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 9d928b0..88278d1 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
+
+	* ctfread.c (new_symbol): Set function address.
+	(read_func_kind_type): Remove incorrect type name setting.
+
 2021-01-29  Tom de Vries  <tdevries@suse.de>
 
 	PR breakpoints/26063
diff --git a/gdb/ctfread.c b/gdb/ctfread.c
index 866977a..5a68d9c 100644
--- a/gdb/ctfread.c
+++ b/gdb/ctfread.c
@@ -462,6 +462,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)
@@ -656,10 +657,6 @@ 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 ()));
-
   type->set_code (TYPE_CODE_FUNC);
   ctf_func_type_info (fp, tid, &cfi);
   rettype = get_tid_type (of, cfi.ctc_return);
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 36ba5d0..3d71354 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
+
+	* gdb.base/ctf-funcreturn.exp: New file.
+
 2021-01-29  Tom de Vries  <tdevries@suse.de>
 
 	PR breakpoints/26063
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] 21+ messages in thread

* [PATCH,V2 2/5] CTF: set up debug info for function arguments
  2021-03-02  1:53 ` [PATCH,V2 1/5] CTF: fix incorrect function return type Weimin Pan
@ 2021-03-02  1:53   ` Weimin Pan
  2021-03-02  1:53     ` [PATCH,V2 3/5] CTF: handle forward reference type Weimin Pan
                       ` (2 more replies)
  2021-03-03 20:37   ` [PATCH,V2 1/5] CTF: fix incorrect function return type Tom Tromey
  1 sibling, 3 replies; 21+ messages in thread
From: Weimin Pan @ 2021-03-02  1:53 UTC (permalink / raw)
  To: gdb-patches

Added this support in read_func_kind_type after gcc started generating
CTF for function arguments.

Expanded gdb.base/ctf-ptype.exp to test function arguments. Also fixed
some typos.

---
 gdb/ChangeLog                        |  4 ++++
 gdb/ctfread.c                        | 29 ++++++++++++++++++++++++++++-
 gdb/testsuite/ChangeLog              |  4 ++++
 gdb/testsuite/gdb.base/ctf-ptype.exp | 19 ++++++++++++++++---
 4 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 88278d1..0a839ba 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
 
+	* ctfread.c (read_func_kind_type): Set up function arguments.
+
+2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
+
 	* ctfread.c (new_symbol): Set function address.
 	(read_func_kind_type): Remove incorrect type name setting.
 
diff --git a/gdb/ctfread.c b/gdb/ctfread.c
index 5a68d9c..75b098c 100644
--- a/gdb/ctfread.c
+++ b/gdb/ctfread.c
@@ -652,8 +652,9 @@ struct ctf_tid_and_type
 {
   struct objfile *of = ccp->of;
   ctf_dict_t *fp = ccp->fp;
-  struct type *type, *rettype;
+  struct type *type, *rettype, *atype;
   ctf_funcinfo_t cfi;
+  uint32_t argc;
 
   type = alloc_type (of);
 
@@ -663,6 +664,32 @@ struct ctf_tid_and_type
   TYPE_TARGET_TYPE (type) = rettype;
   set_type_align (type, ctf_type_align (fp, tid));
 
+  /* Set up function's arguments.  */
+  argc = cfi.ctc_argc;
+  type->set_num_fields (argc);
+  if (cfi.ctc_flags & CTF_FUNC_VARARG)
+    type->set_has_varargs (true);
+
+  if (argc != 0)
+    {
+      std::vector<ctf_id_t> argv (argc);
+      if (ctf_func_type_args (fp, tid, argc, argv.data ()) == CTF_ERR)
+	return NULL;
+
+      type->set_fields
+	((struct field *) TYPE_ZALLOC (type, argc * sizeof (struct field)));
+      struct type *void_type = objfile_type (of)->builtin_void;
+      /* If failed to find the argument type, fill it with void_type.  */
+      for (int iparam = 0; iparam < argc; iparam++)
+	{
+	  atype = get_tid_type (of, argv[iparam]);
+	  if (atype)
+	    type->field (iparam).set_type (atype);
+	  else
+	    type->field (iparam).set_type (void_type);
+	}
+    }
+
   return set_tid_type (of, tid, type);
 }
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 3d71354..9445362 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,9 @@
 2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
 
+	* gdb.base/ctf-ptype.exp: Add function tests and fix typos.
+
+2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
+
 	* gdb.base/ctf-funcreturn.exp: New file.
 
 2021-01-29  Tom de Vries  <tdevries@suse.de>
diff --git a/gdb/testsuite/gdb.base/ctf-ptype.exp b/gdb/testsuite/gdb.base/ctf-ptype.exp
index ffe40f1..056f712 100644
--- a/gdb/testsuite/gdb.base/ctf-ptype.exp
+++ b/gdb/testsuite/gdb.base/ctf-ptype.exp
@@ -63,10 +63,10 @@ gdb_test "ptype struct t_struct" "type = struct t_struct \{.*\[\r\n\]    (unsign
 
 # Test the equivalence between '.' and '->' for struct member references.
 
-if [gdb_test "ptype v_t_struct_p.v_float_member"	"type = float"]<0 then {
+if [gdb_test "ptype v_struct1.v_float_member"	"type = float"]<0 then {
     return -1
 }
-if [gdb_test "ptype v_t_struct_p->v_float_member"	"type = float"]<0 then {
+if [gdb_test "ptype v_struct1->v_float_member"	"type = float"]<0 then {
     return -1
 }
 if [gdb_test "ptype v_t_struct_p.v_float_member"	"type = float"]<0 then {
@@ -211,7 +211,7 @@ gdb_test "ptype the_highest" \
 
 gdb_test "ptype the_highest.anonymous_level_1" \
          "type = struct \{.*\[\r\n\] *int b;.*\[\r\n\] *struct \{.*\[\r\n\] *int c;.*\[\r\n\] *\} anonymous_level_2;.*\[\r\n\]}.*" \
-         "ptype the_highest"
+         "ptype the_highest.anonymous_level_1"
 
 # Print the type of the identifier ID, and check the response:
 # - Expect to see PROTOTYPED as the type.  PROTOTYPED is not a regular
@@ -255,8 +255,21 @@ proc ptype_maybe_prototyped { id prototyped plain { overprototyped "NO-MATCH" }
     }
 }
 
+ptype_maybe_prototyped "func_type" "int (*)(int (*)(int, float), float)" \
+                                   "int (*)()"
 ptype_maybe_prototyped "old_fptr" "double (*)()" "double (*)()" \
                                   "double (*)(void)"
+ptype_maybe_prototyped "new_fptr" "double (*)()" "double (*)()"
+ptype_maybe_prototyped "fptr" "int (*)(int, float)" "int (*)()"
+ptype_maybe_prototyped "fptr2" "int *(*)(int (*)(int, float), float)" \
+                               "int *(*)()"
+ptype_maybe_prototyped "xptr" "int (*)(int (*)(), int (*)(), int)" \
+                              "int (*)()" \
+                              "int (*)(int (*)(void), int (*)(void), int)"
+ptype_maybe_prototyped "ffptr" "int (*(*)(char))(short int)" \
+                               "int (*(*)())()"
+ptype_maybe_prototyped "fffptr" "int (*(*(*)(char))(short int))(long int)" \
+                                "int (*(*(*)())())()"
 
 # Test printing type of string constants and array constants, but
 # requires a running process.  These call malloc, and can take a long
-- 
1.8.3.1


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

* [PATCH,V2 3/5] CTF: handle forward reference type
  2021-03-02  1:53   ` [PATCH,V2 2/5] CTF: set up debug info for function arguments Weimin Pan
@ 2021-03-02  1:53     ` Weimin Pan
  2021-03-02  1:53       ` [PATCH,V2 4/5] CTF: add all members of an enum type to psymtab Weimin Pan
  2021-03-03 20:49       ` [PATCH,V2 3/5] CTF: handle forward reference type Tom Tromey
  2021-03-02 12:23     ` [PATCH,V2 2/5] CTF: set up debug info for function arguments Lancelot SIX
  2021-03-03 20:41     ` Tom Tromey
  2 siblings, 2 replies; 21+ messages in thread
From: Weimin Pan @ 2021-03-02  1:53 UTC (permalink / raw)
  To: gdb-patches

Added function fetch_tid_type which calls get_tid_type and will set up
the type, associated with a tid, if it is not read in yet. Also implement 
function read_forward_type which handles the CTF_K_FORWARD kind.

Expanded gdb.base/ctf-ptype.exp to add cases with forward references. 

---
 gdb/ChangeLog                        |  6 +++
 gdb/ctfread.c                        | 78 ++++++++++++++++++++++++++++++------
 gdb/testsuite/ChangeLog              |  6 +++
 gdb/testsuite/gdb.base/ctf-ptype.c   | 12 ++++++
 gdb/testsuite/gdb.base/ctf-ptype.exp |  2 +
 5 files changed, 91 insertions(+), 13 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0a839ba..dac61a3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
 
+	* ctfread.c (fetch_tid_type): New function, use throughout file.
+	(read_forward_type): New function.
+	(read_type_record): Call read_forward_type.
+
+2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
+
 	* ctfread.c (read_func_kind_type): Set up function arguments.
 
 2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
diff --git a/gdb/ctfread.c b/gdb/ctfread.c
index 75b098c..d3e6ad5 100644
--- a/gdb/ctfread.c
+++ b/gdb/ctfread.c
@@ -186,6 +186,8 @@ static struct type *read_typedef_type (struct ctf_context *cp, ctf_id_t tid,
 static void process_struct_members (struct ctf_context *cp, ctf_id_t tid,
 				    struct type *type);
 
+static struct type *read_forward_type (struct ctf_context *cp, ctf_id_t tid);
+
 static struct symbol *new_symbol (struct ctf_context *cp, struct type *type,
 				  ctf_id_t tid);
 
@@ -269,6 +271,25 @@ struct ctf_tid_and_type
     return NULL;
 }
 
+/* Fetch the type for TID in CCP OF's tid_and_type hash, add the type to
+   context CCP if hash is empty or TID does not have a saved type.  */
+
+static struct type *
+fetch_tid_type (struct ctf_context *ccp, ctf_id_t tid)
+{
+  struct objfile *of = ccp->of;
+  struct type *typ;
+
+  typ = get_tid_type (of, tid);
+  if (typ == NULL)
+    {
+      ctf_add_type_cb (tid, ccp);
+      typ = get_tid_type (of, tid);
+    }
+
+  return typ;
+}
+
 /* Return the size of storage in bits for INTEGER, FLOAT, or ENUM.  */
 
 static int
@@ -368,7 +389,7 @@ struct ctf_tid_and_type
   FIELD_NAME (*fp) = name;
 
   kind = ctf_type_kind (ccp->fp, tid);
-  t = get_tid_type (ccp->of, tid);
+  t = fetch_tid_type (ccp, tid);
   if (t == NULL)
     {
       t = read_type_record (ccp, tid);
@@ -660,7 +681,7 @@ struct ctf_tid_and_type
 
   type->set_code (TYPE_CODE_FUNC);
   ctf_func_type_info (fp, tid, &cfi);
-  rettype = get_tid_type (of, cfi.ctc_return);
+  rettype = fetch_tid_type (ccp, cfi.ctc_return);
   TYPE_TARGET_TYPE (type) = rettype;
   set_type_align (type, ctf_type_align (fp, tid));
 
@@ -787,11 +808,11 @@ struct ctf_tid_and_type
       return NULL;
     }
 
-  element_type = get_tid_type (objfile, ar.ctr_contents);
+  element_type = fetch_tid_type (ccp, ar.ctr_contents);
   if (element_type == NULL)
     return NULL;
 
-  idx_type = get_tid_type (objfile, ar.ctr_index);
+  idx_type = fetch_tid_type (ccp, ar.ctr_index);
   if (idx_type == NULL)
     idx_type = objfile_type (objfile)->builtin_int;
 
@@ -819,7 +840,7 @@ struct ctf_tid_and_type
   struct objfile *objfile = ccp->of;
   struct type *base_type, *cv_type;
 
-  base_type = get_tid_type (objfile, btid);
+  base_type = fetch_tid_type (ccp, btid);
   if (base_type == NULL)
     {
       base_type = read_type_record (ccp, btid);
@@ -843,7 +864,7 @@ struct ctf_tid_and_type
   ctf_dict_t *fp = ccp->fp;
   struct type *base_type, *cv_type;
 
-  base_type = get_tid_type (objfile, btid);
+  base_type = fetch_tid_type (ccp, btid);
   if (base_type == NULL)
     {
       base_type = read_type_record (ccp, btid);
@@ -869,7 +890,7 @@ struct ctf_tid_and_type
   struct objfile *objfile = ccp->of;
   struct type *base_type, *cv_type;
 
-  base_type = get_tid_type (objfile, btid);
+  base_type = fetch_tid_type (ccp, btid);
   if (base_type == NULL)
     {
       base_type = read_type_record (ccp, btid);
@@ -896,7 +917,7 @@ struct ctf_tid_and_type
   char *aname = obstack_strdup (&objfile->objfile_obstack, name);
   this_type = init_type (objfile, TYPE_CODE_TYPEDEF, 0, aname);
   set_tid_type (objfile, tid, this_type);
-  target_type = get_tid_type (objfile, btid);
+  target_type = fetch_tid_type (ccp, btid);
   if (target_type != this_type)
     TYPE_TARGET_TYPE (this_type) = target_type;
   else
@@ -915,7 +936,7 @@ struct ctf_tid_and_type
   struct objfile *of = ccp->of;
   struct type *target_type, *type;
 
-  target_type = get_tid_type (of, btid);
+  target_type = fetch_tid_type (ccp, btid);
   if (target_type == NULL)
     {
       target_type = read_type_record (ccp, btid);
@@ -932,6 +953,34 @@ struct ctf_tid_and_type
   return set_tid_type (of, tid, type);
 }
 
+/* Read all information from a TID of CTF_K_FORWARD.  */
+
+static struct type *
+read_forward_type (struct ctf_context *ccp, ctf_id_t tid)
+{
+  struct objfile *of = ccp->of;
+  ctf_dict_t *fp = ccp->fp;
+  struct type *type;
+  uint32_t kind;
+
+  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 ()));
+
+  kind = ctf_type_kind_forwarded (fp, tid);
+  if (kind == CTF_K_UNION)
+    type->set_code (TYPE_CODE_UNION);
+  else
+    type->set_code (TYPE_CODE_STRUCT);
+
+  TYPE_LENGTH (type) = 0;
+  type->set_is_stub (true);
+
+  return set_tid_type (of, tid, type);
+}
+
 /* Read information associated with type TID.  */
 
 static struct type *
@@ -985,6 +1034,9 @@ struct ctf_tid_and_type
       case CTF_K_ARRAY:
 	type = read_array_type (ccp, tid);
 	break;
+      case CTF_K_FORWARD:
+	type = read_forward_type (ccp, tid);
+	break;
       case CTF_K_UNKNOWN:
 	break;
       default:
@@ -1131,7 +1183,7 @@ struct ctf_tid_and_type
   if ((tid = ctf_lookup_by_symbol (ccp->fp, idx)) == CTF_ERR)
     return NULL;
 
-  type = get_tid_type (ccp->of, tid);
+  type = fetch_tid_type (ccp, tid);
   if (type == NULL)
     return NULL;
 
@@ -1165,7 +1217,7 @@ struct ctf_tid_and_type
     return NULL;
 
   tid = ctf_lookup_by_symbol (ccp->fp, idx);
-  ftype = get_tid_type (ccp->of, tid);
+  ftype = fetch_tid_type (ccp, tid);
   if (finfo.ctc_flags & CTF_FUNC_VARARG)
     ftype->set_has_varargs (true);
   ftype->set_num_fields (argc);
@@ -1179,7 +1231,7 @@ struct ctf_tid_and_type
      to find the argument type.  */
   for (int iparam = 0; iparam < argc; iparam++)
     {
-      atyp = get_tid_type (ccp->of, argv[iparam]);
+      atyp = fetch_tid_type (ccp, argv[iparam]);
       if (atyp)
 	ftype->field (iparam).set_type (atyp);
       else
@@ -1187,7 +1239,7 @@ struct ctf_tid_and_type
     }
 
   sym = new_symbol (ccp, ftype, tid);
-  rettyp = get_tid_type (ccp->of, finfo.ctc_return);
+  rettyp = fetch_tid_type (ccp, finfo.ctc_return);
   if (rettyp != NULL)
     SYMBOL_TYPE (sym) = rettyp;
   else
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 9445362..4f9a3ec 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,11 @@
 2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
 
+	* gdb.base/ctf-ptype.c: Add struct link containing a forward
+	reference type.
+	* gdb.base/ctf-ptype.exp: Add "ptype struct link"
+
+2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
+
 	* gdb.base/ctf-ptype.exp: Add function tests and fix typos.
 
 2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
diff --git a/gdb/testsuite/gdb.base/ctf-ptype.c b/gdb/testsuite/gdb.base/ctf-ptype.c
index cbc7815..51c7c68 100644
--- a/gdb/testsuite/gdb.base/ctf-ptype.c
+++ b/gdb/testsuite/gdb.base/ctf-ptype.c
@@ -124,6 +124,18 @@ struct {
    a symbol.  */
 t_struct3 v_struct3;
 
+/**** 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[3];
+} *s_link;
+
 /**** unions *******/
 
 union t_union {
diff --git a/gdb/testsuite/gdb.base/ctf-ptype.exp b/gdb/testsuite/gdb.base/ctf-ptype.exp
index 056f712..7dd6d95 100644
--- a/gdb/testsuite/gdb.base/ctf-ptype.exp
+++ b/gdb/testsuite/gdb.base/ctf-ptype.exp
@@ -76,6 +76,8 @@ if [gdb_test "ptype v_t_struct_p->v_float_member"	"type = float"]<0 then {
     return -1
 }
 
+gdb_test "ptype struct link" "type = struct link \{\[\r\n\]+\[ \t\]+struct link \\*next;\[\r\n\]+\[ \t\]+struct link \\*\\(\\*linkfunc\\)\\((struct link \\*, int|void|)\\);\[\r\n\]+\[ \t\]+struct t_struct stuff.3.;\[\r\n\]+\}.*" "ptype linked list structure"
+
 #
 # test ptype command with unions
 #
-- 
1.8.3.1


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

* [PATCH,V2 4/5] CTF: add all members of an enum type to psymtab
  2021-03-02  1:53     ` [PATCH,V2 3/5] CTF: handle forward reference type Weimin Pan
@ 2021-03-02  1:53       ` Weimin Pan
  2021-03-02  1:53         ` [PATCH,V2 5/5] CTF: multi-CU and archive support Weimin Pan
  2021-03-03 20:46         ` [PATCH,V2 4/5] CTF: add all members of an enum type to psymtab Tom Tromey
  2021-03-03 20:49       ` [PATCH,V2 3/5] CTF: handle forward reference type Tom Tromey
  1 sibling, 2 replies; 21+ messages in thread
From: Weimin Pan @ 2021-03-02  1:53 UTC (permalink / raw)
  To: gdb-patches

With the following change which was made last April:

    [gdb] Use partial symbol table to find language for main
    commit d3214198119c1a2f9a6a2b8fcc56d8c324e1a245

The ctf reader was modified to enter all members of an enum type,
similar to what the dwarf2 reader did, into the psymtab or gdb
won't be able to find them. In addition, the empty name checking
needed to be moved down so members of a unnamed enum were not left
out. 

---
 gdb/ChangeLog |  5 +++++
 gdb/ctfread.c | 30 +++++++++++++++++++++++++++---
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index dac61a3..31cac5c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
 
+	* ctfread.c (ctf_psymtab_add_enums): New function.
+	(ctf_psymtab_type_cb): call ctf_psymtab_add_enums.
+
+2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
+
 	* ctfread.c (fetch_tid_type): New function, use throughout file.
 	(read_forward_type): New function.
 	(read_type_record): Call read_forward_type.
diff --git a/gdb/ctfread.c b/gdb/ctfread.c
index d3e6ad5..bb73949 100644
--- a/gdb/ctfread.c
+++ b/gdb/ctfread.c
@@ -1294,6 +1294,27 @@ struct ctf_tid_and_type
   return result;
 }
 
+/* Add all members of an enum with type TID to partial symbol table.  */
+
+static void
+ctf_psymtab_add_enums (struct ctf_context *ccp, ctf_id_t tid)
+{
+  int val;
+  const char *ename;
+  ctf_next_t *i = NULL;
+
+  while ((ename = ctf_enum_next (ccp->fp, tid, &i, &val)) != NULL)
+    {
+      ccp->pst->add_psymbol (ename, true,
+			     VAR_DOMAIN, LOC_CONST, -1,
+			     psymbol_placement::GLOBAL,
+			     0, language_c, ccp->of);
+    }
+  if (ctf_errno (ccp->fp) != ECTF_NEXT_END)
+    complaint (_("ctf_enum_next ctf_psymtab_add_enums failed - %s"),
+	       ctf_errmsg (ctf_errno (ccp->fp)));
+}
+
 /* Read in full symbols for PST, and anything it depends on.  */
 
 void
@@ -1415,17 +1436,17 @@ struct ctf_tid_and_type
 
   ccp = (struct ctf_context *) arg;
   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (ccp->fp, tid));
-  if (name == NULL || strlen (name.get ()) == 0)
-    return 0;
 
   domain_enum domain = UNDEF_DOMAIN;
   enum address_class aclass = LOC_UNDEF;
   kind = ctf_type_kind (ccp->fp, tid);
   switch (kind)
     {
+      case CTF_K_ENUM:
+	ctf_psymtab_add_enums (ccp, tid);
+	/* FALL THROUGH */
       case CTF_K_STRUCT:
       case CTF_K_UNION:
-      case CTF_K_ENUM:
 	domain = STRUCT_DOMAIN;
 	aclass = LOC_TYPEDEF;
 	break;
@@ -1456,6 +1477,9 @@ struct ctf_tid_and_type
 	return 0;
     }
 
+  if (name == NULL || strlen (name.get ()) == 0)
+    return 0;
+
   ccp->pst->add_psymbol (name.get (), true,
 			 domain, aclass, section,
 			 psymbol_placement::GLOBAL,
-- 
1.8.3.1


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

* [PATCH,V2 5/5] CTF: multi-CU and archive support
  2021-03-02  1:53       ` [PATCH,V2 4/5] CTF: add all members of an enum type to psymtab Weimin Pan
@ 2021-03-02  1:53         ` Weimin Pan
  2021-03-02 23:15           ` Lancelot SIX
  2021-03-03 20:46         ` [PATCH,V2 4/5] CTF: add all members of an enum type to psymtab Tom Tromey
  1 sibling, 1 reply; 21+ messages in thread
From: Weimin Pan @ 2021-03-02  1:53 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/ChangeLog                             |  15 ++
 gdb/ctfread.c                             | 365 +++++++++++++++++++-----------
 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/multi.exp           |  42 ++++
 15 files changed, 526 insertions(+), 132 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/ChangeLog b/gdb/ChangeLog
index 31cac5c..52a325f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,20 @@
 2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
 
+	* ctfread.c (ctf_per_tu_data): New struct.
+	(add_stt_entries): New funtion.
+	(add_stt_obj): Use it.
+	(add_stt_func): Likewise.
+	(ctf_psymtab_add_stt_entries): New function.
+	(ctf_psymtab_add_stt_obj): Use it.
+	(ctf_psymtab_add_stt_func): Likewise.
+	(ctf_psymtab::expand_psymtab): Call expand_dependencies to expand
+	archives.
+	(scan_partial_symbols): Set up partial symtab list.
+	(build_ctf_archive_member): New function.
+	(elfctf_build_psymtabs): Call Iterater to set up archives.
+
+2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
+
 	* ctfread.c (ctf_psymtab_add_enums): New function.
 	(ctf_psymtab_type_cb): call ctf_psymtab_add_enums.
 
diff --git a/gdb/ctfread.c b/gdb/ctfread.c
index bb73949..9ddd1db 100644
--- a/gdb/ctfread.c
+++ b/gdb/ctfread.c
@@ -80,6 +80,7 @@
 #include "complaints.h"
 #include "block.h"
 #include "ctfread.h"
+#include "objfiles.h"
 #include "psympriv.h"
 
 #if ENABLE_LIBCTF
@@ -116,6 +117,7 @@ struct ctf_context
   ctf_dict_t *fp;
   struct objfile *of;
   partial_symtab *pst;
+  ctf_archive_t *arc;
   struct buildsym_compunit *builder;
 };
 
@@ -162,6 +164,40 @@ 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;
+  // struct partial_symtab *psymtab;
+  /* Push P to the back of IMPORTED_SYMTABS, allocated IMPORTED_SYMTABS
+     first if required.  */
+  void imported_symtabs_push (struct partial_symtab *p)
+  {
+    if (imported_symtabs == nullptr)
+      imported_symtabs = new std::vector <struct partial_symtab *>;
+    imported_symtabs->push_back (p);
+  }
+  /* Return the size of IMPORTED_SYMTABS if it is allocated, otherwise
+     return 0.  */
+  size_t imported_symtabs_size () const
+  {
+    if (imported_symtabs == nullptr)
+      return 0;
+    return imported_symtabs->size ();
+  }
+
+  /* Delete IMPORTED_SYMTABS and set the pointer back to nullptr.  */
+  void imported_symtabs_free ()
+  {
+    delete imported_symtabs;
+    imported_symtabs = nullptr;
+  }
+  std::vector <struct partial_symtab *> *imported_symtabs;
+};
 
 /* Local function prototypes */
 
@@ -241,10 +277,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 == 0)
+    *slot = XOBNEW (&of->objfile_obstack, struct ctf_tid_and_type);
   **slot = ids;
   return typ;
 }
@@ -459,7 +493,7 @@ struct ctf_tid_and_type
   struct symbol *sym = NULL;
 
   gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
-  if (name != NULL)
+  if (name != NULL && strcmp(name.get (), ""))
     {
       sym = new (&objfile->objfile_obstack) symbol;
       OBJSTAT (objfile, n_syms++);
@@ -943,7 +977,7 @@ struct ctf_tid_and_type
       if (target_type == NULL)
 	{
 	  complaint (_("read_pointer_type: NULL target type (%ld)"), btid);
-	  target_type = objfile_type (ccp->of)->builtin_error;
+	  target_type = objfile_type (of)->builtin_error;
 	}
     }
 
@@ -1055,6 +1089,12 @@ struct ctf_tid_and_type
   struct type *type;
   uint32_t kind;
 
+  if (info_verbose)
+    {
+      printf_filtered (_("ctf_add_type_cb adding tid %lx..."), tid);
+      gdb_flush (gdb_stdout);
+    }
+
   /* Check if tid's type has already been defined.  */
   type = get_tid_type (ccp->of, tid);
   if (type != NULL)
@@ -1141,7 +1181,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:
@@ -1171,81 +1212,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 = NULL;
+  const char *tname;
   ctf_id_t tid;
+  struct symbol *sym = NULL;
+  struct type *type;
 
-  if ((tid = ctf_lookup_by_symbol (ccp->fp, idx)) == CTF_ERR)
-    return NULL;
-
-  type = fetch_tid_type (ccp, tid);
-  if (type == NULL)
-    return NULL;
-
-  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 == NULL)
+	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 NULL;
-
-  argc = finfo.ctc_argc;
-  if (ctf_func_args (ccp->fp, idx, argc, argv) == CTF_ERR)
-    return NULL;
-
-  gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (ccp->fp, idx));
-  if (name == NULL)
-    return NULL;
-
-  tid = ctf_lookup_by_symbol (ccp->fp, idx);
-  ftype = fetch_tid_type (ccp, tid);
-  if (finfo.ctc_flags & CTF_FUNC_VARARG)
-    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)));
+  add_stt_entries (ccp, 0);
+}
 
-  /* 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 entries in function info section.  */
 
-  sym = new_symbol (ccp, ftype, tid);
-  rettyp = fetch_tid_type (ccp, finfo.ctc_return);
-  if (rettyp != NULL)
-    SYMBOL_TYPE (sym) = rettyp;
-  else
-    SYMBOL_TYPE (sym) = void_type;
-
-  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.  */
@@ -1268,12 +1276,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, NULL,
 		       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.
@@ -1315,46 +1331,94 @@ 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 = NULL;
+  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, 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)
-    complaint (_("ctf_type_iter psymtab_to_symtab failed - %s"),
+    complaint (_("ctf_type_iter expand_psymtab failed - %s"),
 	       ctf_errmsg (ctf_errno (ccp->fp)));
 
 
   /* Iterate over entries in variable info section.  */
   if (ctf_variable_iter (ccp->fp, ctf_add_var_cb, ccp) == CTF_ERR)
-    complaint (_("ctf_variable_iter psymtab_to_symtab failed - %s"),
+    complaint (_("ctf_variable_iter expand_psymtab failed - %s"),
 	       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 == NULL)
-	{
-	  if (ctf_errno (ccp->fp) == EINVAL
-	      || ctf_errno (ccp->fp) == ECTF_NOSYMTAB)
-	    break;
-	  sym = add_stt_func (ccp, i);
-	}
-      if (sym == NULL)
-	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;
 }
 
@@ -1407,6 +1471,7 @@ struct ctf_tid_and_type
 
 static ctf_psymtab *
 create_partial_symtab (const char *name,
+		       ctf_archive_t *arc,
 		       ctf_dict_t *cfp,
 		       struct objfile *objfile)
 {
@@ -1416,10 +1481,11 @@ struct ctf_tid_and_type
   pst = new ctf_psymtab (name, objfile, 0);
 
   ccx = XOBNEW (&objfile->objfile_obstack, struct ctf_context);
+  ccx->arc = arc;
   ccx->fp = cfp;
+  ctf_ref (cfp);
   ccx->of = objfile;
   ccx->pst = pst;
-  ccx->builder = nullptr;
   pst->context = ccx;
 
   return pst;
@@ -1506,12 +1572,29 @@ struct ctf_tid_and_type
    debugging information is available.  */
 
 static void
-scan_partial_symbols (ctf_dict_t *cfp, struct objfile *of)
+scan_partial_symbols (ctf_dict_t *cfp,
+		      struct ctf_per_tu_data *pcu,
+		      const char *aname)
 {
-  bfd *abfd = of->obfd;
-  const char *name = bfd_get_filename (abfd);
-  ctf_psymtab *pst = create_partial_symtab (name, cfp, of);
+  struct objfile *of = pcu->of;
+  bool isparent = 0;
+
+  if (info_verbose)
+    {
+      printf_filtered (_("Scanning partial symbols for %s..."), aname);
+      gdb_flush (gdb_stdout);
+    }
+
+  if (strcmp (aname, ".ctf") == 0)
+    isparent = 1;
 
+  if (info_verbose)
+    {
+      printf_filtered (_("Creating partial symtab %s..."), aname);
+      gdb_flush (gdb_stdout);
+    }
+
+  ctf_psymtab *pst = create_partial_symtab (aname, pcu->arc, cfp, of);
   struct ctf_context *ccx = pst->context;
 
   if (ctf_type_iter (cfp, ctf_psymtab_type_cb, ccx) == CTF_ERR)
@@ -1525,46 +1608,39 @@ 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;
-	}
-      gdb::unique_xmalloc_ptr<char> tname (ctf_type_aname_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.get (), true,
-			tdomain, aclass, -1,
-			psymbol_placement::STATIC,
-			0, language_c, of);
+  if (isparent)
+    pcu->parent_psymtab = pst;
+  else
+    pcu->imported_symtabs_push (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;
+
+  if (strcmp (name, ".ctf") != 0)
+    {
+      ctf_import (ctf, parent);
     }
 
-  pst->end ();
+  if (info_verbose)
+    {
+      printf_filtered (_("Scanning archive member %s..."), name);
+      gdb_flush (gdb_stdout);
+    }
+
+  scan_partial_symbols (ctf, tup, name);
+
+  return 0;
 }
 
 /* Read CTF debugging information from a BFD section.  This is
@@ -1574,6 +1650,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;
 
@@ -1588,7 +1665,31 @@ struct ctf_tid_and_type
 	   bfd_get_filename (abfd), ctf_errmsg (err));
   ctf_dict_key.emplace (of, fp);
 
-  scan_partial_symbols (fp, of);
+  pcu.fp = fp;
+  pcu.of = of;
+  pcu.arc = arc;
+  pcu.imported_symtabs = nullptr;
+
+  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
+	= of->partial_symtabs->allocate_dependencies (arch_cnt);
+      for (int i = 0; i < arch_cnt; ++i)
+	{
+	  parent_pst->dependencies[i]
+	    = pcu.imported_symtabs->at (i);
+	}
+      pcu.imported_symtabs_free ();
+    }
 }
 
 #else
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 4f9a3ec..3501669 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
 
+	* gdb.ctf/cross-tu-cyclic.exp: New file.
+	* gdb.ctf/multi.exp: New file.
+
+2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
+
 	* gdb.base/ctf-ptype.c: Add struct link containing a forward
 	reference type.
 	* gdb.base/ctf-ptype.exp: Add "ptype struct link"
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..2c5ad8d
--- /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..f514a33
--- /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..50db511
--- /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..8e99f23
--- /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] 21+ messages in thread

* Re: [PATCH,V2 2/5] CTF: set up debug info for function arguments
  2021-03-02  1:53   ` [PATCH,V2 2/5] CTF: set up debug info for function arguments Weimin Pan
  2021-03-02  1:53     ` [PATCH,V2 3/5] CTF: handle forward reference type Weimin Pan
@ 2021-03-02 12:23     ` Lancelot SIX
  2021-03-02 18:12       ` Wei-min Pan
  2021-03-03 20:38       ` Tom Tromey
  2021-03-03 20:41     ` Tom Tromey
  2 siblings, 2 replies; 21+ messages in thread
From: Lancelot SIX @ 2021-03-02 12:23 UTC (permalink / raw)
  To: Weimin Pan; +Cc: gdb-patches

Hi,

I just have style related comments bellow.

Le Mon, Mar 01, 2021 at 08:53:35PM -0500, Weimin Pan via Gdb-patches a écrit :
> Added this support in read_func_kind_type after gcc started generating
> CTF for function arguments.
> 
> Expanded gdb.base/ctf-ptype.exp to test function arguments. Also fixed
> some typos.
> 
> ---
>  gdb/ChangeLog                        |  4 ++++
>  gdb/ctfread.c                        | 29 ++++++++++++++++++++++++++++-
>  gdb/testsuite/ChangeLog              |  4 ++++
>  gdb/testsuite/gdb.base/ctf-ptype.exp | 19 ++++++++++++++++---
>  4 files changed, 52 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 88278d1..0a839ba 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,9 @@
>  2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>  
> +	* ctfread.c (read_func_kind_type): Set up function arguments.
> +
> +2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
> +
>  	* ctfread.c (new_symbol): Set function address.
>  	(read_func_kind_type): Remove incorrect type name setting.
>  
> diff --git a/gdb/ctfread.c b/gdb/ctfread.c
> index 5a68d9c..75b098c 100644
> --- a/gdb/ctfread.c
> +++ b/gdb/ctfread.c
> @@ -652,8 +652,9 @@ struct ctf_tid_and_type
>  {
>    struct objfile *of = ccp->of;
>    ctf_dict_t *fp = ccp->fp;
> -  struct type *type, *rettype;
> +  struct type *type, *rettype, *atype;
>    ctf_funcinfo_t cfi;
> +  uint32_t argc;
>  
>    type = alloc_type (of);
>  
> @@ -663,6 +664,32 @@ struct ctf_tid_and_type
>    TYPE_TARGET_TYPE (type) = rettype;
>    set_type_align (type, ctf_type_align (fp, tid));
>  
> +  /* Set up function's arguments.  */
> +  argc = cfi.ctc_argc;
> +  type->set_num_fields (argc);
> +  if (cfi.ctc_flags & CTF_FUNC_VARARG)
> +    type->set_has_varargs (true);
> +
> +  if (argc != 0)
> +    {
> +      std::vector<ctf_id_t> argv (argc);
> +      if (ctf_func_type_args (fp, tid, argc, argv.data ()) == CTF_ERR)
> +	return NULL;

Could be nullptr instead of NULL (even if I am not sure the standard
enforces that).

> +
> +      type->set_fields
> +	((struct field *) TYPE_ZALLOC (type, argc * sizeof (struct field)));
> +      struct type *void_type = objfile_type (of)->builtin_void;
> +      /* If failed to find the argument type, fill it with void_type.  */
> +      for (int iparam = 0; iparam < argc; iparam++)
> +	{
> +	  atype = get_tid_type (of, argv[iparam]);
> +	  if (atype)

An explicit comparison against nullptr should be done here. See
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Comparison_With_NULL_And_Zero

Thanks,
Lancelot.

> +	    type->field (iparam).set_type (atype);
> +	  else
> +	    type->field (iparam).set_type (void_type);
> +	}
> +    }
> +
>    return set_tid_type (of, tid, type);
>  }
>  
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 3d71354..9445362 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,5 +1,9 @@
>  2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>  
> +	* gdb.base/ctf-ptype.exp: Add function tests and fix typos.
> +
> +2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
> +
>  	* gdb.base/ctf-funcreturn.exp: New file.
>  
>  2021-01-29  Tom de Vries  <tdevries@suse.de>
> diff --git a/gdb/testsuite/gdb.base/ctf-ptype.exp b/gdb/testsuite/gdb.base/ctf-ptype.exp
> index ffe40f1..056f712 100644
> --- a/gdb/testsuite/gdb.base/ctf-ptype.exp
> +++ b/gdb/testsuite/gdb.base/ctf-ptype.exp
> @@ -63,10 +63,10 @@ gdb_test "ptype struct t_struct" "type = struct t_struct \{.*\[\r\n\]    (unsign
>  
>  # Test the equivalence between '.' and '->' for struct member references.
>  
> -if [gdb_test "ptype v_t_struct_p.v_float_member"	"type = float"]<0 then {
> +if [gdb_test "ptype v_struct1.v_float_member"	"type = float"]<0 then {
>      return -1
>  }
> -if [gdb_test "ptype v_t_struct_p->v_float_member"	"type = float"]<0 then {
> +if [gdb_test "ptype v_struct1->v_float_member"	"type = float"]<0 then {
>      return -1
>  }
>  if [gdb_test "ptype v_t_struct_p.v_float_member"	"type = float"]<0 then {
> @@ -211,7 +211,7 @@ gdb_test "ptype the_highest" \
>  
>  gdb_test "ptype the_highest.anonymous_level_1" \
>           "type = struct \{.*\[\r\n\] *int b;.*\[\r\n\] *struct \{.*\[\r\n\] *int c;.*\[\r\n\] *\} anonymous_level_2;.*\[\r\n\]}.*" \
> -         "ptype the_highest"
> +         "ptype the_highest.anonymous_level_1"
>  
>  # Print the type of the identifier ID, and check the response:
>  # - Expect to see PROTOTYPED as the type.  PROTOTYPED is not a regular
> @@ -255,8 +255,21 @@ proc ptype_maybe_prototyped { id prototyped plain { overprototyped "NO-MATCH" }
>      }
>  }
>  
> +ptype_maybe_prototyped "func_type" "int (*)(int (*)(int, float), float)" \
> +                                   "int (*)()"
>  ptype_maybe_prototyped "old_fptr" "double (*)()" "double (*)()" \
>                                    "double (*)(void)"
> +ptype_maybe_prototyped "new_fptr" "double (*)()" "double (*)()"
> +ptype_maybe_prototyped "fptr" "int (*)(int, float)" "int (*)()"
> +ptype_maybe_prototyped "fptr2" "int *(*)(int (*)(int, float), float)" \
> +                               "int *(*)()"
> +ptype_maybe_prototyped "xptr" "int (*)(int (*)(), int (*)(), int)" \
> +                              "int (*)()" \
> +                              "int (*)(int (*)(void), int (*)(void), int)"
> +ptype_maybe_prototyped "ffptr" "int (*(*)(char))(short int)" \
> +                               "int (*(*)())()"
> +ptype_maybe_prototyped "fffptr" "int (*(*(*)(char))(short int))(long int)" \
> +                                "int (*(*(*)())())()"
>  
>  # Test printing type of string constants and array constants, but
>  # requires a running process.  These call malloc, and can take a long
> -- 
> 1.8.3.1
> 

-- 
Lancelot SIX

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

* Re: [PATCH,V2 2/5] CTF: set up debug info for function arguments
  2021-03-02 12:23     ` [PATCH,V2 2/5] CTF: set up debug info for function arguments Lancelot SIX
@ 2021-03-02 18:12       ` Wei-min Pan
  2021-03-03 20:38       ` Tom Tromey
  1 sibling, 0 replies; 21+ messages in thread
From: Wei-min Pan @ 2021-03-02 18:12 UTC (permalink / raw)
  To: Lancelot SIX; +Cc: gdb-patches


On 3/2/2021 4:23 AM, Lancelot SIX wrote:
> Hi,
>
> I just have style related comments bellow.
>
> Le Mon, Mar 01, 2021 at 08:53:35PM -0500, Weimin Pan via Gdb-patches a écrit :
>> Added this support in read_func_kind_type after gcc started generating
>> CTF for function arguments.
>>
>> Expanded gdb.base/ctf-ptype.exp to test function arguments. Also fixed
>> some typos.
>>
>> ---
>>   gdb/ChangeLog                        |  4 ++++
>>   gdb/ctfread.c                        | 29 ++++++++++++++++++++++++++++-
>>   gdb/testsuite/ChangeLog              |  4 ++++
>>   gdb/testsuite/gdb.base/ctf-ptype.exp | 19 ++++++++++++++++---
>>   4 files changed, 52 insertions(+), 4 deletions(-)
>>
>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>> index 88278d1..0a839ba 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>> @@ -1,5 +1,9 @@
>>   2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>>   
>> +	* ctfread.c (read_func_kind_type): Set up function arguments.
>> +
>> +2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>> +
>>   	* ctfread.c (new_symbol): Set function address.
>>   	(read_func_kind_type): Remove incorrect type name setting.
>>   
>> diff --git a/gdb/ctfread.c b/gdb/ctfread.c
>> index 5a68d9c..75b098c 100644
>> --- a/gdb/ctfread.c
>> +++ b/gdb/ctfread.c
>> @@ -652,8 +652,9 @@ struct ctf_tid_and_type
>>   {
>>     struct objfile *of = ccp->of;
>>     ctf_dict_t *fp = ccp->fp;
>> -  struct type *type, *rettype;
>> +  struct type *type, *rettype, *atype;
>>     ctf_funcinfo_t cfi;
>> +  uint32_t argc;
>>   
>>     type = alloc_type (of);
>>   
>> @@ -663,6 +664,32 @@ struct ctf_tid_and_type
>>     TYPE_TARGET_TYPE (type) = rettype;
>>     set_type_align (type, ctf_type_align (fp, tid));
>>   
>> +  /* Set up function's arguments.  */
>> +  argc = cfi.ctc_argc;
>> +  type->set_num_fields (argc);
>> +  if (cfi.ctc_flags & CTF_FUNC_VARARG)
>> +    type->set_has_varargs (true);
>> +
>> +  if (argc != 0)
>> +    {
>> +      std::vector<ctf_id_t> argv (argc);
>> +      if (ctf_func_type_args (fp, tid, argc, argv.data ()) == CTF_ERR)
>> +	return NULL;
> Could be nullptr instead of NULL (even if I am not sure the standard
> enforces that).

nullptr of std::nullptr_t, which is implicitly converted to null pointer 
value of any pointer type,
seems more appropriate. Will do the necessary changes in other places too.

>
>> +
>> +      type->set_fields
>> +	((struct field *) TYPE_ZALLOC (type, argc * sizeof (struct field)));
>> +      struct type *void_type = objfile_type (of)->builtin_void;
>> +      /* If failed to find the argument type, fill it with void_type.  */
>> +      for (int iparam = 0; iparam < argc; iparam++)
>> +	{
>> +	  atype = get_tid_type (of, argv[iparam]);
>> +	  if (atype)
> An explicit comparison against nullptr should be done here. See
> https://urldefense.com/v3/__https://sourceware.org/gdb/wiki/Internals*20GDB-C-Coding-Standards*Comparison_With_NULL_And_Zero__;JSM!!GqivPVa7Brio!IXhIALJgGrsPK1dDbwNKp69KxLTd4Msy6ShNy9Euo18QeN2Iz4dEBg2rhM_3eUmx$

OK, will do.

Thank you for your comments.

> Thanks,
> Lancelot.
>
>> +	    type->field (iparam).set_type (atype);
>> +	  else
>> +	    type->field (iparam).set_type (void_type);
>> +	}
>> +    }
>> +
>>     return set_tid_type (of, tid, type);
>>   }
>>   
>> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
>> index 3d71354..9445362 100644
>> --- a/gdb/testsuite/ChangeLog
>> +++ b/gdb/testsuite/ChangeLog
>> @@ -1,5 +1,9 @@
>>   2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>>   
>> +	* gdb.base/ctf-ptype.exp: Add function tests and fix typos.
>> +
>> +2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>> +
>>   	* gdb.base/ctf-funcreturn.exp: New file.
>>   
>>   2021-01-29  Tom de Vries  <tdevries@suse.de>
>> diff --git a/gdb/testsuite/gdb.base/ctf-ptype.exp b/gdb/testsuite/gdb.base/ctf-ptype.exp
>> index ffe40f1..056f712 100644
>> --- a/gdb/testsuite/gdb.base/ctf-ptype.exp
>> +++ b/gdb/testsuite/gdb.base/ctf-ptype.exp
>> @@ -63,10 +63,10 @@ gdb_test "ptype struct t_struct" "type = struct t_struct \{.*\[\r\n\]    (unsign
>>   
>>   # Test the equivalence between '.' and '->' for struct member references.
>>   
>> -if [gdb_test "ptype v_t_struct_p.v_float_member"	"type = float"]<0 then {
>> +if [gdb_test "ptype v_struct1.v_float_member"	"type = float"]<0 then {
>>       return -1
>>   }
>> -if [gdb_test "ptype v_t_struct_p->v_float_member"	"type = float"]<0 then {
>> +if [gdb_test "ptype v_struct1->v_float_member"	"type = float"]<0 then {
>>       return -1
>>   }
>>   if [gdb_test "ptype v_t_struct_p.v_float_member"	"type = float"]<0 then {
>> @@ -211,7 +211,7 @@ gdb_test "ptype the_highest" \
>>   
>>   gdb_test "ptype the_highest.anonymous_level_1" \
>>            "type = struct \{.*\[\r\n\] *int b;.*\[\r\n\] *struct \{.*\[\r\n\] *int c;.*\[\r\n\] *\} anonymous_level_2;.*\[\r\n\]}.*" \
>> -         "ptype the_highest"
>> +         "ptype the_highest.anonymous_level_1"
>>   
>>   # Print the type of the identifier ID, and check the response:
>>   # - Expect to see PROTOTYPED as the type.  PROTOTYPED is not a regular
>> @@ -255,8 +255,21 @@ proc ptype_maybe_prototyped { id prototyped plain { overprototyped "NO-MATCH" }
>>       }
>>   }
>>   
>> +ptype_maybe_prototyped "func_type" "int (*)(int (*)(int, float), float)" \
>> +                                   "int (*)()"
>>   ptype_maybe_prototyped "old_fptr" "double (*)()" "double (*)()" \
>>                                     "double (*)(void)"
>> +ptype_maybe_prototyped "new_fptr" "double (*)()" "double (*)()"
>> +ptype_maybe_prototyped "fptr" "int (*)(int, float)" "int (*)()"
>> +ptype_maybe_prototyped "fptr2" "int *(*)(int (*)(int, float), float)" \
>> +                               "int *(*)()"
>> +ptype_maybe_prototyped "xptr" "int (*)(int (*)(), int (*)(), int)" \
>> +                              "int (*)()" \
>> +                              "int (*)(int (*)(void), int (*)(void), int)"
>> +ptype_maybe_prototyped "ffptr" "int (*(*)(char))(short int)" \
>> +                               "int (*(*)())()"
>> +ptype_maybe_prototyped "fffptr" "int (*(*(*)(char))(short int))(long int)" \
>> +                                "int (*(*(*)())())()"
>>   
>>   # Test printing type of string constants and array constants, but
>>   # requires a running process.  These call malloc, and can take a long
>> -- 
>> 1.8.3.1
>>

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

* Re: [PATCH,V2 5/5] CTF: multi-CU and archive support
  2021-03-02  1:53         ` [PATCH,V2 5/5] CTF: multi-CU and archive support Weimin Pan
@ 2021-03-02 23:15           ` Lancelot SIX
  2021-03-03  1:01             ` Wei-min Pan
  0 siblings, 1 reply; 21+ messages in thread
From: Lancelot SIX @ 2021-03-02 23:15 UTC (permalink / raw)
  To: Weimin Pan; +Cc: gdb-patches

Hi,

I have few comments spread above.

I hope this help.

Le Mon, Mar 01, 2021 at 08:53:38PM -0500, Weimin Pan via Gdb-patches a écrit :
> 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/ChangeLog                             |  15 ++
>  gdb/ctfread.c                             | 365 +++++++++++++++++++-----------
>  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/multi.exp           |  42 ++++
>  15 files changed, 526 insertions(+), 132 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/ChangeLog b/gdb/ChangeLog
> index 31cac5c..52a325f 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,20 @@
>  2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>  
> +	* ctfread.c (ctf_per_tu_data): New struct.
> +	(add_stt_entries): New funtion.
> +	(add_stt_obj): Use it.
> +	(add_stt_func): Likewise.
> +	(ctf_psymtab_add_stt_entries): New function.
> +	(ctf_psymtab_add_stt_obj): Use it.
> +	(ctf_psymtab_add_stt_func): Likewise.
> +	(ctf_psymtab::expand_psymtab): Call expand_dependencies to expand
> +	archives.
> +	(scan_partial_symbols): Set up partial symtab list.
> +	(build_ctf_archive_member): New function.
> +	(elfctf_build_psymtabs): Call Iterater to set up archives.
> +
> +2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
> +
>  	* ctfread.c (ctf_psymtab_add_enums): New function.
>  	(ctf_psymtab_type_cb): call ctf_psymtab_add_enums.
>  
> diff --git a/gdb/ctfread.c b/gdb/ctfread.c
> index bb73949..9ddd1db 100644
> --- a/gdb/ctfread.c
> +++ b/gdb/ctfread.c
> @@ -80,6 +80,7 @@
>  #include "complaints.h"
>  #include "block.h"
>  #include "ctfread.h"
> +#include "objfiles.h"
>  #include "psympriv.h"
>  
>  #if ENABLE_LIBCTF
> @@ -116,6 +117,7 @@ struct ctf_context
>    ctf_dict_t *fp;
>    struct objfile *of;
>    partial_symtab *pst;
> +  ctf_archive_t *arc;
>    struct buildsym_compunit *builder;
>  };
>  
> @@ -162,6 +164,40 @@ 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;
> +  // struct partial_symtab *psymtab;

I guess this was left here from a previous development iteration and
should be removed.

> +  /* Push P to the back of IMPORTED_SYMTABS, allocated IMPORTED_SYMTABS
> +     first if required.  */
> +  void imported_symtabs_push (struct partial_symtab *p)
> +  {
> +    if (imported_symtabs == nullptr)
> +      imported_symtabs = new std::vector <struct partial_symtab *>;
> +    imported_symtabs->push_back (p);
> +  }
> +  /* Return the size of IMPORTED_SYMTABS if it is allocated, otherwise
> +     return 0.  */
> +  size_t imported_symtabs_size () const
> +  {
> +    if (imported_symtabs == nullptr)
> +      return 0;
> +    return imported_symtabs->size ();
> +  }
> +
> +  /* Delete IMPORTED_SYMTABS and set the pointer back to nullptr.  */
> +  void imported_symtabs_free ()
> +  {
> +    delete imported_symtabs;
> +    imported_symtabs = nullptr;
> +  }
> +  std::vector <struct partial_symtab *> *imported_symtabs;
> +};

From the usage of this struct, it looks like imported_symtaps can be a
std::vector (not pointer to std::vector). This would remove the need of
imported_sympabs_push, imported_smtabs_size and imported symtabs_free
altogether.

The all struct becomes

struct ctf_per_tu_data
{
  ctf_dict_t *fp;
  struct objfile *of;
  ctf_archive_t *arc;
  ctf_psymtab *parent_psymtab;
  std::vector<struct partial_symtab *> imported_symtabs;
}

Then x.imported_symtabs_push (p) becomes
x.imported_symtabs.push_back (p), x.imported_symtabs_size () becomes
x.imported_symtabs.size () and imported_symtabs_free becomes
unnecessary.

>
>  /* Local function prototypes */
>  
> @@ -241,10 +277,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 == 0)
> +    *slot = XOBNEW (&of->objfile_obstack, struct ctf_tid_and_type);
>    **slot = ids;
>    return typ;
>  }
> @@ -459,7 +493,7 @@ struct ctf_tid_and_type
>    struct symbol *sym = NULL;
>  
>    gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
> -  if (name != NULL)
> +  if (name != NULL && strcmp(name.get (), ""))

You missed a space after strcmp and a explicit comparison of the
returned value.

if (name != nullptr && strcmp (name.get (), "") != 0)

>      {
>        sym = new (&objfile->objfile_obstack) symbol;
>        OBJSTAT (objfile, n_syms++);
> @@ -943,7 +977,7 @@ struct ctf_tid_and_type
>        if (target_type == NULL)
>  	{
>  	  complaint (_("read_pointer_type: NULL target type (%ld)"), btid);
> -	  target_type = objfile_type (ccp->of)->builtin_error;
> +	  target_type = objfile_type (of)->builtin_error;
>  	}
>      }
>  
> @@ -1055,6 +1089,12 @@ struct ctf_tid_and_type
>    struct type *type;
>    uint32_t kind;
>  
> +  if (info_verbose)
> +    {
> +      printf_filtered (_("ctf_add_type_cb adding tid %lx..."), tid);
> +      gdb_flush (gdb_stdout);
> +    }
> +
>    /* Check if tid's type has already been defined.  */
>    type = get_tid_type (ccp->of, tid);
>    if (type != NULL)
> @@ -1141,7 +1181,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)

Here also, if (sym != nullptr).

> +	      sym->compute_and_set_names (name, false, ccp->of->per_bfd);
>  	  }
>  	break;
>        case CTF_K_STRUCT:
> @@ -1171,81 +1212,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 = NULL;
> +  const char *tname;
>    ctf_id_t tid;
> +  struct symbol *sym = NULL;
> +  struct type *type;
>  
> -  if ((tid = ctf_lookup_by_symbol (ccp->fp, idx)) == CTF_ERR)
> -    return NULL;
> -
> -  type = fetch_tid_type (ccp, tid);
> -  if (type == NULL)
> -    return NULL;
> -
> -  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 == NULL)
> +	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 NULL;
> -
> -  argc = finfo.ctc_argc;
> -  if (ctf_func_args (ccp->fp, idx, argc, argv) == CTF_ERR)
> -    return NULL;
> -
> -  gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (ccp->fp, idx));
> -  if (name == NULL)
> -    return NULL;
> -
> -  tid = ctf_lookup_by_symbol (ccp->fp, idx);
> -  ftype = fetch_tid_type (ccp, tid);
> -  if (finfo.ctc_flags & CTF_FUNC_VARARG)
> -    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)));
> +  add_stt_entries (ccp, 0);
> +}
>  
> -  /* 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 entries in function info section.  */
>  
> -  sym = new_symbol (ccp, ftype, tid);
> -  rettyp = fetch_tid_type (ccp, finfo.ctc_return);
> -  if (rettyp != NULL)
> -    SYMBOL_TYPE (sym) = rettyp;
> -  else
> -    SYMBOL_TYPE (sym) = void_type;
> -
> -  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.  */
> @@ -1268,12 +1276,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, NULL,
>  		       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.
> @@ -1315,46 +1331,94 @@ 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 = NULL;
> +  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, 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)
> -    complaint (_("ctf_type_iter psymtab_to_symtab failed - %s"),
> +    complaint (_("ctf_type_iter expand_psymtab failed - %s"),
>  	       ctf_errmsg (ctf_errno (ccp->fp)));
>  
>  
>    /* Iterate over entries in variable info section.  */
>    if (ctf_variable_iter (ccp->fp, ctf_add_var_cb, ccp) == CTF_ERR)
> -    complaint (_("ctf_variable_iter psymtab_to_symtab failed - %s"),
> +    complaint (_("ctf_variable_iter expand_psymtab failed - %s"),
>  	       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 == NULL)
> -	{
> -	  if (ctf_errno (ccp->fp) == EINVAL
> -	      || ctf_errno (ccp->fp) == ECTF_NOSYMTAB)
> -	    break;
> -	  sym = add_stt_func (ccp, i);
> -	}
> -      if (sym == NULL)
> -	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;
>  }
>  
> @@ -1407,6 +1471,7 @@ struct ctf_tid_and_type
>  
>  static ctf_psymtab *
>  create_partial_symtab (const char *name,
> +		       ctf_archive_t *arc,
>  		       ctf_dict_t *cfp,
>  		       struct objfile *objfile)
>  {
> @@ -1416,10 +1481,11 @@ struct ctf_tid_and_type
>    pst = new ctf_psymtab (name, objfile, 0);
>  
>    ccx = XOBNEW (&objfile->objfile_obstack, struct ctf_context);
> +  ccx->arc = arc;
>    ccx->fp = cfp;
> +  ctf_ref (cfp);
>    ccx->of = objfile;
>    ccx->pst = pst;
> -  ccx->builder = nullptr;
>    pst->context = ccx;
>  
>    return pst;
> @@ -1506,12 +1572,29 @@ struct ctf_tid_and_type
>     debugging information is available.  */
>  
>  static void
> -scan_partial_symbols (ctf_dict_t *cfp, struct objfile *of)
> +scan_partial_symbols (ctf_dict_t *cfp,
> +		      struct ctf_per_tu_data *pcu,
> +		      const char *aname)
>  {
> -  bfd *abfd = of->obfd;
> -  const char *name = bfd_get_filename (abfd);
> -  ctf_psymtab *pst = create_partial_symtab (name, cfp, of);
> +  struct objfile *of = pcu->of;
> +  bool isparent = 0;
> +
> +  if (info_verbose)
> +    {
> +      printf_filtered (_("Scanning partial symbols for %s..."), aname);
> +      gdb_flush (gdb_stdout);
> +    }
> +
> +  if (strcmp (aname, ".ctf") == 0)
> +    isparent = 1;
>  
> +  if (info_verbose)
> +    {
> +      printf_filtered (_("Creating partial symtab %s..."), aname);
> +      gdb_flush (gdb_stdout);
> +    }
> +
> +  ctf_psymtab *pst = create_partial_symtab (aname, pcu->arc, cfp, of);
>    struct ctf_context *ccx = pst->context;
>  
>    if (ctf_type_iter (cfp, ctf_psymtab_type_cb, ccx) == CTF_ERR)
> @@ -1525,46 +1608,39 @@ 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;
> -	}
> -      gdb::unique_xmalloc_ptr<char> tname (ctf_type_aname_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.get (), true,
> -			tdomain, aclass, -1,
> -			psymbol_placement::STATIC,
> -			0, language_c, of);
> +  if (isparent)
> +    pcu->parent_psymtab = pst;
> +  else
> +    pcu->imported_symtabs_push (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)

I do not see the advantage of having arg passed as a void *.  It is
casted unconditionally to a struct ctf_per_tu_data * and dereferenced
just after that.

Why not just pass the argument as a struct ctf_per_tu_data * directly?

> +{
> +  struct ctf_per_tu_data *tup = (struct ctf_per_tu_data *) arg;
> +  ctf_dict_t *parent = tup->fp;
> +
> +  if (strcmp (name, ".ctf") != 0)
> +    {
> +      ctf_import (ctf, parent);
>      }
>  
> -  pst->end ();
> +  if (info_verbose)
> +    {
> +      printf_filtered (_("Scanning archive member %s..."), name);
> +      gdb_flush (gdb_stdout);
> +    }
> +
> +  scan_partial_symbols (ctf, tup, name);
> +
> +  return 0;
>  }
>  
>  /* Read CTF debugging information from a BFD section.  This is
> @@ -1574,6 +1650,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;
>  
> @@ -1588,7 +1665,31 @@ struct ctf_tid_and_type
>  	   bfd_get_filename (abfd), ctf_errmsg (err));
>    ctf_dict_key.emplace (of, fp);
>  
> -  scan_partial_symbols (fp, of);
> +  pcu.fp = fp;
> +  pcu.of = of;
> +  pcu.arc = arc;
> +  pcu.imported_symtabs = nullptr;
> +
> +  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 ();

I guess it could be declared const.

> +  struct partial_symtab *parent_pst = pcu.parent_psymtab;
> +
> +  if (parent_pst && arch_cnt)

if (parent_pst != nullptr && arch_cnt != 0)

> +    {
> +      /* Fill in the 'dependencies'.  */
> +      parent_pst->number_of_dependencies = arch_cnt;
> +      parent_pst->dependencies
> +	= of->partial_symtabs->allocate_dependencies (arch_cnt);
> +      for (int i = 0; i < arch_cnt; ++i)
> +	{
> +	  parent_pst->dependencies[i]
> +	    = pcu.imported_symtabs->at (i);
> +	}
> +      pcu.imported_symtabs_free ();
> +    }
>  }
>  
>  #else
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 4f9a3ec..3501669 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,5 +1,10 @@
>  2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>  
> +	* gdb.ctf/cross-tu-cyclic.exp: New file.
> +	* gdb.ctf/multi.exp: New file.
> +
> +2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
> +
>  	* gdb.base/ctf-ptype.c: Add struct link containing a forward
>  	reference type.
>  	* gdb.base/ctf-ptype.exp: Add "ptype struct link"
> 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..2c5ad8d
> --- /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" 

The 3 previous lines have trailing spaces.

> diff --git a/gdb/testsuite/gdb.ctf/ctf-a.c b/gdb/testsuite/gdb.ctf/ctf-a.c
> new file mode 100644
> index 0000000..f514a33
> --- /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) 

Trailing space.

> +    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..50db511
> --- /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;
> +} 

Trailing space

> 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..8e99f23
> --- /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" 

You also have trailing spaces here.

Lancelot.

> -- 
> 1.8.3.1
> 

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

* Re: [PATCH,V2 5/5] CTF: multi-CU and archive support
  2021-03-02 23:15           ` Lancelot SIX
@ 2021-03-03  1:01             ` Wei-min Pan
  2021-03-03 17:37               ` Lancelot SIX
  0 siblings, 1 reply; 21+ messages in thread
From: Wei-min Pan @ 2021-03-03  1:01 UTC (permalink / raw)
  To: Lancelot SIX; +Cc: gdb-patches


On 3/2/2021 3:15 PM, Lancelot SIX wrote:
> Hi,
>
> I have few comments spread above.
>
> I hope this help.
>
> Le Mon, Mar 01, 2021 at 08:53:38PM -0500, Weimin Pan via Gdb-patches a écrit :
>> 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/ChangeLog                             |  15 ++
>>   gdb/ctfread.c                             | 365 +++++++++++++++++++-----------
>>   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/multi.exp           |  42 ++++
>>   15 files changed, 526 insertions(+), 132 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/ChangeLog b/gdb/ChangeLog
>> index 31cac5c..52a325f 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>> @@ -1,5 +1,20 @@
>>   2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>>   
>> +	* ctfread.c (ctf_per_tu_data): New struct.
>> +	(add_stt_entries): New funtion.
>> +	(add_stt_obj): Use it.
>> +	(add_stt_func): Likewise.
>> +	(ctf_psymtab_add_stt_entries): New function.
>> +	(ctf_psymtab_add_stt_obj): Use it.
>> +	(ctf_psymtab_add_stt_func): Likewise.
>> +	(ctf_psymtab::expand_psymtab): Call expand_dependencies to expand
>> +	archives.
>> +	(scan_partial_symbols): Set up partial symtab list.
>> +	(build_ctf_archive_member): New function.
>> +	(elfctf_build_psymtabs): Call Iterater to set up archives.
>> +
>> +2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>> +
>>   	* ctfread.c (ctf_psymtab_add_enums): New function.
>>   	(ctf_psymtab_type_cb): call ctf_psymtab_add_enums.
>>   
>> diff --git a/gdb/ctfread.c b/gdb/ctfread.c
>> index bb73949..9ddd1db 100644
>> --- a/gdb/ctfread.c
>> +++ b/gdb/ctfread.c
>> @@ -80,6 +80,7 @@
>>   #include "complaints.h"
>>   #include "block.h"
>>   #include "ctfread.h"
>> +#include "objfiles.h"
>>   #include "psympriv.h"
>>   
>>   #if ENABLE_LIBCTF
>> @@ -116,6 +117,7 @@ struct ctf_context
>>     ctf_dict_t *fp;
>>     struct objfile *of;
>>     partial_symtab *pst;
>> +  ctf_archive_t *arc;
>>     struct buildsym_compunit *builder;
>>   };
>>   
>> @@ -162,6 +164,40 @@ 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;
>> +  // struct partial_symtab *psymtab;
> I guess this was left here from a previous development iteration and
> should be removed.

Yes, it will be removed.

>> +  /* Push P to the back of IMPORTED_SYMTABS, allocated IMPORTED_SYMTABS
>> +     first if required.  */
>> +  void imported_symtabs_push (struct partial_symtab *p)
>> +  {
>> +    if (imported_symtabs == nullptr)
>> +      imported_symtabs = new std::vector <struct partial_symtab *>;
>> +    imported_symtabs->push_back (p);
>> +  }
>> +  /* Return the size of IMPORTED_SYMTABS if it is allocated, otherwise
>> +     return 0.  */
>> +  size_t imported_symtabs_size () const
>> +  {
>> +    if (imported_symtabs == nullptr)
>> +      return 0;
>> +    return imported_symtabs->size ();
>> +  }
>> +
>> +  /* Delete IMPORTED_SYMTABS and set the pointer back to nullptr.  */
>> +  void imported_symtabs_free ()
>> +  {
>> +    delete imported_symtabs;
>> +    imported_symtabs = nullptr;
>> +  }
>> +  std::vector <struct partial_symtab *> *imported_symtabs;
>> +};
>  From the usage of this struct, it looks like imported_symtaps can be a
> std::vector (not pointer to std::vector). This would remove the need of
> imported_sympabs_push, imported_smtabs_size and imported symtabs_free
> altogether.
>
> The all struct becomes
>
> struct ctf_per_tu_data
> {
>    ctf_dict_t *fp;
>    struct objfile *of;
>    ctf_archive_t *arc;
>    ctf_psymtab *parent_psymtab;
>    std::vector<struct partial_symtab *> imported_symtabs;
> }
>
> Then x.imported_symtabs_push (p) becomes
> x.imported_symtabs.push_back (p), x.imported_symtabs_size () becomes
> x.imported_symtabs.size () and imported_symtabs_free becomes
> unnecessary.

This indeed simplifies the struct and saves a call. Nice, will do.

>>   /* Local function prototypes */
>>   
>> @@ -241,10 +277,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 == 0)
>> +    *slot = XOBNEW (&of->objfile_obstack, struct ctf_tid_and_type);
>>     **slot = ids;
>>     return typ;
>>   }
>> @@ -459,7 +493,7 @@ struct ctf_tid_and_type
>>     struct symbol *sym = NULL;
>>   
>>     gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
>> -  if (name != NULL)
>> +  if (name != NULL && strcmp(name.get (), ""))
> You missed a space after strcmp and a explicit comparison of the
> returned value.
>
> if (name != nullptr && strcmp (name.get (), "") != 0)

OK, will fix it.

>>       {
>>         sym = new (&objfile->objfile_obstack) symbol;
>>         OBJSTAT (objfile, n_syms++);
>> @@ -943,7 +977,7 @@ struct ctf_tid_and_type
>>         if (target_type == NULL)
>>   	{
>>   	  complaint (_("read_pointer_type: NULL target type (%ld)"), btid);
>> -	  target_type = objfile_type (ccp->of)->builtin_error;
>> +	  target_type = objfile_type (of)->builtin_error;
>>   	}
>>       }
>>   
>> @@ -1055,6 +1089,12 @@ struct ctf_tid_and_type
>>     struct type *type;
>>     uint32_t kind;
>>   
>> +  if (info_verbose)
>> +    {
>> +      printf_filtered (_("ctf_add_type_cb adding tid %lx..."), tid);
>> +      gdb_flush (gdb_stdout);
>> +    }
>> +
>>     /* Check if tid's type has already been defined.  */
>>     type = get_tid_type (ccp->of, tid);
>>     if (type != NULL)
>> @@ -1141,7 +1181,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)
> Here also, if (sym != nullptr).

OK, will fix it.

>> +	      sym->compute_and_set_names (name, false, ccp->of->per_bfd);
>>   	  }
>>   	break;
>>         case CTF_K_STRUCT:
>> @@ -1171,81 +1212,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 = NULL;
>> +  const char *tname;
>>     ctf_id_t tid;
>> +  struct symbol *sym = NULL;
>> +  struct type *type;
>>   
>> -  if ((tid = ctf_lookup_by_symbol (ccp->fp, idx)) == CTF_ERR)
>> -    return NULL;
>> -
>> -  type = fetch_tid_type (ccp, tid);
>> -  if (type == NULL)
>> -    return NULL;
>> -
>> -  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 == NULL)
>> +	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 NULL;
>> -
>> -  argc = finfo.ctc_argc;
>> -  if (ctf_func_args (ccp->fp, idx, argc, argv) == CTF_ERR)
>> -    return NULL;
>> -
>> -  gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (ccp->fp, idx));
>> -  if (name == NULL)
>> -    return NULL;
>> -
>> -  tid = ctf_lookup_by_symbol (ccp->fp, idx);
>> -  ftype = fetch_tid_type (ccp, tid);
>> -  if (finfo.ctc_flags & CTF_FUNC_VARARG)
>> -    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)));
>> +  add_stt_entries (ccp, 0);
>> +}
>>   
>> -  /* 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 entries in function info section.  */
>>   
>> -  sym = new_symbol (ccp, ftype, tid);
>> -  rettyp = fetch_tid_type (ccp, finfo.ctc_return);
>> -  if (rettyp != NULL)
>> -    SYMBOL_TYPE (sym) = rettyp;
>> -  else
>> -    SYMBOL_TYPE (sym) = void_type;
>> -
>> -  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.  */
>> @@ -1268,12 +1276,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, NULL,
>>   		       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.
>> @@ -1315,46 +1331,94 @@ 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 = NULL;
>> +  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, 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)
>> -    complaint (_("ctf_type_iter psymtab_to_symtab failed - %s"),
>> +    complaint (_("ctf_type_iter expand_psymtab failed - %s"),
>>   	       ctf_errmsg (ctf_errno (ccp->fp)));
>>   
>>   
>>     /* Iterate over entries in variable info section.  */
>>     if (ctf_variable_iter (ccp->fp, ctf_add_var_cb, ccp) == CTF_ERR)
>> -    complaint (_("ctf_variable_iter psymtab_to_symtab failed - %s"),
>> +    complaint (_("ctf_variable_iter expand_psymtab failed - %s"),
>>   	       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 == NULL)
>> -	{
>> -	  if (ctf_errno (ccp->fp) == EINVAL
>> -	      || ctf_errno (ccp->fp) == ECTF_NOSYMTAB)
>> -	    break;
>> -	  sym = add_stt_func (ccp, i);
>> -	}
>> -      if (sym == NULL)
>> -	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;
>>   }
>>   
>> @@ -1407,6 +1471,7 @@ struct ctf_tid_and_type
>>   
>>   static ctf_psymtab *
>>   create_partial_symtab (const char *name,
>> +		       ctf_archive_t *arc,
>>   		       ctf_dict_t *cfp,
>>   		       struct objfile *objfile)
>>   {
>> @@ -1416,10 +1481,11 @@ struct ctf_tid_and_type
>>     pst = new ctf_psymtab (name, objfile, 0);
>>   
>>     ccx = XOBNEW (&objfile->objfile_obstack, struct ctf_context);
>> +  ccx->arc = arc;
>>     ccx->fp = cfp;
>> +  ctf_ref (cfp);
>>     ccx->of = objfile;
>>     ccx->pst = pst;
>> -  ccx->builder = nullptr;
>>     pst->context = ccx;
>>   
>>     return pst;
>> @@ -1506,12 +1572,29 @@ struct ctf_tid_and_type
>>      debugging information is available.  */
>>   
>>   static void
>> -scan_partial_symbols (ctf_dict_t *cfp, struct objfile *of)
>> +scan_partial_symbols (ctf_dict_t *cfp,
>> +		      struct ctf_per_tu_data *pcu,
>> +		      const char *aname)
>>   {
>> -  bfd *abfd = of->obfd;
>> -  const char *name = bfd_get_filename (abfd);
>> -  ctf_psymtab *pst = create_partial_symtab (name, cfp, of);
>> +  struct objfile *of = pcu->of;
>> +  bool isparent = 0;
>> +
>> +  if (info_verbose)
>> +    {
>> +      printf_filtered (_("Scanning partial symbols for %s..."), aname);
>> +      gdb_flush (gdb_stdout);
>> +    }
>> +
>> +  if (strcmp (aname, ".ctf") == 0)
>> +    isparent = 1;
>>   
>> +  if (info_verbose)
>> +    {
>> +      printf_filtered (_("Creating partial symtab %s..."), aname);
>> +      gdb_flush (gdb_stdout);
>> +    }
>> +
>> +  ctf_psymtab *pst = create_partial_symtab (aname, pcu->arc, cfp, of);
>>     struct ctf_context *ccx = pst->context;
>>   
>>     if (ctf_type_iter (cfp, ctf_psymtab_type_cb, ccx) == CTF_ERR)
>> @@ -1525,46 +1608,39 @@ 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;
>> -	}
>> -      gdb::unique_xmalloc_ptr<char> tname (ctf_type_aname_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.get (), true,
>> -			tdomain, aclass, -1,
>> -			psymbol_placement::STATIC,
>> -			0, language_c, of);
>> +  if (isparent)
>> +    pcu->parent_psymtab = pst;
>> +  else
>> +    pcu->imported_symtabs_push (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)
> I do not see the advantage of having arg passed as a void *.  It is
> casted unconditionally to a struct ctf_per_tu_data * and dereferenced
> just after that.
>
> Why not just pass the argument as a struct ctf_per_tu_data * directly?

build_ctf_archive_member is the callback function of ctf_archive_iter 
and is of
type ctf_member_f which is defined in ctf-api.h:

typedef int ctf_member_f (const char *name, ctf_id_t membtype,
                           unsigned long offset, void *arg);

and the last argument "pcu" of type struct ctf_per_tu_data * is being 
passed in call:

         if (ctf_archive_iter (arc, build_ctf_archive_member, &pcu) < 0)

The conversion is necessary.

>> +{
>> +  struct ctf_per_tu_data *tup = (struct ctf_per_tu_data *) arg;
>> +  ctf_dict_t *parent = tup->fp;
>> +
>> +  if (strcmp (name, ".ctf") != 0)
>> +    {
>> +      ctf_import (ctf, parent);
>>       }
>>   
>> -  pst->end ();
>> +  if (info_verbose)
>> +    {
>> +      printf_filtered (_("Scanning archive member %s..."), name);
>> +      gdb_flush (gdb_stdout);
>> +    }
>> +
>> +  scan_partial_symbols (ctf, tup, name);
>> +
>> +  return 0;
>>   }
>>   
>>   /* Read CTF debugging information from a BFD section.  This is
>> @@ -1574,6 +1650,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;
>>   
>> @@ -1588,7 +1665,31 @@ struct ctf_tid_and_type
>>   	   bfd_get_filename (abfd), ctf_errmsg (err));
>>     ctf_dict_key.emplace (of, fp);
>>   
>> -  scan_partial_symbols (fp, of);
>> +  pcu.fp = fp;
>> +  pcu.of = of;
>> +  pcu.arc = arc;
>> +  pcu.imported_symtabs = nullptr;
>> +
>> +  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 ();
> I guess it could be declared const.

Will do.

>> +  struct partial_symtab *parent_pst = pcu.parent_psymtab;
>> +
>> +  if (parent_pst && arch_cnt)
> if (parent_pst != nullptr && arch_cnt != 0)

OK, will fix it.

>> +    {
>> +      /* Fill in the 'dependencies'.  */
>> +      parent_pst->number_of_dependencies = arch_cnt;
>> +      parent_pst->dependencies
>> +	= of->partial_symtabs->allocate_dependencies (arch_cnt);
>> +      for (int i = 0; i < arch_cnt; ++i)
>> +	{
>> +	  parent_pst->dependencies[i]
>> +	    = pcu.imported_symtabs->at (i);
>> +	}
>> +      pcu.imported_symtabs_free ();
>> +    }
>>   }
>>   
>>   #else
>> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
>> index 4f9a3ec..3501669 100644
>> --- a/gdb/testsuite/ChangeLog
>> +++ b/gdb/testsuite/ChangeLog
>> @@ -1,5 +1,10 @@
>>   2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>>   
>> +	* gdb.ctf/cross-tu-cyclic.exp: New file.
>> +	* gdb.ctf/multi.exp: New file.
>> +
>> +2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>> +
>>   	* gdb.base/ctf-ptype.c: Add struct link containing a forward
>>   	reference type.
>>   	* gdb.base/ctf-ptype.exp: Add "ptype struct link"
>> 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..2c5ad8d
>> --- /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 <https://urldefense.com/v3/__http://www.gnu.org/licenses/__;!!GqivPVa7Brio!MlnQ4nwBBbhjsCOlrkvxnKDpvrQ5BtDctjA14y8Sh72MPt3sVujLIe8lqYzl-as2$ >.
>> +
>> +# 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"
> The 3 previous lines have trailing spaces.

OK, will fix them.

>> diff --git a/gdb/testsuite/gdb.ctf/ctf-a.c b/gdb/testsuite/gdb.ctf/ctf-a.c
>> new file mode 100644
>> index 0000000..f514a33
>> --- /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 <https://urldefense.com/v3/__http://www.gnu.org/licenses/__;!!GqivPVa7Brio!MlnQ4nwBBbhjsCOlrkvxnKDpvrQ5BtDctjA14y8Sh72MPt3sVujLIe8lqYzl-as2$ >.  */
>> +
>> +#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)
> Trailing space.

Will fix it.

>> +    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 <https://urldefense.com/v3/__http://www.gnu.org/licenses/__;!!GqivPVa7Brio!MlnQ4nwBBbhjsCOlrkvxnKDpvrQ5BtDctjA14y8Sh72MPt3sVujLIe8lqYzl-as2$ >.  */
>> +
>> +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 <https://urldefense.com/v3/__http://www.gnu.org/licenses/__;!!GqivPVa7Brio!MlnQ4nwBBbhjsCOlrkvxnKDpvrQ5BtDctjA14y8Sh72MPt3sVujLIe8lqYzl-as2$ >.  */
>> +
>> +#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 <https://urldefense.com/v3/__http://www.gnu.org/licenses/__;!!GqivPVa7Brio!MlnQ4nwBBbhjsCOlrkvxnKDpvrQ5BtDctjA14y8Sh72MPt3sVujLIe8lqYzl-as2$ >.  */
>> +
>> +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..50db511
>> --- /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 <https://urldefense.com/v3/__http://www.gnu.org/licenses/__;!!GqivPVa7Brio!MlnQ4nwBBbhjsCOlrkvxnKDpvrQ5BtDctjA14y8Sh72MPt3sVujLIe8lqYzl-as2$ >.  */
>> +
>> +#include "ctf-c.h"
>> +
>> +static struct C c __attribute__((used));
>> +
>> +struct C * foo ()
>> +{
>> +  return &c;
>> +}
> Trailing space

Will fix it.

>> 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 <https://urldefense.com/v3/__http://www.gnu.org/licenses/__;!!GqivPVa7Brio!MlnQ4nwBBbhjsCOlrkvxnKDpvrQ5BtDctjA14y8Sh72MPt3sVujLIe8lqYzl-as2$ >.  */
>> +
>> +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..8e99f23
>> --- /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 <https://urldefense.com/v3/__http://www.gnu.org/licenses/__;!!GqivPVa7Brio!MlnQ4nwBBbhjsCOlrkvxnKDpvrQ5BtDctjA14y8Sh72MPt3sVujLIe8lqYzl-as2$ >.
>> +
>> +# 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"
> You also have trailing spaces here.

Will fix them.

Thanks very much for your comments.

>
> Lancelot.
>
>> -- 
>> 1.8.3.1
>>

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

* Re: [PATCH,V2 5/5] CTF: multi-CU and archive support
  2021-03-03  1:01             ` Wei-min Pan
@ 2021-03-03 17:37               ` Lancelot SIX
  2021-03-03 18:31                 ` Weimin Pan
  0 siblings, 1 reply; 21+ messages in thread
From: Lancelot SIX @ 2021-03-03 17:37 UTC (permalink / raw)
  To: Wei-min Pan; +Cc: gdb-patches

> > > +static int
> > > +build_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
> > I do not see the advantage of having arg passed as a void *.  It is
> > casted unconditionally to a struct ctf_per_tu_data * and dereferenced
> > just after that.
> > 
> > Why not just pass the argument as a struct ctf_per_tu_data * directly?
> 
> build_ctf_archive_member is the callback function of ctf_archive_iter and is
> of
> type ctf_member_f which is defined in ctf-api.h:
> 
> typedef int ctf_member_f (const char *name, ctf_id_t membtype,
>                           unsigned long offset, void *arg);
> 
> and the last argument "pcu" of type struct ctf_per_tu_data * is being passed
> in call:
> 
>         if (ctf_archive_iter (arc, build_ctf_archive_member, &pcu) < 0)
> 
> The conversion is necessary.
> 

Indeed. I went too fast when reading where this function is used. Sorry
for the noise.

Thanks,
Lancelot.

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

* Re: [PATCH,V2 5/5] CTF: multi-CU and archive support
  2021-03-03 17:37               ` Lancelot SIX
@ 2021-03-03 18:31                 ` Weimin Pan
  0 siblings, 0 replies; 21+ messages in thread
From: Weimin Pan @ 2021-03-03 18:31 UTC (permalink / raw)
  To: Lancelot SIX; +Cc: gdb-patches


On 3/3/2021 9:37 AM, Lancelot SIX wrote:
>>>> +static int
>>>> +build_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
>>> I do not see the advantage of having arg passed as a void *.  It is
>>> casted unconditionally to a struct ctf_per_tu_data * and dereferenced
>>> just after that.
>>>
>>> Why not just pass the argument as a struct ctf_per_tu_data * directly?
>> build_ctf_archive_member is the callback function of ctf_archive_iter and is
>> of
>> type ctf_member_f which is defined in ctf-api.h:
>>
>> typedef int ctf_member_f (const char *name, ctf_id_t membtype,
>>                            unsigned long offset, void *arg);
>>
>> and the last argument "pcu" of type struct ctf_per_tu_data * is being passed
>> in call:
>>
>>          if (ctf_archive_iter (arc, build_ctf_archive_member, &pcu) < 0)
>>
>> The conversion is necessary.
>>
> Indeed. I went too fast when reading where this function is used. Sorry
> for the noise.

No problem. I really appreciate your comments.

> Thanks,
> Lancelot.

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

* Re: [PATCH,V2 1/5] CTF: fix incorrect function return type
  2021-03-02  1:53 ` [PATCH,V2 1/5] CTF: fix incorrect function return type Weimin Pan
  2021-03-02  1:53   ` [PATCH,V2 2/5] CTF: set up debug info for function arguments Weimin Pan
@ 2021-03-03 20:37   ` Tom Tromey
  2021-03-03 21:57     ` Wei-min Pan
  1 sibling, 1 reply; 21+ messages in thread
From: Tom Tromey @ 2021-03-03 20:37 UTC (permalink / raw)
  To: Weimin Pan via Gdb-patches; +Cc: Weimin Pan

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

>> +2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>> +
>> +	* ctfread.c (new_symbol): Set function address.
>> +	(read_func_kind_type): Remove incorrect type name setting.

Thanks, this looks good.

>> -    type->set_name (obstack_strdup (&of->objfile_obstack, name.get ()));

It looks like libctf strdup()s the name.  If it returned a pointer to
the debuginfo section, you could just use that pointer without copying.

Otherwise, objfile::intern de-dups names.  Sometimes this is worthwhile.

Tom

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

* Re: [PATCH,V2 2/5] CTF: set up debug info for function arguments
  2021-03-02 12:23     ` [PATCH,V2 2/5] CTF: set up debug info for function arguments Lancelot SIX
  2021-03-02 18:12       ` Wei-min Pan
@ 2021-03-03 20:38       ` Tom Tromey
  2021-03-03 21:58         ` Wei-min Pan
  1 sibling, 1 reply; 21+ messages in thread
From: Tom Tromey @ 2021-03-03 20:38 UTC (permalink / raw)
  To: Lancelot SIX via Gdb-patches; +Cc: Weimin Pan, Lancelot SIX

Lancelot> Could be nullptr instead of NULL (even if I am not sure the standard
Lancelot> enforces that).

IMO "nullptr" is preferred in new code.

Tom

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

* Re: [PATCH,V2 2/5] CTF: set up debug info for function arguments
  2021-03-02  1:53   ` [PATCH,V2 2/5] CTF: set up debug info for function arguments Weimin Pan
  2021-03-02  1:53     ` [PATCH,V2 3/5] CTF: handle forward reference type Weimin Pan
  2021-03-02 12:23     ` [PATCH,V2 2/5] CTF: set up debug info for function arguments Lancelot SIX
@ 2021-03-03 20:41     ` Tom Tromey
  2 siblings, 0 replies; 21+ messages in thread
From: Tom Tromey @ 2021-03-03 20:41 UTC (permalink / raw)
  To: Weimin Pan via Gdb-patches; +Cc: Weimin Pan

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

>> Added this support in read_func_kind_type after gcc started generating
>> CTF for function arguments.

>> Expanded gdb.base/ctf-ptype.exp to test function arguments. Also fixed
>> some typos.

Thanks for the patch.

>> +  if (cfi.ctc_flags & CTF_FUNC_VARARG)

The gdb style is to compare against zero, like:

  if ((cfi.ctc_flags & CTF_FUNC_VARARG) != 0)

This is ok with this change.

Tom

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

* Re: [PATCH,V2 4/5] CTF: add all members of an enum type to psymtab
  2021-03-02  1:53       ` [PATCH,V2 4/5] CTF: add all members of an enum type to psymtab Weimin Pan
  2021-03-02  1:53         ` [PATCH,V2 5/5] CTF: multi-CU and archive support Weimin Pan
@ 2021-03-03 20:46         ` Tom Tromey
  2021-03-03 22:27           ` Wei-min Pan
  1 sibling, 1 reply; 21+ messages in thread
From: Tom Tromey @ 2021-03-03 20:46 UTC (permalink / raw)
  To: Weimin Pan via Gdb-patches; +Cc: Weimin Pan

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

>> With the following change which was made last April:
>>     [gdb] Use partial symbol table to find language for main
>>     commit d3214198119c1a2f9a6a2b8fcc56d8c324e1a245

>> The ctf reader was modified to enter all members of an enum type,
>> similar to what the dwarf2 reader did, into the psymtab or gdb
>> won't be able to find them. In addition, the empty name checking
>> needed to be moved down so members of a unnamed enum were not left
>> out. 

Thanks for the patch.

>> +  ctf_next_t *i = NULL;
>> +
>> +  while ((ename = ctf_enum_next (ccp->fp, tid, &i, &val)) != NULL)

Should probably use nullptr here.

This is ok with this change.

Tom

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

* Re: [PATCH,V2 3/5] CTF: handle forward reference type
  2021-03-02  1:53     ` [PATCH,V2 3/5] CTF: handle forward reference type Weimin Pan
  2021-03-02  1:53       ` [PATCH,V2 4/5] CTF: add all members of an enum type to psymtab Weimin Pan
@ 2021-03-03 20:49       ` Tom Tromey
  2021-03-03 22:01         ` Wei-min Pan
  1 sibling, 1 reply; 21+ messages in thread
From: Tom Tromey @ 2021-03-03 20:49 UTC (permalink / raw)
  To: Weimin Pan via Gdb-patches; +Cc: Weimin Pan

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

>> Added function fetch_tid_type which calls get_tid_type and will set up
>> the type, associated with a tid, if it is not read in yet. Also implement 
>> function read_forward_type which handles the CTF_K_FORWARD kind.

Are forward types always filled in?
Or are they incomplete types (like in C)?

>> +  TYPE_LENGTH (type) = 0;
>> +  type->set_is_stub (true);

I ask because I think you'd only want to do this for incomplete types.

Tom

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

* Re: [PATCH,V2 1/5] CTF: fix incorrect function return type
  2021-03-03 20:37   ` [PATCH,V2 1/5] CTF: fix incorrect function return type Tom Tromey
@ 2021-03-03 21:57     ` Wei-min Pan
  0 siblings, 0 replies; 21+ messages in thread
From: Wei-min Pan @ 2021-03-03 21:57 UTC (permalink / raw)
  To: Tom Tromey, Weimin Pan via Gdb-patches


On 3/3/2021 12:37 PM, Tom Tromey wrote:
>>>>>> ">" == Weimin Pan via Gdb-patches <gdb-patches@sourceware.org> writes:
>>> +2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>>> +
>>> +	* ctfread.c (new_symbol): Set function address.
>>> +	(read_func_kind_type): Remove incorrect type name setting.
> Thanks, this looks good.

This is in response to your comment from the original series.

>>> -    type->set_name (obstack_strdup (&of->objfile_obstack, name.get ()));
> It looks like libctf strdup()s the name.  If it returned a pointer to
> the debuginfo section, you could just use that pointer without copying.
>
> Otherwise, objfile::intern de-dups names.  Sometimes this is worthwhile.

Given that line of code is deleted, it causes no harm. But will keep 
your comment in mind.

Thanks.

>
> Tom

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

* Re: [PATCH,V2 2/5] CTF: set up debug info for function arguments
  2021-03-03 20:38       ` Tom Tromey
@ 2021-03-03 21:58         ` Wei-min Pan
  0 siblings, 0 replies; 21+ messages in thread
From: Wei-min Pan @ 2021-03-03 21:58 UTC (permalink / raw)
  To: Tom Tromey, Lancelot SIX via Gdb-patches; +Cc: Lancelot SIX


On 3/3/2021 12:38 PM, Tom Tromey wrote:
> Lancelot> Could be nullptr instead of NULL (even if I am not sure the standard
> Lancelot> enforces that).
>
> IMO "nullptr" is preferred in new code.

OK, done in multiple places.

> Tom

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

* Re: [PATCH,V2 3/5] CTF: handle forward reference type
  2021-03-03 20:49       ` [PATCH,V2 3/5] CTF: handle forward reference type Tom Tromey
@ 2021-03-03 22:01         ` Wei-min Pan
  0 siblings, 0 replies; 21+ messages in thread
From: Wei-min Pan @ 2021-03-03 22:01 UTC (permalink / raw)
  To: Tom Tromey, Weimin Pan via Gdb-patches


On 3/3/2021 12:49 PM, Tom Tromey wrote:
>>>>>> ">" == Weimin Pan via Gdb-patches <gdb-patches@sourceware.org> writes:
>>> Added function fetch_tid_type which calls get_tid_type and will set up
>>> the type, associated with a tid, if it is not read in yet. Also implement
>>> function read_forward_type which handles the CTF_K_FORWARD kind.
> Are forward types always filled in?
> Or are they incomplete types (like in C)?

No, these types are not always filled in and can be incomplete.

>
>>> +  TYPE_LENGTH (type) = 0;
>>> +  type->set_is_stub (true);
> I ask because I think you'd only want to do this for incomplete types.
>
> Tom

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

* Re: [PATCH,V2 4/5] CTF: add all members of an enum type to psymtab
  2021-03-03 20:46         ` [PATCH,V2 4/5] CTF: add all members of an enum type to psymtab Tom Tromey
@ 2021-03-03 22:27           ` Wei-min Pan
  0 siblings, 0 replies; 21+ messages in thread
From: Wei-min Pan @ 2021-03-03 22:27 UTC (permalink / raw)
  To: Tom Tromey, Weimin Pan via Gdb-patches


On 3/3/2021 12:46 PM, Tom Tromey wrote:
>>>>>> ">" == Weimin Pan via Gdb-patches <gdb-patches@sourceware.org> writes:
>>> With the following change which was made last April:
>>>      [gdb] Use partial symbol table to find language for main
>>>      commit d3214198119c1a2f9a6a2b8fcc56d8c324e1a245
>>> The ctf reader was modified to enter all members of an enum type,
>>> similar to what the dwarf2 reader did, into the psymtab or gdb
>>> won't be able to find them. In addition, the empty name checking
>>> needed to be moved down so members of a unnamed enum were not left
>>> out.
> Thanks for the patch.
>
>>> +  ctf_next_t *i = NULL;
>>> +
>>> +  while ((ename = ctf_enum_next (ccp->fp, tid, &i, &val)) != NULL)
> Should probably use nullptr here.

Yes, will do.

> This is ok with this change.

Thanks.

> Tom

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

end of thread, other threads:[~2021-03-03 22:27 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-02  1:53 [PATCH,V2 0/5] CTF: bug fixes and new features Weimin Pan
2021-03-02  1:53 ` [PATCH,V2 1/5] CTF: fix incorrect function return type Weimin Pan
2021-03-02  1:53   ` [PATCH,V2 2/5] CTF: set up debug info for function arguments Weimin Pan
2021-03-02  1:53     ` [PATCH,V2 3/5] CTF: handle forward reference type Weimin Pan
2021-03-02  1:53       ` [PATCH,V2 4/5] CTF: add all members of an enum type to psymtab Weimin Pan
2021-03-02  1:53         ` [PATCH,V2 5/5] CTF: multi-CU and archive support Weimin Pan
2021-03-02 23:15           ` Lancelot SIX
2021-03-03  1:01             ` Wei-min Pan
2021-03-03 17:37               ` Lancelot SIX
2021-03-03 18:31                 ` Weimin Pan
2021-03-03 20:46         ` [PATCH,V2 4/5] CTF: add all members of an enum type to psymtab Tom Tromey
2021-03-03 22:27           ` Wei-min Pan
2021-03-03 20:49       ` [PATCH,V2 3/5] CTF: handle forward reference type Tom Tromey
2021-03-03 22:01         ` Wei-min Pan
2021-03-02 12:23     ` [PATCH,V2 2/5] CTF: set up debug info for function arguments Lancelot SIX
2021-03-02 18:12       ` Wei-min Pan
2021-03-03 20:38       ` Tom Tromey
2021-03-03 21:58         ` Wei-min Pan
2021-03-03 20:41     ` Tom Tromey
2021-03-03 20:37   ` [PATCH,V2 1/5] CTF: fix incorrect function return type Tom Tromey
2021-03-03 21:57     ` 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).