Bug 59656 - weak_ptr::lock function crashes when compiling with -fno-exceptions flag
Summary: weak_ptr::lock function crashes when compiling with -fno-exceptions flag
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 4.8.1
: P3 normal
Target Milestone: 4.9.0
Assignee: Jonathan Wakely
: 59869 (view as bug list)
Depends on:
Reported: 2014-01-01 19:20 UTC by chus_flores
Modified: 2014-01-28 19:08 UTC (History)
1 user (show)

See Also:
Known to work:
Known to fail:
Last reconfirmed: 2014-01-24 00:00:00

test case (2.07 KB, text/x-c++src)
2014-01-21 21:05 UTC, Paul Pluzhnikov
don't use _M_add_ref_lock() when exceptions are disabled (1.59 KB, patch)
2014-01-22 13:31 UTC, Jonathan Wakely
Details | Diff
Alternate fix from Kyle Lippincott (1.89 KB, text/plain)
2014-01-22 19:20 UTC, Paul Pluzhnikov

Note You need to log in before you can comment on or make changes to this bug.
Description chus_flores 2014-01-01 19:20:13 UTC
weak_ptr::lock crashes when the code is compiled with the -fno-exceptions flag and the data pointed by the weak_ptr expires during the execution of the lock function itself:

(from http://gcc.gnu.org/onlinedocs/gcc-4.8.2/libstdc++/api/a01518_source.html)

  494       lock() const noexcept
  495       {
  496 #ifdef __GTHREADS
  497     if (this->expired())
  498       return shared_ptr<_Tp>();
  500     __try
  501       {
  502         return shared_ptr<_Tp>(*this);
  503       }
  504     __catch(const bad_weak_ptr&)
  505       {
  506         return shared_ptr<_Tp>();
  507       }
  508 #else
  509     return this->expired() ? shared_ptr<_Tp>() : shared_ptr<_Tp>(*this);
  510 #endif
  511       }

If the data is valid when line 497 is executed and the data is released in a different thread just before executing line 502, the program will crash because it will try to throw an exception (exceptions are disabled because of the flag -fno-exceptions). This code only works when exceptions are enabled because the try/catch will resolve the problem, but not otherwise.

The standard definition says that this function must return safely and it doesn't throw any exception. I presume this must apply even if the exceptions are not enabled.
Comment 1 Jonathan Wakely 2014-01-06 17:34:43 UTC
The standard says exceptions are always enabled, if you disable them you are no longer using standard C++, so what it says in the standard doesn't necessarily apply.

I don't think it would be simple to make this work with -fno-exceptions
Comment 2 Paul Pluzhnikov 2014-01-18 16:55:23 UTC
(In reply to Jonathan Wakely from comment #1)
> The standard says exceptions are always enabled, if you disable them you are
> no longer using standard C++, so what it says in the standard doesn't
> necessarily apply.

It's a quality of implementation issue: effectively current std::weak_ptr is not usable in any multithreaded program built with -fno-exceptions.

> I don't think it would be simple to make this work with -fno-exceptions

We have a patch that makes it work.
Comment 3 Paul Pluzhnikov 2014-01-18 16:57:58 UTC
*** Bug 59869 has been marked as a duplicate of this bug. ***
Comment 4 Paul Pluzhnikov 2014-01-21 21:05:14 UTC
Created attachment 31911 [details]
test case

Copy of test case from PR 59869
Comment 5 Jonathan Wakely 2014-01-22 11:21:50 UTC
Thanks Paul. You mentioned a proposed patch, are you working on that? From a quick look I didn't see how to fix it without adding a _Sp_counted_base::_M_add_ref_lock_nothrow() function and adding new constructors taking an additional std::nothrow_t argument to the whole class hierarchy. Did you have a simpler fix in mind?
Comment 6 Jonathan Wakely 2014-01-22 13:31:20 UTC
Created attachment 31920 [details]
don't use _M_add_ref_lock() when exceptions are disabled

Here's a patch which fixes Paul's testcase
Comment 7 Paul Pluzhnikov 2014-01-22 19:20:35 UTC
Created attachment 31924 [details]
Alternate fix from Kyle Lippincott

Alternate patch, courtesy Kyle Lippincott <spectral@google.com>.

AFAIU, Kyle's patch uses roughly the same idea as Jonathan's,
but no #ifdef's.

Should we take these patches to gcc-patches list?
Comment 8 Jonathan Wakely 2014-01-24 17:43:32 UTC
Thanks, yes the general idea is identical. I prefer some parts of Kyle's fix but I don't want the new constructor to be public, so I'll combine the two patches and post it to gcc-patches next week.
Comment 9 Jonathan Wakely 2014-01-28 10:23:59 UTC
Author: redi
Date: Tue Jan 28 10:23:27 2014
New Revision: 207180

URL: http://gcc.gnu.org/viewcvs?rev=207180&root=gcc&view=rev
2014-01-28  Jonathan Wakely  <jwakely@redhat.com>
	    Kyle Lippincott  <spectral@google.com>

	PR libstdc++/59656
	* include/bits/shared_ptr.h (shared_ptr): Add new non-throwing
	constructor and grant friendship to weak_ptr.
	(weak_ptr::lock()): Use new constructor.
	* include/bits/shared_ptr_base.h
	(_Sp_counted_base::_M_add_ref_lock_nothrow()): Declare new function
	and define specializations.
	(__shared_count): Add new non-throwing constructor.
	(__shared_ptr): Add new non-throwing constructor and grant friendship
	to __weak_ptr.
	(__weak_ptr::lock()): Use new constructor.
	* testsuite/20_util/shared_ptr/cons/43820_neg.cc: Adjust dg-error.
	* testsuite/20_util/shared_ptr/cons/void_neg.cc: Likewise.

Comment 10 Jonathan Wakely 2014-01-28 10:24:34 UTC
Fixed on trunk
Comment 11 ppluzhnikov 2014-01-28 19:08:33 UTC
Author: ppluzhnikov
Date: Tue Jan 28 19:08:00 2014
New Revision: 207201

URL: http://gcc.gnu.org/viewcvs?rev=207201&root=gcc&view=rev
For Google b/12533273 and PR 59656, partially backport r207180 from trunk.

(The new test: libstdc++-v3/testsuite/20_util/shared_ptr/cons/void_neg.cc
is not added, because it succeeds to compile with gcc-4_8.)