Re: [RFC] examine for TYPE_CODE_REF in pascal language PR 11349

Re: [RFC] examine for TYPE_CODE_REF in pascal language PR 11349

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre>   -inside x_command, a TYPE_CODE_REF generates a call 
Pierre> to value_ind, but this generates possibly several dereferencing
Pierre> if the target type is also a pointer.

Pierre>   I do not know what is the expectation for C language
Pierre> here, and I don't even know how to generate code that 
Pierre> has variables of type TYPE_CODE_REF from C source.

You need C++ to make a TYPE_CODE_REF.  It corresponds to a C++
reference.

Pierre>   Should this change be extended to other languages
Pierre> or to all languages?

I think coerce_ref is the right thing to do here for C++.  If an
expression evaluates to a reference to a pointer, I would expect the
reference to be ignored and the referred-to pointer to be used instead.

However, the fact that the code is explicitly written to check
TYPE_CODE_REF and use value_ind gives me pause.  If you change it to
unconditionally use coerce_ref, do any tests fail?

Tom

[RFA] examine for TYPE_CODE_REF (PR 11349)

[Pierre Muller]

> Pierre>   -inside x_command, a TYPE_CODE_REF generates a call
> Pierre> to value_ind, but this generates possibly several dereferencing
> Pierre> if the target type is also a pointer.
> 
> Pierre>   I do not know what is the expectation for C language
> Pierre> here, and I don't even know how to generate code that
> Pierre> has variables of type TYPE_CODE_REF from C source.
> 
> You need C++ to make a TYPE_CODE_REF.  It corresponds to a C++
> reference.

  OK, I looked into gdb.cp and found uses in ref-types.exp
 
> Pierre>   Should this change be extended to other languages
> Pierre> or to all languages?
> 
> I think coerce_ref is the right thing to do here for C++.  If an
> expression evaluates to a reference to a pointer, I would expect the
> reference to be ignored and the referred-to pointer to be used instead.

  Ok, so let's try to use coerce_ref for all languages!
 
> However, the fact that the code is explicitly written to check
> TYPE_CODE_REF and use value_ind gives me pause.  If you change it to
> unconditionally use coerce_ref, do any tests fail?
  I first did a simple test without the condition,
nothing special came out.
  After this, I tried to modify the gdb.cp/ref-types.exp
to add a test for this modification.

  Here is a new patch:
the new test fails on current CVS GDB
but passes with the change in printcmd.c below.

Is this patch OK?

Pierre

 
2010-05-03  Pierre Muller  <muller@ics.u-strasbg.fr>

	* printcmd.c (x_command): Only dereference once implicitly for
	TYPE_CODE_REF.

2010-05-03  Pierre Muller  <muller@ics.u-strasbg.fr>

	* testsuite/gdb.cp/ref-types.exp: Add test to examine
	use a reference local variable.

Index: src/gdb/printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.176
diff -u -p -r1.176 printcmd.c
--- src/gdb/printcmd.c	22 Apr 2010 23:15:41 -0000	1.176
+++ src/gdb/printcmd.c	2 May 2010 22:21:02 -0000
@@ -1420,7 +1420,7 @@ x_command (char *exp, int from_tty)
       old_chain = make_cleanup (free_current_contents, &expr);
       val = evaluate_expression (expr);
       if (TYPE_CODE (value_type (val)) == TYPE_CODE_REF)
-	val = value_ind (val);
+	val = coerce_ref (val);
       /* In rvalue contexts, such as this, functions are coerced into
          pointers to functions.  This makes "x/i main" work.  */
       if (/* last_format == 'i'  && */ 
Index: src/gdb/testsuite/gdb.cp/ref-types.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/ref-types.exp,v
retrieving revision 1.12
diff -u -p -r1.12 ref-types.exp
--- src/gdb/testsuite/gdb.cp/ref-types.exp	1 Jan 2010 07:32:01 -0000
1.12
+++ src/gdb/testsuite/gdb.cp/ref-types.exp	2 May 2010 22:21:03 -0000
@@ -200,6 +200,19 @@ gdb_expect {
     timeout           { fail "(timeout) print value of *rps" }
   }
 
+# GDB had a bug about dereferencing a pointer type
+# that would lead to wrong results
+# if we try to examine memory at pointer value.
+
+send_gdb "x /hd rps\n"
+gdb_expect {
+    -re ".* -1.*$gdb_prompt $" {
+        pass "examine value at rps"
+      }
+    -re ".*$gdb_prompt $" { fail "examine value at rps" }
+    timeout           { fail "(timeout) examine value at rps" }
+  }
+
 
 send_gdb "ptype rps\n"
 gdb_expect {

Re: [RFA] examine for TYPE_CODE_REF (PR 11349)

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre> +send_gdb "x /hd rps\n"
Pierre> +gdb_expect {
Pierre> +    -re ".* -1.*$gdb_prompt $" {
Pierre> +        pass "examine value at rps"
Pierre> +      }
Pierre> +    -re ".*$gdb_prompt $" { fail "examine value at rps" }
Pierre> +    timeout           { fail "(timeout) examine value at rps" }
Pierre> +  }
Pierre> +

These days we're trying to use gdb_test rather than send_gdb.
The patch is ok with that change.

Tom

RE: [RFA] examine for TYPE_CODE_REF (PR 11349)

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Tom Tromey
> Envoyé : Monday, May 03, 2010 7:40 PM
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFA] examine for TYPE_CODE_REF (PR 11349)
> 
> >>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr>
> writes:
> 
> Pierre> +send_gdb "x /hd rps\n"
> Pierre> +gdb_expect {
> Pierre> +    -re ".* -1.*$gdb_prompt $" {
> Pierre> +        pass "examine value at rps"
> Pierre> +      }
> Pierre> +    -re ".*$gdb_prompt $" { fail "examine value at rps" }
> Pierre> +    timeout           { fail "(timeout) examine value at rps"
> }
> Pierre> +  }
> Pierre> +
> 
> These days we're trying to use gdb_test rather than send_gdb.
> The patch is ok with that change.

  It's just that all the other tests around this one
where also written using gdb_expect, and I simply copy
and adapted one...

  Thanks for the approval,
patch committed
http://sourceware.org/ml/gdb-cvs/2010-05/msg00026.html

Pierre

PS: I had one point for which I was unsure:
I use '/h' modifier which means 16 bit size,
are there any systems for which C type 'short' is not 16 bit in size?
On such systems, the test might fail ...


For the testsuite, this is how I modified it:
Index: testsuite/gdb.cp/ref-types.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/ref-types.exp,v
retrieving revision 1.12
diff -u -p -r1.12 ref-types.exp
--- testsuite/gdb.cp/ref-types.exp	1 Jan 2010 07:32:01 -0000	1.12
+++ testsuite/gdb.cp/ref-types.exp	4 May 2010 06:47:56 -0000
@@ -200,6 +200,11 @@ gdb_expect {
     timeout           { fail "(timeout) print value of *rps" }
   }
 
+# GDB had a bug about dereferencing a pointer type
+# that would lead to wrong results
+# if we try to examine memory at pointer value.
+
+gdb_test "x /hd rps" "$hex:\[ \t\]*-1" "examine value at rps"
 
 send_gdb "ptype rps\n"
 gdb_expect {

Re: [RFA] examine for TYPE_CODE_REF (PR 11349)

[Joel Brobecker]

> It's just that all the other tests around this one
> where also written using gdb_expect, and I simply copy
> and adapted one...

I guessed that this was the case ;-).  But using send_gdb/gdb_expect
is bad, because the gdb_expect block typically does not handle all known
error situations.  That's why we have gdb_test and gdb_test_multiple.
In the future, you can assume that it is required that new test be
written using gdb_test/gdb_test_multiple, even if the tests already
written around the new one still use send_gdb. Conversions of old uses
of send_gdb/gdb_expect will also be very much appreciated.  In fact,
I'll add that to the list of possible projects in the Wiki - it's an
easy task for someone who does not know GDB but wants to contribute :).

-- 
Joel