(len % 1) != 0

Hello,

In gdb/gdbserver/server.c,

there is a line:

if ((len % 1) != 0 || unhexify (mon, own_buf + 6, len / 2)
!= len / 2)

I am curious that (len % 1) != 0 is always false, why put
here?

Thanks.
Bin

Hi Daniel,

Is it a bug?

On Sun, Feb 17, 2008 at 10:12 PM, Eli Zaretskii <elizgnu.org> wrote:
> > Date: Sun, 17 Feb 2008 13:57:31 +0800
>  > From: "Bin Chen" <binary.chengmail.com>
>
>
> >
>  > Hello,
>  >
>  > In gdb/gdbserver/server.c,
>  >
>  > there is a line:
>  >
>  > if ((len % 1) != 0 || unhexify (mon, own_buf + 6,
len / 2) != len / 2)
>  >
>  > I am curious that (len % 1) != 0 is always false,
why put here?
>
>  It's probably a bug: they meant len % 2.
>

On Sun, Feb 17, 2008 at 10:41:36PM +0800, Bin Chen wrote:
> Hi Daniel,
> 
> Is it a bug?

Yes.  Fixed as below.

-- 
Daniel Jacobowitz
CodeSourcery

2008-02-17  Daniel Jacobowitz  <dancodesourcery.com>

	* server.c (handle_query): Correct length check.

Index: server.c
============================================================
=======
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.64
diff -u -p -r1.64 server.c
--- server.c	14 Feb 2008 16:42:55 -0000	1.64
+++ server.c	17 Feb 2008 15:25:22 -0000
 -649,7
+649,7  handle_query (char *own_buf, int packet_
       char *mon = malloc (PBUFSIZ);
       int len = strlen (own_buf + 6);
 
-      if ((len % 1) != 0 || unhexify (mon, own_buf + 6, len
/ 2) != len / 2)
+      if ((len % 2) != 0 || unhexify (mon, own_buf + 6, len
/ 2) != len / 2)
 	{
 	  write_enn (own_buf);
 	  free (mon);

On 19 February 2008 16:30, Sheng-Liang Song wrote:

> or
> 
>  (len  & 1) != 0   <=>  (len % 2) != 0
> 

  That would have the advantage of not requiring a divide
operation 

    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....

or

 (len  & 1) != 0   <=>  (len % 2) != 0


Daniel Jacobowitz wrote:
> On Sun, Feb 17, 2008 at 10:41:36PM +0800, Bin Chen
wrote:
>   
>> Hi Daniel,
>>
>> Is it a bug?
>>     
>
> Yes.  Fixed as below.
>
>   

Dave Korn wrote:
> On 19 February 2008 16:30, Sheng-Liang Song wrote:
> 
>> or
>>
>>  (len  & 1) != 0   <=>  (len % 2) != 0
>>
> 
>   That would have the advantage of not requiring a
divide operation 
> 

But GCC will fold it to the bitwise and form, so it doesn't
really matter.

David Daney

On 19 February 2008 18:35, David Daney wrote:

> Dave Korn wrote:
>> On 19 February 2008 18:23, Paul Koning wrote:
>> 
>>>>>>>> "Dave" == Dave
Korn <dave.kornartimi.com> writes:
>>>  Dave> On 19 February 2008 16:30,
Sheng-Liang Song wrote:  >> or
>>>  >>
>>>  >> (len & 1) != 0 <=> (len %
2) != 0
>>>  >>
>>> 
>>>  Dave> That would have the advantage of not
requiring a divide  Dave>
>>> operation  
>>> 
>>> It shouldn't matter -- the optimizer will do
the right thing, as I
>>> recall.
>> 
>>   NB len == signed int.
>> 
>>   Yes, it can simplify it to a bunch of shifts and
sign extends and masks
>> without using an explicit divide, but it's not as
good as a simple AND
>> operation.  (Maybe VRP in 4.x could handle that by
knowing that the result
>> of strlen has to be >= 0, but 3.x series won't
do it).
>> 
> 
> ??  Not to be pedantic, but on 3.4.3 for mipsel-linux I
get:
> 
> $ cat j.c
> int f1 (int a)
> {
>      return (a % 2) != 0;
> }
> $ mipsel-linux-gcc -c -O3 j.c
> $ mipsel-linux-objdump -d j.o
> 
> j.o:     file format elf32-tradlittlemips
> 
> Disassembly of section .text:
> 
> 00000000 <f1>:
>     0:   03e00008        jr      ra
>     4:   30820001        andi    v0,a0,0x1

   <lightbulb>   Ah, it's the !=0 that allows the
compiler to go that last
step of the way, I was just testing 


int bar (int len)
{
  return len % 2;
}

int baz (int len)
{
  return len & 1;
}


  Hmm, 3.4.4 on x86 still doesn't optimise it quite as
well:

/artimi/boards $ gcc -xc -S -oo.s -O2 -


int bar (int len)
{
  return len % 2;
}

int baz (int len)
{
  return len & 1;
}

int bat (int len)
{
  return (len % 2) != 0;
}

int quux (int len)
{
  return (len & 1) != 0;
}

/artimi/boards $ cat o.s
        .file   ""
        .text
.globl _bar
        .def    _bar;   .scl    2;      .type   32;    
.endef
_bar:
        pushl   %ebp
        movl    %esp, %ebp
        movl    8(%ebp), %eax
        popl    %ebp
        movl    %eax, %edx
        shrl    $31, %edx
        leal    (%eax,%edx), %edx
        andl    $-2, %edx
        subl    %edx, %eax
        ret
        .p2align 4,,15
.globl _baz
        .def    _baz;   .scl    2;      .type   32;    
.endef
_baz:
        pushl   %ebp
        movl    %esp, %ebp
        movl    8(%ebp), %eax
        popl    %ebp
        andl    $1, %eax
        ret
        .p2align 4,,15
.globl _bat
        .def    _bat;   .scl    2;      .type   32;    
.endef
_bat:
        pushl   %ebp
        xorl    %eax, %eax
        movl    %esp, %ebp
        testb   $1, 8(%ebp)
        popl    %ebp
        setne   %al
        ret
        .p2align 4,,15
.globl _quux
        .def    _quux;  .scl    2;      .type   32;    
.endef
_quux:
        pushl   %ebp
        movl    %esp, %ebp
        movl    8(%ebp), %eax
        popl    %ebp
        andl    $1, %eax
        ret

/artimi/boards/Kitsman/Test Boards/A200_TPCB002 $

    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....

Dave Korn wrote:
> On 19 February 2008 18:23, Paul Koning wrote:
> 
>>>>>>> "Dave" == Dave Korn
<dave.kornartimi.com> writes:
>>  Dave> On 19 February 2008 16:30, Sheng-Liang
Song wrote:
>>  >> or
>>  >>
>>  >> (len & 1) != 0 <=> (len % 2) !=
0
>>  >>
>>
>>  Dave> That would have the advantage of not
requiring a divide
>>  Dave> operation 
>>
>> It shouldn't matter -- the optimizer will do the
right thing, as I
>> recall.
> 
>   NB len == signed int.
> 
>   Yes, it can simplify it to a bunch of shifts and sign
extends and masks
> without using an explicit divide, but it's not as good
as a simple AND
> operation.  (Maybe VRP in 4.x could handle that by
knowing that the result of
> strlen has to be >= 0, but 3.x series won't do it).
> 

??  Not to be pedantic, but on 3.4.3 for mipsel-linux I
get:

$ cat j.c
int f1 (int a)
{
     return (a % 2) != 0;
}
$ mipsel-linux-gcc -c -O3 j.c
$ mipsel-linux-objdump -d j.o

j.o:     file format elf32-tradlittlemips

Disassembly of section .text:

00000000 <f1>:
    0:   03e00008        jr      ra
    4:   30820001        andi    v0,a0,0x1

"Dave Korn" <dave.kornartimi.com> writes:

> On 19 February 2008 16:30, Sheng-Liang Song wrote:
>
>> or
>> 
>>  (len  & 1) != 0   <=>  (len % 2) != 0
>> 
>
>   That would have the advantage of not requiring a
divide operation 

The divide operation is already required and can be combined
with the
modulo operation.

And this is by no means performance critical.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwabsuse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg,
Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5
214B 8276 4ED5
"Premature optimization is the root of all evil."

On 19 February 2008 18:23, Paul Koning wrote:

>>>>>> "Dave" == Dave Korn
<dave.kornartimi.com> writes:
> 
>  Dave> On 19 February 2008 16:30, Sheng-Liang Song
wrote:
>  >> or
>  >>
>  >> (len & 1) != 0 <=> (len % 2) != 0
>  >>
> 
>  Dave> That would have the advantage of not
requiring a divide
>  Dave> operation 
> 
> It shouldn't matter -- the optimizer will do the right
thing, as I
> recall.

  NB len == signed int.

  Yes, it can simplify it to a bunch of shifts and sign
extends and masks
without using an explicit divide, but it's not as good as a
simple AND
operation.  (Maybe VRP in 4.x could handle that by knowing
that the result of
strlen has to be >= 0, but 3.x series won't do it).


    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....