public inbox for gcc-prs@sourceware.org
help / color / mirror / Atom feed
* Re: c/5593: GCC miscompiles bitshifts on unsigned struct members when creating a 64-bit value
@ 2002-11-05  8:15 bangerth
  0 siblings, 0 replies; 6+ messages in thread
From: bangerth @ 2002-11-05  8:15 UTC (permalink / raw)
  To: gandalf, gcc-bugs, gcc-prs, neil

Synopsis: GCC miscompiles bitshifts on unsigned struct members when creating a 64-bit value

State-Changed-From-To: open->analyzed
State-Changed-By: bangerth
State-Changed-When: Tue Nov  5 08:15:02 2002
State-Changed-Why:
    I can confirm this. However, I'm not sure whether what you
    do is specified at all: it all boils down to this function:
    long long equation4(struct field *data)
    {
      return ((long long)data->num << 32)|
              (data->flags << 16)|
              (data->container << 8)|
              data->quantity;
    }
    and that data->flags is a 16-bit integer. I think, shifting
    it by 16 is invoking undefined behavior, and you should
    not be surprised. But then, I'm not a language lawyer and
    leave this to someone else.

http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=5593


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: c/5593: GCC miscompiles bitshifts on unsigned struct members when creating a 64-bit value
@ 2002-11-09  4:16 neil
  0 siblings, 0 replies; 6+ messages in thread
From: neil @ 2002-11-09  4:16 UTC (permalink / raw)
  To: gandalf, gcc-bugs, gcc-prs, neil

Synopsis: GCC miscompiles bitshifts on unsigned struct members when creating a 64-bit value

State-Changed-From-To: analyzed->closed
State-Changed-By: neil
State-Changed-When: Sat Nov  9 04:16:33 2002
State-Changed-Why:
    Request of submitter.

http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=5593


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: c/5593: GCC miscompiles bitshifts on unsigned struct members when creating a 64-bit value
@ 2002-11-08 20:46 Byron Stanoszek
  0 siblings, 0 replies; 6+ messages in thread
From: Byron Stanoszek @ 2002-11-08 20:46 UTC (permalink / raw)
  To: neil; +Cc: gcc-prs

The following reply was made to PR c/5593; it has been noted by GNATS.

From: Byron Stanoszek <gandalf@winds.org>
To: Christian Ehrhardt <ehrhardt@mathematik.uni-ulm.de>
Cc: bangerth@dealii.org, <gcc-bugs@gcc.gnu.org>, <gcc-prs@gcc.gnu.org>,
   <neil@gcc.gnu.org>, <gcc-gnats@gcc.gnu.org>
Subject: Re: c/5593: GCC miscompiles bitshifts on unsigned struct members
 when creating a 64-bit value
Date: Fri, 8 Nov 2002 23:44:04 -0500 (EST)

 On Fri, 8 Nov 2002, Christian Ehrhardt wrote:
 
 > On Tue, Nov 05, 2002 at 04:15:03PM -0000, bangerth@dealii.org wrote:
 > > Synopsis: GCC miscompiles bitshifts on unsigned struct members when creating a 64-bit value
 > > 
 > >     I can confirm this. However, I'm not sure whether what you
 > >     do is specified at all: it all boils down to this function:
 > >     long long equation4(struct field *data)
 > >     {
 > >       return ((long long)data->num << 32)|
 > >               (data->flags << 16)|
 > >               (data->container << 8)|
 > >               data->quantity;
 > >     }
 > >     and that data->flags is a 16-bit integer. I think, shifting
 > >     it by 16 is invoking undefined behavior, and you should
 > >     not be surprised. But then, I'm not a language lawyer and
 > >     leave this to someone else.
 > 
 > b) A type is promoted to int if the whole range can be represented in
 >    an int no matter what the sign of the original type was (6.3.1.1[#2])
 >    This is the culprit!
 
 This does appear to be the culprit. Modifing the function so that we have
 'unsigned short flags=0x8000' or 'unsigned char flags=0x80' has the same effect
 in 'equation 1' to promote the shift to a signed int.
 
 Both pieces of code function similarly in a 64-bit environment (e.g. Alpha) so
 I'm pretty much declaring this to be not a bug at all.  Thanks for pointing out
 the C spec.
 
  -Byron
 
 > c) According to 6.5.7[#2] it is actually unspecified what happens
 >    if an overflow occurs in a left shift of a signed integer. This is
 >    the undefined behaviour invoked here but I think it is clear what
 >    the _right_ behaviour is.
 > 
 > This means (assuming a 16 Bit short and a 32 Bit int) the standard
 > says that the equation below always holds. Actually all the casts on
 > the rhs aren't necessary .
 > 
 > unsigned short a;
 > a << 16 == (int)(((int)a)<<16);
 > 
 > The actual value of the rhs is still unspecified according to the standard
 > if the value of a is greater than 0x7fff. But again I don't think it is
 > unspecified what gcc does in this case.
 > 
 > Now looking at the bitwise or with a 64 Bit operand:
 > a) 6.5.11[#3] states that the usual arithmetic conversions are performed
 >    on the operands of ``|'' before the operator is applied.
 > b) The usual arithmetic conversions defined in 6.3.1.8 state in [#1]
 >    for this case:
 >         Otherwise, if both operands have signed integer types or both
 >         have unsigned integer types, the operand with the type of lesser
 >         integer conversion rank is converted to the type of the operand
 >         with greater rank [which is long long in the case of int and
 > 	long long].
 >    and converting a signed integer to a signed integer of another type
 >    is defined in 6.3.1.3[#1]:
 >         When a value with integer type is converted to another integer
 >         type other than _Bool,if the value can be represented by the new
 >         type, it is unchanged.
 > 
 > De facto this means that the operand of the smaller type is sign extended.
 > 
 > This means that we get these implicit casts assuming 64 Bit long long:
 > 
 > long long ll; unsigned short a;
 > ll | (a << 16) == ll | (long long)(int)((int)a << (int) 16)
 > 
 > Now using your values (ll = 0x123400000000, a = 0x8000) in this
 > expression yields:
 >     ll                         | (long long)(int)((int)a << (int) 16)
 > ==  (long long)0x123400000000  | (long long)(int)((int)0x8000 << 16)    (*)
 > ==  (long long)0x123400000000  | (long long)(int)0x80000000             (**)
 > ==  (long long)0x123400000000  | (long long)(-2147483648)
 > ==  (long long)0x123400000000  | (long long)0xffffffff80000000
 > ==  (long long)0xffffffff80000000
 > 
 > The step from (*) to (**) is the only place where undefined behaviour
 > is invoked and I think we all agree that we'd consider it a bug if this
 > calculation did anything else than what I did above.
 > 
 >     regards   Christian
 > 
 > http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=5593
 > 
 > 
 
 -- 
 Byron Stanoszek                         Ph: (330) 644-3059
 Systems Programmer                      Fax: (330) 644-8110
 Commercial Timesharing Inc.             Email: byron@comtime.com
 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: c/5593: GCC miscompiles bitshifts on unsigned struct members when creating a 64-bit value
@ 2002-11-08  1:06 Christian Ehrhardt
  0 siblings, 0 replies; 6+ messages in thread
From: Christian Ehrhardt @ 2002-11-08  1:06 UTC (permalink / raw)
  To: neil; +Cc: gcc-prs

The following reply was made to PR c/5593; it has been noted by GNATS.

From: "Christian Ehrhardt" <ehrhardt@mathematik.uni-ulm.de>
To: bangerth@dealii.org, gandalf@winds.org, gcc-bugs@gcc.gnu.org,
  gcc-prs@gcc.gnu.org, neil@gcc.gnu.org, gcc-gnats@gcc.gnu.org
Cc:  
Subject: Re: c/5593: GCC miscompiles bitshifts on unsigned struct members when creating a 64-bit value
Date: Fri, 8 Nov 2002 10:00:28 +0100

 On Tue, Nov 05, 2002 at 04:15:03PM -0000, bangerth@dealii.org wrote:
 > Synopsis: GCC miscompiles bitshifts on unsigned struct members when creating a 64-bit value
 > 
 >     I can confirm this. However, I'm not sure whether what you
 >     do is specified at all: it all boils down to this function:
 >     long long equation4(struct field *data)
 >     {
 >       return ((long long)data->num << 32)|
 >               (data->flags << 16)|
 >               (data->container << 8)|
 >               data->quantity;
 >     }
 >     and that data->flags is a 16-bit integer. I think, shifting
 >     it by 16 is invoking undefined behavior, and you should
 >     not be surprised. But then, I'm not a language lawyer and
 >     leave this to someone else.
 
 I'm not a language lawyer, either, but as far as I can see this is not
 a bug. The standard (that I got myself today, so don't read too much into
 this) explicitly says that:
 
 a) Operands of << are integer promoted before the Operator
    is applied (6.5.7[#3]) and the result type is the type of the
    promoted left operand.
 b) A type is promoted to int if the whole range can be represented in
    an int no matter what the sign of the original type was (6.3.1.1[#2])
    This is the culprit!
 c) According to 6.5.7[#2] it is actually unspecified what happens
    if an overflow occurs in a left shift of a signed integer. This is
    the undefined behaviour invoked here but I think it is clear what
    the _right_ behaviour is.
 
 This means (assuming a 16 Bit short and a 32 Bit int) the standard
 says that the equation below always holds. Actually all the casts on
 the rhs aren't necessary .
 
 unsigned short a;
 a << 16 == (int)(((int)a)<<16);
 
 The actual value of the rhs is still unspecified according to the standard
 if the value of a is greater than 0x7fff. But again I don't think it is
 unspecified what gcc does in this case.
 
 Now looking at the bitwise or with a 64 Bit operand:
 a) 6.5.11[#3] states that the usual arithmetic conversions are performed
    on the operands of ``|'' before the operator is applied.
 b) The usual arithmetic conversions defined in 6.3.1.8 state in [#1]
    for this case:
         Otherwise, if both operands have signed integer types or both
         have unsigned integer types, the operand with the type of lesser
         integer conversion rank is converted to the type of the operand
         with greater rank [which is long long in the case of int and
 	long long].
    and converting a signed integer to a signed integer of another type
    is defined in 6.3.1.3[#1]:
         When a value with integer type is converted to another integer
         type other than _Bool,if the value can be represented by the new
         type, it is unchanged.
 
 De facto this means that the operand of the smaller type is sign extended.
 
 This means that we get these implicit casts assuming 64 Bit long long:
 
 long long ll; unsigned short a;
 ll | (a << 16) == ll | (long long)(int)((int)a << (int) 16)
 
 Now using your values (ll = 0x123400000000, a = 0x8000) in this
 expression yields:
     ll                         | (long long)(int)((int)a << (int) 16)
 ==  (long long)0x123400000000  | (long long)(int)((int)0x8000 << 16)    (*)
 ==  (long long)0x123400000000  | (long long)(int)0x80000000             (**)
 ==  (long long)0x123400000000  | (long long)(-2147483648)
 ==  (long long)0x123400000000  | (long long)0xffffffff80000000
 ==  (long long)0xffffffff80000000
 
 The step from (*) to (**) is the only place where undefined behaviour
 is invoked and I think we all agree that we'd consider it a bug if this
 calculation did anything else than what I did above.
 
     regards   Christian
 
 http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=5593
 
 -- 
 THAT'S ALL FOLKS!


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: c/5593: GCC miscompiles bitshifts on unsigned struct members when creating a 64-bit value
@ 2002-02-10 19:02 rth
  0 siblings, 0 replies; 6+ messages in thread
From: rth @ 2002-02-10 19:02 UTC (permalink / raw)
  To: gandalf, gcc-bugs, gcc-prs, neil, nobody

Synopsis: GCC miscompiles bitshifts on unsigned struct members when creating a 64-bit value

Responsible-Changed-From-To: unassigned->neil
Responsible-Changed-By: rth
Responsible-Changed-When: Sun Feb 10 19:02:32 2002
Responsible-Changed-Why:
    Pretty sure this is more fodder for your bitfield patch.

http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=5593


^ permalink raw reply	[flat|nested] 6+ messages in thread

* c/5593: GCC miscompiles bitshifts on unsigned struct members when creating a 64-bit value
@ 2002-02-05 11:46 gandalf
  0 siblings, 0 replies; 6+ messages in thread
From: gandalf @ 2002-02-05 11:46 UTC (permalink / raw)
  To: gcc-gnats


>Number:         5593
>Category:       c
>Synopsis:       GCC miscompiles bitshifts on unsigned struct members when creating a 64-bit value
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    unassigned
>State:          open
>Class:          wrong-code
>Submitter-Id:   net
>Arrival-Date:   Tue Feb 05 11:46:01 PST 2002
>Closed-Date:
>Last-Modified:
>Originator:     gandalf@winds.org
>Release:        gcc-2.95.3, gcc-3.0.3, gcc-3.1.0 (as of 20020205)
>Organization:
>Environment:
Error occurs on both i686 (IA32) and Alpha (64-bit) environments
>Description:
Four equations in the following code attempt to leftshift the 'flags' field to
create a 64-bit 'long long' value. The function equation4() is in error,
producing a value that has the upper 32 bits set to '1' due to an unexpected
problem with signed 32-bit integers.

This problem is observed under both -O0 and -O3 optimizations.


/* Begin Code */

struct field {
  int num;
  unsigned int flags:16;
  unsigned int container:8;
  unsigned int quantity:8;
};

long long equation1(void)
{
  int num=0x1234;
  unsigned int flags=0x8000;
  unsigned int container=0;
  unsigned int quantity=0;

  return ((long long)num << 32)|
          (flags << 16)|
          (container << 8)|
          quantity;
}

long long equation2(struct field *data)
{
  return ((long long)data->num << 32)|
          (unsigned int)(data->flags << 16)|
          (data->container << 8)|
          data->quantity;
}

long long equation3(struct field *data)
{
  return ((long long)data->num << 32)|
          (data->flags << 12)|
          (data->container << 8)|
          data->quantity;
}

long long equation4(struct field *data)
{
  return ((long long)data->num << 32)|
          (data->flags << 16)|
          (data->container << 8)|
          data->quantity;
}

int main(int argc, char *argv[])
{
  struct field test={0x1234, 0x8000, 0, 0};

  printf("0x%016qx\n", equation1());
  printf("0x%016qx\n", equation2(&test));
  printf("0x%016qx\n", equation3(&test));
  printf("0x%016qx\n", equation4(&test));
}

/* End Code */


Program Output:
---------------

0x0000123480000000
0x0000123480000000
0x0000123408000000
0xffffffff80000000



Analysis:
---------

Equation4() above is the equation with the error. The other three equations are
provided as a comparison to hone-in on what the actual problem is, as per the
following descriptions:

Equation1() is different from equation 4 in that the variables being left-
shifted are local, unsigned variables declared using 'char' and 'short' instead
of being members of a structure that are all defined as 'unsigned int' with a
number-of-bits limitation (the :8 and :16).

Equation2() is different in that the expression (data->flags << 16) is prefixed
by an (unsigned int) cast.

Equation3() is different in that the data->flags field is being left-shifted by
12 instead of by 16.

Whatever the case, because 'data->flags' is an unsigned 16-bit integer, left-
shifting by 16 in a 64-bit value is erroneously being sign-extended unless
the entire shift operation is explicitly casted as unsigned.
  
---
Byron Stanoszek                         Ph: (330) 644-3059
Systems Programmer                      Fax: (330) 644-8110
Commercial Timesharing Inc.             Email: bstanoszek@comtime.com
>How-To-Repeat:

>Fix:

>Release-Note:
>Audit-Trail:
>Unformatted:
----gnatsweb-attachment----
Content-Type: application/octet-stream; name="error.c"
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="error.c"

c3RydWN0IGZpZWxkIHsKICBpbnQgbnVtOwogIHVuc2lnbmVkIGludCBmbGFnczoxNjsKICB1bnNp
Z25lZCBpbnQgY29udGFpbmVyOjg7CiAgdW5zaWduZWQgaW50IHF1YW50aXR5Ojg7Cn07Cgpsb25n
IGxvbmcgZXF1YXRpb24xKHZvaWQpCnsKICBpbnQgbnVtPTB4MTIzNDsKICB1bnNpZ25lZCBpbnQg
ZmxhZ3M9MHg4MDAwOwogIHVuc2lnbmVkIGludCBjb250YWluZXI9MDsKICB1bnNpZ25lZCBpbnQg
cXVhbnRpdHk9MDsKCiAgcmV0dXJuICgobG9uZyBsb25nKW51bSA8PCAzMil8CiAgICAgICAgICAo
ZmxhZ3MgPDwgMTYpfAogICAgICAgICAgKGNvbnRhaW5lciA8PCA4KXwKICAgICAgICAgIHF1YW50
aXR5Owp9Cgpsb25nIGxvbmcgZXF1YXRpb24yKHN0cnVjdCBmaWVsZCAqZGF0YSkKewogIHJldHVy
biAoKGxvbmcgbG9uZylkYXRhLT5udW0gPDwgMzIpfAogICAgICAgICAgKHVuc2lnbmVkIGludCko
ZGF0YS0+ZmxhZ3MgPDwgMTYpfAogICAgICAgICAgKGRhdGEtPmNvbnRhaW5lciA8PCA4KXwKICAg
ICAgICAgIGRhdGEtPnF1YW50aXR5Owp9Cgpsb25nIGxvbmcgZXF1YXRpb24zKHN0cnVjdCBmaWVs
ZCAqZGF0YSkKewogIHJldHVybiAoKGxvbmcgbG9uZylkYXRhLT5udW0gPDwgMzIpfAogICAgICAg
ICAgKGRhdGEtPmZsYWdzIDw8IDEyKXwKICAgICAgICAgIChkYXRhLT5jb250YWluZXIgPDwgOCl8
CiAgICAgICAgICBkYXRhLT5xdWFudGl0eTsKfQoKbG9uZyBsb25nIGVxdWF0aW9uNChzdHJ1Y3Qg
ZmllbGQgKmRhdGEpCnsKICByZXR1cm4gKChsb25nIGxvbmcpZGF0YS0+bnVtIDw8IDMyKXwKICAg
ICAgICAgIChkYXRhLT5mbGFncyA8PCAxNil8CiAgICAgICAgICAoZGF0YS0+Y29udGFpbmVyIDw8
IDgpfAogICAgICAgICAgZGF0YS0+cXVhbnRpdHk7Cn0KCmludCBtYWluKGludCBhcmdjLCBjaGFy
ICphcmd2W10pCnsKICBzdHJ1Y3QgZmllbGQgdGVzdD17MHgxMjM0LCAweDgwMDAsIDAsIDB9OwoK
ICBwcmludGYoIjB4JTAxNnF4XG4iLCBlcXVhdGlvbjEoKSk7CiAgcHJpbnRmKCIweCUwMTZxeFxu
IiwgZXF1YXRpb24yKCZ0ZXN0KSk7CiAgcHJpbnRmKCIweCUwMTZxeFxuIiwgZXF1YXRpb24zKCZ0
ZXN0KSk7CiAgcHJpbnRmKCIweCUwMTZxeFxuIiwgZXF1YXRpb240KCZ0ZXN0KSk7Cn0K


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2002-11-09 12:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-11-05  8:15 c/5593: GCC miscompiles bitshifts on unsigned struct members when creating a 64-bit value bangerth
  -- strict thread matches above, loose matches on Subject: below --
2002-11-09  4:16 neil
2002-11-08 20:46 Byron Stanoszek
2002-11-08  1:06 Christian Ehrhardt
2002-02-10 19:02 rth
2002-02-05 11:46 gandalf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).