[m-rev.] for review: Don't print value of errno in MR_fatal_error.

Peter Wang novalazy at gmail.com
Sun Aug 19 12:21:24 AEST 2018


The majority of calls to MR_fatal_error do not follow an operation that
sets errno, so printing out an error message unrelated to the reason for
the fatal error will lead to confusion. It can also cause test failures
if errno happens to be set to non-zero some time prior to an expected
call to MR_fatal_error. Fixes bug #464.

runtime/mercury_misc.c:
    Don't print value of errno in MR_fatal_error.

runtime/mercury_context.c:
runtime/mercury_thread.c:
    Pass strerror strings to MR_fatal_error where appropriate.

runtime/mercury_memory_zones.c:
runtime/mercury_memory_zones.h:
    Pass strerror strings to MR_fatal_error following failures of
    MR_protect_pages. Document that this assumes MR_protect_pages sets
    errno on error.

    Skip unnecessary call to sprintf before MR_fatal_error.

runtime/mercury_deep_profiling.c:
    Skip unnecessary call to sprintf before MR_fatal_error.

    Reduce size of some buffers.

runtime/mercury_overflow.c:
runtime/mercury_stack_trace.c:
    Pass a fixed format string to MR_fatal_error just in case
    the message string may contain percentage signs.

runtime/mercury_tabling.c:
    Skip unnecessary call to sprintf before MR_fatal_error.

deep_profiler/timeout.m:
library/thread.m:
mdbcomp/shared_utilities.m:
    Pass strerror strings to MR_fatal_error where appropriate.

trace/mercury_trace.c:
    Skip unnecessary call to sprintf before MR_fatal_error.

trace/mercury_trace_external.c:
    Pass a fixed format string to MR_fatal_error just in case.
---
 deep_profiler/timeout.m          | 21 ++++++++---
 library/thread.m                 |  4 +-
 mdbcomp/shared_utilities.m       |  4 +-
 runtime/mercury_context.c        |  5 ++-
 runtime/mercury_deep_profiling.c |  8 ++--
 runtime/mercury_memory_zones.c   | 65 +++++++++++++++++---------------
 runtime/mercury_memory_zones.h   |  4 ++
 runtime/mercury_misc.c           |  7 ----
 runtime/mercury_overflow.c       |  2 +-
 runtime/mercury_stack_trace.c    |  4 +-
 runtime/mercury_tabling.c        |  6 +--
 runtime/mercury_thread.c         |  5 ++-
 trace/mercury_trace.c            | 11 ++----
 trace/mercury_trace_external.c   |  8 ++--
 14 files changed, 82 insertions(+), 72 deletions(-)

diff --git a/deep_profiler/timeout.m b/deep_profiler/timeout.m
index 26f733a8d..5d93417ec 100644
--- a/deep_profiler/timeout.m
+++ b/deep_profiler/timeout.m
@@ -388,21 +388,24 @@ MP_handle_timeout(void)
             }
         }
 #endif
 
         (void) alarm(MP_timeout_seconds);
         return;
     }
 
     dir = opendir(MP_timeout_want_dir);
     if (dir == NULL) {
-        MR_fatal_error(""MP_handle_timeout: opendir failed"");
+        char    errbuf[MR_STRERROR_BUF_SIZE];
+
+        MR_fatal_error(""MP_handle_timeout: opendir failed: %s"",
+            MR_strerror(errno, errbuf, sizeof(errbuf)));
     }
 
     while ((dirent = readdir(dir)) != NULL) {
         if (MR_strneq(dirent->d_name, MP_timeout_want_prefix, matchlen)) {
 
 #ifdef  MP_DEBUG_LOCKS
             {
                 FILE    *debug_fp;
 
                 debug_fp = fopen(""/tmp/deep_locks"", ""a"");
@@ -532,21 +535,24 @@ MP_do_try_get_lock(const char *mutex_file)
         debug_fp = fopen(""/tmp/deep_locks"", ""a"");
         if (debug_fp != NULL) {
             fprintf(debug_fp, ""pid %d try: no lock %s\\n"",
                 getpid(), mutex_file);
             fclose(debug_fp);
         }
 #endif
 
         success = MR_FALSE;
     } else {
-        MR_fatal_error(""MP_do_try_get_lock failed"");
+        char    errbuf[MR_STRERROR_BUF_SIZE];
+
+        MR_fatal_error(""MP_do_try_get_lock failed: %s"",
+            MR_strerror(errno, errbuf, sizeof(errbuf)));
     }
 
     return success;
 }
 
 void
 MP_do_get_lock(const char *mutex_file)
 {
     int res;
 
@@ -575,21 +581,24 @@ MP_do_get_lock(const char *mutex_file)
             if (debug_fp != NULL) {
                 fprintf(debug_fp, ""pid %d trying to lock %s ...\\n"",
                     getpid(), mutex_file);
                 fclose(debug_fp);
             }
 #endif
 
             sleep(5);
             continue;
         } else {
-            MR_fatal_error(""MP_do_get_lock failed"");
+            char    errbuf[MR_STRERROR_BUF_SIZE];
+
+            MR_fatal_error(""MP_do_get_lock failed: %s"",
+                MR_strerror(errno, errbuf, sizeof(errbuf)));
         }
     }
 }
 
 void
 MP_do_release_lock(const char *mutex_file)
 {
 #ifdef  MP_DEBUG_LOCKS
     FILE    *debug_fp;
 
@@ -766,25 +775,27 @@ release_lock(Debug, MutexFile, !IO) :-
 #else
     MR_fatal_error(""deep profiler not enabled"");
 #endif
 ").
 
 :- pragma foreign_proc("C",
     make_want_file(WantFileName::in, _S0::di, _S::uo),
     [will_not_call_mercury, promise_pure],
 "
 #ifdef  MR_DEEP_PROFILER_ENABLED
-    int fd;
+    int     fd;
+    char    errbuf[MR_STRERROR_BUF_SIZE];
 
     fd = open(WantFileName, O_CREAT, 0);
     if (fd < 0) {
-        MR_fatal_error(""make_want_file: open failed"");
+        MR_fatal_error(""make_want_file: open failed: %s"",
+            MR_strerror(errno, errbuf, sizeof(errbuf)));
     }
     (void) close(fd);
     MP_register_cleanup_file(WantFileName);
 #else
     MR_fatal_error(""deep profiler not enabled"");
 #endif
 ").
 
 :- pragma foreign_proc("C",
     remove_want_file(WantFileName::in, _S0::di, _S::uo),
diff --git a/library/thread.m b/library/thread.m
index d50828aa8..69bc5c68e 100644
--- a/library/thread.m
+++ b/library/thread.m
@@ -595,22 +595,22 @@ INIT mercury_sys_init_thread_modules
         *error_msg = MR_make_string(alloc_id, ""pthread_create failed: %s"",
           MR_strerror(thread_errno, errbuf, sizeof(errbuf)));
     } else {
         MR_LOCK(&args.mutex, ""ML_create_exclusive_thread"");
         while (args.thread_state == ML_THREAD_NOT_READY) {
             int cond_err = MR_COND_WAIT(&args.cond, &args.mutex,
                 ""ML_create_exclusive_thread"");
             // EINTR should not be possible, but it has happened before.
             if (cond_err != 0 && errno != EINTR) {
                 MR_fatal_error(
-                    ""ML_create_exclusive_thread: MR_COND_WAIT error %d"",
-                    errno);
+                    ""ML_create_exclusive_thread: MR_COND_WAIT error: %s"",
+                    MR_strerror(errno, errbuf, sizeof(errbuf)));
             }
         }
         MR_UNLOCK(&args.mutex, ""ML_create_exclusive_thread"");
 
         if (args.thread_state == ML_THREAD_START_ERROR) {
             *error_msg =
                 MR_make_string_const(""Error setting up engine for thread."");
         }
     }
 
diff --git a/mdbcomp/shared_utilities.m b/mdbcomp/shared_utilities.m
index 6078a7d77..abd48f0ac 100644
--- a/mdbcomp/shared_utilities.m
+++ b/mdbcomp/shared_utilities.m
@@ -45,23 +45,25 @@
 
 ").
 
 :- pragma foreign_proc("C",
     unlimit_stack(_S0::di, _S::uo),
     [will_not_call_mercury, promise_pure],
 "
 #if defined(MR_HAVE_SETRLIMIT)
     struct rlimit   limit_struct;
     rlim_t          max_value;
+    char            errbuf[MR_STRERROR_BUF_SIZE];
 
     if (getrlimit(RLIMIT_STACK, &limit_struct) != 0) {
-        MR_fatal_error(""could not get current stack limit"");
+        MR_fatal_error(""could not get current stack limit: %s"",
+            MR_strerror(errno, errbuf, sizeof(errbuf)));
     }
 
     max_value = limit_struct.rlim_max;
     limit_struct.rlim_cur = limit_struct.rlim_max;
     /* If this fails, we have no recourse, so ignore any failure. */
     (void) setrlimit(RLIMIT_STACK, &limit_struct);
 #endif
 ").
 
     % Clause for non-C backends.
diff --git a/runtime/mercury_context.c b/runtime/mercury_context.c
index cbbf1b4c6..106f91e68 100644
--- a/runtime/mercury_context.c
+++ b/runtime/mercury_context.c
@@ -62,20 +62,21 @@ ENDINIT
 
 #ifdef MR_WIN32_GETSYSTEMINFO
   #include "mercury_windows.h"
 #endif
 
 #include "mercury_memory_handlers.h"
 #include "mercury_context.h"
 #include "mercury_engine.h"             // for `MR_memdebug'
 #include "mercury_threadscope.h"        // for data types and posting events
 #include "mercury_reg_workarounds.h"    // for `MR_fd*' stuff
+#include "mercury_runtime_util.h"
 
 #ifdef MR_PROFILE_PARALLEL_EXECUTION_SUPPORT
 #define MR_PROFILE_PARALLEL_EXECUTION_FILENAME "parallel_execution_profile.txt"
 #endif
 
 ////////////////////////////////////////////////////////////////////////////
 
 static void     MR_init_context_maybe_generator(MR_Context *c, const char *id,
                     MR_GeneratorPtr gen);
 
@@ -1597,20 +1598,21 @@ MR_sched_yield(void)
 #ifndef MR_HIGHLEVEL_CODE
 // Check to see if any contexts that blocked on IO have become runnable.
 // Return the number of contexts that are still blocked.
 // The parameter specifies whether or not the call to select should block.
 
 static int
 MR_check_pending_contexts(MR_bool block)
 {
 #ifdef  MR_CAN_DO_PENDING_IO
     int                 err;
+    char                errbuf[MR_STRERROR_BUF_SIZE];
     int                 max_fd;
     int                 num_fds;
     int                 n_ids;
     fd_set              rd_set0;
     fd_set              wr_set0;
     fd_set              ex_set0;
     fd_set              rd_set;
     fd_set              wr_set;
     fd_set              ex_set;
     struct timeval      timeout;
@@ -1672,21 +1674,22 @@ MR_check_pending_contexts(MR_bool block)
             rd_set = rd_set0;
             wr_set = wr_set0;
             ex_set = ex_set0;
             timeout.tv_sec = 0;
             timeout.tv_usec = 0;
             err = select(num_fds, &rd_set, &wr_set, &ex_set, &timeout);
         } while (err == -1 && MR_is_eintr(errno));
     }
 
     if (err < 0) {
-        MR_fatal_error("select failed!");
+        MR_fatal_error("select failed: %s",
+            MR_strerror(errno, errbuf, sizeof(errbuf)));
     }
 
     n_ids = 0;
     for (pctxt = MR_pending_contexts; pctxt; pctxt = pctxt -> next) {
         n_ids++;
         if (    ((pctxt->waiting_mode & MR_PENDING_READ)
                 && FD_ISSET(pctxt->fd, &rd_set))
             ||  ((pctxt->waiting_mode & MR_PENDING_WRITE)
                 && FD_ISSET(pctxt->fd, &wr_set))
             ||  ((pctxt->waiting_mode & MR_PENDING_EXEC)
diff --git a/runtime/mercury_deep_profiling.c b/runtime/mercury_deep_profiling.c
index 561b66f2c..c8a579832 100644
--- a/runtime/mercury_deep_profiling.c
+++ b/runtime/mercury_deep_profiling.c
@@ -143,45 +143,43 @@ int     MR_deep_prof_call_builtin_old = 0;
 
 #ifdef MR_DEEP_PROFILING_LOG
 FILE    *MR_deep_prof_log_file = NULL;
 #endif
 
 void
 MR_deep_assert_failed(const MR_CallSiteDynamic *csd, const MR_ProcLayout *pl,
     const MR_ProcStatic *ps, const char *cond,
     const char *filename, int linenumber)
 {
-    char    buf[1024];
-    char    bufcsd[1024];
-    char    bufps[1024];
+    char    bufcsd[64];
+    char    bufps[64];
 
     if (csd != NULL) {
         sprintf(bufcsd, ", csd %p\n", csd);
     } else {
         strcpy(bufcsd, "");
     }
 
     if (pl != NULL) {
         sprintf(bufps, ", pl %p\n", pl);
     } else {
         strcpy(bufps, "");
     }
 
     if (ps != NULL) {
         sprintf(bufps, ", ps %p\n", ps);
     } else {
         strcpy(bufps, "");
     }
 
-    sprintf(buf, "Deep profiling assertion failed, %s:%d\n%s%s%s\n",
+    MR_fatal_error("Deep profiling assertion failed, %s:%d\n%s%s%s\n",
         filename, linenumber, cond, bufcsd, bufps);
-    MR_fatal_error(buf);
 }
 
 void
 MR_setup_callback(void *entry)
 {
     MR_CallSiteDynList  *csd_list;
     MR_CallSiteDynamic  *csd;
 
     MR_enter_instrumentation();
     csd_list = *MR_current_callback_site;
diff --git a/runtime/mercury_memory_zones.c b/runtime/mercury_memory_zones.c
index 8d7ec67ab..2e51ebe49 100644
--- a/runtime/mercury_memory_zones.c
+++ b/runtime/mercury_memory_zones.c
@@ -50,20 +50,21 @@
 #ifdef  MR_HAVE_SYS_UCONTEXT_H
   #include <sys/ucontext.h>
 #endif
 
 #ifdef MR_THREAD_SAFE
   #include "mercury_thread.h"
   #include "mercury_atomic_ops.h"
 #endif
 
 #include "mercury_memory_handlers.h"
+#include "mercury_runtime_util.h"
 
 // This macro can be used to update a high water mark of a statistic.
 
 #define MR_UPDATE_HIGHWATER(max, cur)                                       \
     do {                                                                    \
         if ((max) < (cur)) {                                                \
             (max) = (cur);                                                  \
         }                                                                   \
     } while (0);
 
@@ -392,26 +393,29 @@ MR_add_zone_to_used_list(MR_MemoryZone *zone)
 
     MR_UNLOCK(&memory_zones_lock, "MR_add_zone_to_used_list");
 }
 
 static void
 MR_free_zone(MR_MemoryZone *zone)
 {
 #ifdef MR_CHECK_OVERFLOW_VIA_MPROTECT
     size_t          redsize;
     int             res;
+    char            errbuf[MR_STRERROR_BUF_SIZE];
 
     redsize = zone->MR_zone_redzone_size;
     res = MR_protect_pages((char *) zone->MR_zone_redzone,
         redsize + MR_page_size, NORMAL_PROT);
     if (res) {
-        MR_fatal_error("Could not unprotect memory pages in MR_free_zone");
+        MR_fatal_error(
+            "Could not unprotect memory pages in MR_free_zone: %s",
+            MR_strerror(errno, errbuf, sizeof(errbuf)));
     }
 #endif
 
 #ifdef MR_PROFILE_ZONES
     MR_LOCK(&memory_zones_stats_lock, "MR_free_zone");
     MR_num_zones--;
     MR_total_zone_size_net -= zone->MR_zone_desired_size;
     MR_total_zone_size_gross -=
         (MR_Integer) zone->MR_zone_top - (MR_Integer) zone->MR_zone_bottom;
     MR_UNLOCK(&memory_zones_stats_lock, "MR_free_zone");
@@ -624,20 +628,21 @@ MR_create_new_zone(size_t desired_size, size_t redzone_size, size_t offset)
 MR_Integer
 MR_extend_zone(MR_MemoryZone *zone, size_t new_size)
 {
     void            *old_base;
     void            *new_base;
     size_t          offset;
     size_t          copy_size;
     size_t          new_total_size;
     MR_Integer      base_incr;
     int             res;
+    char            errbuf[MR_STRERROR_BUF_SIZE];
 #ifdef MR_PROFILE_ZONES
     size_t          size_delta;
 #endif
 
     if (zone == NULL) {
         MR_fatal_error("MR_extend_zone called with NULL pointer");
     }
 
     // XXX: This value is strange for new_total_size, it is a page bigger than
     // it needs to be. However, this allows for a hardzone in some cases.
@@ -660,36 +665,34 @@ MR_extend_zone(MR_MemoryZone *zone, size_t new_size)
         ((MR_Integer) zone->MR_zone_top - (MR_Integer) zone->MR_zone_bottom);
     MR_UNLOCK(&memory_zones_stats_lock, "MR_extend_zone");
 #endif
 
 #ifdef  MR_CHECK_OVERFLOW_VIA_MPROTECT
     // Unprotect the entire zone area.
     res = MR_protect_pages((char *) zone->MR_zone_bottom,
         ((char *) zone->MR_zone_top) - ((char *) zone->MR_zone_bottom),
         NORMAL_PROT);
     if (res < 0) {
-        char buf[2560];
-        sprintf(buf, "unable to reset %s#%" MR_INTEGER_LENGTH_MODIFIER
-                "d total area\nbase=%p, redzone=%p",
+        MR_fatal_error(
+            "unable to reset %s#%" MR_INTEGER_LENGTH_MODIFIER
+                "d total area\nbase=%p, redzone=%p, errno=%s",
             zone->MR_zone_name, zone->MR_zone_id,
-            zone->MR_zone_bottom, zone->MR_zone_top);
-        MR_fatal_error(buf);
+            zone->MR_zone_bottom, zone->MR_zone_top,
+            MR_strerror(errno, errbuf, sizeof(errbuf)));
     }
 #endif  // MR_CHECK_OVERFLOW_VIA_MPROTECT
 
     new_base = MR_realloc_zone_memory(old_base, copy_size, new_size);
     if (new_base == NULL) {
-        char buf[2560];
-        sprintf(buf, "unable reallocate memory zone: %s#%"
+        MR_fatal_error("unable reallocate memory zone: %s#%"
                 MR_INTEGER_LENGTH_MODIFIER "d",
             zone->MR_zone_name, zone->MR_zone_id);
-        MR_fatal_error(buf);
     }
 
     // XXX The casts to MR_Integer are here because this code was relying
     // on the gcc extension that allows arithmetic on void pointers;
     // this breaks when compiling with Visual C - juliensf.
 
     base_incr = (MR_Integer) new_base - (MR_Integer) old_base;
 
     zone->MR_zone_desired_size = new_size;
     zone->MR_zone_bottom = new_base;
@@ -742,68 +745,69 @@ MR_configure_redzone_size(MR_MemoryZone *zone, size_t redsize)
         (MR_Unsigned) zone->MR_zone_top);
 }
 #endif
 
 static void
 MR_setup_redzones(MR_MemoryZone *zone)
 {
     size_t      size;
     size_t      redsize;
     int         res;
+    char        errbuf[MR_STRERROR_BUF_SIZE];
 
     size = zone->MR_zone_desired_size;
     redsize = zone->MR_zone_redzone_size;
 
     assert(size > redsize);
 
 #ifdef MR_DEBUG_CONTEXT_CREATION_SPEED
     MR_debug_log_message("Setting up redzone of size: 0x%x.", redsize);
 #endif
 
     // Setup the redzone.
 
 #ifdef MR_CHECK_OVERFLOW_VIA_MPROTECT
     MR_configure_redzone_size(zone, redsize);
 
     res = MR_protect_pages((char *) zone->MR_zone_redzone,
         redsize + MR_page_size, REDZONE_PROT);
     if (res < 0) {
-        char buf[2560];
         if (zone->MR_zone_name == NULL) {
             zone->MR_zone_name = "unknown";
         }
-        sprintf(buf, "unable to set %s#%" MR_INTEGER_LENGTH_MODIFIER
-                "d redzone\nbase=%p, redzone=%p",
+        MR_fatal_error(
+            "unable to set %s#%" MR_INTEGER_LENGTH_MODIFIER
+              "d redzone\nbase=%p, redzone=%p, errno=%s",
             zone->MR_zone_name, zone->MR_zone_id,
-            zone->MR_zone_bottom, zone->MR_zone_redzone);
-        MR_fatal_error(buf);
+            zone->MR_zone_bottom, zone->MR_zone_redzone,
+            MR_strerror(errno, errbuf, sizeof(errbuf)));
     }
 #endif // MR_CHECK_OVERFLOW_VIA_MPROTECT
 
     // Setup the hardzone.
 
 #if defined(MR_PROTECTPAGE)
     // The % MR_page_size is to ensure page alignment.
     zone->MR_zone_hardmax = (MR_Word *)((MR_Unsigned) zone->MR_zone_top -
             MR_page_size - ((MR_Unsigned) zone->MR_zone_top % MR_page_size));
     res = MR_protect_pages((char *) zone->MR_zone_hardmax, MR_page_size,
         REDZONE_PROT);
     if (res < 0) {
-        char buf[2560];
         if (zone->MR_zone_name == NULL) {
             zone->MR_zone_name = "unknown";
         }
-        sprintf(buf, "unable to set %s#%" MR_INTEGER_LENGTH_MODIFIER
-                "d hardmax\nbase=%p, hardmax=%p top=%p",
+        MR_fatal_error(
+            "unable to set %s#%" MR_INTEGER_LENGTH_MODIFIER
+                "d hardmax\nbase=%p, hardmax=%p top=%p, errno=%s",
             zone->MR_zone_name, zone->MR_zone_id,
-            zone->MR_zone_bottom, zone->MR_zone_hardmax, zone->MR_zone_top);
-        MR_fatal_error(buf);
+            zone->MR_zone_bottom, zone->MR_zone_hardmax, zone->MR_zone_top,
+            MR_strerror(errno, errbuf, sizeof(errbuf)));
     }
 #endif  // MR_PROTECTPAGE
 
 #if defined(MR_NATIVE_GC) && defined(MR_HIGHLEVEL_CODE)
     zone->MR_zone_gc_threshold = (char *) zone->MR_zone_end
         - MR_heap_margin_size;
 #endif
 
 #if defined(MR_STACK_SEGMENTS) && !defined(MR_HIGHLEVEL_CODE)
     zone->MR_zone_extend_threshold =
@@ -833,47 +837,48 @@ MR_setup_redzones(MR_MemoryZone *zone)
   #ifdef MR_CHECK_OVERFLOW_VIA_MPROTECT
     assert((MR_Word *) zone->MR_zone_extend_threshold < zone->MR_zone_redzone);
   #endif
 #endif
 }
 
 void
 MR_reset_redzone(MR_MemoryZone *zone)
 {
 #ifdef  MR_CHECK_OVERFLOW_VIA_MPROTECT
-    int res;
+    int     res;
+    char    errbuf[MR_STRERROR_BUF_SIZE];
 
     zone->MR_zone_redzone = zone->MR_zone_redzone_base;
 
     // Unprotect the non-redzone area.
     res = MR_protect_pages((char *) zone->MR_zone_bottom,
         ((char *) zone->MR_zone_redzone) - ((char *) zone->MR_zone_bottom),
         NORMAL_PROT);
     if (res < 0) {
-        char buf[2560];
-        sprintf(buf, "unable to reset %s#%" MR_INTEGER_LENGTH_MODIFIER
-                "d normal area\nbase=%p, redzone=%p",
+        MR_fatal_error(
+            "unable to reset %s#%" MR_INTEGER_LENGTH_MODIFIER
+                "d normal area\nbase=%p, redzone=%p, errno=%s",
             zone->MR_zone_name, zone->MR_zone_id,
-            zone->MR_zone_bottom, zone->MR_zone_redzone);
-        MR_fatal_error(buf);
+            zone->MR_zone_bottom, zone->MR_zone_redzone,
+            MR_strerror(errno, errbuf, sizeof(errbuf)));
     }
     // Protect the redzone area.
     res = MR_protect_pages((char *) zone->MR_zone_redzone,
         ((char *) zone->MR_zone_top) - ((char *) zone->MR_zone_redzone),
         REDZONE_PROT);
     if (res < 0) {
-        char buf[2560];
-        sprintf(buf, "unable to reset %s#%" MR_INTEGER_LENGTH_MODIFIER
-                "d redzone\nbase=%p, redzone=%p",
+        MR_fatal_error(
+            "unable to reset %s#%" MR_INTEGER_LENGTH_MODIFIER
+                "d redzone\nbase=%p, redzone=%p, errno=%s",
             zone->MR_zone_name, zone->MR_zone_id,
-            zone->MR_zone_bottom, zone->MR_zone_redzone);
-        MR_fatal_error(buf);
+            zone->MR_zone_bottom, zone->MR_zone_redzone,
+            MR_strerror(errno, errbuf, sizeof(errbuf)));
     }
 #endif  // MR_CHECK_OVERFLOW_VIA_MPROTECT
 }
 
 MR_MemoryZone *
 MR_get_used_memory_zones_readonly(void)
 {
     return used_memory_zones;
 }
 
diff --git a/runtime/mercury_memory_zones.h b/runtime/mercury_memory_zones.h
index dc45e2914..a7bda4b98 100644
--- a/runtime/mercury_memory_zones.h
+++ b/runtime/mercury_memory_zones.h
@@ -191,20 +191,24 @@ struct MR_MemoryZonesFree_Struct {
       #define PROT_NONE  0x0000
     #endif
     #ifndef PROT_READ
       #define PROT_READ  0x0001
     #endif
     #ifndef PROT_WRITE
       #define PROT_WRITE 0x0002
     #endif
   #endif
 
+// MR_protect_pages() is currently only a wrapper around mprotect() so callers
+// may expect errno to be set appropriately on error to provide better error
+// messages. If MR_protect_pages() gains another implementation then it may be
+// necessary to update callers to not depend on errno being set.
 extern int      MR_protect_pages(void *addr, size_t size, int prot_flags);
 
 #endif
 
 // MR_init_zones() initializes the memory zone pool and the offset generator.
 // It should be used before any zones are created or offsets requested.
 
 extern void     MR_init_zones(void);
 
 // MR_create_zone(Name, Id, Size, Offset, RedZoneSize, FaultHandler)
diff --git a/runtime/mercury_misc.c b/runtime/mercury_misc.c
index 77662bfd8..605be85c8 100644
--- a/runtime/mercury_misc.c
+++ b/runtime/mercury_misc.c
@@ -4,21 +4,20 @@
 // Copyright (C) 2014, 2016, 2018 The Mercury team.
 // This file is distributed under the terms specified in COPYING.LIB.
 
 #include    "mercury_conf.h"
 #ifndef MR_HIGHLEVEL_CODE
   #include  "mercury_imp.h"
 #endif
 #include    "mercury_string.h"
 #include    "mercury_misc.h"
 #include    "mercury_array_macros.h"
-#include    "mercury_runtime_util.h"
 
 #include    <stdio.h>
 #include    <stdarg.h>
 #include    <errno.h>
 
 static void MR_print_warning(const char *prog, const char *fmt, va_list args);
 static void MR_do_perror(const char *prog, const char *message);
 
 void
 MR_warning(const char *fmt, ...)
@@ -75,29 +74,23 @@ MR_do_perror(const char *prog, const char *message)
     perror(message);
 }
 
 // XXX will need to modify this to kill other threads if MR_THREAD_SAFE
 // (and cleanup resources, etc....)
 
 void
 MR_fatal_error(const char *fmt, ...)
 {
     va_list args;
-    int error = errno;
-    char errbuf[MR_STRERROR_BUF_SIZE];
 
     fflush(stdout);     // In case stdout and stderr are the same.
 
-    if (error != 0) {
-        fprintf(stderr, "Errno = %d: %s\n", error,
-            MR_strerror(error, errbuf, sizeof(errbuf)));
-    }
     fprintf(stderr, "Mercury runtime: ");
     va_start(args, fmt);
     vfprintf(stderr, fmt, args);
     va_end(args);
     fprintf(stderr, "\n");
 
 #ifndef MR_HIGHLEVEL_CODE
     MR_trace_report(stderr);
 #endif
 
diff --git a/runtime/mercury_overflow.c b/runtime/mercury_overflow.c
index 8bd27ee84..45c670297 100644
--- a/runtime/mercury_overflow.c
+++ b/runtime/mercury_overflow.c
@@ -73,12 +73,12 @@ MR_fatal_zone_error(MR_OverflowZone ptr_kind,
 
     fprintf(stderr, "\n%s: ", zone_name);
     MR_print_zone(stderr, zone);
     MR_print_zones(stderr, zones);
 
     if (where != NULL) {
         fprintf(stderr, "error occurred in %s\n", where);
     }
 #endif
 
-    MR_fatal_error(error);
+    MR_fatal_error("%s", error);
 }
diff --git a/runtime/mercury_stack_trace.c b/runtime/mercury_stack_trace.c
index 4ff26c25b..f28db366d 100644
--- a/runtime/mercury_stack_trace.c
+++ b/runtime/mercury_stack_trace.c
@@ -1380,21 +1380,21 @@ MR_traverse_nondet_stack_from_layout(MR_Word *base_maxfr,
             MR_record_temp_redoip(base_maxfr);
         } else if (frame_size == MR_DET_TEMP_SIZE) {
             // do nothing
         } else {
             level_number++;
             if (MR_above_bottom_nondet_frame(base_maxfr)) {
                 problem = MR_step_over_nondet_frame(
                     MR_traverse_nondet_stack_frame, &func_info,
                     level_number, base_maxfr);
                 if (problem != NULL) {
-                    MR_fatal_error(problem);
+                    MR_fatal_error("%s", problem);
                 }
             }
         }
 
         base_maxfr = MR_prevfr_slot(base_maxfr);
         frames_traversed_so_far++;
     }
 }
 
 static void
@@ -1430,21 +1430,21 @@ MR_init_nondet_branch_infos(MR_Word *base_maxfr,
 
     // Skip past any model_det frames.
     do {
         proc_layout = label_layout->MR_sll_entry;
         if (!MR_DETISM_DET_STACK(proc_layout->MR_sle_detism)) {
             break;
         }
         result = MR_stack_walk_step(proc_layout, &label_layout,
             &stack_pointer, &current_frame, &reused_frames, &problem);
         if (result == MR_STEP_ERROR_BEFORE || result == MR_STEP_ERROR_AFTER) {
-            MR_fatal_error(problem);
+            MR_fatal_error("%s", problem);
         }
 
     } while (label_layout != NULL);
 
     // Double-check that we didn't skip any model_non frames.
     assert(current_frame == base_curfr);
 
     if (label_layout != NULL) {
         MR_ensure_room_for_next(MR_nondet_branch_info, MR_NondetBranchInfo,
             MR_INIT_NONDET_BRANCH_ARRAY_SIZE);
diff --git a/runtime/mercury_tabling.c b/runtime/mercury_tabling.c
index 97b147ca0..b6b7df89b 100644
--- a/runtime/mercury_tabling.c
+++ b/runtime/mercury_tabling.c
@@ -1660,19 +1660,15 @@ void mercury_sys_init_table_modules_write_out_proc_statics(FILE *fp)
     // of model_non memo tabled predicates.
 }
 #endif
 
 ////////////////////////////////////////////////////////////////////////////
 
 #ifdef MR_TABLE_DEBUG
 static void
 MR_table_assert_failed(const char *file, unsigned line)
 {
-    char    buf[256];
-
-    MR_snprintf(buf, sizeof(buf), "assertion failed: file %s, line %d",
-        file, line);
-    MR_fatal_error(buf);
+    MR_fatal_error("assertion failed: file %s, line %d", file, line);
 }
 #endif
 
 ////////////////////////////////////////////////////////////////////////////
diff --git a/runtime/mercury_thread.c b/runtime/mercury_thread.c
index 09e8f3c84..ac4cfbebe 100644
--- a/runtime/mercury_thread.c
+++ b/runtime/mercury_thread.c
@@ -2,20 +2,21 @@
 
 // Copyright (C) 1997-2001, 2003, 2005-2007, 2009-2011 The University of Melbourne.
 // Copyright (C) 2014, 2016, 2018 The Mercury team.
 // This file is distributed under the terms specified in COPYING.LIB.
 
 #include "mercury_imp.h"
 #include "mercury_regs.h"
 #include "mercury_engine.h"
 #include "mercury_memory.h"
 #include "mercury_context.h"    // for MR_do_runnext
+#include "mercury_runtime_util.h"
 #include "mercury_thread.h"
 #include "mercury_threadscope.h"
 
 #include <stdio.h>
 #include <errno.h>
 
 #ifdef  MR_THREAD_SAFE
   MercuryThread     MR_primordial_thread;
 
   MercuryThreadKey  MR_exception_handler_key;
@@ -58,39 +59,41 @@ MR_shutdown_engine_for_threads(MercuryEngine *eng);
 #ifdef MR_LL_PARALLEL_CONJ
 static void *
 MR_create_worksteal_thread_2(void *goal);
 
 MercuryThread *
 MR_create_worksteal_thread(void)
 {
     MercuryThread   *thread;
     pthread_attr_t  attrs;
     int             err;
+    char            errbuf[MR_STRERROR_BUF_SIZE];
 
     assert(!MR_thread_equal(MR_primordial_thread, MR_null_thread()));
 
     // Create threads in the detached state so that resources will be
     // automatically freed when threads terminate (we don't call
     // pthread_join() anywhere).
 
     thread = MR_GC_NEW_ATTRIB(MercuryThread, MR_ALLOC_SITE_RUNTIME);
     pthread_attr_init(&attrs);
     pthread_attr_setdetachstate(&attrs, PTHREAD_CREATE_DETACHED);
     err = pthread_create(thread, &attrs, MR_create_worksteal_thread_2, NULL);
     pthread_attr_destroy(&attrs);
 
 #if 0
     fprintf(stderr, "pthread_create returned %d (errno = %d)\n", err, errno);
 #endif
 
     if (err != 0) {
-        MR_fatal_error("error creating thread");
+        MR_fatal_error("error creating thread: %s",
+            MR_strerror(err, errbuf, sizeof(errbuf)));
     }
 
     return thread;
 }
 
 static void *
 MR_create_worksteal_thread_2(void *arg)
 {
   #ifdef MR_HAVE_THREAD_PINNING
      // TODO: We may use the cpu value returned to determine which CPUs
diff --git a/trace/mercury_trace.c b/trace/mercury_trace.c
index f9f75a7d2..05617fe7e 100644
--- a/trace/mercury_trace.c
+++ b/trace/mercury_trace.c
@@ -1699,28 +1699,23 @@ MR_maybe_record_call_table(const MR_ProcLayout *level_layout,
 
         return;
 
     case MR_EVAL_METHOD_TABLE_IO:
     case MR_EVAL_METHOD_TABLE_IO_DECL:
     case MR_EVAL_METHOD_TABLE_IO_UNITIZE:
     case MR_EVAL_METHOD_TABLE_IO_UNITIZE_DECL:
         return;
     }
 
-    {
-        char    buf[256];
-
-        sprintf(buf,
-            "unknown evaluation method %d in MR_maybe_record_call_table",
-            MR_sle_eval_method(level_layout));
-        MR_fatal_error(buf);
-    }
+    MR_fatal_error(
+        "unknown evaluation method %d in MR_maybe_record_call_table",
+        MR_sle_eval_method(level_layout));
 }
 
 static void
 MR_reset_call_table_array(void)
 {
     int i;
 
     for (i = 0; i < MR_call_table_ptr_next; i++) {
 #ifdef  MR_DEBUG_RETRY
         printf("resetting call table ptr %d (%x)\n",
diff --git a/trace/mercury_trace_external.c b/trace/mercury_trace_external.c
index 43a3c69d8..569d2f047 100644
--- a/trace/mercury_trace_external.c
+++ b/trace/mercury_trace_external.c
@@ -1029,21 +1029,21 @@ MR_trace_make_var_list(void)
 
     var_count = MR_trace_var_count();
 
     MR_TRACE_USE_HP(
         var_list = MR_list_empty();
     );
 
     for (i = var_count; i > 0; i--) {
         problem = MR_trace_return_var_info(i, NULL, &type_info, &value);
         if (problem != NULL) {
-            MR_fatal_error(problem);
+            MR_fatal_error("%s", problem);
         }
 
         MR_TRACE_USE_HP(
             MR_new_univ_on_hp(univ, type_info, value);
         );
 
         MR_TRACE_USE_HP(
             var_list = MR_univ_list_cons(univ, var_list);
         );
     }
@@ -1069,21 +1069,21 @@ MR_trace_make_var_names_list(void)
 
     var_count = MR_trace_var_count();
 
     MR_TRACE_USE_HP(
         var_names_list = MR_list_empty();
     );
 
     for (i = var_count; i > 0; i--) {
         problem = MR_trace_return_var_info(i, &name, NULL, NULL);
         if (problem != NULL) {
-            MR_fatal_error(problem);
+            MR_fatal_error("%s", problem);
         }
 
         MR_TRACE_USE_HP(
             var_names_list = MR_string_list_cons((MR_Word) name,
                 var_names_list);
         );
     }
 
     return var_names_list;
 }
@@ -1106,21 +1106,21 @@ MR_trace_make_type_list(void)
 
     var_count = MR_trace_var_count();
 
     MR_TRACE_USE_HP(
         type_list = MR_list_empty();
     );
 
     for (i = var_count; i > 0; i--) {
         problem = MR_trace_return_var_info(i, NULL, &type_info, NULL);
         if (problem != NULL) {
-            MR_fatal_error(problem);
+            MR_fatal_error("%s", problem);
         }
 
         MR_TRACE_CALL_MERCURY(
             type_info_string = ML_type_name((MR_Word) type_info);
         );
         MR_TRACE_USE_HP(
             type_list = MR_string_list_cons((MR_Word) type_info_string,
                 type_list);
         );
     }
@@ -1145,21 +1145,21 @@ MR_trace_make_nth_var(MR_Word debugger_request)
 
     problem = MR_trace_return_var_info(var_number, NULL, &type_info, &value);
     if (problem == NULL) {
         MR_TRACE_USE_HP(
             MR_new_univ_on_hp(univ, type_info, value);
         );
     } else {
         // Should never occur since we check in the external debugger process
         // if a variable is live before retrieving it.
 
-        MR_fatal_error(problem);
+        MR_fatal_error("%s", problem);
     }
 
     return univ;
 }
 
 // This function is called only when debugger_request = current_nth_var(n).
 // It returns the integer 'n'.
 
 static int
 MR_get_var_number(MR_Word debugger_request)
-- 
2.18.0



More information about the reviews mailing list