RFA: fix address in call to val_print

RFA: fix address in call to val_print

[Tom Tromey]

This has been on the Python branch for a while.  Paul wrote it, but
since I ran into it today while doing a merge, I decided to submit it.

val_print_array_elements passes 0 as the address to val_print.
However, this is not correct, and will cause problems if val_print
ever uses this value.

I don't have a test case for this against the trunk.  I would still
like to check it in, as I think it is fairly obviously correct.

Built and regtested on x86-64 (compile farm).
Ok?

Tom

2008-11-20  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* valprint.c (val_print_array_elements): Pass correct
	element address to val_print.

diff --git a/gdb/valprint.c b/gdb/valprint.c
index 5086a70..6bcb2f8 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -1112,8 +1112,8 @@ val_print_array_elements (struct type *type, const gdb_byte *valaddr,
 
       if (reps > options->repeat_count_threshold)
 	{
-	  val_print (elttype, valaddr + i * eltlen, 0, 0, stream,
-		     recurse + 1, options, current_language);
+	  val_print (elttype, valaddr + i * eltlen, 0, address + i * eltlen,
+		     stream, recurse + 1, options, current_language);
 	  annotate_elt_rep (reps);
 	  fprintf_filtered (stream, " <repeats %u times>", reps);
 	  annotate_elt_rep_end ();
@@ -1123,8 +1123,8 @@ val_print_array_elements (struct type *type, const gdb_byte *valaddr,
 	}
       else
 	{
-	  val_print (elttype, valaddr + i * eltlen, 0, 0, stream,
-		     recurse + 1, options, current_language);
+	  val_print (elttype, valaddr + i * eltlen, 0, address + i * eltlen,
+		     stream, recurse + 1, options, current_language);
 	  annotate_elt ();
 	  things_printed++;
 	}

Re: RFA: fix address in call to val_print

[Joel Brobecker]

> 2008-11-20  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
> 	* valprint.c (val_print_array_elements): Pass correct
> 	element address to val_print.

Sure! It looks pretty clearly correct to me as well.

I'm wondering about the testcase. As I understand it, it wouldn't cause
any problem with the current FSF version of GDB. But if it's easy to
create one, then I think it'd still be a useful test to have, just to
make sure it doesn't regress in the future.

-- 
Joel

Re: RFA: fix address in call to val_print

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

>> * valprint.c (val_print_array_elements): Pass correct
>> element address to val_print.

Joel> I'm wondering about the testcase. As I understand it, it wouldn't cause
Joel> any problem with the current FSF version of GDB. But if it's easy to
Joel> create one, then I think it'd still be a useful test to have, just to
Joel> make sure it doesn't regress in the future.

Ok, I will hold off on this patch until I can make a test case.

Tom

Re: RFA: fix address in call to val_print

[Joel Brobecker]

> Ok, I will hold off on this patch until I can make a test case.

No no, don't, that's not what I meant - the patch looked good so let's
put it in now. If you had a testcase, then let's have it now rather than
later, even if it already passed before you applied the patch. If not,
oh well. Sorry, I realize I'm not always clear when I express myself.

-- 
Joel

Re: RFA: fix address in call to val_print

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

>> Ok, I will hold off on this patch until I can make a test case.

Joel> No no, don't, that's not what I meant - the patch looked good so let's
Joel> put it in now. If you had a testcase, then let's have it now rather than
Joel> later, even if it already passed before you applied the patch. If not,
Joel> oh well. Sorry, I realize I'm not always clear when I express myself.

Sorry, I was too brief.

My reasoning was that I do not know whether it is easy to make a test
case or not.  I would have to look into it.

IMO, if it is possible, it makes the most sense to do this before
committing the patch.  Otherwise, I will have to undo the patch to
test it, which is a pain (not so bad in this instance, but in
general).

Tom

Re: RFA: fix address in call to val_print

[Joel Brobecker]

> My reasoning was that I do not know whether it is easy to make a test
> case or not.  I would have to look into it.

Aha, I see.

What I understood from your original patch was that Paul encountered
an actual problem in the Archer GDB, which prompted the patch. The
scenario in which Archer GDB fails before the patch and passes after
was all that I was wondering about. If we can put that example and
stuff it in a testcase now, then let's do it if you have the time.
If it depends on some functionality that's still only in the Archer
repo, then let's defer that until that piece of functionaly is in
the FSF repo.

In the meantime, the code patch is OK to go in (unless you'd prefer
to hold off on it, but I would imagine that the fewer local changes
in the Archer repo, the better).

-- 
Joel

Re: RFA: fix address in call to val_print

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> If it depends on some functionality that's still only in the Archer
Joel> repo, then let's defer that until that piece of functionaly is in
Joel> the FSF repo.

Yeah, the only case I know of depends on other code from the python
branch which is not yet in gdb.  Specifically, when pretty-printing we
reconstruct a "struct value" from the arguments to val_print; in this
case the address being 0 causes problems later when trying to take the
address of an object -- gdb complains that it does not have an
address.

Joel> In the meantime, the code patch is OK to go in (unless you'd prefer
Joel> to hold off on it, but I would imagine that the fewer local changes
Joel> in the Archer repo, the better).

Ok, I will check it in.  Thanks for clarifying this.

And yeah, I'm trying to flush out the various independent bug fixes
and changes from the python branch.  I am trying to set the stage to
submit the pretty-printing feature.

Tom