Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix foreground process group on restart #904

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/nosyscallsreal.c
Original file line number Diff line number Diff line change
Expand Up @@ -276,17 +276,19 @@ _real_tcsetpgrp(int fd, pid_t pgrp)
REAL_FUNC_PASSTHROUGH(tcsetpgrp) (fd, pgrp);
}

int
pid_t
_real_tcgetpgrp(int fd)
{
REAL_FUNC_PASSTHROUGH(tcgetpgrp) (fd);
REAL_FUNC_PASSTHROUGH_PID_T(tcgetpgrp) (fd);
}

#if 0
pid_t
_real_getpgrp(void)
{
REAL_FUNC_PASSTHROUGH_PID_T(getpgrp) ();
}
#endif

pid_t
_real_setpgrp(void)
Expand Down
30 changes: 24 additions & 6 deletions src/plugin/pid/pid_syscallsreal.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <dlfcn.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/ioctl.h>
#include <sys/syscall.h>
#include <unistd.h>
#include "dmtcp.h"
Expand Down Expand Up @@ -137,23 +138,27 @@ _real_dlsym(void *handle, const char *symbol)
return (void *)(*_libc_dlsym_fnptr)(handle, symbol);
}

// Also copied into src/threadlist.cpp, so that libdmtcp.sp
// won't depend on libdmtcp_pid.sp
// Also copied into src/threadlist.cpp, so that libdmtcp.so
// won't depend on libdmtcp_pid.so
LIB_PRIVATE
pid_t
_real_getpid(void)
{
// libc caches pid of the process and hence after restart, libc:getpid()
// returns the pre-ckpt value.
// returns the pre-ckpt value. So, we call _real_syscall instead of
// REAL_FUNC_PASSTHROUGH(getpid).
return (pid_t)_real_syscall(SYS_getpid);
}

// Also copied into src/syscallsreal.c, so that libdmtcp.so
// won't depend on libdmtcp_pid.so
LIB_PRIVATE
pid_t
_real_getppid(void)
{
// libc caches ppid of the process and hence after restart, libc:getppid()
// returns the pre-ckpt value.
// returns the pre-ckpt value. So, we call _real_syscall instead of
// REAL_FUNC_PASSTHROUGH(getppid).
return (pid_t)_real_syscall(SYS_getppid);
}

Expand All @@ -175,7 +180,20 @@ LIB_PRIVATE
pid_t
_real_tcgetpgrp(int fd)
{
REAL_FUNC_PASSTHROUGH(tcgetpgrp) (fd);
// REAL_FUNC_PASSTHROUGH(tcgetpgrp) (fd);
// SYS_tcgetpgrp doesn't exist; use _real_ioctl, not _real_syscall
pid_t arg;
_real_ioctl(fd, TIOCGPGRP, &arg);
return arg;
}

// FIXME: libdmtcp.so:_libc_getpgrp() depends on this.
// It should `call dmtcp_get_libc_addr() instead.
// Remove this _libc_getpgrp() when that works.
pid_t
_libc_getpgrp(void)
{
REAL_FUNC_PASSTHROUGH_TYPED(pid_t, getpgrp) ();
}

LIB_PRIVATE
Expand Down Expand Up @@ -328,7 +346,7 @@ _real_tgkill(int tgid, int tid, int sig)
return (int)_real_syscall(SYS_tgkill, tgid, tid, sig);
}

LIB_PRIVATE
// libdmtcp.so:_real_syscall() will call here, which will relay to libc.so
long
_real_syscall(long sys_num, ...)
{
Expand Down
43 changes: 43 additions & 0 deletions src/syscallsreal.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <sys/select.h>
#include <sys/syscall.h>
Expand All @@ -48,6 +49,7 @@
#include "constants.h"
#include "syscallwrappers.h" /* glibc > ver. 2.33: redefines xstat to stat */
#include "trampolines.h"
#include "../include/dmtcp.h"

typedef int (*funcptr_t) ();
typedef pid_t (*funcptr_pid_t) ();
Expand Down Expand Up @@ -258,6 +260,7 @@ dmtcp_prepare_wrappers(void)

#define REAL_FUNC_PASSTHROUGH(name) REAL_FUNC_PASSTHROUGH_TYPED(int, name)

// FIXME: This should take two parameters: name and fn
#define REAL_FUNC_PASSTHROUGH_WORK(name) \
if (fn == NULL) { \
if (_real_func_addr[ENUM(name)] == NULL) { \
Expand Down Expand Up @@ -621,6 +624,44 @@ _real_closelog(void)
REAL_FUNC_PASSTHROUGH_VOID(closelog) ();
}

LIB_PRIVATE
int
_real_ioctl(int d, unsigned long int request, ...)
{
void *arg;
va_list ap;

// Most calls to ioctl take 'void *', 'int' or no extra argument
// A few specialized ones take more args, but we don't need to handle those.
va_start(ap, request);
arg = va_arg(ap, void *);
va_end(ap);

///usr/include/unistd.h says syscall returns long int (contrary to man page)
REAL_FUNC_PASSTHROUGH_TYPED(int, ioctl) (d, request, arg);
}

// FIXME: This depends on libdmtcp_pid.so:_libc_getpgrp().
// It should `call dmtcp_get_libc_addr() instead.
LIB_PRIVATE
pid_t
_libc_getpgrp()
{
// This introduces a dependence of libdmtcp.so on libdmtcp_pid.so
REAL_FUNC_PASSTHROUGH(_libc_getpgrp) ();
}

LIB_PRIVATE
pid_t
_real_tcgetpgrp(int fd)
{
// REAL_FUNC_PASSTHROUGH(tcgetpgrp) (fd);
// SYS_tcgetpgrp doesn't exist; use _real_ioctl, not _real_syscall
pid_t arg;
_real_ioctl(fd, TIOCGPGRP, &arg);
return arg;
}

// set the handler
LIB_PRIVATE
sighandler_t
Expand Down Expand Up @@ -888,6 +929,8 @@ _real_syscall(long sys_num, ...)
va_end(ap);

///usr/include/unistd.h says syscall returns long int (contrary to man page)
// This goes to libdmtcp_pid.so:syscall() which virtualizes pids,
// or directly to libc.so if ther is no libdmtcp_pid.so.
REAL_FUNC_PASSTHROUGH_TYPED(long, syscall) (sys_num, arg[0], arg[1],
arg[2], arg[3], arg[4],
arg[5], arg[6]);
Expand Down
4 changes: 4 additions & 0 deletions src/syscallwrappers.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ LIB_PRIVATE extern __thread int thread_performing_dlopen_dlsym;
MACRO(tcgetpgrp) \
MACRO(tcsetpgrp) \
MACRO(getpgrp) \
MACRO(_libc_getpgrp) /* src/plugin/pid/pid_syscallsreal.c */ \
Comment on lines 159 to +160
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
MACRO(getpgrp) \
MACRO(_libc_getpgrp) /* src/plugin/pid/pid_syscallsreal.c */ \

MACRO(setpgrp) \
\
MACRO(getpgid) \
Expand Down Expand Up @@ -380,6 +381,9 @@ int _real_socketpair(int d, int type, int protocol, int sv[2]);
void _real_openlog(const char *ident, int option, int facility);
void _real_closelog(void);

int _real_tcgetpgrp(int fd);
int _libc_getpgrp(void);

// Despite what 'man signal' says, signal.h already defines sighandler_t
// But signal.h defines this only because we define GNU_SOURCE (or __USE_GNU_
typedef void (*sighandler_t)(int); /* POSIX has user define this type */
Expand Down
54 changes: 45 additions & 9 deletions src/terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#endif // ifdef HAS_PR_SET_PTRACER
#include "../jalib/jassert.h"
#include "config.h"
#include "syscallwrappers.h"
#include "dmtcp.h"

/*************************************************************************
Expand All @@ -19,6 +20,7 @@ namespace dmtcp
static int saved_termios_exists = 0;
static struct termios saved_termios;
static struct winsize win;
static bool pgrp_is_foreground = false;

static void save_term_settings();
static void restore_term_settings();
Expand All @@ -34,6 +36,10 @@ save_term_settings()
&& tcgetattr(STDIN_FILENO, &saved_termios) >= 0);
if (saved_termios_exists) {
ioctl(STDIN_FILENO, TIOCGWINSZ, (char *)&win);
// NOTE: There can be at most one foreground process for a terminal.
// Normally, the foreground process group leader is also pgrp leader.
// It can be in this DMTCP computation group, or outside of DMTCP.
pgrp_is_foreground = (_real_tcgetpgrp(STDIN_FILENO) == _libc_getpgrp());
}
}

Expand All @@ -60,22 +66,48 @@ safe_tcsetattr(int fd, int optional_actions, const struct termios *termios_p)
return 0;
}

// FIXME: Handle Virtual Pids
static int
get_parent_pid(int pid)
{
char buf[8192];
// /proc/PID/stat is: PID (COMMAND) STATE PPID ...
snprintf(buf, sizeof(buf), "/proc/%d/stat", pid);
FILE* fp = fopen(buf, "r");
JASSERT(fp != NULL)(pid)(JASSERT_ERRNO).Text("fopen");
int rc = -1;
if (fp) {
size_t size = fread(buf, 1, sizeof(buf), fp);
if (size > 0 && strstr(buf, ") ")) {
rc = strtol(strstr(strstr(buf, ") ")+2, " "), NULL, 10);
}
}
JASSERT(rc != -1)(pid).Text("parent pid not found in /proc/*/stat");
return rc;
}

// See: info libc -> Job Control -> Implementing a Shell ->
// Foreground and Background
static void
restore_term_settings()
{
if (saved_termios_exists) {
/* First check if we are in foreground. If not, skip this and print
* warning. If we try to call tcsetattr in background, we will hang up.
* In some shells, the shell itself will be a pgrp that includes is
* first child process, instead of the first child forming a pgrp.
*/
int foreground = (tcgetpgrp(STDIN_FILENO) == getpgrp());
JTRACE("restore terminal attributes, check foreground status first")
(foreground);
if (foreground) {
if ((!isatty(STDIN_FILENO)
|| safe_tcsetattr(STDIN_FILENO, TCSANOW, &saved_termios) == -1)) {
JWARNING(false).Text("failed to restore terminal");
} else {
(pgrp_is_foreground)
(_libc_getpgrp()) (_real_tcgetpgrp(STDIN_FILENO))
(get_parent_pid(_real_tcgetpgrp(STDIN_FILENO)));
if (pgrp_is_foreground && isatty(STDIN_FILENO)) {
// Set login shell (not under DMTCP control) to be the foreground process,
// instead of this process.
JASSERT(tcsetpgrp(STDIN_FILENO, getpgrp()) == 0)(JASSERT_ERRNO);
JTRACE("The process group ID of this process is the\n"
" the process group ID for the foreground process group.\n"
"Make current process group the new foreground process group.");
if (safe_tcsetattr(STDIN_FILENO, TCSANOW, &saved_termios) != -1) {
struct winsize cur_win;
JTRACE("restored terminal");
ioctl(STDIN_FILENO, TIOCGWINSZ, (char *)&cur_win);
Expand All @@ -87,10 +119,14 @@ restore_term_settings()
if (cur_win.ws_row == 0 && cur_win.ws_col == 0) {
ioctl(STDIN_FILENO, TIOCSWINSZ, (char *)&win);
}
} else {
JWARNING(false).Text("failed to restore terminal attributes");
}
} else if (pgrp_is_foreground) {
JWARNING(false).Text("failed to restore terminal. STDIN not a tty");
} else {
JWARNING(false)
.Text(":skip restore terminal step -- we are in BACKGROUND");
.Text("skip restore terminal step -- we are in BACKGROUND");
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/threadlist.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ static void Thread_SaveSigState(Thread *th);
static void Thread_RestoreSigState(Thread *th);

// Copied from src/plugin/pid/pid_syscallsreal.c
// Without this, libdmtcp.so will depend on libdmtcp_plugin.so being loaded
// Without this, libdmtcp.so will depend on libdmtcp_pid.so being loaded
static pid_t _real_getpid(void)
{
JWARNING("_real_getpid")
Expand Down
3 changes: 2 additions & 1 deletion util/gdb-dmtcp-utils
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ class ShowFilenameAtAddress(gdb.Command):
if getpid() == 0:
gdb.execute('print "Process not yet started"', False, False)
else:
memory_region = "%s (r-x): 0x%x-0x%x" % \
memory_region = "%s (r[-w]x): 0x%x-0x%x" % \
memory_region_at_address(memory_address)
gdb.execute('print "' + memory_region + '"', False, False)
# This will add the new gdb command: show-filename-at-address MEMORY_ADDRESS
Expand Down Expand Up @@ -352,6 +352,7 @@ def procmap_filename_address(line):
return (triple[-1],) + \
tuple([int("0x"+elt, 16) for elt in triple[0].split("-")])
def is_text_segment(procmap_line):
print(procmap_line)
return "r-x" in procmap_line and \
(procmap_line == "" or procmap_line.isspace() or '/' in procmap_line)
def memory_regions():
Expand Down