GNU bug report logs - #15924
[PATCH] dfa: avoid undefined behavior of "1 << 31"

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 15924 in the body.
You can then email your comments to 15924 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: bug-grep <at> gnu.org
Subject: [PATCH] dfa: avoid undefined behavior of "1 << 31"
Date: Mon, 18 Nov 2013 17:54:56 -0800

[Message part 1 (text/plain, inline)]

FYI:

[k.txt (text/plain, attachment)]

Message #8 received at 15924 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Jim Meyering <jim <at> meyering.net>, 15924 <at> debbugs.gnu.org
Subject: Re: bug#15924: [PATCH] dfa: avoid undefined behavior of "1 << 31"
Date: Mon, 18 Nov 2013 18:16:10 -0800

Jim Meyering wrote:
>  static int
>  tstbit (unsigned int b, charclass const c)
>  {
> -  return c[b / INTBITS] & 1 << b % INTBITS;
> +  return c[b / INTBITS] & 1U << b % INTBITS;
>  }

On a machine with 32-bit int and where b % INTBITS is 31,
the expression c[b / INTBITS] & 1U << b % INTBITS
is of type 'unsigned' and can have the value 2**31, and
this will overflow when tstbit converts that value as an int,
leading to implementation-defined behavior, which can include
raising a signal.

Better would be something like this:

static bool
tstbit (unsigned int b, charclass const c)
{
  return c[b / INTBITS] >> b % INTBITS & 1;
}

and it'd probably be better to encourage this style in
other places where the problem occurs, e.g., quotearg.

Message #11 received at 15924 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 15924 <at> debbugs.gnu.org
Subject: Re: bug#15924: [PATCH] dfa: avoid undefined behavior of "1 << 31"
Date: Mon, 18 Nov 2013 21:25:25 -0800

On Mon, Nov 18, 2013 at 6:16 PM, Paul Eggert <eggert <at> cs.ucla.edu> wrote:
> Jim Meyering wrote:
>>  static int
>>  tstbit (unsigned int b, charclass const c)
>>  {
>> -  return c[b / INTBITS] & 1 << b % INTBITS;
>> +  return c[b / INTBITS] & 1U << b % INTBITS;
>>  }
>
> On a machine with 32-bit int and where b % INTBITS is 31,
> the expression c[b / INTBITS] & 1U << b % INTBITS
> is of type 'unsigned' and can have the value 2**31, and
> this will overflow when tstbit converts that value as an int,
> leading to implementation-defined behavior, which can include
> raising a signal.
>
> Better would be something like this:
>
> static bool
> tstbit (unsigned int b, charclass const c)
> {
>   return c[b / INTBITS] >> b % INTBITS & 1;
> }
>
> and it'd probably be better to encourage this style in
> other places where the problem occurs, e.g., quotearg.

Good point.  "bool" is a better return type, too.
I will adjust.

Thanks, Paul.

Message #14 received at 15924 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 15924 <at> debbugs.gnu.org
Subject: Re: bug#15924: [PATCH] dfa: avoid undefined behavior of "1 << 31"
Date: Wed, 20 Nov 2013 21:03:42 -0800

[Message part 1 (text/plain, inline)]

On Mon, Nov 18, 2013 at 9:25 PM, Jim Meyering <jim <at> meyering.net> wrote:
> On Mon, Nov 18, 2013 at 6:16 PM, Paul Eggert <eggert <at> cs.ucla.edu> wrote:
>> Jim Meyering wrote:
>>>  static int
>>>  tstbit (unsigned int b, charclass const c)
>>>  {
>>> -  return c[b / INTBITS] & 1 << b % INTBITS;
>>> +  return c[b / INTBITS] & 1U << b % INTBITS;
>>>  }
>>
>> On a machine with 32-bit int and where b % INTBITS is 31,
>> the expression c[b / INTBITS] & 1U << b % INTBITS
>> is of type 'unsigned' and can have the value 2**31, and
>> this will overflow when tstbit converts that value as an int,
>> leading to implementation-defined behavior, which can include
>> raising a signal.
>>
>> Better would be something like this:
>>
>> static bool
>> tstbit (unsigned int b, charclass const c)
>> {
>>   return c[b / INTBITS] >> b % INTBITS & 1;
>> }
>>
>> and it'd probably be better to encourage this style in
>> other places where the problem occurs, e.g., quotearg.
>
> Good point.  "bool" is a better return type, too.
> I will adjust.

Here's an updated patch:

[k.txt (text/plain, attachment)]

Message #19 received at 15924-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 15924-done <at> debbugs.gnu.org
Subject: Re: bug#15924: [PATCH] dfa: avoid undefined behavior of "1 << 31"
Date: Fri, 22 Nov 2013 08:56:40 -0800

Pushed.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.