Avoid reverse dependencies in JS library code handling time. NFC (#16118)

Specifically this change removes `_get_daylight`, `_get_daylight`
and `_get_tzname` which are native symbols that were used by
`tzset` which has a lot of different call sites.

This made reverse dependencies on these symbols fairly common
and easy to miss.  See #15958 for a recent example.

This change removes these functions completely in favor or just
passing these addresses in as arguments.

Fixes: #15958

Author: sbc100

src/library.js

@@ -464,10 +464,8 @@ LibraryManager.library = {
     return time1 - time0;
   },
 
-  mktime__deps: ['tzset'],
-  mktime__sig: 'ii',
-  mktime: function(tmPtr) {
-    _tzset();
+  _mktime_js__sig: 'ii',
+  _mktime_js: function(tmPtr) {
     var date = new Date({{{ makeGetValue('tmPtr', C_STRUCTS.tm.tm_year, 'i32') }}} + 1900,
                         {{{ makeGetValue('tmPtr', C_STRUCTS.tm.tm_mon, 'i32') }}},
                         {{{ makeGetValue('tmPtr', C_STRUCTS.tm.tm_mday, 'i32') }}},
@@ -507,7 +505,6 @@ LibraryManager.library = {
 
     return (date.getTime() / 1000)|0;
   },
-  timelocal: 'mktime',
 
   _gmtime_js__sig: 'iii',
   _gmtime_js: function(time, tmPtr) {
@@ -524,10 +521,8 @@ LibraryManager.library = {
     {{{ makeSetValue('tmPtr', C_STRUCTS.tm.tm_yday, 'yday', 'i32') }}};
   },
 
-  timegm__deps: ['tzset'],
-  timegm__sig: 'ii',
-  timegm: function(tmPtr) {
-    _tzset();
+  _timegm_js__sig: 'ii',
+  _timegm_js: function(tmPtr) {
     var time = Date.UTC({{{ makeGetValue('tmPtr', C_STRUCTS.tm.tm_year, 'i32') }}} + 1900,
                         {{{ makeGetValue('tmPtr', C_STRUCTS.tm.tm_mon, 'i32') }}},
                         {{{ makeGetValue('tmPtr', C_STRUCTS.tm.tm_mday, 'i32') }}},
@@ -545,10 +540,8 @@ LibraryManager.library = {
     return (date.getTime() / 1000)|0;
   },
 
-  localtime_r__deps: ['tzset'],
-  localtime_r__sig: 'iii',
-  localtime_r: function(time, tmPtr) {
-    _tzset();
+  _localtime_js__sig: 'iii',
+  _localtime_js: function(time, tmPtr) {
     var date = new Date({{{ makeGetValue('time', 0, 'i32') }}}*1000);
     {{{ makeSetValue('tmPtr', C_STRUCTS.tm.tm_sec, 'date.getSeconds()', 'i32') }}};
     {{{ makeSetValue('tmPtr', C_STRUCTS.tm.tm_min, 'date.getMinutes()', 'i32') }}};
@@ -568,16 +561,9 @@ LibraryManager.library = {
     var winterOffset = start.getTimezoneOffset();
     var dst = (summerOffset != winterOffset && date.getTimezoneOffset() == Math.min(winterOffset, summerOffset))|0;
     {{{ makeSetValue('tmPtr', C_STRUCTS.tm.tm_isdst, 'dst', 'i32') }}};
-
-    var zonePtr = {{{ makeGetValue('__get_tzname()', 'dst ? ' + Runtime.POINTER_SIZE + ' : 0', 'i32') }}};
-    {{{ makeSetValue('tmPtr', C_STRUCTS.tm.tm_zone, 'zonePtr', 'i32') }}};
-
-    return tmPtr;
   },
-  __localtime_r: 'localtime_r',
 
   // musl-internal function used to implement both `asctime` and `asctime_r`
-  __asctime_r__deps: ['mktime'],
   __asctime_r__sig: 'iii',
   __asctime_r: function(tmPtr, buf) {
     var date = {
@@ -622,23 +608,24 @@ LibraryManager.library = {
 
   // TODO: Initialize these to defaults on startup from system settings.
   // Note: glibc has one fewer underscore for all of these. Also used in other related functions (timegm)
-  tzset__deps: ['tzset_impl'],
-  tzset__sig: 'v',
-  tzset: function() {
+  _tzset_js__deps: ['tzset_impl'],
+  _tzset_js__sig: 'viii',
+  _tzset_js: function(timezone, daylight, tzname) {
     // TODO: Use (malleable) environment variables instead of system settings.
-    if (_tzset.called) return;
-    _tzset.called = true;
-    _tzset_impl();
+    if (__tzset_js.called) return;
+    __tzset_js.called = true;
+    _tzset_impl(timezone, daylight, tzname);
   },
 
+  tzset_impl__internal: true,
   tzset_impl__proxy: 'sync',
-  tzset_impl__sig: 'v',
-  tzset_impl__deps: ['_get_daylight', '_get_timezone', '_get_tzname',
+  tzset_impl__sig: 'viii',
+  tzset_impl__deps: [
 #if MINIMAL_RUNTIME
     '$allocateUTF8'
 #endif
   ],
-  tzset_impl: function() {
+  tzset_impl: function(timezone, daylight, tzname) {
     var currentYear = new Date().getFullYear();
     var winter = new Date(currentYear, 0, 1);
     var summer = new Date(currentYear, 6, 1);
@@ -655,9 +642,9 @@ LibraryManager.library = {
     // Coordinated Universal Time (UTC) and local standard time."), the same
     // as returned by stdTimezoneOffset.
     // See http://pubs.opengroup.org/onlinepubs/009695399/functions/tzset.html
-    {{{ makeSetValue('__get_timezone()', '0', 'stdTimezoneOffset * 60', 'i32') }}};
+    {{{ makeSetValue('timezone', '0', 'stdTimezoneOffset * 60', 'i32') }}};
 
-    {{{ makeSetValue('__get_daylight()', '0', 'Number(winterOffset != summerOffset)', 'i32') }}};
+    {{{ makeSetValue('daylight', '0', 'Number(winterOffset != summerOffset)', 'i32') }}};
 
     function extractZone(date) {
       var match = date.toTimeString().match(/\(([A-Za-z ]+)\)$/);
@@ -669,11 +656,11 @@ LibraryManager.library = {
     var summerNamePtr = allocateUTF8(summerName);
     if (summerOffset < winterOffset) {
       // Northern hemisphere
-      {{{ makeSetValue('__get_tzname()', '0', 'winterNamePtr', 'i32') }}};
-      {{{ makeSetValue('__get_tzname()', Runtime.POINTER_SIZE, 'summerNamePtr', 'i32') }}};
+      {{{ makeSetValue('tzname', '0', 'winterNamePtr', POINTER_TYPE) }}};
+      {{{ makeSetValue('tzname', Runtime.POINTER_SIZE, 'summerNamePtr', POINTER_TYPE) }}};
     } else {
-      {{{ makeSetValue('__get_tzname()', '0', 'summerNamePtr', 'i32') }}};
-      {{{ makeSetValue('__get_tzname()', Runtime.POINTER_SIZE, 'winterNamePtr', 'i32') }}};
+      {{{ makeSetValue('tzname', '0', 'summerNamePtr', POINTER_TYPE) }}};
+      {{{ makeSetValue('tzname', Runtime.POINTER_SIZE, 'winterNamePtr', POINTER_TYPE) }}};
     }
   },
 

system/lib/libc/emscripten_time.c

@@ -0,0 +1,61 @@
+/*
+ * Copyright 2022 The Emscripten Authors.  All rights reserved.
+ * Emscripten is available under two separate licenses, the MIT license and the
+ * University of Illinois/NCSA Open Source License.  Both these licenses can be
+ * found in the LICENSE file.
+ */
+
+#include <time.h>
+#include <stdbool.h>
+#include "libc.h"
+
+// Replaces musl's __tz.c
+
+__attribute__((__weak__)) long  timezone = 0;
+__attribute__((__weak__)) int   daylight = 0;
+__attribute__((__weak__)) char *tzname[2] = { 0, 0 };
+
+void _tzset_js(long* timezone, int* daylight, char** tzname);
+time_t _timegm_js(struct tm *tm);
+time_t _mktime_js(struct tm *tm);
+void _localtime_js(const time_t *restrict t, struct tm *restrict tm);
+void _gmtime_js(const time_t *restrict t, struct tm *restrict tm);
+
+__attribute__((__weak__))
+void tzset() {
+  _tzset_js(&timezone, &daylight, tzname);
+}
+
+__attribute__((__weak__))
+time_t timegm(struct tm *tm) {
+  tzset();
+  return _timegm_js(tm);
+}
+
+__attribute__((__weak__))
+time_t mktime(struct tm *tm) {
+  tzset();
+  return _mktime_js(tm);
+}
+
+__attribute__((__weak__))
+struct tm *__localtime_r(const time_t *restrict t, struct tm *restrict tm) {
+  tzset();
+  _localtime_js(t, tm);
+  // __localtime_js sets everything but the tmzone pointer
+  tm->__tm_zone = tm->tm_isdst ? tzname[1] :tzname[0];
+  return tm;
+}
+
+__attribute__((__weak__))
+struct tm *__gmtime_r(const time_t *restrict t, struct tm *restrict tm) {
+  tzset();
+  _gmtime_js(t, tm);
+  tm->tm_isdst = 0;
+  tm->__tm_gmtoff = 0;
+  tm->__tm_zone = "GMT";
+  return tm;
+}
+
+weak_alias(__gmtime_r, gmtime_r);
+weak_alias(__localtime_r, localtime_r);

system/lib/libc/extras.c

@@ -5,26 +5,6 @@
  * found in the LICENSE file.
  */
 
-// Extra libc helper functions
-
-char *tzname[2];
-
-void* _get_tzname() {
-  return (void*)tzname;
-}
-
-int daylight;
-
-int* _get_daylight() {
-  return &daylight;
-}
-
-long timezone;
-
-long* _get_timezone() {
-  return &timezone;
-}
-
 // Musl lock internals. As we assume wasi is single-threaded for now, these
 // are no-ops.
 

system/lib/libc/musl/src/time/gmtime_r.c

@@ -1,26 +1,15 @@
 #include "time_impl.h"
 #include <errno.h>
 
-#ifdef __EMSCRIPTEN__
-void _gmtime_js(const time_t *restrict t, struct tm *restrict tm);
-#endif
-
 struct tm *__gmtime_r(const time_t *restrict t, struct tm *restrict tm)
 {
-#ifndef __EMSCRIPTEN__
 	if (__secs_to_tm(*t, tm) < 0) {
 		errno = EOVERFLOW;
 		return 0;
 	}
 	tm->tm_isdst = 0;
 	tm->__tm_gmtoff = 0;
 	tm->__tm_zone = __utc;
-#else
-	_gmtime_js(t, tm);
-	tm->tm_isdst = 0;
-	tm->__tm_gmtoff = 0;
-	tm->__tm_zone = "GMT";
-#endif
 	return tm;
 }
 

tests/test_other.py

@@ -1187,29 +1187,30 @@ def test_export_all(self):
 
   def test_export_all_and_exported_functions(self):
     # EXPORT_ALL should not export library functions by default.
-    # This mans that to export library function you also need to explicitly
+    # This means that to export library function you also need to explicitly
     # list them in EXPORTED_FUNCTIONS.
     lib = r'''
       #include <stdio.h>
       #include <emscripten.h>
       EMSCRIPTEN_KEEPALIVE void libfunc() { puts("libfunc\n"); }
+      void libfunc2() { puts("libfunc2\n"); }
     '''
     create_file('lib.c', lib)
     create_file('main.js', '''
       var Module = {
         onRuntimeInitialized: function() {
           _libfunc();
-          __get_daylight();
+          _libfunc2();
         }
       };
     ''')
 
-    # __get_daylight should not be linked by default, even with EXPORT_ALL
+    # libfunc2 should not be linked by default, even with EXPORT_ALL
     self.emcc('lib.c', ['-sEXPORT_ALL', '--pre-js', 'main.js'], output_filename='a.out.js')
     err = self.run_js('a.out.js', assert_returncode=NON_ZERO)
-    self.assertContained('__get_daylight is not defined', err)
+    self.assertContained('_libfunc2 is not defined', err)
 
-    self.emcc('lib.c', ['-sEXPORTED_FUNCTIONS=__get_daylight', '-sEXPORT_ALL', '--pre-js', 'main.js'], output_filename='a.out.js')
+    self.emcc('lib.c', ['-sEXPORTED_FUNCTIONS=_libfunc2', '-sEXPORT_ALL', '--pre-js', 'main.js'], output_filename='a.out.js')
     self.assertContained('libfunc\n', self.run_js('a.out.js'))
 
   def test_stdin(self):

tools/deps_info.py

@@ -73,8 +73,9 @@
   'alcGetString': ['malloc'],
   'bind': ['ntohs', 'htons'],
   'connect': ['ntohs', 'htons'],
-  'ctime': ['_get_tzname', '_get_daylight', '_get_timezone', 'malloc'],
-  'ctime_r': ['_get_tzname', '_get_daylight', '_get_timezone', 'malloc'],
+  'ctime': ['malloc'],
+  'gmtime': ['malloc'],
+  'ctime_r': ['malloc'],
   'dladdr': ['malloc'],
   'dlopen': ['__dl_seterr'],
   'dlsym': ['__dl_seterr'],
@@ -167,9 +168,9 @@
   'glfwInit': ['malloc', 'free'],
   'glfwSleep': ['sleep'],
   'glfwGetMonitors': ['malloc'],
-  'localtime': ['_get_tzname', '_get_daylight', '_get_timezone', 'malloc'],
-  'localtime_r': ['_get_tzname', '_get_daylight', '_get_timezone', 'malloc'],
-  'mktime': ['_get_tzname', '_get_daylight', '_get_timezone', 'malloc'],
+  'localtime': ['malloc'],
+  'localtime_r': ['malloc'],
+  'mktime': ['malloc'],
   'mmap': ['memalign'],
   'munmap': ['free'],
   'pthread_create': ['malloc', 'free', 'emscripten_main_thread_process_queued_calls'],
@@ -187,10 +188,10 @@
   # dependency.
   'setjmp': ['malloc', 'free', 'saveSetjmp', 'setThrew'],
   'setprotoent': ['malloc'],
-  'syslog': ['ntohs', 'htons'],
-  'vsyslog': ['ntohs', 'htons'],
-  'timegm': ['_get_tzname', '_get_daylight', '_get_timezone', 'malloc'],
-  'tzset': ['_get_tzname', '_get_daylight', '_get_timezone', 'malloc'],
+  'syslog': ['malloc', 'ntohs', 'htons'],
+  'vsyslog': ['malloc', 'ntohs', 'htons'],
+  'timegm': ['malloc'],
+  'tzset': ['malloc'],
   'uuid_compare': ['memcmp'],
   'uuid_copy': ['memcpy'],
   'wgpuBufferGetMappedRange': ['malloc', 'free'],

tools/system_libs.py

@@ -906,7 +906,6 @@ def get_files(self):
           'asctime.c',
           'ctime.c',
           'gmtime.c',
-          'gmtime_r.c',
           'localtime.c',
           'nanosleep.c',
           'clock_nanosleep.c',
@@ -971,6 +970,7 @@ def get_files(self):
           'sigtimedwait.c',
           'pthread_sigmask.c',
           'emscripten_console.c',
+          'emscripten_time.c',
         ])
 
     libc_files += files_in_path(
@@ -1577,7 +1577,9 @@ def get_files(self):
                    'difftime.c',
                    'gettimeofday.c',
                    'localtime_r.c',
+                   'gmtime_r.c',
                    'mktime.c',
+                   'timegm.c',
                    'time.c'])
     # It is more efficient to use JS for __assert_fail, as it avoids always
     # including fprintf etc.