diff mbox series

[RFC] Return intptr when dereferencing integer

Message ID 20230726215205.3995028-1-rnovds@gmail.com
State New
Headers
Series [RFC] Return intptr when dereferencing integer |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 fail Testing failed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Revunov Denis July 26, 2023, 9:52 p.m. UTC 
  The code which makes default dereference type an int was intruduced in
gdb-3.4(1c997a4ae869) in 1989, 10 years before AMD64 was released. With
current adoption of 64-bit it makes sense to change default dereference
size to match the native address size, which will enable us to easily
inspect .got entries and follow chains of pointers on 64-bit systems
without excessive casting. This change won't affect 32-and-less bit
systems since intptr is the same as int there.
---
 gdb/eval.c     | 7 +++----
 gdb/gdbtypes.c | 3 +++
 gdb/gdbtypes.h | 1 +
 3 files changed, 7 insertions(+), 4 deletions(-)

Comments

Tom Tromey July 27, 2023, 4:15 p.m. UTC | #1 
>>>>> "Denis" == Denis Revunov via Gdb-patches <gdb-patches@sourceware.org> writes:

Denis> The code which makes default dereference type an int was intruduced in
Denis> gdb-3.4(1c997a4ae869) in 1989, 10 years before AMD64 was released. With
Denis> current adoption of 64-bit it makes sense to change default dereference
Denis> size to match the native address size, which will enable us to easily
Denis> inspect .got entries and follow chains of pointers on 64-bit systems
Denis> without excessive casting. This change won't affect 32-and-less bit
Denis> systems since intptr is the same as int there.

Does this impact any test?
If not, can a test be written for it?

I guess from context the scenario is something like: "print *23",
and here gdb acts like you typed "print *(int*)23"?

Tom
Revunov Denis July 29, 2023, 12:14 a.m. UTC | #2 
>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
writes:

Tom> Does this impact any test?

I cannot be 100% sure because i get slightly different test results on
consecutive runs even without any changes, but it seems the only failures
appearing in gdb.sum diff (with/without the patch) are timeouts and some
server stuff, and there's like 2-5 of them. So we're probably fine.

Tom> If not, can a test be written for it?

Sure, see a diff in the end.

Tom> I guess from context the scenario is something like: "print *23",
Tom> and here gdb acts like you typed "print *(int*)23"?

That's right, and my point is to make it act like "print *(intptr_t*)23".
Because int is intptr_t everywhere except 64-bit.

Here's a diff for the test:

---
 gdb/testsuite/gdb.base/pointers.c   | 5 ++++-
 gdb/testsuite/gdb.base/pointers.exp | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/pointers.c
b/gdb/testsuite/gdb.base/pointers.c
index 96f2b52f5e8..d3a9689b46a 100644
--- a/gdb/testsuite/gdb.base/pointers.c
+++ b/gdb/testsuite/gdb.base/pointers.c
@@ -1,4 +1,4 @@
-
+#include <stdint.h>
 #if !defined (__STDC__) && !defined (_AIX)
 #define signed  /**/
 #endif
@@ -71,6 +71,8 @@ float ** ptr_to_ptr_to_float;

 int y;

+const intptr_t intptr_max = INTPTR_MAX;
+intptr_t intptr_p;

 typedef long k[5];

@@ -170,6 +172,7 @@ void dummy()

   v_float_array[0] = v_float;
   v_double_array[0] = v_double;
+  intptr_p = (intptr_t)&intptr_max;

 }

diff --git a/gdb/testsuite/gdb.base/pointers.exp
b/gdb/testsuite/gdb.base/pointers.exp
index d72fa5ad613..5787b873cca 100644
--- a/gdb/testsuite/gdb.base/pointers.exp
+++ b/gdb/testsuite/gdb.base/pointers.exp
@@ -202,6 +202,10 @@ gdb_test "print *( *(matrix+1) +2)" " = 5" \
 gdb_test "print **ptr_to_ptr_to_float" " = 100" \
     "print through ptr to ptr"

+# when we attempt to dereference an integer,
+# it should be treated as a pointer to intptr_t.
+gdb_test "print *intptr_p == intptr_max" " = 1" "deref size is intptr"
+
 # tests for pointers
 # with elementary type variables and pointers.
 #
Revunov Denis Aug. 13, 2023, 4:35 p.m. UTC | #3 
Ping
diff mbox series

Patch

diff --git a/gdb/eval.c b/gdb/eval.c
index 457a6697923..23568d8ed3c 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -1672,11 +1672,10 @@  eval_op_ind (struct type *expect_type, struct expression *exp,
     }
 
   /* Allow * on an integer so we can cast it to whatever we want.
-     This returns an int, which seems like the most C-like thing to
-     do.  "long long" variables are rare enough that
-     BUILTIN_TYPE_LONGEST would seem to be a mistake.  */
+     This returns an intptr, which has the same size as a native
+     address and thus can be treated as a pointer on any machine. */
   if (type->code () == TYPE_CODE_INT)
-    return value_at_lazy (builtin_type (exp->gdbarch)->builtin_int,
+    return value_at_lazy (builtin_type (exp->gdbarch)->builtin_intptr,
 			  (CORE_ADDR) value_as_address (arg1));
   return value_ind (arg1);
 }
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index c5272979cb9..9deacd94ce2 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -6026,6 +6026,9 @@  create_gdbtypes_data (struct gdbarch *gdbarch)
     = init_integer_type (alloc, 128, 0, "int128_t");
   builtin_type->builtin_uint128
     = init_integer_type (alloc, 128, 1, "uint128_t");
+  builtin_type->builtin_intptr
+    = init_integer_type (alloc, gdbarch_addr_bit (gdbarch), 0,
+			 "intptr_t");
 
   builtin_type->builtin_int8->set_instance_flags
     (builtin_type->builtin_int8->instance_flags ()
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index aedaf53cd5d..222f92815a9 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -2079,6 +2079,7 @@  struct builtin_type
   struct type *builtin_uint64 = nullptr;
   struct type *builtin_int128 = nullptr;
   struct type *builtin_uint128 = nullptr;
+  struct type *builtin_intptr = nullptr;
 
   /* Wide character types.  */
   struct type *builtin_char16 = nullptr;