Trivial bug in valarith.c

Trivial bug in valarith.c

[Jim Ingham]

Hi, all...

There's a trivial bug in valarith.c, in value_sub.   Here is the patch:

Index: valarith.c
===================================================================
RCS file: /cvs/src/src/gdb/valarith.c,v
retrieving revision 1.9
diff -c -w -r1.9 valarith.c
*** valarith.c  2001/09/24 17:16:53     1.9
--- valarith.c  2001/10/13 05:03:06
***************
*** 104,110 ****
         {
           /* pointer - integer.  */
           LONGEST sz = TYPE_LENGTH (check_typedef (TYPE_TARGET_TYPE 
(type1)));
!         return value_from_pointer (VALUE_TYPE (arg1),
                                      (value_as_pointer (arg1)
                                       - (sz * value_as_long (arg2))));
         }
--- 104,110 ----
         {
           /* pointer - integer.  */
           LONGEST sz = TYPE_LENGTH (check_typedef (TYPE_TARGET_TYPE 
(type1)));
!         return value_from_pointer (type1,
                                      (value_as_pointer (arg1)
                                       - (sz * value_as_long (arg2))));
         }

You need to pass the type that has passed through check_typedef (that's 
what type1 is) rather than VALUE_TYPE(arg1) or you might pass a typedef 
to value_from_pointer, which will then issue a scary internal error 
warning.  Here is an example that will show this:

$cat test.c
typedef char * charPtr;

main () {
   charPtr myPtr = (charPtr) "some data";

     printf("Stupid thing to do, huh?\n");

}
$ cc -g -o test test.c
$ gdb test
GNU gdb 5.0-20001113 (Apple version gdb-200) (Mon Sep  3 02:43:52 GMT 
2001) (UI_OUT)
Copyright 2000 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you 
are
welcome to change it and/or distribute copies of it under certain 
conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for 
details.
This GDB was configured as "powerpc-apple-macos10".
Reading symbols for shared libraries .. done
(gdb) break main
Breakpoint 1 at 0x1e80: file test.c, line 4.
(gdb) run
Starting program: /tmp/test
[Switching to thread 1 (process 5994 thread 0x1603)]

Breakpoint 1, main () at test.c:4
4         charPtr myPtr = (charPtr) "some data";
(gdb) n
6           printf("Stupid thing to do, huh?\n");
(gdb) x/4x myPtr-4
gdb-internal-error: findvar.c (store_typed_address): type is not a 
pointer or reference

Jim
_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-
Jim Ingham                                                           
jingham@apple.com
Developer Tools - gdb

Re: Trivial bug in valarith.c

[Andrew Cagney]

JimB,

While this looks, er obvious, can you give it a quick peek (I think you 
last touched that code ;-).  Looks like this problem is also on the 5.1 
branch :-/

JimI,

Do you have the ChangeLog entry?  Any chance of a testcase?  BTW, I guess:
	(gdb) x/b myPtr+1
also causes the crash - that is slightly more valid.

enjoy,
Andrew


> Hi, all...
> 
> There's a trivial bug in valarith.c, in value_sub.   Here is the patch:
> 
> Index: valarith.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/valarith.c,v
> retrieving revision 1.9
> diff -c -w -r1.9 valarith.c
> *** valarith.c  2001/09/24 17:16:53     1.9
> --- valarith.c  2001/10/13 05:03:06
> ***************
> *** 104,110 ****
>         {
>           /* pointer - integer.  */
>           LONGEST sz = TYPE_LENGTH (check_typedef (TYPE_TARGET_TYPE (type1)));
> !         return value_from_pointer (VALUE_TYPE (arg1),
>                                      (value_as_pointer (arg1)
>                                       - (sz * value_as_long (arg2))));
>         }
> --- 104,110 ----
>         {
>           /* pointer - integer.  */
>           LONGEST sz = TYPE_LENGTH (check_typedef (TYPE_TARGET_TYPE (type1)));
> !         return value_from_pointer (type1,
>                                      (value_as_pointer (arg1)
>                                       - (sz * value_as_long (arg2))));
>         }

Re: Trivial bug in valarith.c

[Jim Ingham]

Andrew,

> JimB,
>
> While this looks, er obvious, can you give it a quick peek (I think you 
> last touched that code ;-).  Looks like this problem is also on the 5.1 
> branch :-/

cvs tattle points the finger at Jim...  He changed value_from_longest - 
which presumably doesn't care whether it is handed a pointer or a 
typedef, to value_from_pointer which does.  That change is correct, you 
just have to be careful about the arguments.

Another possible way to fix this, BTW, is to have value_from_pointer 
call check_typedef before whinging about the type.  Since check_typedef 
can sometimes take a bit of work, however, I thought it was better to 
keep the contract that you have to pass value_from_pointer a real 
pointer, and let the callers do the work - which in most cases they will 
have done already anyway.

>
> JimI,
>
> Do you have the ChangeLog entry?  Any chance of a testcase?

Oops, forgot the ChangeLog.  Here it is:

2001-10-12  Jim Ingham  <jingham@inghji.apple.com>

         * valarith.c (value_sub): Don't pass a raw type to
         value_from_pointer, it has to go through check_typedef first.

If I don't get distracted by some other emergency, I will whip up a 
testcase on Monday.

> BTW, I guess:
> 	(gdb) x/b myPtr+1
> also causes the crash - that is slightly more valid.

Yes, that will cause the problem too.  Anything that calls value_sub to 
handle a pointer +- integer where the pointer's type is a typedef 
instead of a direct pointer will fall over - it doesn't matter how you 
format the output.
The original case I was sent was a pointer to some opaque Foundation 
type, and I guess x/x made sense in that case, I have no idea.  I just 
changed the example to something that would not require GnuStep or MacOS 
X...

Jim

>
> enjoy,
> Andrew
>
>
>> Hi, all...
>> There's a trivial bug in valarith.c, in value_sub.   Here is the patch:
>> Index: valarith.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/valarith.c,v
>> retrieving revision 1.9
>> diff -c -w -r1.9 valarith.c
>> *** valarith.c  2001/09/24 17:16:53     1.9
>> --- valarith.c  2001/10/13 05:03:06
>> ***************
>> *** 104,110 ****
>>         {
>>           /* pointer - integer.  */
>>           LONGEST sz = TYPE_LENGTH (check_typedef (TYPE_TARGET_TYPE 
>> (type1)));
>> !         return value_from_pointer (VALUE_TYPE (arg1),
>>                                      (value_as_pointer (arg1)
>>                                       - (sz * value_as_long (arg2))));
>>         }
>> --- 104,110 ----
>>         {
>>           /* pointer - integer.  */
>>           LONGEST sz = TYPE_LENGTH (check_typedef (TYPE_TARGET_TYPE 
>> (type1)));
>> !         return value_from_pointer (type1,
>>                                      (value_as_pointer (arg1)
>>                                       - (sz * value_as_long (arg2))));
>>         }
>
>
>
_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-
Jim Ingham                                                           
jingham@apple.com
Developer Tools - gdb

Re: Trivial bug in valarith.c

[Andrew Cagney]

> Jim Ingham <jingham@apple.com> writes:
> 
>> Andrew,
> 
>> > JimB,
>> >
>> > While this looks, er obvious, can you give it a quick peek (I think you 
>> > last touched that code  [;-)] .  Looks like this problem is also on the 5.1 
>> > branch :-/
> 
>> 
>> cvs tattle points the finger at Jim...  He changed value_from_longest - 
>> which presumably doesn't care whether it is handed a pointer or a 
>> typedef, to value_from_pointer which does.  That change is correct, you 
>> just have to be careful about the arguments.
> 
> 
> I think Jim's change is correct.


I've checked it in (I've another change that this one affects sitting in 
my pending queue).

Andrew