From 19b53ac3d79c77d2454385365347bff48d2877e1 Mon Sep 17 00:00:00 2001
From: Ilya Verbin <iverbin@gmail.com>
Date: Mon, 3 Apr 2023 21:23:46 +0300
Subject: [PATCH] cmake: always define TARGET_OS_* to 0 or 1
There is a system include file TargetConditionals.h on macOS, which
defines TARGET_OS_LINUX (and others) to 0 or 1. On the other side,
TARGET_OS_LINUX is also defined by trivia/config.h.cmake, but there it
has another possible values: undefined or 1. This inconsistency causes
issues like #8445, when TARGET_OS_LINUX is defined (to 0) in one file
and undefined in another. Let's always define it to 0 or 1.
Closes #8445
NO_DOC=bugfix
---
.../gh-8445-crash-during-crash-report.md | 4 ++
src/box/module_cache.c | 2 +-
src/lib/core/crash.c | 2 +-
src/lib/core/crash.h | 2 +-
src/lib/core/fio.c | 2 +-
src/lib/core/popen.c | 10 ++--
src/lib/core/say.c | 2 +-
src/lib/core/sio.c | 2 +-
src/main.cc | 6 +--
src/trivia/config.h.cmake | 20 +++----
...gh_8445_crash_during_crash_report_test.lua | 54 +++++++++++++++++++
11 files changed, 82 insertions(+), 24 deletions(-)
create mode 100644 changelogs/unreleased/gh-8445-crash-during-crash-report.md
create mode 100644 test/app-luatest/gh_8445_crash_during_crash_report_test.lua
diff --git a/changelogs/unreleased/gh-8445-crash-during-crash-report.md b/changelogs/unreleased/gh-8445-crash-during-crash-report.md
new file mode 100644
index 0000000000..7f9ce6c508
--- /dev/null
+++ b/changelogs/unreleased/gh-8445-crash-during-crash-report.md
@@ -0,0 +1,4 @@
+## bugfix/core
+
+* Fixed a crash that could happen when preparing a crash report on macOS
+ (gh-8445).
diff --git a/src/box/module_cache.c b/src/box/module_cache.c
index 3d9b1883df..bb51623f23 100644
--- a/src/box/module_cache.c
+++ b/src/box/module_cache.c
@@ -261,7 +261,7 @@ module_attr_fill(struct module_attr *attr, struct stat *st)
attr->st_dev = (uint64_t)st->st_dev;
attr->st_ino = (uint64_t)st->st_ino;
attr->st_size = (uint64_t)st->st_size;
-#ifdef TARGET_OS_DARWIN
+#if TARGET_OS_DARWIN
attr->tv_sec = (uint64_t)st->st_mtimespec.tv_sec;
attr->tv_nsec = (uint64_t)st->st_mtimespec.tv_nsec;
#else
diff --git a/src/lib/core/crash.c b/src/lib/core/crash.c
index cd434b8eca..6e324997c9 100644
--- a/src/lib/core/crash.c
+++ b/src/lib/core/crash.c
@@ -184,7 +184,7 @@ crash_signal_cb(int signo, siginfo_t *siginfo, void *context)
crash_callback(cinfo);
} else {
/* Got a signal while running the handler. */
- fprintf(stderr, "Fatal %d while backtracing", signo);
+ fprintf(stderr, "Fatal %d while backtracing\n", signo);
}
/* Try to dump a core */
diff --git a/src/lib/core/crash.h b/src/lib/core/crash.h
index 58e14d5f09..255a2d313b 100644
--- a/src/lib/core/crash.h
+++ b/src/lib/core/crash.h
@@ -13,7 +13,7 @@
extern "C" {
#endif /* defined(__cplusplus) */
-#if defined(TARGET_OS_LINUX) && defined(__x86_64__)
+#if TARGET_OS_LINUX && defined(__x86_64__)
# define HAS_GREG
#endif
diff --git a/src/lib/core/fio.c b/src/lib/core/fio.c
index 4388b9a534..ddaf00972e 100644
--- a/src/lib/core/fio.c
+++ b/src/lib/core/fio.c
@@ -46,7 +46,7 @@
const char *
fio_filename(int fd)
{
-#ifdef TARGET_OS_LINUX
+#if TARGET_OS_LINUX
int save_errno = errno;
char *proc_path = tt_static_buf();
diff --git a/src/lib/core/popen.c b/src/lib/core/popen.c
index 1716edc134..5911be6f9d 100644
--- a/src/lib/core/popen.c
+++ b/src/lib/core/popen.c
@@ -19,7 +19,7 @@
#include "say.h"
#include "tarantool_ev.h"
-#ifdef TARGET_OS_DARWIN
+#if TARGET_OS_DARWIN
# include <sys/ioctl.h>
#endif
@@ -861,7 +861,7 @@ popen_delete(struct popen_handle *handle)
static int
make_pipe(int pfd[2])
{
-#ifdef TARGET_OS_LINUX
+#if TARGET_OS_LINUX
if (pipe2(pfd, O_CLOEXEC)) {
diag_set(SystemError, "Can't create pipe2");
return -1;
@@ -895,7 +895,7 @@ make_pipe(int pfd[2])
static int
close_inherited_fds(int *skip_fds, size_t nr_skip_fds)
{
-# if defined(TARGET_OS_LINUX)
+# if TARGET_OS_LINUX
static const char path[] = "/proc/self/fd";
# else
static const char path[] = "/dev/fd";
@@ -1004,7 +1004,7 @@ signal_reset(void)
}
}
-#ifdef TARGET_OS_DARWIN
+#if TARGET_OS_DARWIN
/**
* Wait until a child process will become a group leader or will
* complete.
@@ -1312,7 +1312,7 @@ popen_new(struct popen_opts *opts)
signal_reset();
if (opts->flags & POPEN_FLAG_SETSID) {
-#ifndef TARGET_OS_DARWIN
+#if !TARGET_OS_DARWIN
if (setsid() == -1) {
say_syserror("child: setsid failed");
goto exit_child;
diff --git a/src/lib/core/say.c b/src/lib/core/say.c
index 1600665282..042a90dab9 100644
--- a/src/lib/core/say.c
+++ b/src/lib/core/say.c
@@ -451,7 +451,7 @@ log_pipe_init(struct log *log, const char *init_str)
diag_set(SystemError, "can't start logger: %s", init_str);
return -1;
}
-#ifndef TARGET_OS_DARWIN
+#if !TARGET_OS_DARWIN
/*
* A courtesy to a DBA who might have
* misconfigured the logger option: check whether
diff --git a/src/lib/core/sio.c b/src/lib/core/sio.c
index 2264742c7c..7fde2d9046 100644
--- a/src/lib/core/sio.c
+++ b/src/lib/core/sio.c
@@ -128,7 +128,7 @@ sio_option_name(int option)
static int
sio_listen_backlog()
{
-#ifdef TARGET_OS_LINUX
+#if TARGET_OS_LINUX
FILE *proc = fopen("/proc/sys/net/core/somaxconn", "r");
if (proc) {
int backlog;
diff --git a/src/main.cc b/src/main.cc
index a07f892787..6f48982fbd 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -47,7 +47,7 @@
#include <locale.h>
#include <libgen.h>
#include <sysexits.h>
-#if defined(TARGET_OS_LINUX) && defined(HAVE_PRCTL_H)
+#if TARGET_OS_LINUX && defined(HAVE_PRCTL_H)
# include <sys/prctl.h>
#endif
#include "fiber.h"
@@ -398,7 +398,7 @@ load_cfg(void)
say_syserror("setrlimit");
exit(EX_OSERR);
}
-#if defined(TARGET_OS_LINUX) && defined(HAVE_PRCTL_H)
+#if TARGET_OS_LINUX && defined(HAVE_PRCTL_H)
if (prctl(PR_SET_DUMPABLE, 1, 0, 0, 0) < 0) {
say_syserror("prctl");
exit(EX_OSERR);
@@ -415,7 +415,7 @@ load_cfg(void)
if (!small_test_feature(SMALL_FEATURE_DONTDUMP)) {
static const char strip_msg[] =
"'strip_core' is set but unsupported";
-#ifdef TARGET_OS_LINUX
+#if TARGET_OS_LINUX
/*
* Linux is known to support madvise(DONT_DUMP)
* feature, thus warn on this platform only. The
diff --git a/src/trivia/config.h.cmake b/src/trivia/config.h.cmake
index 9d8bbb9129..7c5c79e592 100644
--- a/src/trivia/config.h.cmake
+++ b/src/trivia/config.h.cmake
@@ -27,16 +27,16 @@
/** \endcond public */
#define PACKAGE "@PACKAGE@"
-/* Defined if building for Linux */
-#cmakedefine TARGET_OS_LINUX 1
-/* Defined if building for FreeBSD */
-#cmakedefine TARGET_OS_FREEBSD 1
-/* Defined if building for NetBSD */
-#cmakedefine TARGET_OS_NETBSD 1
-/* Defined if building for Darwin */
-#cmakedefine TARGET_OS_DARWIN 1
-
-#ifdef TARGET_OS_DARWIN
+/* Defined to 1 if building for Linux, or to 0 otherwise. */
+#cmakedefine01 TARGET_OS_LINUX
+/* Defined to 1 if building for FreeBSD, or to 0 otherwise. */
+#cmakedefine01 TARGET_OS_FREEBSD
+/* Defined to 1 if building for NetBSD, or to 0 otherwise. */
+#cmakedefine01 TARGET_OS_NETBSD
+/* Defined to 1 if building for Darwin, or to 0 otherwise. */
+#cmakedefine01 TARGET_OS_DARWIN
+
+#if TARGET_OS_DARWIN
#define TARANTOOL_LIBEXT "dylib"
#else
#define TARANTOOL_LIBEXT "so"
diff --git a/test/app-luatest/gh_8445_crash_during_crash_report_test.lua b/test/app-luatest/gh_8445_crash_during_crash_report_test.lua
new file mode 100644
index 0000000000..ae07fe9584
--- /dev/null
+++ b/test/app-luatest/gh_8445_crash_during_crash_report_test.lua
@@ -0,0 +1,54 @@
+local t = require('luatest')
+local g = t.group('gh-8445')
+
+g.before_all(function(cg)
+ -- TODO(gh-8572)
+ t.skip_if(jit.arch == 'arm64' and jit.os == 'Linux',
+ 'Disabled on AArch64 Linux due to #8572')
+ local server = require('luatest.server')
+ cg.server = server:new({alias = 'gh-8445'})
+end)
+
+g.after_all(function(cg)
+ cg.server:drop()
+end)
+
+-- Check that forked Tarantool doesn't crash when preparing a crash report.
+g.test_crash_during_crash_report = function(cg)
+ local ffi = require('ffi')
+ local popen = require('popen')
+
+ -- Use `cd' and `shell = true' due to lack of cwd option in popen (gh-5633).
+ local exe = arg[-1]
+ local dir = cg.server.workdir
+ local cmd = [[
+ cd %s && %s -e "box.cfg{} require('log').info('pid = ' .. box.info.pid)"
+ ]]
+ local ph = popen.new({string.format(cmd, dir, exe)},
+ {stdout = popen.opts.DEVNULL,
+ stderr = popen.opts.PIPE, shell = true})
+ t.assert(ph)
+
+ -- Wait for box.cfg{} completion.
+ local output = ''
+ t.helpers.retrying({}, function()
+ local chunk = ph:read({stderr = true, timeout = 0.05})
+ if chunk ~= nil then
+ output = output .. chunk
+ end
+ t.assert_str_contains(output, "pid = ")
+ end)
+
+ -- ph:info().pid won't work, because it returns pid of the shell rather than
+ -- pid of the Tarantool.
+ local pid = tonumber(string.match(output, "pid = (%d+)"))
+ t.assert(pid)
+ ffi.C.kill(pid, popen.signal.SIGILL)
+ ph:wait()
+ output = ph:read({stderr = true})
+ t.assert_str_contains(output, "Please file a bug")
+
+ -- Check that there were no fatal signals during crash report.
+ t.assert_not_str_contains(output, "Fatal 11 while backtracing")
+ ph:close()
+end
--
GitLab