Bug#921717: cyrus-imapd: FTBFS on arm*, i386, mipsel, ppc64el and s390x
Hi Xavier! On 5/12/20 10:09 AM, Xavier wrote: >>> The problem here is va_list. On some architectures, the calling convention >>> doesn't seem to allow comparing va_list against NULL due to the way va_list >>> is implemented on a particular architecture. >> Correction: The va_list problem seems to affect ARM architectures only. >> >> The other architectures have testsuite failures which seem unrelated. >> >> Adrian > > Thanks a lot! I had to modify your patch: I had to declare a va_list > "noargs" variable to fix the problem. Glad it works and thanks for the additional modifications and for testing it. It was actually a shot in the dark yesterday shortly before I went to bed :-). I'll have a look whether I can help with the testsuite failures as well in the evening or tomorrow. Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Bug#921717: cyrus-imapd: FTBFS on arm*, i386, mipsel, ppc64el and s390x
Le 12/05/2020 à 01:04, John Paul Adrian Glaubitz a écrit :
> On 5/12/20 1:01 AM, John Paul Adrian Glaubitz wrote:
>> On 5/11/20 11:56 PM, Xavier wrote:
>>> Could someone help us here ? I forwarded this bug to upstream ([1]) but
>>> didn't receive any response for now.
>>>
>>> (Cc to RFH bug)
>>
>> The problem here is va_list. On some architectures, the calling convention
>> doesn't seem to allow comparing va_list against NULL due to the way va_list
>> is implemented on a particular architecture.
> Correction: The va_list problem seems to affect ARM architectures only.
>
> The other architectures have testsuite failures which seem unrelated.
>
> Adrian
Thanks a lot! I had to modify your patch: I had to declare a va_list
"noargs" variable to fix the problem.
Cheers,
Xavier
Description: fix for non x86 arch
The problem here is va_list. On some architectures, the calling convention
doesn't seem to allow comparing va_list against NULL due to the way va_list
is implemented on a particular architecture.
Author: John Paul Adrian Glaubitz
Bug: https://github.com/cyrusimap/cyrus-imapd/issues/3040
Bug-Debian: https://bugs.debian.org/960263
Forwarded: https://github.com/cyrusimap/cyrus-imapd/issues/3040
Reviewed-By: Xavier Guimard
Last-Update: 2020-05-12
--- a/imap/httpd.c
+++ b/imap/httpd.c
@@ -2350,7 +2350,7 @@
simple_hdr(txn, "Access-Control-Expose-Headers", hdr)
static void comma_list_body(struct buf *buf,
-const char *vals[], unsigned flags, va_list args)
+const char *vals[], unsigned flags, int has_args, va_list args)
{
const char *sep = "";
int i;
@@ -2358,11 +2358,11 @@
for (i = 0; vals[i]; i++) {
if (flags & (1 << i)) {
buf_appendcstr(buf, sep);
-if (args) buf_vprintf(buf, vals[i], args);
+if (has_args) buf_vprintf(buf, vals[i], args);
else buf_appendcstr(buf, vals[i]);
sep = ", ";
}
-else if (args) {
+else if (has_args) {
/* discard any unused args */
vsnprintf(NULL, 0, vals[i], args);
}
@@ -2377,7 +2377,7 @@
va_start(args, flags);
-comma_list_body(, vals, flags, args);
+comma_list_body(, vals, flags, 1, args);
va_end(args);
@@ -2512,6 +2512,7 @@
int i;
time_t now;
char datestr[30];
+va_list noargs;
double cmdtime, nettime;
const char **hdr, *sep;
struct auth_challenge_t *auth_chal = >auth_chal;
@@ -3077,17 +3078,17 @@
}
if (code == HTTP_SWITCH_PROT || code == HTTP_UPGRADE) {
buf_printf(logbuf, "%supgrade=", sep);
-comma_list_body(logbuf, upgrd_tokens, txn->flags.upgrade, NULL);
+comma_list_body(logbuf, upgrd_tokens, txn->flags.upgrade, 0, noargs);
sep = "; ";
}
if (txn->flags.te) {
buf_printf(logbuf, "%stx-encoding=", sep);
-comma_list_body(logbuf, te, txn->flags.te, NULL);
+comma_list_body(logbuf, te, txn->flags.te, 0, noargs);
sep = "; ";
}
if (txn->resp_body.enc.proc) {
buf_printf(logbuf, "%scnt-encoding=", sep);
-comma_list_body(logbuf, ce, txn->resp_body.enc.type, NULL);
+comma_list_body(logbuf, ce, txn->resp_body.enc.type, 0, noargs);
sep = "; ";
}
if (txn->location) {
Bug#921717: cyrus-imapd: FTBFS on arm*, i386, mipsel, ppc64el and s390x
On 2020-05-12 01:04 +0200, John Paul Adrian Glaubitz wrote: > On 5/12/20 1:01 AM, John Paul Adrian Glaubitz wrote: > > On 5/11/20 11:56 PM, Xavier wrote: > >> Could someone help us here ? I forwarded this bug to upstream ([1]) but > >> didn't receive any response for now. > >> > >> (Cc to RFH bug) > > > > The problem here is va_list. On some architectures, the calling convention > > doesn't seem to allow comparing va_list against NULL due to the way va_list > > is implemented on a particular architecture. > Correction: The va_list problem seems to affect ARM architectures only. This is a classic 'porting to arm' issue which used to catch people out regularly 15 years ago because it was something where you could do technically incorrect things that worked fine on other architectures. It's a very long time since I saw something hit this issue. Thanks for the patch Adrian. Wookey -- Principal hats: Linaro, Debian, Wookware, ARM http://wookware.org/ signature.asc Description: PGP signature
Bug#921717: cyrus-imapd: FTBFS on arm*, i386, mipsel, ppc64el and s390x
On Tue, May 12, 2020 at 01:04:06AM +0200, John Paul Adrian Glaubitz wrote: > On 5/12/20 1:01 AM, John Paul Adrian Glaubitz wrote: > > On 5/11/20 11:56 PM, Xavier wrote: > >> Could someone help us here ? I forwarded this bug to upstream ([1]) but > >> didn't receive any response for now. > >> > >> (Cc to RFH bug) > > > > The problem here is va_list. On some architectures, the calling convention > > doesn't seem to allow comparing va_list against NULL due to the way va_list > > is implemented on a particular architecture. > Correction: The va_list problem seems to affect ARM architectures only According to the standard va_list is a complete object type. There is therefore no guarantee it can be compared to NULL and it is only by accident (implementation detail) that is possible to do so on some architectures. Cheers, Michael.
Bug#921717: cyrus-imapd: FTBFS on arm*, i386, mipsel, ppc64el and s390x
On 5/12/20 1:01 AM, John Paul Adrian Glaubitz wrote: > On 5/11/20 11:56 PM, Xavier wrote: >> Could someone help us here ? I forwarded this bug to upstream ([1]) but >> didn't receive any response for now. >> >> (Cc to RFH bug) > > The problem here is va_list. On some architectures, the calling convention > doesn't seem to allow comparing va_list against NULL due to the way va_list > is implemented on a particular architecture. Correction: The va_list problem seems to affect ARM architectures only. The other architectures have testsuite failures which seem unrelated. Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Bug#921717: cyrus-imapd: FTBFS on arm*, i386, mipsel, ppc64el and s390x
Hi Xavier!
On 5/11/20 11:56 PM, Xavier wrote:
> Could someone help us here ? I forwarded this bug to upstream ([1]) but
> didn't receive any response for now.
>
> (Cc to RFH bug)
The problem here is va_list. On some architectures, the calling convention
doesn't seem to allow comparing va_list against NULL due to the way va_list
is implemented on a particular architecture.
You could maybe add an auxiliary boolean variable to control whether
"va_list args" is empty or not, i.e. something like this:
Index: cyrus-imapd-3.2.0/imap/httpd.c
===
--- cyrus-imapd-3.2.0.orig/imap/httpd.c
+++ cyrus-imapd-3.2.0/imap/httpd.c
@@ -2350,7 +2350,7 @@ EXPORTED void simple_hdr(struct transact
simple_hdr(txn, "Access-Control-Expose-Headers", hdr)
static void comma_list_body(struct buf *buf,
-const char *vals[], unsigned flags, va_list args)
+const char *vals[], unsigned flags, int has_args,
va_list args)
{
const char *sep = "";
int i;
@@ -2358,11 +2358,11 @@ static void comma_list_body(struct buf *
for (i = 0; vals[i]; i++) {
if (flags & (1 << i)) {
buf_appendcstr(buf, sep);
-if (args) buf_vprintf(buf, vals[i], args);
+if (has_args) buf_vprintf(buf, vals[i], args);
else buf_appendcstr(buf, vals[i]);
sep = ", ";
}
-else if (args) {
+else if (has_args) {
/* discard any unused args */
vsnprintf(NULL, 0, vals[i], args);
}
@@ -2377,7 +2377,7 @@ EXPORTED void comma_list_hdr(struct tran
va_start(args, flags);
-comma_list_body(, vals, flags, args);
+comma_list_body(, vals, flags, 1, args);
va_end(args);
@@ -3077,17 +3077,17 @@ EXPORTED void response_header(long code,
}
if (code == HTTP_SWITCH_PROT || code == HTTP_UPGRADE) {
buf_printf(logbuf, "%supgrade=", sep);
-comma_list_body(logbuf, upgrd_tokens, txn->flags.upgrade, NULL);
+comma_list_body(logbuf, upgrd_tokens, txn->flags.upgrade, 0, 0);
sep = "; ";
}
if (txn->flags.te) {
buf_printf(logbuf, "%stx-encoding=", sep);
-comma_list_body(logbuf, te, txn->flags.te, NULL);
+comma_list_body(logbuf, te, txn->flags.te, 0, 0);
sep = "; ";
}
if (txn->resp_body.enc.proc) {
buf_printf(logbuf, "%scnt-encoding=", sep);
-comma_list_body(logbuf, ce, txn->resp_body.enc.type, NULL);
+comma_list_body(logbuf, ce, txn->resp_body.enc.type, 0, 0);
sep = "; ";
}
if (txn->location) {
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - glaub...@debian.org
`. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
`-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Description: Don't try to compare va_list against NULL
Author: John Paul Adrian Glaubitz
Last-Update: 2020-05-12
Index: cyrus-imapd-3.2.0/imap/httpd.c
===
--- cyrus-imapd-3.2.0.orig/imap/httpd.c
+++ cyrus-imapd-3.2.0/imap/httpd.c
@@ -2350,7 +2350,7 @@ EXPORTED void simple_hdr(struct transact
simple_hdr(txn, "Access-Control-Expose-Headers", hdr)
static void comma_list_body(struct buf *buf,
-const char *vals[], unsigned flags, va_list args)
+const char *vals[], unsigned flags, int has_args, va_list args)
{
const char *sep = "";
int i;
@@ -2358,11 +2358,11 @@ static void comma_list_body(struct buf *
for (i = 0; vals[i]; i++) {
if (flags & (1 << i)) {
buf_appendcstr(buf, sep);
-if (args) buf_vprintf(buf, vals[i], args);
+if (has_args) buf_vprintf(buf, vals[i], args);
else buf_appendcstr(buf, vals[i]);
sep = ", ";
}
-else if (args) {
+else if (has_args) {
/* discard any unused args */
vsnprintf(NULL, 0, vals[i], args);
}
@@ -2377,7 +2377,7 @@ EXPORTED void comma_list_hdr(struct tran
va_start(args, flags);
-comma_list_body(, vals, flags, args);
+comma_list_body(, vals, flags, 1, args);
va_end(args);
@@ -3077,17 +3077,17 @@ EXPORTED void response_header(long code,
}
if (code == HTTP_SWITCH_PROT || code == HTTP_UPGRADE) {
buf_printf(logbuf, "%supgrade=", sep);
-comma_list_body(logbuf, upgrd_tokens, txn->flags.upgrade, NULL);
+comma_list_body(logbuf, upgrd_tokens, txn->flags.upgrade, 0, 0);
sep = "; ";
}
if (txn->flags.te) {
buf_printf(logbuf, "%stx-encoding=", sep);
-comma_list_body(logbuf, te, txn->flags.te, NULL);
+comma_list_body(logbuf, te, txn->flags.te, 0, 0);
sep = "; ";
}
if (txn->resp_body.enc.proc) {
buf_printf(logbuf, "%scnt-encoding=", sep);
-
Bug#921717: cyrus-imapd: FTBFS on arm*, i386, mipsel, ppc64el and s390x
> cyrus-imapd failed to build on almost all architectures. See > https://buildd.debian.org/status/fetch.php?pkg=cyrus-imapd=i386=3.2.0-1=1588581560=0 > and > https://buildd.debian.org/status/fetch.php?pkg=cyrus-imapd=arm64=3.2.0-1=1588581432=0 > for examples. Hi, Could someone help us here ? I forwarded this bug to upstream ([1]) but didn't receive any response for now. (Cc to RFH bug) Cheers, Xavier [1]: https://github.com/cyrusimap/cyrus-imapd/issues/3040
