public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: "Sharma, Alok Kumar" <AlokKumar.Sharma@amd.com>
To: Andrew Burgess <andrew.burgess@embecosm.com>,
	"gdb-patches@sourceware.org"	<gdb-patches@sourceware.org>
Subject: RE: [PATCHv2] gdb/fortran: Fix printing of logical true values for Flang
Date: Tue, 03 Mar 2020 04:47:00 -0000	[thread overview]
Message-ID: <DM6PR12MB312910FCE9AF38277CD111699EE40@DM6PR12MB3129.namprd12.prod.outlook.com> (raw)
In-Reply-To: <20200302182152.12819-1-andrew.burgess@embecosm.com>

Hi Andrew,

I strongly agree and accept your comment. Please let me know if patch need to be updated by me? In case you have already incorporated the comments, please push it.

Regards,
Alok

-----Original Message-----
From: Andrew Burgess <andrew.burgess@embecosm.com> 
Sent: Monday, March 2, 2020 11:52 PM
To: gdb-patches@sourceware.org
Cc: Sharma, Alok Kumar <AlokKumar.Sharma@amd.com>; Andrew Burgess <andrew.burgess@embecosm.com>
Subject: [PATCHv2] gdb/fortran: Fix printing of logical true values for Flang

[CAUTION: External Email]

Alok,

Thanks for looking into this.  I think that the best solution right now will be to handle TYPE_CODE_BOOL in f_val_print rather than modifying generic_val_print_bool.

The other possibility would be, I think, to add a new field to 'struct language_defn' and use this in generic_val_print_bool instead of comparing the value of current_lanuage directly, however, this isn't the common approach, so I'd prefer to handle this case just like other
TYPE_CODE_* are handled for now.

I know that in places within GDB we do compare the value of current_lanuage to the know language structures, but I'd like to move us away from doing this.

I've gone ahead and added some tests too.

Let me know what you think of this.  If you're happy then I'll go ahead and push this.

Thanks
Andrew

---

GDB is not able to print logical true values for Flang compiler.

Actual result:

  (gdb) p l
  $1 = 4294967295

Expected result:

  (gdb) p l
  $1 = .TRUE.

This is due to GDB expecting representation of true value being 1.
The Fortran standard doesnt specify how LOGICAL types are represented.
Different compilers use different non-zero values to represent logical true. The gfortran compiler uses 1 to represent logical true and the flang compiler uses -1. GDB should accept all the non-zero values as true.

This is achieved by handling TYPE_CODE_BOOL in f_val_print and printing any non-zero value as true.

gdb/ChangeLog:

        * f-valprint.c (f_val_print): Handle TYPE_CODE_BOOL, any non-zero
        value should be printed as true.

gdb/testsuite/ChangeLog:

        * gdb.fortran/logical.exp: Add tests that any non-zero value is
        printed as true.
---
 gdb/ChangeLog                         |  6 ++++++
 gdb/f-valprint.c                      | 25 ++++++++++++++++++++++++-
 gdb/testsuite/ChangeLog               |  5 +++++
 gdb/testsuite/gdb.fortran/logical.exp | 18 ++++++++++++++++++
 4 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/gdb/f-valprint.c b/gdb/f-valprint.c index a25e6147322..0393ddfa8ab 100644
--- a/gdb/f-valprint.c
+++ b/gdb/f-valprint.c
@@ -357,6 +357,30 @@ f_val_print (struct type *type, int embedded_offset,
       fprintf_filtered (stream, " )");
       break;

+    case TYPE_CODE_BOOL:
+      if (options->format || options->output_format)
+       {
+         struct value_print_options opts = *options;
+         opts.format = (options->format ? options->format
+                        : options->output_format);
+         val_print_scalar_formatted (type, embedded_offset,
+                                     original_value, &opts, 0, stream);
+       }
+      else
+       {
+         int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);
+         LONGEST val
+           = unpack_long (type, valaddr + embedded_offset * unit_size);
+         /* The Fortran standard doesn't specify how logical types are
+            represented.  Different compilers use different non zero
+            values to represent logical true.  */
+         if (val == 0)
+           fputs_filtered (f_decorations.false_name, stream);
+         else
+           fputs_filtered (f_decorations.true_name, stream);
+       }
+      break;
+
     case TYPE_CODE_REF:
     case TYPE_CODE_FUNC:
     case TYPE_CODE_FLAGS:
@@ -366,7 +390,6 @@ f_val_print (struct type *type, int embedded_offset,
     case TYPE_CODE_RANGE:
     case TYPE_CODE_UNDEF:
     case TYPE_CODE_COMPLEX:
-    case TYPE_CODE_BOOL:
     case TYPE_CODE_CHAR:
     default:
       generic_val_print (type, embedded_offset, address, diff --git a/gdb/testsuite/gdb.fortran/logical.exp b/gdb/testsuite/gdb.fortran/logical.exp
index f0028159e59..96e6f8f9559 100644
--- a/gdb/testsuite/gdb.fortran/logical.exp
+++ b/gdb/testsuite/gdb.fortran/logical.exp
@@ -33,3 +33,21 @@ gdb_test "p l1" " = \\.TRUE\\."
 gdb_test "p l2" " = \\.TRUE\\."
 gdb_test "p l4" " = \\.TRUE\\."
 gdb_test "p l8" " = \\.TRUE\\."
+
+# Different Fortran compilers use different values for logical true.
+# Check how GDB handles this by modifying the underlying value for our 
+# logical variables and check they still print as true.
+foreach_with_prefix var { l l1 l2 l4 l8 } {
+    set len [get_integer_valueof "sizeof (${var})" "get sizeof ${var}"]
+    set addr [get_hexadecimal_valueof "&l" "get address of ${var}"]
+
+    for { set i 0 } { $i < $len } { incr i } {
+       with_test_prefix "byte $i" {
+           gdb_test_no_output "set *((uint8_t *) ${addr}) = 0xff" \
+               "set contents of byte at offset $i"
+           gdb_test "p l" " = \\.TRUE\\."
+           incr addr
+           set addr [format "0x%x" $addr]
+       }
+    }
+}
--
2.14.5

  reply	other threads:[~2020-03-03  4:47 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-02 17:00 [PATCH] gdb/fortran Fixed " Sharma, Alok Kumar
2020-03-02 18:21 ` [PATCHv2] gdb/fortran: Fix " Andrew Burgess
2020-03-03  4:47   ` Sharma, Alok Kumar [this message]
2020-03-03 18:21     ` Andrew Burgess
2020-03-04  8:48       ` [gdb/testsuite] Fix missing uint8_t in gdb.fortran/logical.exp Tom de Vries
2020-03-11 10:24         ` Andrew Burgess
2020-03-11 12:00           ` Tom de Vries
2020-03-03 16:32   ` [PATCHv2] gdb/fortran: Fix printing of logical true values for Flang Tom Tromey
2020-03-03 17:24     ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR12MB312910FCE9AF38277CD111699EE40@DM6PR12MB3129.namprd12.prod.outlook.com \
    --to=alokkumar.sharma@amd.com \
    --cc=andrew.burgess@embecosm.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).