Re: [gdb-7.0 release] 2009-09-02 status and proposed plan

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> Date: Fri, 4 Sep 2009 17:25:20 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> 
> Hui,
> 
> > I had post a patch for it.  It still in discussion.
> > http://sourceware.org/ml/gdb-patches/2009-08/msg00574.html
> 
> As far as I can tell, you received some feedback from one of the
> maintainers (Mark Kettenis) about your patch:
> 
>     http://sourceware.org/ml/gdb-patches/2009-08/msg00584.html
> 
> I also looked at your patch before looking at the replies, and I had
> the same comments as Jiang and Mark. The casts in this raised a red
> flag, and I don't see why we should need them.
> 
> Would it make sense to define a type syscall_t that's either an int
> or an unsigned int, and use that consistently throughout?  Otherwise,
> another simpler option would be to just use either int or unsigned int
> without using a typedef.

I'm not a big fan of typedefs like this.  They hide the signedness of
the type, which makes it more likely we'll end up with signed ->
unsigned conversions again or messed up range checks.

Unless people are aware of an operating system that uses negative
numbers for system calls, I don't think it matters very much whether
we use a signed or an unsigned type.  However, we have to pick one and
use it consistently.  We fail to do that for line numbers, and as a
result we still have bugs in our code.

> In the meantime, I think you can get away from this all by using
> regcache_raw_write_signed. Read the syscall ID as a signed number,
> all should be fine. I'm attaching a patch that fixes the build
> issue an illustrates this suggestion. Can you please give it a
> test and resubmit if it works for you?

This would be almost ok, but you'll need a check that syscall_num
isn't < 0 as well.

> --GvXjxJ+pjyke8COw
> Content-Type: text/x-diff; charset=us-ascii
> Content-Disposition: attachment; filename="syscall-cygwin-64bit.diff"
> 
> Index: i386-linux-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/i386-linux-tdep.c,v
> retrieving revision 1.66
> diff -u -p -r1.66 i386-linux-tdep.c
> --- i386-linux-tdep.c	10 Aug 2009 03:04:44 -0000	1.66
> +++ i386-linux-tdep.c	5 Sep 2009 00:24:25 -0000
> @@ -367,18 +367,19 @@ static int
>  i386_linux_intx80_sysenter_record (struct regcache *regcache)
>  {
>    int ret;
> -  uint32_t tmpu32;
> +  LONGEST syscall_num;
>  
> -  regcache_raw_read (regcache, I386_EAX_REGNUM, (gdb_byte *) &tmpu32);
> +  regcache_raw_read_signed (regcache, I386_EAX_REGNUM, &syscall_num);
>  
> -  if (tmpu32 > 499)
> +  if (syscall_num > 499)
>      {
> -      printf_unfiltered (_("Process record and replay target doesn't "
> -                           "support syscall number %u\n"), tmpu32);
> +      printf_unfiltered (_("Process record and replay target does not "
> +                           "support syscall number %s\n"),
> +                         plongest (syscall_num));
>        return -1;
>      }
>  
> -  ret = record_linux_system_call (tmpu32, regcache,
> +  ret = record_linux_system_call (syscall_num, regcache,
>  				  &i386_linux_record_tdep);
>    if (ret)
>      return ret;
> 
> --GvXjxJ+pjyke8COw--
>