test failures on 32 bit architectures.

test failures on 32 bit architectures.

[David Bremner]

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

Re: test failures on 32 bit architectures.

[David Bremner]

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

Re: test failures on 32 bit architectures.

[David Bremner]

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.

[PATCH] lib: avoid casting gint64 to time_t

[David Bremner]

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

Re: [PATCH] lib: avoid casting gint64 to time_t

[David Bremner]

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

[PATCH 1/2] configure: detect 64 bit time_t

[David Bremner]

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

[PATCH 2/2] test: mark two tests broken on machines with 32 bit time_t

[David Bremner]

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

Re: [PATCH 2/2] test: mark two tests broken on machines with 32 bit time_t

[David Bremner]

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

Re: [PATCH 1/2] configure: detect 64 bit time_t

[Tomi Ollila]

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

Re: [PATCH 1/2] configure: detect 64 bit time_t

[David Bremner]

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