[Bug c++/54897] New: [4.8 Regression]: 23_containers/bitset/45713.cc (test for excess errors)

[Bug c++/54897] New: [4.8 Regression]: 23_containers/bitset/45713.cc (test for excess errors)

[hp at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54897

             Bug #: 54897
           Summary: [4.8 Regression]: 23_containers/bitset/45713.cc (test
                    for excess errors)
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Keywords: ABI
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: hp@gcc.gnu.org
              Host: x64_86-unknown-linux-gnu
            Target: cris-axis-elf


With revision 192137 this test passed.
>From revision 192147 and on, this test has failed as follows:

Running
/tmp/hpautotest-gcc1/gcc/libstdc++-v3/testsuite/libstdc++-dg/conformance.exp
...
FAIL: 23_containers/bitset/45713.cc (test for excess errors)

With the message in the logfile being:

/tmp/hpautotest-gcc1/gcc/libstdc++-v3/testsuite/23_containers/bitset/45713.cc:24:55:
error: size of array 'test' is not\
 an integral constant-expression^M
 int test[sizeof(std::bitset<0xffffffff>) != 1 ? 1 : -1];^M
                                                       ^^M
compiler exited with status 1

FWIW, the cris-axis-elf ABI is effectively __attribute__ ((__packed__)).

Author of the only suspect patch in this revision range CC:ed.
(Did this patch change the ABI for some targets, Jakub?)

I'll attach a preprocessed file.

[Bug c++/54897] [4.8 Regression]: 23_containers/bitset/45713.cc (test for excess errors)

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54897

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-10-11 07:38:19 UTC ---
The problem is that IMHO the testcase is just invalid on 32-bit HWI targets.
On 32-bit targets with 64-bit HWI, bitsizetype is 64-bit and thus
std::bitset<0xffffffff> has size 0x20000000, and bitsize 0x100000000.  But
32-bit HWI targets use 32-bit bitsizetype, thereforestd::bitset<0xffffffff> is
larger than the largest supportable type, bitsize of the array is 0 overflow,
and both bitsize and unit size of the bitset are therefore set to 0 overflow.
Before my changes, sizeof(std::bitset<0xffffffff>) yielded 0 overflow, so the
test magically passed even when sizeof 0 is as wrong as sizeof 1.  With the
patch, maybe_constant_size is used to fold the SIZEOF_EXPR, but as it is
overflown, it is not considered a valid integer constant expression, thus we
end up with size unfolded and therefore the error (or considering the array a
VLA).

[Bug c++/54897] [4.8 Regression]: 23_containers/bitset/45713.cc (test for excess errors)

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54897

--- Comment #2 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-10-11 09:48:39 UTC ---
See if changing the test to the following (unconditional) works for you:

int test[sizeof(std::bitset<__SIZE_MAX__>) != 1 ? 1 : -1];

It passes on x86_64-linux, -m32 and -m64. In case, you are welcome to commit
the change.

I think it does still test for the original issue on the affected targets.
Anyway, it was only a quick try of mine at adapting the testcase provided by
submitter, the actual issue was just a thinko, IMO we shouldn't spend too much
time on this.

[Bug c++/54897] [4.8 Regression]: 23_containers/bitset/45713.cc (test for excess errors)

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54897

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-10-11 10:11:52 UTC ---
Actually, I was wrong about 32-bit HWI, the actual problem is
MAX_FIXED_MODE_SIZE.
  bprecision
    = MIN (precision + BITS_PER_UNIT_LOG + 1, MAX_FIXED_MODE_SIZE);
  bprecision
    = GET_MODE_PRECISION (smallest_mode_for_size (bprecision, MODE_INT));
  if (bprecision > HOST_BITS_PER_DOUBLE_INT)
    bprecision = HOST_BITS_PER_DOUBLE_INT;
In cris case that is MIN (32 + 3 + 1, 32), while i?86/x86_64 have 64 resp. 128
(-m32 resp. -m64).  So, such change isn't going to improve anything for cris,
where simply any variables etc. of size 0x20000000 and larger can't be
supported.
Targets with similar issues: avr, h8300, mcore, moxie.
I'd say the testcase should just be skipped for those targets.

[Bug c++/54897] [4.8 Regression]: 23_containers/bitset/45713.cc (test for excess errors)

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54897

--- Comment #4 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-10-11 10:19:39 UTC ---
Ah! I'm Ok with xfailing - I'm leaving that to you - or we can just remove the
test, isn't a big deal.

[Bug c++/54897] [4.8 Regression]: 23_containers/bitset/45713.cc (test for excess errors)

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54897

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-10-11 10:31:31 UTC ---
I'd go with
--- libstdc++-v3/testsuite/23_containers/bitset/45713.cc    2010-09-22
17:15:42.000000000 +0200
+++ libstdc++-v3/testsuite/23_containers/bitset/45713.cc    2012-10-11
12:28:49.865370623 +0200
@@ -1,4 +1,4 @@
-// Copyright (C) 2010 Free Software Foundation, Inc.
+// Copyright (C) 2010, 2012 Free Software Foundation, Inc.
 //
 // This file is part of the GNU ISO C++ Library.  This library is free
 // software; you can redistribute it and/or modify it under the
@@ -15,7 +15,10 @@
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.

-// { dg-do compile }
+// The testcase requires bitsizetype to be wider than sizetype,
+// otherwise types/vars with 0x20000000 bytes or larger can't be used.
+// See http://gcc.gnu.org/PR54897
+// { dg-do compile { target { ! { avr*-* cris*-* h8300*-* mcore*-* moxie*-* }
} } }

 #include <bitset>


but don't have time to test it on those targets...

[Bug c++/54897] [4.8 Regression]: 23_containers/bitset/45713.cc (test for excess errors)

[hp at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54897

--- Comment #6 from Hans-Peter Nilsson <hp at gcc dot gnu.org> 2012-10-11 10:55:23 UTC ---
(In reply to comment #3)
> Actually, I was wrong about 32-bit HWI, the actual problem is

> In cris case that is MIN (32 + 3 + 1, 32), while i?86/x86_64 have 64 resp. 128
> (-m32 resp. -m64).  So, such change isn't going to improve anything for cris,
> where simply any variables etc. of size 0x20000000 and larger can't be
> supported.

I hope you mean the actual size, not holding that value and larger.

> Targets with similar issues: avr, h8300, mcore, moxie.
> I'd say the testcase should just be skipped for those targets.

Ok: I'm happy with a skipping patch until I find time to investigate fallout
(and if it'd make sense) changing MAX_FIXED_MODE_SIZE for CRIS. The docs are a
bit misleading, DImode works fine, for instance.  And of course, it would be
right for remaining targets anyway.  If there are more tests like this, we can
consider an effective-target.  I'll test and commit.  Thanks.

[Bug testsuite/54897] [4.8 Regression]: 23_containers/bitset/45713.cc (test for excess errors)

[hp at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54897

Hans-Peter Nilsson <hp at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c++                         |testsuite

--- Comment #7 from Hans-Peter Nilsson <hp at gcc dot gnu.org> 2012-10-11 11:34:47 UTC ---
Correcting component as per conclusion.

[Bug testsuite/54897] [4.8 Regression]: 23_containers/bitset/45713.cc (test for excess errors)

[hp at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54897

--- Comment #8 from Hans-Peter Nilsson <hp at gcc dot gnu.org> 2012-10-11 11:36:49 UTC ---
Author: hp
Date: Thu Oct 11 11:36:39 2012
New Revision: 192354

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=192354
Log:
    PR testsuite/54897
    * testsuite/23_containers/bitset/45713.cc: Skip for avr*-*-*
    cris*-*-* h8300*-*-* mcore*-*-* moxie*-*-*.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/testsuite/23_containers/bitset/45713.cc

[Bug testsuite/54897] [4.8 Regression]: 23_containers/bitset/45713.cc (test for excess errors)

[hp at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54897

Hans-Peter Nilsson <hp at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED

--- Comment #9 from Hans-Peter Nilsson <hp at gcc dot gnu.org> 2012-10-11 11:40:40 UTC ---
...and closing.