[1/8] gdb: Add $_cimag and $_creal internal functions

Message ID 854230b716fa6838bd2d8e73f1fd2d342a7a75ed.1552913183.git.andrew.burgess@embecosm.com
State New
Headers show
Series
  • Series of Fortran type printing patches
Related show

Commit Message

Andrew Burgess March 18, 2019, 12:52 p.m.
Add two new internal functions $_cimag and $_creal that extract the
imaginary and real parts of a complex value.

These internal functions can take a complex value of any type 'float
complex', 'double complex', or 'long double complex' and return a
suitable floating point value 'float', 'double', or 'long double'.
So we can now do this:

    (gdb) p z1
    $1 = 1.5 + 4.5 * I
    (gdb) p $_cimag (z1)
    $4 = 4.5
    (gdb) p $_creal (z1)
    $4 = 1.5

The components of a complex value are not strictly named types in
DWARF, as the complex type is itself the base type.  However, once we
are able to extract the components it makes sense to be able to ask
what the type of these components is and get a sensible answer back,
rather than the error we would currently get.  Currently GDB says:

    (gdb) ptype z1
    type = complex double
    (gdb) p $_cimag (z1)
    $4 = 4.5
    (gdb) ptype $
    type = <invalid type code 9>

With the changes in dwarf2read.c, GDB now says:

    (gdb) ptype z1
    type = complex double
    (gdb) p $_cimag (z1)
    $4 = 4.5
    (gdb) ptype $
    type = double

Which seems to make more sense.

gdb/ChangeLog:

	* NEWS: Mention new internal functions.
	* dwarf2read.c (dwarf2_init_complex_target_type): New function.
	(read_base_type): Use dwarf2_init_complex_target_type.
	* value.c (creal_internal_fn): New function.
	(cimag_internal_fn): New function.
	(_initialize_values): Register new internal functions.

gdb/doc/ChangeLog:

	* gdb.texinfo (Convenience Funs): Document '$_creal' and
	'$_cimag'.

gdb/testsuite/ChangeLog:

	* gdb.base/complex-parts.c: New file.
	* gdb.base/complex-parts.exp: New file.
---
 gdb/ChangeLog                            |  9 +++++
 gdb/NEWS                                 |  3 ++
 gdb/doc/ChangeLog                        |  5 +++
 gdb/doc/gdb.texinfo                      | 15 ++++++++
 gdb/dwarf2read.c                         | 36 ++++++++++++++++++-
 gdb/testsuite/ChangeLog                  |  5 +++
 gdb/testsuite/gdb.base/complex-parts.c   | 50 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/complex-parts.exp | 62 ++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/default.exp       |  2 ++
 gdb/value.c                              | 52 +++++++++++++++++++++++++++
 10 files changed, 238 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.base/complex-parts.c
 create mode 100644 gdb/testsuite/gdb.base/complex-parts.exp

-- 
2.14.5

Comments

Eli Zaretskii March 18, 2019, 5:20 p.m. | #1
> From: Andrew Burgess <andrew.burgess@embecosm.com>

> Cc: Richard Bunt <Richard.Bunt@arm.com>,	Andrew Burgess <andrew.burgess@embecosm.com>

> Date: Mon, 18 Mar 2019 12:52:16 +0000

> 

> gdb/ChangeLog:

> 

> 	* NEWS: Mention new internal functions.

> 	* dwarf2read.c (dwarf2_init_complex_target_type): New function.

> 	(read_base_type): Use dwarf2_init_complex_target_type.

> 	* value.c (creal_internal_fn): New function.

> 	(cimag_internal_fn): New function.

> 	(_initialize_values): Register new internal functions.

> 

> gdb/doc/ChangeLog:

> 

> 	* gdb.texinfo (Convenience Funs): Document '$_creal' and

> 	'$_cimag'.


The documentation parts are okay, with a couple of comments:

> +@item $_cimag(@var{value})

> +@findex $_cimag@r{, convenience function}

> +Return the imaginary part of a complex number @var{value}.

> +

> +The type of the imaginary part depends on the type of the complex

> +number, a @code{float complex} will return an imaginary part of type

> +@code{float}.

> +

> +@item $_creal(@var{value})

> +@findex $_creal@r{, convenience function}

> +Return the real part of a complex number @var{value}.

> +

> +The type of the real part depends on the type of the complex number, a

> +@code{float complex} will return an real part of type @code{float}.


There's no need to repeat the sentence about the type being dependent
on the original type; you can say that just once for both.

Also, the "a @code{float complex} will return ..." part is better
preceded by "e.g.", as this is an example, right?

And finally, "a real part", not "an real part".

> +Return the real part of a complex number, the type depends on the\n\

> +type of a complex number."),

           ^
"the", not "a".

> +Return the imaginary part of a complex number, the type depends on the\n\

> +type of a complex number."),


Likewise.

Thanks.
Tom Tromey March 19, 2019, 7:47 p.m. | #2
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:


Andrew> Add two new internal functions $_cimag and $_creal that extract the
Andrew> imaginary and real parts of a complex value.

Thanks.  I had one nit.

Andrew> +  if (argc != 1)
Andrew> +    error (_("You must provide one argument for $_real_from_complex."));
[...]
Andrew> +  if (argc != 1)
Andrew> +    error (_("You must provide one argument for $_real_from_complex."));

I think these should use the names $_creal and $_cimag instead.

Tom
Andrew Burgess March 29, 2019, 10:41 p.m. | #3
* Eli Zaretskii <eliz@gnu.org> [2019-03-18 19:20:26 +0200]:

> > From: Andrew Burgess <andrew.burgess@embecosm.com>

> > Cc: Richard Bunt <Richard.Bunt@arm.com>,	Andrew Burgess <andrew.burgess@embecosm.com>

> > Date: Mon, 18 Mar 2019 12:52:16 +0000

> > 

> > gdb/ChangeLog:

> > 

> > 	* NEWS: Mention new internal functions.

> > 	* dwarf2read.c (dwarf2_init_complex_target_type): New function.

> > 	(read_base_type): Use dwarf2_init_complex_target_type.

> > 	* value.c (creal_internal_fn): New function.

> > 	(cimag_internal_fn): New function.

> > 	(_initialize_values): Register new internal functions.

> > 

> > gdb/doc/ChangeLog:

> > 

> > 	* gdb.texinfo (Convenience Funs): Document '$_creal' and

> > 	'$_cimag'.

> 

> The documentation parts are okay, with a couple of comments:

> 

> > +@item $_cimag(@var{value})

> > +@findex $_cimag@r{, convenience function}

> > +Return the imaginary part of a complex number @var{value}.

> > +

> > +The type of the imaginary part depends on the type of the complex

> > +number, a @code{float complex} will return an imaginary part of type

> > +@code{float}.

> > +

> > +@item $_creal(@var{value})

> > +@findex $_creal@r{, convenience function}

> > +Return the real part of a complex number @var{value}.

> > +

> > +The type of the real part depends on the type of the complex number, a

> > +@code{float complex} will return an real part of type @code{float}.

> 

> There's no need to repeat the sentence about the type being dependent

> on the original type; you can say that just once for both.


Thanks for your feedback.

What I've done is group the @item entries for $_creal and $_cimag
together.  My thinking is that if a user looks up either of these then
the documentation should be complete.  If I left the @item entries
separate, and removed the sentence as you suggested then I think I'd
have to add a cross reference.

Hopefully this will be OK.

All your other feedback has been addressed.  A new version of the
patch is below for your consideration.

Thanks,
Andrew

> 

> Also, the "a @code{float complex} will return ..." part is better

> preceded by "e.g.", as this is an example, right?

> 

> And finally, "a real part", not "an real part".

> 

> > +Return the real part of a complex number, the type depends on the\n\

> > +type of a complex number."),

>            ^

> "the", not "a".

> 

> > +Return the imaginary part of a complex number, the type depends on the\n\

> > +type of a complex number."),

> 

> Likewise.


---

    gdb: Add $_cimag and $_creal internal functions
    
    Add two new internal functions $_cimag and $_creal that extract the
    imaginary and real parts of a complex value.
    
    These internal functions can take a complex value of any type 'float
    complex', 'double complex', or 'long double complex' and return a
    suitable floating point value 'float', 'double', or 'long double'.
    So we can now do this:
    
        (gdb) p z1
        $1 = 1.5 + 4.5 * I
        (gdb) p $_cimag (z1)
        $4 = 4.5
        (gdb) p $_creal (z1)
        $4 = 1.5
    
    The components of a complex value are not strictly named types in
    DWARF, as the complex type is itself the base type.  However, once we
    are able to extract the components it makes sense to be able to ask
    what the type of these components is and get a sensible answer back,
    rather than the error we would currently get.  Currently GDB says:
    
        (gdb) ptype z1
        type = complex double
        (gdb) p $_cimag (z1)
        $4 = 4.5
        (gdb) ptype $
        type = <invalid type code 9>
    
    With the changes in dwarf2read.c, GDB now says:
    
        (gdb) ptype z1
        type = complex double
        (gdb) p $_cimag (z1)
        $4 = 4.5
        (gdb) ptype $
        type = double
    
    Which seems to make more sense.
    
    gdb/ChangeLog:
    
            * NEWS: Mention new internal functions.
            * dwarf2read.c (dwarf2_init_complex_target_type): New function.
            (read_base_type): Use dwarf2_init_complex_target_type.
            * value.c (creal_internal_fn): New function.
            (cimag_internal_fn): New function.
            (_initialize_values): Register new internal functions.
    
    gdb/doc/ChangeLog:
    
            * gdb.texinfo (Convenience Funs): Document '$_creal' and
            '$_cimag'.
    
    gdb/testsuite/ChangeLog:
    
            * gdb.base/complex-parts.c: New file.
            * gdb.base/complex-parts.exp: New file.

diff --git a/gdb/NEWS b/gdb/NEWS
index edcc9c951a0..6d700d1cf32 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -10,6 +10,9 @@
 
 * Support for Pointer Authentication on AArch64 Linux.
 
+* Two new convernience functions $_cimag and $_creal that extract the
+  imaginary and real parts respectively from complex numbers.
+
 *** Changes in GDB 8.3
 
 * GDB and GDBserver now support access to additional registers on
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 4c1d5e8294f..6dda71d83c0 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -11390,6 +11390,17 @@
 Visiting node of type NODE_INTEGER
 @end smallexample
 
+@item $_cimag(@var{value})
+@item $_creal(@var{value})
+@findex $_cimag@r{, convenience function}
+@findex $_creal@r{, convenience function}
+Return the imaginary (@code{$_cimag}) or real (@code{$_creal}) part of
+the complex number @var{value}.
+
+The type of the imaginary or real part depends on the type of the
+complex number, e.g., using @code{$_cimag} on a @code{float complex}
+will return an imaginary part of type @code{float}.
+
 @end table
 
 @value{GDBN} provides the ability to list and get help on
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 658c86264bf..e742931ad53 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -17530,6 +17530,40 @@ dwarf2_init_integer_type (struct dwarf2_cu *cu, struct objfile *objfile,
   return type;
 }
 
+/* Initialise and return a floating point type of size BITS suitable for
+   use as a component of a complex number.  The NAME_HINT is passed through
+   when initialising the floating point type and is the name of the complex
+   type.
+
+   As DWARF doesn't currently provide an explicit name for the components
+   of a complex number, but it can be helpful to have these components
+   named, we try to select a suitable name based on the size of the
+   component.  */
+static struct type *
+dwarf2_init_complex_target_type (struct dwarf2_cu *cu,
+				 struct objfile *objfile,
+				 int bits, const char *name_hint)
+{
+  gdbarch *gdbarch = get_objfile_arch (objfile);
+  struct type *tt = nullptr;
+
+  switch (bits)
+    {
+    case 32:
+      tt = builtin_type (gdbarch)->builtin_float;
+      break;
+    case 64:
+      tt = builtin_type (gdbarch)->builtin_double;
+      break;
+    case 128:
+      tt = builtin_type (gdbarch)->builtin_long_double;
+      break;
+    }
+
+  const char *name = (tt == nullptr) ? nullptr : TYPE_NAME (tt);
+  return dwarf2_init_float_type (objfile, bits, name, name_hint);
+}
+
 /* Find a representation of a given base type and install
    it in the TYPE field of the die.  */
 
@@ -17569,7 +17603,7 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
 	type = init_boolean_type (objfile, bits, 1, name);
 	break;
       case DW_ATE_complex_float:
-	type = dwarf2_init_float_type (objfile, bits / 2, NULL, name);
+	type = dwarf2_init_complex_target_type (cu, objfile, bits / 2, name);
 	type = init_complex_type (objfile, name, type);
 	break;
       case DW_ATE_decimal_float:
diff --git a/gdb/testsuite/gdb.base/complex-parts.c b/gdb/testsuite/gdb.base/complex-parts.c
new file mode 100644
index 00000000000..243caee0695
--- /dev/null
+++ b/gdb/testsuite/gdb.base/complex-parts.c
@@ -0,0 +1,50 @@
+/* Copyright 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+/* Unlike the other 'complex.c' test, this one uses the "standard" header
+   file to pull in the complex types.  The testing is around printing the
+   complex numbers, and using the convenience function $_cimag and $_creal
+   to extract the parts of the complex numbers.  */
+
+#include <complex.h>
+
+void
+keep_around (volatile void *ptr)
+{
+  asm ("" ::: "memory");
+}
+
+int
+main (void)
+{
+  double complex z1 = 1.5 + 4.5 * I;
+  float complex z2 = 2.5 - 5.5 * I;
+  long double complex z3 = 3.5 + 6.5 * I;
+
+  double d1 = 1.5;
+  float f1 = 2.5;
+  int i1 = 3;
+
+  keep_around (&z1);
+  keep_around (&z2);
+  keep_around (&z3);
+  keep_around (&d1);
+  keep_around (&f1);
+  keep_around (&i1);
+
+  return 0;	/* Break Here.  */
+}
diff --git a/gdb/testsuite/gdb.base/complex-parts.exp b/gdb/testsuite/gdb.base/complex-parts.exp
new file mode 100644
index 00000000000..ce8f4277a8b
--- /dev/null
+++ b/gdb/testsuite/gdb.base/complex-parts.exp
@@ -0,0 +1,62 @@
+# Copyright 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+if { ![runto_main] } then {
+    fail "can't run to main"
+    return 0
+}
+
+gdb_breakpoint [gdb_get_line_number "Break Here"]
+gdb_continue_to_breakpoint "breakpt" ".* Break Here\\. .*"
+
+gdb_test "p z1" " = 1.5 \\+ 4.5 \\* I"
+gdb_test "p z2" " = 2.5 \\+ -5.5 \\* I"
+gdb_test "p z3" " = 3.5 \\+ 6.5 \\* I"
+
+gdb_test "ptype z1" " = complex double"
+gdb_test "ptype z2" " = complex float"
+gdb_test "ptype z3" " = complex long double"
+
+gdb_test "p \$_cimag (z1)" " = 4.5"
+gdb_test "ptype \$" " = double"
+
+gdb_test "p \$_cimag (z2)" " = -5.5"
+gdb_test "ptype \$" " = float"
+
+gdb_test "p \$_cimag (z3)" " = 6.5"
+gdb_test "ptype \$" " = long double"
+
+gdb_test "p \$_creal (z1)" " = 1.5"
+gdb_test "ptype \$" " = double"
+
+gdb_test "p \$_creal (z2)" " = 2.5"
+gdb_test "ptype \$" " = float"
+
+gdb_test "p \$_creal (z3)" " = 3.5"
+gdb_test "ptype \$" " = long double"
+
+gdb_test "p \$_cimag (d1)" "expected a complex number"
+gdb_test "p \$_cimag (f1)" "expected a complex number"
+gdb_test "p \$_cimag (i1)" "expected a complex number"
+
+gdb_test "p \$_creal (d1)" "expected a complex number"
+gdb_test "p \$_creal (f1)" "expected a complex number"
+gdb_test "p \$_creal (i1)" "expected a complex number"
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index ece1428e617..bdae0d471e7 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -601,6 +601,8 @@ set show_conv_list \
 	{$_probe_arg9 = <error: No frame selected>} \
 	{$_probe_arg10 = <error: No frame selected>} \
 	{$_probe_arg11 = <error: No frame selected>} \
+	{$_cimag = <internal function _cimag>} \
+	{$_creal = <internal function _creal>} \
 	{$_isvoid = <internal function _isvoid>} \
     }
 if ![skip_python_tests] {
diff --git a/gdb/value.c b/gdb/value.c
index dc297dfe0f9..d011b396986 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3933,6 +3933,44 @@ isvoid_internal_fn (struct gdbarch *gdbarch,
   return value_from_longest (builtin_type (gdbarch)->builtin_int, ret);
 }
 
+/* Implementation of the convenience function $_cimag.  Extracts the
+   real part from a complex number.  */
+
+static struct value *
+creal_internal_fn (struct gdbarch *gdbarch,
+		   const struct language_defn *language,
+		   void *cookie, int argc, struct value **argv)
+{
+  if (argc != 1)
+    error (_("You must provide one argument for $_creal."));
+
+  value *cval = argv[0];
+  type *ctype = check_typedef (value_type (cval));
+  if (TYPE_CODE (ctype) != TYPE_CODE_COMPLEX)
+    error (_("expected a complex number"));
+  return value_from_component (cval, TYPE_TARGET_TYPE (ctype), 0);
+}
+
+/* Implementation of the convenience function $_cimag.  Extracts the
+   imaginary part from a complex number.  */
+
+static struct value *
+cimag_internal_fn (struct gdbarch *gdbarch,
+		   const struct language_defn *language,
+		   void *cookie, int argc,
+		   struct value **argv)
+{
+  if (argc != 1)
+    error (_("You must provide one argument for $_cimag."));
+
+  value *cval = argv[0];
+  type *ctype = check_typedef (value_type (cval));
+  if (TYPE_CODE (ctype) != TYPE_CODE_COMPLEX)
+    error (_("expected a complex number"));
+  return value_from_component (cval, TYPE_TARGET_TYPE (ctype),
+			       TYPE_LENGTH (TYPE_TARGET_TYPE (ctype)));
+}
+
 #if GDB_SELF_TEST
 namespace selftests
 {
@@ -4114,6 +4152,20 @@ Usage: $_isvoid (expression)\n\
 Return 1 if the expression is void, zero otherwise."),
 			 isvoid_internal_fn, NULL);
 
+  add_internal_function ("_creal", _("\
+Extract the real part of a complex number.\n\
+Usage: $_creal (expression)\n\
+Return the real part of a complex number, the type depends on the\n\
+type of a complex number."),
+			 creal_internal_fn, NULL);
+
+  add_internal_function ("_cimag", _("\
+Extract the imaginary part of a complex number.\n\
+Usage: $_cimag (expression)\n\
+Return the imaginary part of a complex number, the type depends on the\n\
+type of a complex number."),
+			 cimag_internal_fn, NULL);
+
   add_setshow_zuinteger_unlimited_cmd ("max-value-size",
 				       class_support, &max_value_size, _("\
 Set maximum sized value gdb will load from the inferior."), _("\
Eli Zaretskii March 30, 2019, 7:15 a.m. | #4
> Date: Fri, 29 Mar 2019 22:41:16 +0000

> From: Andrew Burgess <andrew.burgess@embecosm.com>

> Cc: gdb-patches@sourceware.org, Richard.Bunt@arm.com

> 

> What I've done is group the @item entries for $_creal and $_cimag

> together.  My thinking is that if a user looks up either of these then

> the documentation should be complete.  If I left the @item entries

> separate, and removed the sentence as you suggested then I think I'd

> have to add a cross reference.

> 

> Hopefully this will be OK.


I have only one comment:

> +@item $_cimag(@var{value})

> +@item $_creal(@var{value})


The second @item should be @itemx.

Thanks.
Sergio Durigan Junior April 12, 2019, 8:23 p.m. | #5
On Monday, March 18 2019, Andrew Burgess wrote:

> Add two new internal functions $_cimag and $_creal that extract the

> imaginary and real parts of a complex value.

[...]

Hi Andrew,

The new testcase gdb.base/complex-parts.exp has one new FAIL when
testing x86_64 GDB against unix/-m32.  I.e.:

  make check-gdb TESTS=gdb.base/complex-parts.exp RUNTESTFLAGS='--target_board unix/-m32'

Here's what I'm seeing:

  ptype $
  type = <invalid type code 9>
  (gdb) FAIL: gdb.base/complex-parts.exp: ptype $
  ...
  ptype $
  type = <invalid type code 9>
  (gdb) FAIL: gdb.base/complex-parts.exp: ptype $

The BuildBot run:

  https://gdb-build.sergiodj.net/builders/Fedora-x86_64-m32/builds/12210

The logs:

  https://gdb-build.sergiodj.net/results/Fedora-x86_64-m32/8b/8bdc16587e26100282094c8eaa8e83180ba57afd/

Let me know if you need help investigating this.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
Andrew Burgess April 13, 2019, 12:02 a.m. | #6
* Sergio Durigan Junior <sergiodj@redhat.com> [2019-04-12 16:23:57 -0400]:

> On Monday, March 18 2019, Andrew Burgess wrote:

> 

> > Add two new internal functions $_cimag and $_creal that extract the

> > imaginary and real parts of a complex value.

> [...]

> 

> Hi Andrew,

> 

> The new testcase gdb.base/complex-parts.exp has one new FAIL when

> testing x86_64 GDB against unix/-m32.  I.e.:

> 

>   make check-gdb TESTS=gdb.base/complex-parts.exp RUNTESTFLAGS='--target_board unix/-m32'

> 

> Here's what I'm seeing:

> 

>   ptype $

>   type = <invalid type code 9>

>   (gdb) FAIL: gdb.base/complex-parts.exp: ptype $

>   ...

>   ptype $

>   type = <invalid type code 9>

>   (gdb) FAIL: gdb.base/complex-parts.exp: ptype $

> 

> The BuildBot run:

> 

>   https://gdb-build.sergiodj.net/builders/Fedora-x86_64-m32/builds/12210

> 

> The logs:

> 

>   https://gdb-build.sergiodj.net/results/Fedora-x86_64-m32/8b/8bdc16587e26100282094c8eaa8e83180ba57afd/

> 

> Let me know if you need help investigating this.


Thanks for pointing this out.

I've pushed the patch below to resolve this issue.

Thanks,
Andrew

--

[PATCH] gdb: Fix failure in gdb.base/complex-parts.exp for x86-32

The x86-32 ABI specifies 96-bit long double, this was causing a
failure on the test gdb.base/complex-parts.exp.

The problem is that GDB tries to find a builtin floating point type of
the correct size in order to reuse the name of that type as the name
for the components of the complex type being built.

Previously GDB was only aware of floating point types sized 32, 64, or
128 bits.  This patch teaches GDB how to handle 96 bit floating point
type.

gdb/ChangeLog:

	* dwarf2read.c (dwarf2_init_complex_target_type): Handle complex
	target types of size 96-bits, add some additional comments, and
	check that the builtin type we found was the correct size.
---
 gdb/ChangeLog    |  6 ++++++
 gdb/dwarf2read.c | 10 ++++++++++
 2 files changed, 16 insertions(+)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index b718192cb12..0873028e438 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -17546,6 +17546,9 @@ dwarf2_init_complex_target_type (struct dwarf2_cu *cu,
   gdbarch *gdbarch = get_objfile_arch (objfile);
   struct type *tt = nullptr;
 
+  /* Try to find a suitable floating point builtin type of size BITS.
+     We're going to use the name of this type as the name for the complex
+     target type that we are about to create.  */
   switch (bits)
     {
     case 32:
@@ -17554,11 +17557,18 @@ dwarf2_init_complex_target_type (struct dwarf2_cu *cu,
     case 64:
       tt = builtin_type (gdbarch)->builtin_double;
       break;
+    case 96:	/* The x86-32 ABI specifies 96-bit long double.  */
     case 128:
       tt = builtin_type (gdbarch)->builtin_long_double;
       break;
     }
 
+  /* If the type we found doesn't match the size we were looking for, then
+     pretend we didn't find a type at all, the complex target type we
+     create will then be nameless.  */
+  if (TYPE_LENGTH (tt) * TARGET_CHAR_BIT != bits)
+    tt = nullptr;
+
   const char *name = (tt == nullptr) ? nullptr : TYPE_NAME (tt);
   return dwarf2_init_float_type (objfile, bits, name, name_hint);
 }
-- 
2.14.5
Tom Tromey April 16, 2019, 6:30 p.m. | #7
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:


Andrew> The problem is that GDB tries to find a builtin floating point type of
Andrew> the correct size in order to reuse the name of that type as the name
Andrew> for the components of the complex type being built.

This patch caused a crash on an internal test case.
Let me know what you think of the appended.

thanks,
Tom

commit bf3507f7fcb6331401f9fd8eb7f1fad25ebfdf23
Author: Tom Tromey <tromey@adacore.com>
Date:   Tue Apr 16 12:12:09 2019 -0600

    Avoid crash in dwarf2_init_complex_target_type
    
    After commit 35add35 ("gdb: Fix failure in gdb.base/complex-parts.exp
    for x86-32"), dwarf2_init_complex_target_type can crash if "tt" is
    nullptr.  This patch avoids the problem by checking for this case.
    
    No test case because I don't know a good way to write one; it was
    found by an internal AdaCore test case that apparently uses a 16 bit
    floating point type.
    
    gdb/ChangeLog:
            * dwarf2read.c (dwarf2_init_complex_target_type): Check "tt"
            against nullptr before use.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ba1300d57ef..9281d822ade 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2019-04-16  Tom Tromey  <tromey@adacore.com>
+
+	* dwarf2read.c (dwarf2_init_complex_target_type): Check "tt"
+	against nullptr before use.
+
 2019-04-15  Leszek Swirski  <leszeks@google.com>
 
 	* amd64-tdep.c (amd64_classify_aggregate): Use cp_pass_by_reference
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 0873028e438..16bf2404a21 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -17566,7 +17566,7 @@ dwarf2_init_complex_target_type (struct dwarf2_cu *cu,
   /* If the type we found doesn't match the size we were looking for, then
      pretend we didn't find a type at all, the complex target type we
      create will then be nameless.  */
-  if (TYPE_LENGTH (tt) * TARGET_CHAR_BIT != bits)
+  if (tt != nullptr && TYPE_LENGTH (tt) * TARGET_CHAR_BIT != bits)
     tt = nullptr;
 
   const char *name = (tt == nullptr) ? nullptr : TYPE_NAME (tt);
Andrew Burgess April 17, 2019, 12:03 a.m. | #8
* Tom Tromey <tom@tromey.com> [2019-04-16 12:30:48 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

> 

> Andrew> The problem is that GDB tries to find a builtin floating point type of

> Andrew> the correct size in order to reuse the name of that type as the name

> Andrew> for the components of the complex type being built.

> 

> This patch caused a crash on an internal test case.

> Let me know what you think of the appended.

> 

> thanks,

> Tom

> 

> commit bf3507f7fcb6331401f9fd8eb7f1fad25ebfdf23

> Author: Tom Tromey <tromey@adacore.com>

> Date:   Tue Apr 16 12:12:09 2019 -0600

> 

>     Avoid crash in dwarf2_init_complex_target_type

>     

>     After commit 35add35 ("gdb: Fix failure in gdb.base/complex-parts.exp

>     for x86-32"), dwarf2_init_complex_target_type can crash if "tt" is

>     nullptr.  This patch avoids the problem by checking for this case.

>     

>     No test case because I don't know a good way to write one; it was

>     found by an internal AdaCore test case that apparently uses a 16 bit

>     floating point type.

>     

>     gdb/ChangeLog:

>             * dwarf2read.c (dwarf2_init_complex_target_type): Check "tt"

>             against nullptr before use.

> 

> diff --git a/gdb/ChangeLog b/gdb/ChangeLog

> index ba1300d57ef..9281d822ade 100644

> --- a/gdb/ChangeLog

> +++ b/gdb/ChangeLog

> @@ -1,3 +1,8 @@

> +2019-04-16  Tom Tromey  <tromey@adacore.com>

> +

> +	* dwarf2read.c (dwarf2_init_complex_target_type): Check "tt"

> +	against nullptr before use.

> +

>  2019-04-15  Leszek Swirski  <leszeks@google.com>

>  

>  	* amd64-tdep.c (amd64_classify_aggregate): Use cp_pass_by_reference

> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c

> index 0873028e438..16bf2404a21 100644

> --- a/gdb/dwarf2read.c

> +++ b/gdb/dwarf2read.c

> @@ -17566,7 +17566,7 @@ dwarf2_init_complex_target_type (struct dwarf2_cu *cu,

>    /* If the type we found doesn't match the size we were looking for, then

>       pretend we didn't find a type at all, the complex target type we

>       create will then be nameless.  */

> -  if (TYPE_LENGTH (tt) * TARGET_CHAR_BIT != bits)

> +  if (tt != nullptr && TYPE_LENGTH (tt) * TARGET_CHAR_BIT != bits)

>      tt = nullptr;

>  

>    const char *name = (tt == nullptr) ? nullptr : TYPE_NAME (tt);


Thank you, that looks great.

Feel free to push this.

Andrew

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0c39c190475..705bcaff49f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@ 
+2019-03-18  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* NEWS: Mention new internal functions.
+	* dwarf2read.c (dwarf2_init_complex_target_type): New function.
+	(read_base_type): Use dwarf2_init_complex_target_type.
+	* value.c (creal_internal_fn): New function.
+	(cimag_internal_fn): New function.
+	(_initialize_values): Register new internal functions.
+
 2019-03-17  Sergei Trofimovich <siarheit@google.com>
 
 	* unittests/string_view-selftests.c: Define
diff --git a/gdb/NEWS b/gdb/NEWS
index c45b3134064..98ab9fe8f99 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,9 @@ 
 
 *** Changes since GDB 8.3
 
+* Two new convernience functions $_cimag and $_creal that extract the
+  imaginary and real parts respectively from complex numbers.
+
 *** Changes in GDB 8.3
 
 * GDB and GDBserver now support access to additional registers on
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 6b9911c5436..00e2a2e0ed6 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@ 
+2019-03-18  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.texinfo (Convenience Funs): Document '$_creal' and
+	'$_cimag'.
+
 2019-03-14  Simon Marchi  <simon.marchi@efficios.com>
 
 	* gdb.texinfo (GDB/MI Development and Front Ends): Fix closing
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 271739b6cdc..8e7e622e1c5 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -11390,6 +11390,21 @@ 
 Visiting node of type NODE_INTEGER
 @end smallexample
 
+@item $_cimag(@var{value})
+@findex $_cimag@r{, convenience function}
+Return the imaginary part of a complex number @var{value}.
+
+The type of the imaginary part depends on the type of the complex
+number, a @code{float complex} will return an imaginary part of type
+@code{float}.
+
+@item $_creal(@var{value})
+@findex $_creal@r{, convenience function}
+Return the real part of a complex number @var{value}.
+
+The type of the real part depends on the type of the complex number, a
+@code{float complex} will return an real part of type @code{float}.
+
 @end table
 
 @value{GDBN} provides the ability to list and get help on
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 7c07a10e8c0..d865ff1a2b2 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -17530,6 +17530,40 @@  dwarf2_init_integer_type (struct dwarf2_cu *cu, struct objfile *objfile,
   return type;
 }
 
+/* Initialise and return a floating point type of size BITS suitable for
+   use as a component of a complex number.  The NAME_HINT is passed through
+   when initialising the floating point type and is the name of the complex
+   type.
+
+   As DWARF doesn't currently provide an explicit name for the components
+   of a complex number, but it can be helpful to have these components
+   named, we try to select a suitable name based on the size of the
+   component.  */
+static struct type *
+dwarf2_init_complex_target_type (struct dwarf2_cu *cu,
+				 struct objfile *objfile,
+				 int bits, const char *name_hint)
+{
+  gdbarch *gdbarch = get_objfile_arch (objfile);
+  struct type *tt = nullptr;
+
+  switch (bits)
+    {
+    case 32:
+      tt = builtin_type (gdbarch)->builtin_float;
+      break;
+    case 64:
+      tt = builtin_type (gdbarch)->builtin_double;
+      break;
+    case 128:
+      tt = builtin_type (gdbarch)->builtin_long_double;
+      break;
+    }
+
+  const char *name = (tt == nullptr) ? nullptr : TYPE_NAME (tt);
+  return dwarf2_init_float_type (objfile, bits, name, name_hint);
+}
+
 /* Find a representation of a given base type and install
    it in the TYPE field of the die.  */
 
@@ -17569,7 +17603,7 @@  read_base_type (struct die_info *die, struct dwarf2_cu *cu)
 	type = init_boolean_type (objfile, bits, 1, name);
 	break;
       case DW_ATE_complex_float:
-	type = dwarf2_init_float_type (objfile, bits / 2, NULL, name);
+	type = dwarf2_init_complex_target_type (cu, objfile, bits / 2, name);
 	type = init_complex_type (objfile, name, type);
 	break;
       case DW_ATE_decimal_float:
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 523013bf9b2..b471f374271 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@ 
+2019-03-18  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.base/complex-parts.c: New file.
+	* gdb.base/complex-parts.exp: New file.
+
 2019-03-14  Tom Tromey  <tromey@adacore.com>
 
 	* gdb.base/style.exp: Add "set style sources" test.
diff --git a/gdb/testsuite/gdb.base/complex-parts.c b/gdb/testsuite/gdb.base/complex-parts.c
new file mode 100644
index 00000000000..243caee0695
--- /dev/null
+++ b/gdb/testsuite/gdb.base/complex-parts.c
@@ -0,0 +1,50 @@ 
+/* Copyright 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+/* Unlike the other 'complex.c' test, this one uses the "standard" header
+   file to pull in the complex types.  The testing is around printing the
+   complex numbers, and using the convenience function $_cimag and $_creal
+   to extract the parts of the complex numbers.  */
+
+#include <complex.h>
+
+void
+keep_around (volatile void *ptr)
+{
+  asm ("" ::: "memory");
+}
+
+int
+main (void)
+{
+  double complex z1 = 1.5 + 4.5 * I;
+  float complex z2 = 2.5 - 5.5 * I;
+  long double complex z3 = 3.5 + 6.5 * I;
+
+  double d1 = 1.5;
+  float f1 = 2.5;
+  int i1 = 3;
+
+  keep_around (&z1);
+  keep_around (&z2);
+  keep_around (&z3);
+  keep_around (&d1);
+  keep_around (&f1);
+  keep_around (&i1);
+
+  return 0;	/* Break Here.  */
+}
diff --git a/gdb/testsuite/gdb.base/complex-parts.exp b/gdb/testsuite/gdb.base/complex-parts.exp
new file mode 100644
index 00000000000..ce8f4277a8b
--- /dev/null
+++ b/gdb/testsuite/gdb.base/complex-parts.exp
@@ -0,0 +1,62 @@ 
+# Copyright 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+if { ![runto_main] } then {
+    fail "can't run to main"
+    return 0
+}
+
+gdb_breakpoint [gdb_get_line_number "Break Here"]
+gdb_continue_to_breakpoint "breakpt" ".* Break Here\\. .*"
+
+gdb_test "p z1" " = 1.5 \\+ 4.5 \\* I"
+gdb_test "p z2" " = 2.5 \\+ -5.5 \\* I"
+gdb_test "p z3" " = 3.5 \\+ 6.5 \\* I"
+
+gdb_test "ptype z1" " = complex double"
+gdb_test "ptype z2" " = complex float"
+gdb_test "ptype z3" " = complex long double"
+
+gdb_test "p \$_cimag (z1)" " = 4.5"
+gdb_test "ptype \$" " = double"
+
+gdb_test "p \$_cimag (z2)" " = -5.5"
+gdb_test "ptype \$" " = float"
+
+gdb_test "p \$_cimag (z3)" " = 6.5"
+gdb_test "ptype \$" " = long double"
+
+gdb_test "p \$_creal (z1)" " = 1.5"
+gdb_test "ptype \$" " = double"
+
+gdb_test "p \$_creal (z2)" " = 2.5"
+gdb_test "ptype \$" " = float"
+
+gdb_test "p \$_creal (z3)" " = 3.5"
+gdb_test "ptype \$" " = long double"
+
+gdb_test "p \$_cimag (d1)" "expected a complex number"
+gdb_test "p \$_cimag (f1)" "expected a complex number"
+gdb_test "p \$_cimag (i1)" "expected a complex number"
+
+gdb_test "p \$_creal (d1)" "expected a complex number"
+gdb_test "p \$_creal (f1)" "expected a complex number"
+gdb_test "p \$_creal (i1)" "expected a complex number"
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index ece1428e617..bdae0d471e7 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -601,6 +601,8 @@  set show_conv_list \
 	{$_probe_arg9 = <error: No frame selected>} \
 	{$_probe_arg10 = <error: No frame selected>} \
 	{$_probe_arg11 = <error: No frame selected>} \
+	{$_cimag = <internal function _cimag>} \
+	{$_creal = <internal function _creal>} \
 	{$_isvoid = <internal function _isvoid>} \
     }
 if ![skip_python_tests] {
diff --git a/gdb/value.c b/gdb/value.c
index dc297dfe0f9..eb48094dbe7 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3933,6 +3933,44 @@  isvoid_internal_fn (struct gdbarch *gdbarch,
   return value_from_longest (builtin_type (gdbarch)->builtin_int, ret);
 }
 
+/* Implementation of the convenience function $_cimag.  Extracts the
+   real part from a complex number.  */
+
+static struct value *
+creal_internal_fn (struct gdbarch *gdbarch,
+		   const struct language_defn *language,
+		   void *cookie, int argc, struct value **argv)
+{
+  if (argc != 1)
+    error (_("You must provide one argument for $_real_from_complex."));
+
+  value *cval = argv[0];
+  type *ctype = check_typedef (value_type (cval));
+  if (TYPE_CODE (ctype) != TYPE_CODE_COMPLEX)
+    error (_("expected a complex number"));
+  return value_from_component (cval, TYPE_TARGET_TYPE (ctype), 0);
+}
+
+/* Implementation of the convenience function $_cimag.  Extracts the
+   imaginary part from a complex number.  */
+
+static struct value *
+cimag_internal_fn (struct gdbarch *gdbarch,
+		   const struct language_defn *language,
+		   void *cookie, int argc,
+		   struct value **argv)
+{
+  if (argc != 1)
+    error (_("You must provide one argument for $_real_from_complex."));
+
+  value *cval = argv[0];
+  type *ctype = check_typedef (value_type (cval));
+  if (TYPE_CODE (ctype) != TYPE_CODE_COMPLEX)
+    error (_("expected a complex number"));
+  return value_from_component (cval, TYPE_TARGET_TYPE (ctype),
+			       TYPE_LENGTH (TYPE_TARGET_TYPE (ctype)));
+}
+
 #if GDB_SELF_TEST
 namespace selftests
 {
@@ -4114,6 +4152,20 @@  Usage: $_isvoid (expression)\n\
 Return 1 if the expression is void, zero otherwise."),
 			 isvoid_internal_fn, NULL);
 
+  add_internal_function ("_creal", _("\
+Extract the real part of a complex number.\n\
+Usage: $_real_from_complex (expression)\n\
+Return the real part of a complex number, the type depends on the\n\
+type of a complex number."),
+			 creal_internal_fn, NULL);
+
+  add_internal_function ("_cimag", _("\
+Extract the imaginary part of a complex number.\n\
+Usage: $_imaginary_from_complex (expression)\n\
+Return the imaginary part of a complex number, the type depends on the\n\
+type of a complex number."),
+			 cimag_internal_fn, NULL);
+
   add_setshow_zuinteger_unlimited_cmd ("max-value-size",
 				       class_support, &max_value_size, _("\
 Set maximum sized value gdb will load from the inferior."), _("\