On Fr, Mai 25, 2012 at 01:35:09 (CEST), Guillem Jover wrote:
> On Thu, 2012-05-24 at 08:12:50 +0200, Reinhard Tartler wrote:
>> Package: ftp.debian.org
>> Severity: normal
>
>> The package aspectc++ does not build on kfreebsd since April now. The
>> reason for this is, as I suspect, a bug or insufficienty in the fnctl()
>> implementation on kfreebsd. In order to speed up compilation (ac++ is
>> dead slow), the compilation is run in parallel where possible. However,
>> each compilation step needs to store information about the found join
>> points and woven advice instantiation in project wide "aspect
>> repository", which needs to be guarded against concurrent updates from
>> ac++ processes. And this locking now seems to fail fatally since
>> upstream release 1.1.
>>
>> Before 1.1, AFAIUI locking didn't work either, and you ended up with a
>> corrupt aspect repository -- rending the package pretty much useless
>> IMO, but YMMV.
>>
>> This has been documented as #633402, and porters have already been asked
>> for help. Unfortunately, no solution could be found. Therefore, I have
>> to ask for an architecture specific removal in order to allow the
>> package to migrate to testing.
>
> I've just quickly checked the aspectc++ source code and found out the
> problem is in there, and as such this removal request is not warranted.
> All struct flock initializers in (at least) Puma/src/basics/SysCall.cc
> are bogus, as it assumes the layout of the struct flock whose members
> are explicitly defined as not having a specific order. The code should
> switch to use named initializers or simple member assignments [0]. I've
> not checked or tested if there's other portability issues though.
>
> [0]
>
> struct flock fl = { .l_type = A, .l_whence = B, ... };
>
> or
>
> struct flock fl;
>
> fl.l_type = A;
> fl.l_whence = B;
> ...
Thanks Guillem, you are absolutely right. As always :-)
What I find a bit irritating is that while using the same glibc version,
Debian/kFreeBSD ships a different /usr/include/bits/fnctl.h to the other
Debian architectures.
The kFreeBSD Version reads:
struct flock
{
__off_t l_start; /* Offset where the lock begins. */
__off_t l_len; /* Size of the locked area; zero means until EOF. */
__pid_t l_pid; /* Process holding the lock. */
short int l_type; /* Type of lock: F_RDLCK, F_WRLCK, or F_UNLCK. */
short int l_whence; /* Where `l_start' is relative to (like `lseek'). */
int __l_sysid; /* remote system id or zero for local */
};
While the Linux version reads:
struct flock
{
short int l_type; /* Type of lock: F_RDLCK, F_WRLCK, or F_UNLCK. */
short int l_whence; /* Where `l_start' is relative to (like `lseek'). */
#ifndef __USE_FILE_OFFSET64
__off_t l_start; /* Offset where the lock begins. */
__off_t l_len; /* Size of the locked area; zero means until EOF. */
#else
__off64_t l_start; /* Offset where the lock begins. */
__off64_t l_len; /* Size of the locked area; zero means until EOF. */
#endif
__pid_t l_pid; /* Process holding the lock. */
};
Why on earth has the kFreeBSD version of glibc decided to shuffle around
the members of the flock struct? While discussing the order in detail,
the POSIX spec [1] does mention 'struct flock' having a order of members
that matches the Linux version.
[1] http://pubs.opengroup.org/onlinepubs/009695399/basedefs/fcntl.h.html
The FreeBSD *kernel* on the other hand does not seem to obey this
ordering [2]. Obviously, glibc has to model the flock structure after
what is being passed down from the kernel. *sigh*
[2] http://fxr.watson.org/fxr/source/sys/fcntl.h
Anyways, I've come up with the following patch:
Index: SysCall.cc
===================================================================
--- SysCall.cc (revision 742)
+++ SysCall.cc (working copy)
@@ -158,7 +158,10 @@
int fd = open (file, flags, err);
#if defined(__GLIBC__)
if (!(fd < 0)) {
- flock lock = { F_WRLCK, SEEK_SET, 0, 0, 0 };
+ flock lock;
+ memset (&lock, 0, sizeof lock);
+ lock.l_type = F_WRLCK;
+ lock.l_whence = SEEK_SET;
if (fcntl (fd, F_SETLKW, &lock) == -1)
printerror (err, "fcntl lock", file);
}
@@ -186,7 +189,10 @@
int fd = create (file, mode, err);
#if defined(__GLIBC__)
if (!(fd < 0)) {
- flock lock = { F_WRLCK, SEEK_SET, 0, 0, 0 };
+ flock lock;
+ memset (&lock, 0, sizeof lock);
+ lock.l_type = F_WRLCK;
+ lock.l_whence = SEEK_SET;
if (fcntl (fd, F_SETLKW, &lock) == -1)
printerror (err, "fcntl lock", file);
}
@@ -205,7 +211,10 @@
bool SysCall::close_excl (int fd, ErrorSink *err) {
#if defined(__GLIBC__)
- flock lock = { F_UNLCK, SEEK_SET, 0, 0, 0 };
+ flock lock;
+ memset (&lock, 0, sizeof lock);
+ lock.l_type = F_UNLCK;
+ lock.l_whence = SEEK_SET;
if (fcntl (fd, F_SETLKW, &lock) == -1) {
printerror (err, "fcntl unlock", fd);
return false;
Unfortunately, using named initializers is not an option, as Puma is a
C++ project.
I've now tested that a Puma with this patch is able to boostrap itself
on the 48-core machine that I have access to here. While I did not test
this patch on kFreeBSD, I'm confident that this should unbreak it. I'm
therefore going to commit this upstream shortly.
Cheers,
Reinhard
--
Gruesse/greetings,
Reinhard Tartler, KeyID 945348A4