* [PATCH, libbacktrace] Don't call __sync_lock_test_and_set if we don't have sync functions
@ 2012-12-09 19:08 John David Anglin
2012-12-30 16:28 ` John David Anglin
2013-01-01 16:15 ` Ian Lance Taylor
0 siblings, 2 replies; 3+ messages in thread
From: John David Anglin @ 2012-12-09 19:08 UTC (permalink / raw)
To: gcc-patches
On hppa*-*-hpux*, we don't have sync functions. However,
__sync_lock_test_and_set is called in backtrace_alloc and
backtrace_free. This causes an abort before ICE proccessing
is fully complete.
hppa64 is an ELF target and backtraces are nominally supported.
The attached change avoids calling __sync_lock_test_and_set
if we don't have sync functions. As a result, the memory is
leaked.
This fixes the btest failure.
OK for trunk?
Dave
--
J. David Anglin dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada (613) 990-0752 (FAX: 952-6602)
2012-12-09 John David Anglin <dave.anglin@nrc-cnrc.gc.ca>
* mmap.c: Define HAVE_SYNC_FUNCTIONS if not defined.
(backtrace_alloc): Don't call __sync_lock_test_and_set if we don't
have sync functions.
(backtrace_free): Likewise.
Index: mmap.c
===================================================================
--- mmap.c (revision 194055)
+++ mmap.c (working copy)
@@ -49,6 +49,10 @@
#define MAP_ANONYMOUS MAP_ANON
#endif
+#ifndef HAVE_SYNC_FUNCTIONS
+#define HAVE_SYNC_FUNCTIONS 0
+#endif
+
/* A list of free memory blocks. */
struct backtrace_freelist_struct
@@ -96,7 +100,7 @@
using mmap. __sync_lock_test_and_set returns the old state of
the lock, so we have acquired it if it returns 0. */
- if (!__sync_lock_test_and_set (&state->lock_alloc, 1))
+ if (HAVE_SYNC_FUNCTIONS && !__sync_lock_test_and_set (&state->lock_alloc, 1))
{
for (pp = &state->freelist; *pp != NULL; pp = &(*pp)->next)
{
@@ -158,7 +162,7 @@
If we can't acquire the lock, just leak the memory.
__sync_lock_test_and_set returns the old state of the lock, so we
have acquired it if it returns 0. */
- if (!__sync_lock_test_and_set (&state->lock_alloc, 1))
+ if (HAVE_SYNC_FUNCTIONS && !__sync_lock_test_and_set (&state->lock_alloc, 1))
{
backtrace_free_locked (state, addr, size);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH, libbacktrace] Don't call __sync_lock_test_and_set if we don't have sync functions
2012-12-09 19:08 [PATCH, libbacktrace] Don't call __sync_lock_test_and_set if we don't have sync functions John David Anglin
@ 2012-12-30 16:28 ` John David Anglin
2013-01-01 16:15 ` Ian Lance Taylor
1 sibling, 0 replies; 3+ messages in thread
From: John David Anglin @ 2012-12-30 16:28 UTC (permalink / raw)
To: John David Anglin; +Cc: gcc-patches
Ping.
On 9-Dec-12, at 2:08 PM, John David Anglin wrote:
> On hppa*-*-hpux*, we don't have sync functions. However,
> __sync_lock_test_and_set is called in backtrace_alloc and
> backtrace_free. This causes an abort before ICE proccessing
> is fully complete.
>
> hppa64 is an ELF target and backtraces are nominally supported.
>
> The attached change avoids calling __sync_lock_test_and_set
> if we don't have sync functions. As a result, the memory is
> leaked.
>
> This fixes the btest failure.
>
> OK for trunk?
>
> Dave
> --
> J. David Anglin dave.anglin@nrc-cnrc.gc.ca
> National Research Council of Canada (613) 990-0752
> (FAX: 952-6602)
>
> 2012-12-09 John David Anglin <dave.anglin@nrc-cnrc.gc.ca>
>
> * mmap.c: Define HAVE_SYNC_FUNCTIONS if not defined.
> (backtrace_alloc): Don't call __sync_lock_test_and_set if we don't
> have sync functions.
> (backtrace_free): Likewise.
>
> Index: mmap.c
> ===================================================================
> --- mmap.c (revision 194055)
> +++ mmap.c (working copy)
> @@ -49,6 +49,10 @@
> #define MAP_ANONYMOUS MAP_ANON
> #endif
>
> +#ifndef HAVE_SYNC_FUNCTIONS
> +#define HAVE_SYNC_FUNCTIONS 0
> +#endif
> +
> /* A list of free memory blocks. */
>
> struct backtrace_freelist_struct
> @@ -96,7 +100,7 @@
> using mmap. __sync_lock_test_and_set returns the old state of
> the lock, so we have acquired it if it returns 0. */
>
> - if (!__sync_lock_test_and_set (&state->lock_alloc, 1))
> + if (HAVE_SYNC_FUNCTIONS && !__sync_lock_test_and_set (&state-
> >lock_alloc, 1))
> {
> for (pp = &state->freelist; *pp != NULL; pp = &(*pp)->next)
> {
> @@ -158,7 +162,7 @@
> If we can't acquire the lock, just leak the memory.
> __sync_lock_test_and_set returns the old state of the lock, so we
> have acquired it if it returns 0. */
> - if (!__sync_lock_test_and_set (&state->lock_alloc, 1))
> + if (HAVE_SYNC_FUNCTIONS && !__sync_lock_test_and_set (&state-
> >lock_alloc, 1))
> {
> backtrace_free_locked (state, addr, size);
>
>
--
John David Anglin dave.anglin@bell.net
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH, libbacktrace] Don't call __sync_lock_test_and_set if we don't have sync functions
2012-12-09 19:08 [PATCH, libbacktrace] Don't call __sync_lock_test_and_set if we don't have sync functions John David Anglin
2012-12-30 16:28 ` John David Anglin
@ 2013-01-01 16:15 ` Ian Lance Taylor
1 sibling, 0 replies; 3+ messages in thread
From: Ian Lance Taylor @ 2013-01-01 16:15 UTC (permalink / raw)
To: John David Anglin; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 894 bytes --]
On Sun, Dec 9, 2012 at 11:08 AM, John David Anglin
<dave@hiauly1.hia.nrc.ca> wrote:
> On hppa*-*-hpux*, we don't have sync functions. However,
> __sync_lock_test_and_set is called in backtrace_alloc and
> backtrace_free. This causes an abort before ICE proccessing
> is fully complete.
>
> hppa64 is an ELF target and backtraces are nominally supported.
>
> The attached change avoids calling __sync_lock_test_and_set
> if we don't have sync functions. As a result, the memory is
> leaked.
Sorry for the delay. I think it's better to fix this the same way we
handle the other sync functions, as in this patch. Bootstrapped and
ran libbacktrace and go testsuites on x86_64-unknown-linux-gnu.
Committed to mainline.
Ian
2013-01-01 Ian Lance Taylor <iant@google.com>
PR other/55536
* mmap.c (backtrace_alloc): Don't call sync functions if not
threaded.
(backtrace_free): Likewise.
[-- Attachment #2: foo.patch --]
[-- Type: application/octet-stream, Size: 1818 bytes --]
Index: mmap.c
===================================================================
--- mmap.c (revision 194764)
+++ mmap.c (working copy)
@@ -84,6 +84,7 @@ backtrace_alloc (struct backtrace_state
void *data)
{
void *ret;
+ int locked;
struct backtrace_freelist_struct **pp;
size_t pagesize;
size_t asksize;
@@ -96,7 +97,12 @@ backtrace_alloc (struct backtrace_state
using mmap. __sync_lock_test_and_set returns the old state of
the lock, so we have acquired it if it returns 0. */
- if (!__sync_lock_test_and_set (&state->lock_alloc, 1))
+ if (!state->threaded)
+ locked = 1;
+ else
+ locked = __sync_lock_test_and_set (&state->lock_alloc, 1) == 0;
+
+ if (locked)
{
for (pp = &state->freelist; *pp != NULL; pp = &(*pp)->next)
{
@@ -120,7 +126,8 @@ backtrace_alloc (struct backtrace_state
}
}
- __sync_lock_release (&state->lock_alloc);
+ if (state->threaded)
+ __sync_lock_release (&state->lock_alloc);
}
if (ret == NULL)
@@ -154,15 +161,24 @@ backtrace_free (struct backtrace_state *
backtrace_error_callback error_callback ATTRIBUTE_UNUSED,
void *data ATTRIBUTE_UNUSED)
{
+ int locked;
+
/* If we can acquire the lock, add the new space to the free list.
If we can't acquire the lock, just leak the memory.
__sync_lock_test_and_set returns the old state of the lock, so we
have acquired it if it returns 0. */
- if (!__sync_lock_test_and_set (&state->lock_alloc, 1))
+
+ if (!state->threaded)
+ locked = 1;
+ else
+ locked = __sync_lock_test_and_set (&state->lock_alloc, 1) == 0;
+
+ if (locked)
{
backtrace_free_locked (state, addr, size);
- __sync_lock_release (&state->lock_alloc);
+ if (state->threaded)
+ __sync_lock_release (&state->lock_alloc);
}
}
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-01-01 16:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-09 19:08 [PATCH, libbacktrace] Don't call __sync_lock_test_and_set if we don't have sync functions John David Anglin
2012-12-30 16:28 ` John David Anglin
2013-01-01 16:15 ` Ian Lance Taylor
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).