unofficial mirror of notmuch@notmuchmail.org
 help / color / Atom feed
* test failures on 32 bit architectures.
@ 2020-06-22 14:18 David Bremner
  2020-06-22 23:12 ` David Bremner
  0 siblings, 1 reply; 10+ messages in thread
From: David Bremner @ 2020-06-22 14:18 UTC (permalink / raw)
  To: notmuch


I know, I know, I don't use 32 bit architectures either. However...

Looking at [1], it looks like there two tests consistently failing on 32
bit architectures (and also mips64el, FWIW).

    T160-json: Testing --format=json output
     FAIL   Search message: json, 64-bit timestamp
            --- T160-json.8.expected	2020-06-22 12:29:35.053363072 +0000
            +++ T160-json.8.output	2020-06-22 12:29:35.053363072 +0000
            @@ -1,7 +1,7 @@
             [
                 {
                     "authors": "Notmuch Test Suite",
            -        "date_relative": "the future",
            +        "date_relative": "1970-01-01",
                     "matched": 1,
                     "query": [
                         "id:msg-005@notmuch-test-suite",
            @@ -13,7 +13,7 @@
                         "unread"
                     ],
                     "thread": "XXX",
            -        "timestamp": 32472187200,
            +        "timestamp": 0,
                     "total": 1
                 }
             ]

    T355-smime: Testing S/MIME signature verification and decryption
     FAIL   Verify signature on PKCS#7 SignedData message
            expires: value not equal: data[0][0][0]["crypto"]["signed"]["status"][0]["expires"] = 2145914603 != 2611032858

I haven't tried to debug yet, but the first bug looks like a timestamp
overflowing 32 bits, becoming negative, and being clamped to 0.
No idea about the second.

[1]: https://buildd.debian.org/status/package.php?p=notmuch&suite=experimental

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: test failures on 32 bit architectures.
  2020-06-22 14:18 test failures on 32 bit architectures David Bremner
@ 2020-06-22 23:12 ` David Bremner
  2020-06-24  0:39   ` David Bremner
  0 siblings, 1 reply; 10+ messages in thread
From: David Bremner @ 2020-06-22 23:12 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

> I know, I know, I don't use 32 bit architectures either. However...
>
> Looking at [1], it looks like there two tests consistently failing on 32
> bit architectures (and also mips64el, FWIW).

Hmm. Somewhere between when I sent this message and now, some upgrade in
debian unstable made these failures go away for me on i386. So maybe
I'll wait a day or so for the autobuilders to catch up, and retry the
more exotic architectures.

d

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: test failures on 32 bit architectures.
  2020-06-22 23:12 ` David Bremner
@ 2020-06-24  0:39   ` David Bremner
  2020-06-24 11:04     ` [PATCH] lib: avoid casting gint64 to time_t David Bremner
  0 siblings, 1 reply; 10+ messages in thread
From: David Bremner @ 2020-06-24  0:39 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

> David Bremner <david@tethera.net> writes:
>
>> I know, I know, I don't use 32 bit architectures either. However...
>>
>> Looking at [1], it looks like there two tests consistently failing on 32
>> bit architectures (and also mips64el, FWIW).
>
> Hmm. Somewhere between when I sent this message and now, some upgrade in
> debian unstable made these failures go away for me on i386. So maybe
> I'll wait a day or so for the autobuilders to catch up, and retry the
> more exotic architectures.

I managed to duplicate the bug on the debian i386 porterbox. Here the
timestamp is 0 in the database, so if some mangling happened, it
happened during indexing.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] lib: avoid casting gint64 to time_t
  2020-06-24  0:39   ` David Bremner
@ 2020-06-24 11:04     ` David Bremner
  2020-06-24 12:40       ` David Bremner
  0 siblings, 1 reply; 10+ messages in thread
From: David Bremner @ 2020-06-24 11:04 UTC (permalink / raw)
  To: David Bremner, notmuch

This is a partial fix for problems with 64 bit timestamps on 32 bit
architectures. In certain (not completely understood by me) cases this
casting causes 32bit overflows and yields negative timestamps for
times in the far future.

In g_mime_utils_header_decode_date_unix, there is really no reason to
cast to and from time_t.

In _notmuch_message_set_header_values, we rely on implicit casting of
the argument to Xapian::sortable_serialise to double.
---

 This is not a complete fix, but at least the timestamp ends up
 aparently correct in the database. It looks like there are still
 wonky conversions on reading the large timestamp from the database.

 lib/message.cc     | 2 +-
 util/gmime-extra.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 0fa0eb3a..948626bd 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1217,7 +1217,7 @@ _notmuch_message_set_header_values (notmuch_message_t *message,
 				    const char *from,
 				    const char *subject)
 {
-    time_t time_value;
+    gint64 time_value;
 
     /* GMime really doesn't want to see a NULL date, so protect its
      * sensibilities. */
diff --git a/util/gmime-extra.c b/util/gmime-extra.c
index 04d8ed3d..af7b6d52 100644
--- a/util/gmime-extra.c
+++ b/util/gmime-extra.c
@@ -192,7 +192,7 @@ gint64
 g_mime_utils_header_decode_date_unix (const char *date)
 {
     GDateTime *parsed_date = g_mime_utils_header_decode_date (date);
-    time_t ret;
+    gint64 ret;
 
     if (parsed_date) {
 	ret = g_date_time_to_unix (parsed_date);
-- 
2.27.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] lib: avoid casting gint64 to time_t
  2020-06-24 11:04     ` [PATCH] lib: avoid casting gint64 to time_t David Bremner
@ 2020-06-24 12:40       ` David Bremner
  2020-06-24 14:32         ` [PATCH 1/2] configure: detect 64 bit time_t David Bremner
  0 siblings, 1 reply; 10+ messages in thread
From: David Bremner @ 2020-06-24 12:40 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:
>
>  This is not a complete fix, but at least the timestamp ends up
>  aparently correct in the database. It looks like there are still
>  wonky conversions on reading the large timestamp from the database.

There is another cast that is harder to avoid:

notmuch_message_get_date casts the output of
Xapian::sortable_unserialise to time_t.

Changing that would require an API/ABI change.

There are apparently still architectures where time_t is 32bits (it
seems like these machines with failing tests are among them). For 0.30 I
suspect the best thing is to mark the tests broken on architectures with
32 bit time_t. For the future I'm not sure, but maybe we should avoid
time_t completely.

d

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] configure: detect 64 bit time_t
  2020-06-24 12:40       ` David Bremner
@ 2020-06-24 14:32         ` David Bremner
  2020-06-24 19:38           ` Tomi Ollila
                             ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: David Bremner @ 2020-06-24 14:32 UTC (permalink / raw)
  To: David Bremner, notmuch

Certain tests involving timestamps > 32bits cannot pass with the
current libnotmuch API. We will avoid this isssue for now by disabling
those tests on "old" architectures with 32bit time_t.
---
 configure | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 05ade05b..ecb0bdb7 100755
--- a/configure
+++ b/configure
@@ -1045,6 +1045,34 @@ else
 fi
 rm -f compat/have_timegm
 
+cat <<EOF > _time_t.c
+#include <time.h>
+#include <stdio.h>
+int main(int argc, char**argv) {
+  printf("%d\n",sizeof(time_t) >= 8);
+}
+EOF
+
+printf "Checking for 64 bit time_t... "
+if ${CC} _time_t.c -o _time_t > /dev/null 2>&1;
+then
+    have_64bit_time_t=$(./_time_t)
+    if [ $have_64bit_time_t -eq 1 ]
+    then
+        printf "Yes.\n"
+    else
+        printf "No.\n"
+    fi
+else
+    cat <<EOF
+
+*** Error: compilation failed.  Please try running configure again in
+a clean environment, and if the problem persists, report a bug.
+EOF
+    rm -f _time_t _time_t.c exit 1
+fi
+
+
 printf "Checking for dirent.d_type... "
 if ${CC} -o compat/have_d_type "$srcdir"/compat/have_d_type.c > /dev/null 2>&1
 then
@@ -1128,7 +1156,7 @@ for flag in -Wmissing-declarations; do
 done
 printf "\n\t%s\n" "${WARN_CFLAGS}"
 
-rm -f minimal minimal.c _libversion.c _libversion _libversion.sh _check_session_keys.c _check_session_keys _check_x509_validity.c _check_x509_validity
+rm -f minimal minimal.c _time_t _time_t.c _libversion.c _libversion _libversion.sh _check_session_keys.c _check_session_keys _check_x509_validity.c _check_x509_validity
 
 # construct the Makefile.config
 cat > Makefile.config <<EOF
@@ -1429,6 +1457,9 @@ NOTMUCH_HAVE_MAN=$((have_sphinx))
 NOTMUCH_HAVE_BASH=${have_bash}
 NOTMUCH_BASH_ABSOLUTE=${bash_absolute}
 
+# Whether time_t is 64 bits (or more)
+NOTMUCH_HAVE_64BIT_TIME_T=${have_64bit_time_t}
+
 # Whether perl exists, and if so where
 NOTMUCH_HAVE_PERL=${have_perl}
 NOTMUCH_PERL_ABSOLUTE=${perl_absolute}
-- 
2.27.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] test: mark two tests broken on machines with 32 bit time_t
       [not found]           ` <20200624143234.540526-2-david@tethera.net>
@ 2020-06-24 14:32             ` David Bremner
  2020-06-27  1:25             ` David Bremner
  1 sibling, 0 replies; 10+ messages in thread
From: David Bremner @ 2020-06-24 14:32 UTC (permalink / raw)
  To: David Bremner, notmuch

I haven't traced the code path as exhaustively for the SMIME test, but
the expiry date in question is larger then representable in a signed
32 bit integer.
---
 test/T160-json.sh  | 3 +++
 test/T355-smime.sh | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/test/T160-json.sh b/test/T160-json.sh
index d975efa7..e8b75605 100755
--- a/test/T160-json.sh
+++ b/test/T160-json.sh
@@ -65,6 +65,9 @@ test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
  \"unread\"]}]"
 
 test_begin_subtest "Search message: json, 64-bit timestamp"
+if [ $NOTMUCH_HAVE_64BIT_TIME_T -ne 1 ]; then
+    test_subtest_known_broken
+fi
 add_message "[subject]=\"json-search-64bit-timestamp-subject\"" "[date]=\"Tue, 01 Jan 2999 12:00:00 -0000\"" "[body]=\"json-search-64bit-timestamp-message\""
 output=$(notmuch search --format=json "json-search-64bit-timestamp-message" | notmuch_search_sanitize)
 test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index 170f8649..f8cec62c 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -176,6 +176,9 @@ output=$(notmuch show --format=json id:smime-onepart-signed@protected-headers.ex
 test_valid_json "$output"
 
 test_begin_subtest "Verify signature on PKCS#7 SignedData message"
+if [ $NOTMUCH_HAVE_64BIT_TIME_T -ne 1 ]; then
+    test_subtest_known_broken
+fi
 output=$(notmuch show --format=json id:smime-onepart-signed@protected-headers.example)
 
 test_json_nodes <<<"$output" \
-- 
2.27.0

_______________________________________________
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] configure: detect 64 bit time_t
  2020-06-24 14:32         ` [PATCH 1/2] configure: detect 64 bit time_t David Bremner
@ 2020-06-24 19:38           ` Tomi Ollila
  2020-06-27  1:25           ` David Bremner
       [not found]           ` <20200624143234.540526-2-david@tethera.net>
  2 siblings, 0 replies; 10+ messages in thread
From: Tomi Ollila @ 2020-06-24 19:38 UTC (permalink / raw)
  To: David Bremner, notmuch

On Wed, Jun 24 2020, David Bremner wrote:

> Certain tests involving timestamps > 32bits cannot pass with the
> current libnotmuch API. We will avoid this isssue for now by disabling

s/isssue/issue/

> those tests on "old" architectures with 32bit time_t.
> ---
>  configure | 33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 05ade05b..ecb0bdb7 100755
> --- a/configure
> +++ b/configure
> @@ -1045,6 +1045,34 @@ else
>  fi
>  rm -f compat/have_timegm
>  
> +cat <<EOF > _time_t.c
> +#include <time.h>
> +#include <stdio.h>
> +int main(int argc, char**argv) {
> +  printf("%d\n",sizeof(time_t) >= 8);

Since notmuch now (since fall 2018) requires c11 compiler, static_assert
would be simpler alternative:

    #include <assert.h>
    #include <time.h>

    static_assert(sizeof(time_t) >= 8, "sizeof(time_t) < 8");

and

    if ${CC} -c _time_t.c -o /dev/null > /dev/null 2>&1
    then 
         printf "Yes.\n"
         have_64bit_time_t=1
    else 
         printf "No.\n"
         have_64bit_time_t=0
    fi

but if not... 

> +}
> +EOF
> +
> +printf "Checking for 64 bit time_t... "
> +if ${CC} _time_t.c -o _time_t > /dev/null 2>&1;
> +then
> +    have_64bit_time_t=$(./_time_t)

anyway would be better for _time_t just exit 0 or 1 and
set have_64bit_time_t as (inverse =D) of that (like above),
... but still, static_assert FTW \0/ ;)

> +    if [ $have_64bit_time_t -eq 1 ]
> +    then
> +        printf "Yes.\n"
> +    else
> +        printf "No.\n"
> +    fi
> +else
> +    cat <<EOF
> +
> +*** Error: compilation failed.  Please try running configure again in
> +a clean environment, and if the problem persists, report a bug.
> +EOF
> +    rm -f _time_t _time_t.c exit 1
> +fi
> +
> +
>  printf "Checking for dirent.d_type... "
>  if ${CC} -o compat/have_d_type "$srcdir"/compat/have_d_type.c > /dev/null 2>&1
>  then
> @@ -1128,7 +1156,7 @@ for flag in -Wmissing-declarations; do
>  done
>  printf "\n\t%s\n" "${WARN_CFLAGS}"
>  
> -rm -f minimal minimal.c _libversion.c _libversion _libversion.sh _check_session_keys.c _check_session_keys _check_x509_validity.c _check_x509_validity
> +rm -f minimal minimal.c _time_t _time_t.c _libversion.c _libversion _libversion.sh _check_session_keys.c _check_session_keys _check_x509_validity.c _check_x509_validity
>  
>  # construct the Makefile.config
>  cat > Makefile.config <<EOF
> @@ -1429,6 +1457,9 @@ NOTMUCH_HAVE_MAN=$((have_sphinx))
>  NOTMUCH_HAVE_BASH=${have_bash}
>  NOTMUCH_BASH_ABSOLUTE=${bash_absolute}
>  
> +# Whether time_t is 64 bits (or more)
> +NOTMUCH_HAVE_64BIT_TIME_T=${have_64bit_time_t}
> +
>  # Whether perl exists, and if so where
>  NOTMUCH_HAVE_PERL=${have_perl}
>  NOTMUCH_PERL_ABSOLUTE=${perl_absolute}
> -- 
> 2.27.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] configure: detect 64 bit time_t
  2020-06-24 14:32         ` [PATCH 1/2] configure: detect 64 bit time_t David Bremner
  2020-06-24 19:38           ` Tomi Ollila
@ 2020-06-27  1:25           ` David Bremner
       [not found]           ` <20200624143234.540526-2-david@tethera.net>
  2 siblings, 0 replies; 10+ messages in thread
From: David Bremner @ 2020-06-27  1:25 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

> Certain tests involving timestamps > 32bits cannot pass with the
> current libnotmuch API. We will avoid this isssue for now by disabling
> those tests on "old" architectures with 32bit time_t.

pushed an updated version based on Tomi's comments

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] test: mark two tests broken on machines with 32 bit time_t
       [not found]           ` <20200624143234.540526-2-david@tethera.net>
  2020-06-24 14:32             ` [PATCH 2/2] test: mark two tests broken on machines with 32 " David Bremner
@ 2020-06-27  1:25             ` David Bremner
  1 sibling, 0 replies; 10+ messages in thread
From: David Bremner @ 2020-06-27  1:25 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

> I haven't traced the code path as exhaustively for the SMIME test, but
> the expiry date in question is larger then representable in a signed
> 32 bit integer.

pushed

d

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-22 14:18 test failures on 32 bit architectures David Bremner
2020-06-22 23:12 ` David Bremner
2020-06-24  0:39   ` David Bremner
2020-06-24 11:04     ` [PATCH] lib: avoid casting gint64 to time_t David Bremner
2020-06-24 12:40       ` David Bremner
2020-06-24 14:32         ` [PATCH 1/2] configure: detect 64 bit time_t David Bremner
2020-06-24 19:38           ` Tomi Ollila
2020-06-27  1:25           ` David Bremner
     [not found]           ` <20200624143234.540526-2-david@tethera.net>
2020-06-24 14:32             ` [PATCH 2/2] test: mark two tests broken on machines with 32 " David Bremner
2020-06-27  1:25             ` David Bremner

unofficial mirror of notmuch@notmuchmail.org

Archives are clonable:
	git clone --mirror https://yhetil.org/notmuch/0 notmuch/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 notmuch notmuch/ https://yhetil.org/notmuch \
		notmuch@notmuchmail.org
	public-inbox-index notmuch

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.mail.notmuch.general
	nntp://news.gmane.io/gmane.mail.notmuch.general


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git