[Bug libstdc++/53578] New: include/ext/concurrence.h relies on ill-formed narrowing conversions

[Bug libstdc++/53578] New: include/ext/concurrence.h relies on ill-formed narrowing conversions

[richard-gccbugzilla at metafoo dot co.uk]

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

             Bug #: 53578
           Summary: include/ext/concurrence.h relies on ill-formed
                    narrowing conversions
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: richard-gccbugzilla@metafoo.co.uk


>From around line 271 of ext/concurrence.h:

// matches a gthr-win32.h recursive mutex
template<typename _Rm>
static typename __enable_if<sizeof(&_Rm::sema), void>::__type
_S_destroy(_Rm* __mx)

This SFINAE device doesn't work in Clang in C++11 mode, because the sizeof
expression evaluates to a value greater than one, and thus the implicit
conversion to bool for the first template parameter is a narrowing conversion.

I assume g++ will hit this too once it adds support for the narrowing check in
converted constant expressions.

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-06-04 23:55:04 UTC ---
> I assume g++ will hit this too once it adds support for the narrowing check in
> converted constant expressions.

IIRC g++ had the check for narrowing in converted constant expressions and it
was removed because the standard had a defect.

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-06-05 00:00:18 UTC ---
(In reply to comment #1)
> > I assume g++ will hit this too once it adds support for the narrowing check in
> > converted constant expressions.
> 
> IIRC g++ had the check for narrowing in converted constant expressions and it
> was removed because the standard had a defect.

Actually I take that back.  GCC already implements the error for narrowing for
converted constant expressions.

Also ext/concurrence.h is an extension to the C++ standard so I don't think we
should worry about other compilers besides GCC here.

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jwakely.gcc at gmail dot
                   |                            |com

--- Comment #3 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-06-05 00:04:59 UTC ---
Let's ask Jon to have a look, I think he figured out rather recently these
bits.

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[redi at gcc dot gnu.org]

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-06-05 00:24:31 UTC ---
That stuff was a hack to get it working in a hurry, ideally we want a
__gthread_mutex_destroy function. Otherwise, I can fix the sfinae check to work
properly. Presumably just comparing to 0 would avoid a narrowing conversion.

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[vanboxem.ruben at gmail dot com]

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

Ruben Van Boxem <vanboxem.ruben at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vanboxem.ruben at gmail dot
                   |                            |com

--- Comment #5 from Ruben Van Boxem <vanboxem.ruben at gmail dot com> 2012-06-06 10:25:26 UTC ---
This is not only a <ext/*> problem, but also turns up with `std::unique_ptr`
(so the header <memory>) on Windows. This error is caught by Clang in c++11
mode:
http://llvm.org/bugs/show_bug.cgi?id=12707

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-06-06 10:43:22 UTC ---
That's because ext/concurrence.h is included throughout the library.

I plan to add __gthread_recursive_mutex_destroy on trunk but any fix on the
release branches will have to be less intrusive.

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2012-06-06
         AssignedTo|unassigned at gcc dot       |redi at gcc dot gnu.org
                   |gnu.org                     |
   Target Milestone|---                         |4.6.4
     Ever Confirmed|0                           |1

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[vanboxem.ruben at gmail dot com]

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

--- Comment #7 from Ruben Van Boxem <vanboxem.ruben at gmail dot com> 2012-06-06 11:10:34 UTC ---
Changing this line:
      static typename __enable_if<sizeof(&_Rm::sema), void>::__type

to read:
      static typename __enable_if<static_cast<bool>(sizeof(&_Rm::sema)),
void>::__type

makes the unique_ptr problem go away (see
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-June/021863.html).

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[redi at gcc dot gnu.org]

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

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-06-06 11:21:30 UTC ---
Yes I know, but I want to get rid of that code entirely on trunk, and even a
small tweak to fix the narrowing conversion might not be suitable for the
release branches as it's not a GCC regression.  But I'll try to get it fixed on
the 4.6 and 4.7 branches.

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[redi at gcc dot gnu.org]

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

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-06-15 00:27:35 UTC ---
Author: redi
Date: Fri Jun 15 00:27:29 2012
New Revision: 188646

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188646
Log:
    PR libstdc++/53578
    * include/ext/concurrence.h (__recursive_mutex::_S_destroy): Fix
    narrowing conversion.
    * include/std/mutex (__recursive_mutex_base::_S_destroy): Likewise.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/ext/concurrence.h
    trunk/libstdc++-v3/include/std/mutex

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[redi at gcc dot gnu.org]

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

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-06-15 00:27:35 UTC ---
Author: redi
Date: Fri Jun 15 00:27:29 2012
New Revision: 188646

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188646
Log:
    PR libstdc++/53578
    * include/ext/concurrence.h (__recursive_mutex::_S_destroy): Fix
    narrowing conversion.
    * include/std/mutex (__recursive_mutex_base::_S_destroy): Likewise.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/ext/concurrence.h
    trunk/libstdc++-v3/include/std/mutex

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-06-15 00:28:36 UTC ---
Should be fixed on trunk, please test.

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[redi at gcc dot gnu.org]

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

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-06-15 00:28:36 UTC ---
Should be fixed on trunk, please test.

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |WAITING

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-06-29 08:17:21 UTC ---
Ruben, any chance you could test trunk with clang?

I'm not putting untested changes on the 4.6 and 4.7 release branches, so unless
someone tests it on an affected platform and confirms it's fixed then it stays
broken.

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[internet at 123gen dot com]

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

--- Comment #12 from Zouzou <internet at 123gen dot com> 2012-06-29 14:16:16 UTC ---
applying the changes in rev 188646 to a MinGW with GCC 4.7.0 works fine and
doesn't reproduce the problem detailed in bug 46455.

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[redi at gcc dot gnu.org]

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

--- Comment #13 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-07-07 18:13:24 UTC ---
Author: redi
Date: Sat Jul  7 18:13:19 2012
New Revision: 189351

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189351
Log:
    PR libstdc++/53578
    * include/ext/concurrence.h (__recursive_mutex::_S_destroy): Fix
    narrowing conversion.
    * include/std/mutex (__recursive_mutex_base::_S_destroy): Likewise.

Modified:
    branches/gcc-4_7-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_7-branch/libstdc++-v3/include/ext/concurrence.h
    branches/gcc-4_7-branch/libstdc++-v3/include/std/mutex

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[redi at gcc dot gnu.org]

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

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-07-07 18:35:58 UTC ---
Author: redi
Date: Sat Jul  7 18:35:52 2012
New Revision: 189352

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189352
Log:
    PR libstdc++/53578
    * include/ext/concurrence.h (__recursive_mutex::_S_destroy): Fix
    narrowing conversion.
    * include/std/mutex (__recursive_mutex_base::_S_destroy): Likewise.

Modified:
    branches/gcc-4_6-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_6-branch/libstdc++-v3/include/ext/concurrence.h
    branches/gcc-4_6-branch/libstdc++-v3/include/std/mutex

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

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

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-07-07 18:35:58 UTC ---
Author: redi
Date: Sat Jul  7 18:35:52 2012
New Revision: 189352

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189352
Log:
    PR libstdc++/53578
    * include/ext/concurrence.h (__recursive_mutex::_S_destroy): Fix
    narrowing conversion.
    * include/std/mutex (__recursive_mutex_base::_S_destroy): Likewise.

Modified:
    branches/gcc-4_6-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_6-branch/libstdc++-v3/include/ext/concurrence.h
    branches/gcc-4_6-branch/libstdc++-v3/include/std/mutex

--- Comment #15 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-07-07 18:36:27 UTC ---
This should be fixed on all active branches then.

[Bug libstdc++/53578] include/ext/concurrence.h relies on ill-formed narrowing conversions

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

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

--- Comment #15 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-07-07 18:36:27 UTC ---
This should be fixed on all active branches then.