1812604 - time_t isn't 4 bytes on many BSDs

Status: RESOLVED FIXED

(Core :: IPC, defect, P3)

Description

[Landry Breuil (:gaston)]

from https://bugzilla.mozilla.org/show_bug.cgi?id=1766848#c9 and after https://bugzilla.mozilla.org/show_bug.cgi?id=1766848#c5

a bit late to the party, but i recently updated seamonkey to 2.53.15 on OpenBSD (which includes this bug/commit via https://gitlab.com/seamonkey-project/seamonkey-2.53-mozilla/-/commit/a178a8e5d1fed581b0ffe32cb20e70b210062946), and it fails to build on OpenBSD/i386:

/pobj/seamonkey-2.53.15/seamonkey-2.53.15/ipc/chromium/src/base/message_pump_libevent.cc:43:1: error: static_assert failed due to requirement '4 == sizeof(long long)' "bad EVENT__SIZEOF_TIME_T"

on OpenBSD, time_t is a long long on all archs, so i think https://searchfox.org/mozilla-central/source/ipc/chromium/src/third_party/libevent/bsd/event2/event-config.h#514 is wrong for OpenBSD

i'll test a local fix:

-#if defined(__LP64__)
+#if defined(__LP64__) || defined(__OpenBSD__)
#define EVENT__SIZEOF_TIME_T 8

from various feedback, time_t is also 8 bytes on 32-bits NetBSD, and at least FreeBSD/powerpc (per https://lists.freebsd.org/pipermail/freebsd-ppc/2017-June/008907.html)

low-hanging fruit as only seamonkey still builds natively on OpenBSD/i386, firefox/thunderbird need too much resources to link.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1766848

:RyanVM, since you are the author of the regressor, bug 1766848, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Comment 2

[Martin Husemann]

time_t is 64 bit on all NetBSD architectures (since 2009 or so)

Comment 3

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

It looks like the only use of EVENT__SIZEOF_TIME_T is in a test case that I don't think we're building, which might explain pkgsrc's patch which just comments out the assertions.

Comment 4

[anonymous0000007]

Hello mentor,

As mentioned above and in my research OpenBSD(v5.5 and later on) and NetBSD(v6.0 and later on) fixed time_t to 64 bit. FreeBSD and other BSD distros not mentioned yet anything on their official documentation of time_t.

Fixed should be according to me is:
#if defined(_LP64_) || defined(_OpenBSD_) || defined(_NetBSD_) || (powerpc condition)
#define EVENT__SIZEOF_TIME_T 8
#else
#define EVENT__SIZEOF_TIME_T 4

Need HELP:

1. can you help me in powerpc handling condition(should it be defined(powerpc64)?).
2. can you help me on submit patch and verify fix is correct or need to work more.

Comment 5

[anonymous0000007]

(In reply to anonymous0000007 from comment #4)

Hello mentor,

As mentioned above and in my research OpenBSD(v5.5 and later on) and NetBSD(v6.0 and later on) fixed time_t to 64 bit. FreeBSD and other BSD distros not mentioned yet anything on their official documentation of time_t.

Fixed should be according to me is:
#if defined(_LP64_) || defined(_OpenBSD_) || defined(_NetBSD_) || (powerpc condition)
#define EVENT__SIZEOF_TIME_T 8
#else
#define EVENT__SIZEOF_TIME_T 4

Need HELP:

1. can you help me in powerpc handling condition(should it be defined(powerpc64)?).
2. can you help me on submit patch and verify fix is correct or need to work more.

Hello mentor,

sorry to forgot to mentioned that i am a new comer. and i want to contribute in firefox. i'm searching good first bug and i looked into this. so can you help me as mentioned above to fix this bug?

Comment 6

[Gian-Carlo Pascutto [:gcp]]

can you help me on submit patch

Hi! You can find the documentation about this here:
https://firefox-source-docs.mozilla.org/contributing/how_to_submit_a_patch.html

verify fix is correct or need to work more.

Hmm, that's harder for us to help you with as most of us don't run OpenBSD. I wonder if landry can give pointers here.

Comment 7

[Landry Breuil (:gaston)]

i dunno for the FreeBSD/powerpc thing (jan?), but so far here's the patch the OpenBSD port for seamonkey has been using : http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/www/seamonkey/patches/patch-ipc_chromium_src_third_party_libevent_bsd_event2_event-config_h?rev=1.1&content-type=text/x-cvsweb-markup

Comment 8

[anonymous0000007]

Attachment: Bug 1812604 - time_t isn't 4 bytes on many BSDs r=jld,RyanVM

Comment 9

[Jan Beich]

(Sorry, I can't reply in Phabricator due to bug 1536716.)

I'd prefer the following instead. DragonFly only supports amd64. FreeBSD only uses 32-bit time_t on i386 (Tier1 -> Tier2 since 13.0) due to historic and ABI-compat reasons.

#if defined(__FreeBSD__) && defined(__i386___)
#define EVENT__SIZEOF_TIME_T 4
#else
#define EVENT__SIZEOF_TIME_T 8
#endif

$ cd /usr/src
$ rg -g _types.h time_t
sys/arm/include/_types.h
93:typedef      __int64_t       __time_t;               /* time()... */

sys/arm64/include/_types.h
79:typedef      __int64_t       __time_t;               /* time()... */

sys/mips/include/_types.h
122:typedef     __int64_t       __time_t;               /* time()... */

sys/powerpc/include/_types.h
112:typedef     __int64_t       __time_t;               /* time()... */

sys/riscv/include/_types.h
79:typedef      __int64_t       __time_t;               /* time()... */

sys/sparc64/include/_types.h
81:typedef      __int64_t       __time_t;               /* time()... */

sys/x86/include/_types.h
108:typedef     __int64_t       __time_t;               /* time()... */
117:typedef     __int32_t       __time_t;

See also https://www.freebsd.org/platforms/
https://lists.freebsd.org/archives/freebsd-arch/2023-April/000366.html

Comment 10

[anonymous0000007]

Hello @Jan Beich,

#if defined(__LP64__) || defined(__OpenBSD__) || defined(__NetBSD__) || (defined(__FreeBSD__) && !defined(__i386___))
#define EVENT__SIZEOF_TIME_T 8
#else
#define EVENT__SIZEOF_TIME_T 4
#endif

1. your freebsd suggestion implemented
2. dragonfly bsd uses 48bit and 32bit unsigned(for backward compatibility) - https://www.dragonflybsd.org/release56/ (no need to change in condition)

Can you please check this condition looks good to you or not?

Comment 11

[anonymous0000007]

(In reply to anonymous0000007 from comment #10)

Hello @jbeich,

#if defined(__LP64__) || defined(__OpenBSD__) || defined(__NetBSD__) || (defined(__FreeBSD__) && !defined(__i386___))
#define EVENT__SIZEOF_TIME_T 8
#else
#define EVENT__SIZEOF_TIME_T 4
#endif

1. your freebsd suggestion implemented
2. dragonfly bsd uses 48bit and 32bit unsigned(for backward compatibility) - https://www.dragonflybsd.org/release56/ (no need to change in condition)

Can you please check this condition looks good to you or not?

Comment 12

[anonymous0000007]

Hi @jbeich can you verify this from your end to resolve this bug.

/* --------------------------------------------------------
----------------------------- */
/* MOZILLA NOTE: some BSD system increased time_t size from 32-bit to 64-bit             */
/* OpenBSD https://www.openbsd.org/55.html                                               */
/* NetBSD https://man.netbsd.org/NetBSD-9.1/time.3                                       */
/* FreeBSD/powerpc https://lists.freebsd.org/pipermail/freebsd-ppc/2017-June/008907.html */
/* ------------------------------------------------------------------------------------- */
/* The size of `time_t', as computed by sizeof. */
#ifdef __LP64__
#if defined(__LP64__) || defined(__OpenBSD__) || defined(__NetBSD__) || (defined(__FreeBSD__) && !defined(__i386__))
#define EVENT__SIZEOF_TIME_T 8
#else
#define EVENT__SIZEOF_TIME_T 4
#endif

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit BugBot documentation.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Jan, any chance you can take a look at the latest patch and give it your blessing?

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Landry, can you rubberstamp this since Jan isn't responsive?

Comment 16

[Landry Breuil (:gaston)]

Given https://bugzilla.mozilla.org/show_bug.cgi?id=1812604#c9 that should be good.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/325967acc8f2
time_t isn't 4 bytes on many BSDs r=RyanVM,gaston

Comment 18

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/325967acc8f2

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Would backporting this to ESR115 be useful?

Comment 20

[Landry Breuil (:gaston)]

i've now reread the logic in this bug - as far as OpenBSD is concerned that is only relevant for seamonkey on i386, since thunderbird/firefox don't build/link anymore on this platform (we only do native builds, not cross-compiles).

And as seamonkey uses an old mozilla esr codebase with backports on top... im not sure it's worth backporting to ESR115. As for FreeBSD i have no idea if they still natively build on i386.