Bug 13844 - `nptl/sysdeps/unix/sysv/linux/sparc/sparc32/libc-lowlevellock.c' missing in glibc sources
Summary: `nptl/sysdeps/unix/sysv/linux/sparc/sparc32/libc-lowlevellock.c' missing in g...
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: build (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: 2.16
Assignee: Carlos O'Donell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-14 13:05 UTC by Ilya Yu. Malakhov
Modified: 2014-06-26 13:59 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
carlos_odonell: review+
carlos_odonell: review+
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Yu. Malakhov 2012-03-14 13:05:36 UTC
Hi.

 Because of the absence of the sparc32-linux specific version of this file, `nptl/sysdeps/unix/sysv/linux/libc-lowlevellock.c' is fetched when building `libc-lowlevellock.o{,s}'. The latter contains just
. . .
#include "lowlevellock.c"

which leads to `nptl/sysdeps/unix/sysv/linux/lowlevellock.c' being actually compiled instead of `nptl/sysdeps/unix/sysv/linux/sparc/sparc32/lowlevellock.c'.

 As a consequence a futex can be accessed in a non-atomic way when using libc's internal locks, e.g. when performing output to the same stream from different threads.

 For example, when the following test is run on a V8 host, this typically leads to a deadlock when all threads are waiting for a futex to become free and there is no one to release it. This happens particularly soon when it's executed this way at a multi-CPU host:

$ ./test.sparc32 5 > /dev/null


$ cat test.c

#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>

void *
handler (void *arg)
{
  for (;;)
    printf ("Thread #%ld\n", (long) arg);

  return NULL;
}

int
main (int argc, char **argv)
{
  size_t i;
  size_t nthreads = (size_t) atoi (argv[1]);
  pthread_t *threads = (pthread_t *) malloc (nthreads * sizeof (pthread_t));

  for (i = 0; i < nthreads; i++)
    pthread_create (&threads[i], NULL, handler, (void *) i);

  for (i = 0; i < nthreads; i++)
    pthread_join (threads[i], NULL);

  free (threads);
  return 0;
}
Comment 1 Carlos O'Donell 2012-03-14 15:51:23 UTC
(1) I've added Dave Miller (Sparc maintainer) to the CC.

(2) Please follow "What to put in a report": http://www.gnu.org/software/libc/bugs.html

(3) No architecture needs to provide libc-lowlevellock.c. The normal sysdep override mechanism should have selected the sparc32 version of lowlevellock.c *unless* your configure options failed to cause the selection of sparc32. Given that you have not provided a full report we don't know why sparc32 was not selected.

Please include the full log of running `./configure ...` since that will contain which sysdep directories were selected and in what override order.

Thanks.
Comment 2 Ilya Yu. Malakhov 2012-03-15 06:31:22 UTC
(In reply to comment #1)
. . .
> (3) . . . The normal sysdep
> override mechanism should have selected the sparc32 version of lowlevellock.c
> *unless* your configure options failed to cause the selection of sparc32. . .
. . .
 And it actually did when compiling lowlevellock.o{,s}, however `#include "lowlevellock.c"' in the generic version of libc-lowlevellock.c causes the generic version of lowlevellock.c to be included (because they are in the same directory) rather than the sparc32 one.

 If you still need a full configure log, I'll provide it a bit later.
Comment 3 Carlos O'Donell 2012-03-15 15:14:53 UTC
(In reply to comment #2)
> (In reply to comment #1)
> . . .
> > (3) . . . The normal sysdep
> > override mechanism should have selected the sparc32 version of lowlevellock.c
> > *unless* your configure options failed to cause the selection of sparc32. . .
> . . .
>  And it actually did when compiling lowlevellock.o{,s}, however `#include
> "lowlevellock.c"' in the generic version of libc-lowlevellock.c causes the
> generic version of lowlevellock.c to be included (because they are in the same
> directory) rather than the sparc32 one.
> 
>  If you still need a full configure log, I'll provide it a bit later.

No, that makes more sense to me.

I see HPPA has it's own version of libc-lowlevellock.c probably because of this issue.

I also think ARM might suffer the same problem, it has a lowlevellock.c but no overriding libc-lowlevellock.c like HPPA.

OK, confirmed, on ARM we are also using the *default* lowlevellock.c e.g.
~~~
nptl/libc-lowlevellock.os:     file format elf32-littlearm


Disassembly of section .text:

00000000 <__lll_lock_wait_private>:
__lll_lock_wait_private():
/opt/codesourcery/arm-verifone-linux-gnueabi/src/glibc/nptl/../nptl/sysdeps/unix/sysv/linux/lowlevellock.c:29
   0:   e92d40f0        push    {r4, r5, r6, r7, lr}
~~~

OK, some big-picture questions.

Dave, Why does sparc have a custom version of lowlevellock.c? Why do any architectures? The custom versions all seem vaguely similar to the generic linux version.

If architectures *do* need to override the generic lowlevellock.c without overriding libc-lowlevellock.c then I think the right solution would be to make libc-lowlevellock.c use `#include <lowlevellock.c>` such that the sysdep include mechanism includes the right file.

Il'ya, Would you mind testing that change?
Comment 4 David S. Miller 2012-03-16 03:51:14 UTC
Sparc needs a custom lowlevellock.c on 32-bit sparc because
we have a situation where we don't know if we have the
proper atomic instructions available or not.

For example, if we don't have the v9 compare-and-swap available
we use spinlocks instead.  And for futexes we use a special
24-bit atomic, which embeds the spinlock in the remaining
8-bits.  That's what all of those atomic_compare_and_exchange_val_24_acq
etc. routines are all about.

I'll just add the necessary sparc32/libc-lowlevellock.c file
to fix this, thanks for the report.
Comment 5 Carlos O'Donell 2012-03-16 14:27:49 UTC
Joseph,

It looks like ARM also needs a libc-lowlevellock.c? Could you please review?
Comment 6 joseph@codesourcery.com 2012-03-16 14:32:50 UTC
I think we should use #include <> in the generic libc-lowlevellock.c 
rather than accumulating copies.
Comment 7 David S. Miller 2012-03-17 04:00:51 UTC
That's what I ended up doing in the end.