Denys Vlasenko [Thu, 25 Aug 2011 08:23:00 +0000 (10:23 +0200)]
Split syscall_fixup into enter/exit pair of functions
* syscall.c: Create syscall_fixup_on_sysexit() which is a copy of
syscall_fixup().
(trace_syscall_exiting): Call syscall_fixup_on_sysexit() instead of
syscall_fixup().
Denys Vlasenko [Wed, 24 Aug 2011 23:27:59 +0000 (01:27 +0200)]
Optimize tabto()
tabto is used in many lines of strace output.
On glibc, tprintf("%*s", col - curcol, "") is noticeably slow
compared to tprintf(" "). Use the latter.
Observed ~15% reduction of time spent in userspace.
* defs.h: Drop extern declaration of acolumn. Make tabto()
take no parameters.
* process.c (sys_exit): Call tabto() with no parameters.
* syscall.c (trace_syscall_exiting): Call tabto() with no parameters.
* strace.c: Make acolumn static, add static char *acolumn_spaces.
(main): Allocate acolumn_spaces as a string of spaces.
(printleader): Call tabto() with no parameters.
(tabto): Use simpler method to print lots of spaces.
Denys Vlasenko [Wed, 24 Aug 2011 23:13:43 +0000 (01:13 +0200)]
Opotimize "scno >= 0 && scno < nsyscalls" check
gcc can't figure out on its own that this check can be done with
single compare, and does two compares. We can help it by casting
scno to unsigned long: ((unsigned long)(scno) < nsyscalls)
* defs.h: New macro SCNO_IN_RANGE(long_var).
* count.c (count_syscall): Use SCNO_IN_RANGE() instead of open-coded check.
* syscall.c (getrval2): Use SCNO_IN_RANGE() instead of open-coded check.
This fixes a bug: missing check for scno < 0 and scno > nsyscalls
instead of scno >= nsyscalls.
(get_scno): Use SCNO_IN_RANGE() instead of open-coded check.
This fixes a bug: scno > nsyscalls instead of scno >= nsyscalls.
(known_scno): Use SCNO_IN_RANGE() instead of open-coded check.
(internal_syscall): Likewise.
(syscall_enter): Likewise.
(trace_syscall_entering): Likewise.
(get_error): Likewise.
(trace_syscall_exiting): Likewise.
Denys Vlasenko [Wed, 24 Aug 2011 15:25:32 +0000 (17:25 +0200)]
Unify per-architecture post-execve SIGTRAP check.
Move post-execve SIGTRAP check from get_scno_on_sysenter
(multitude of places on many architectures) to a single location
in trace_syscall_entering. This loosens the logic for some arches,
since many of them had additional checks such as scno == 0.
However, on non-ancient Linux kernels we should never have post-execve
SIGTRAP in the first place, by virtue of using PTRACE_O_TRACEEXEC.
Denys Vlasenko [Wed, 24 Aug 2011 14:59:23 +0000 (16:59 +0200)]
Speed up x86 by avoiding EAX read on syscall entry
on x86, EAX read on syscall entry is not necessary if we know
that post-execve SIGTRAP is disabled by PTRACE_O_TRACEEXEC ptrace option.
This patch (a) moves EAX retrieval from syscall_fixup
to get_scno_on_sysexit, and (b) perform EAX retrieval in syscall_fixup
only if we are in syscall entry and PTRACE_O_TRACEEXEC option is not on.
* syscall.c (get_scno_on_sysexit): On I386 and X86_64, read eax/rax
which contain syscall return value.
(syscall_fixup): On I386 and X86_64, read eax/rax only on syscall enter
and only if PTRACE_O_TRACEEXEC is not in effect.
Denys Vlasenko [Wed, 24 Aug 2011 14:56:03 +0000 (16:56 +0200)]
Do not read syscall no in get_scno_on_sysexit
* syscall.c (get_scno_on_sysexit): Remove scno retrieval code, since
we don't save it anyway. This is the first real logic change
which should make strace faster: for example, on x64 ORIG_EAX
is no longer read in each syscall exit.
Denys Vlasenko [Wed, 24 Aug 2011 14:47:32 +0000 (16:47 +0200)]
get_scno is an unholy mess, make it less horrible
Currently, get_scno does *much* more than "get syscall no".
It checks for post-execve SIGTRAP. It checks for changes
in personality. It retrieves params on entry and registers on exit.
Worse still, it is different in different architectures: for example,
for AVR32 regs are fetched in get_scno(), while for e.g. I386
it is done in syscall_enter().
Another problem is that get_scno() is called on both syscall entry and
syscall exit, which is stupid: we don't need to know scno on syscall
exit, it is already known from last syscall entry and stored in
tcp->scno! In essence, get_scno() does two completely different things
on syscall entry and on exit, they are just mixed into one bottle, like
shampoo and conditioner.
The following patches will try to improve this situation.
This change duplicates get_scno into identical get_scno_on_sysenter,
get_scno_on_sysexit functions. Call them in syscall enter and syscall
exit, correspondingly.
* defs.h: Rename get_scno to get_scno_on_sysenter; declare it only
if USE_PROCFS.
* strace.c (proc_open): Call get_scno_on_sysenter instead of get_scno.
* syscall.c (get_scno): Split into two (so far identical) functions
get_scno_on_sysenter and get_scno_on_sysexit.
(trace_syscall_entering): Call get_scno_on_sysenter instead of get_scno.
(trace_syscall_exiting): Call get_scno_on_sysexit instead of get_scno.
Dmitry V. Levin [Tue, 23 Aug 2011 16:24:20 +0000 (16:24 +0000)]
Reduce code redundancy in syscall_enter()
* syscall.c [LINUX] (syscall_enter): Move tcp->u_nargs initialization
from arch-specific ifdefs to common code. Always cache tcp->u_nargs in
a local variable and use it in for() loops.
[IA64, AVR32] Rewrite tcp->u_arg[] initialization using a loop.
Denys Vlasenko [Tue, 23 Aug 2011 16:04:25 +0000 (18:04 +0200)]
Define MAX_ARGS to 6 for all Linux arches
* defs.h: Define MAX_ARGS to 6 for all Linux arches.
* linux/ia64/syscallent.h: Change all 8-argument printargs
to MA (MAX_ARGS).
linux/mips/syscallent.h: Change all two 7-argument printargs
to MA (MAX_ARGS).
Denys Vlasenko [Tue, 23 Aug 2011 11:32:38 +0000 (13:32 +0200)]
Cache tcp->u_nargs in a local variable for for() loops
Loops of the form "for (i = 0; i < tcp->u_nargs; i++) ..."
need to fetch tcp->u_nargs from memory on every iteration
if "..." part has a function call (gcc doesn't know that
tcp->u_nargs won't change). This can be sped up
by putting tcp->u_nargs in a local variable, which might
go into a CPU register.
* syscall.c (decode_subcall): Cache tcp->u_nargs in a local variable
as for() loop limit value.
(syscall_enter): Likewise.
Denys Vlasenko [Tue, 23 Aug 2011 11:24:17 +0000 (13:24 +0200)]
Stop using nargs == -1 in syscallent tables
Usage -1 as argument count in syscallent tables
necessitates the check for it, a-la:
if (sysent[tcp->scno].nargs != -1)
tcp->u_nargs = sysent[tcp->scno].nargs;
else
tcp->u_nargs = MAX_ARGS;
which is stupid: we waste cycles checking something which
is constant and known at compile time.
* defs.h: Make struct sysent::nargs unsigned.
* freebsd/i386/syscallent.h: Replace nargs of -1 with MA.
* linux/s390/syscallent.h: Likewise.
* linux/s390x/syscallent.h: Likewise.
* svr4/syscallent.h: Likewise.
* freebsd/syscalls.pl: Likewise in generator script.
* syscallent.sh: Likewise in generator script.
* syscall.c: Add define MA MAX_ARGS / undef MA around includes
of syscallent[N].h.
Denys Vlasenko [Mon, 22 Aug 2011 00:06:35 +0000 (02:06 +0200)]
Fix -z display.
Before this patch, the following:
open("qwerty", O_RDONLY) = -1 ENOENT
write(2, "wc: qwerty: No such file or dire"..., 38) = 38
was shown totally wrongly with -z:
open("qwerty", O_RDONLY) = 38
(yes, that's right, write syscall is lost!)
Now it is shown "less wrongly" as:
open("qwerty", O_RDONLY <unfinished ...>
write(2, "wc: qwerty: No such file or dire"..., 38) = 38
* syscall.c (trace_syscall_exiting): Use common TCB_INSYSCALL clearing
via "goto ret". This fixes totally broken display of -z, but even now
it is not working as intended. Add a comment about that.
(trace_syscall_entering): Use common TCB_INSYSCALL setting
via "goto ret".
Denys Vlasenko [Sun, 21 Aug 2011 16:03:23 +0000 (18:03 +0200)]
Straighten up confused comments/messages about post-execve SIGTRAP handling
* defs.h: Explain TCB_INSYSCALL and TCB_WAITEXECVE bits in detail.
* strace.c (choose_pfd): Use entering/exiting macros instead of direct check
for TCB_INSYSCALL.
* syscall.c (get_scno): Use entering/exiting macros instead of direct check
for TCB_INSYSCALL. Fix comments about post-execve SIGTRAP.
(syscall_fixup): Use entering/exiting instead of direct check
for TCB_INSYSCALL. Add a comment what "not a syscall entry" message
usually means. Change wrong "stray syscall exit" messages into
"not a syscall entry" ones.
Denys Vlasenko [Sun, 21 Aug 2011 15:47:40 +0000 (17:47 +0200)]
count_syscall() always returns 0, optimize it
* defs.h (count_syscall): Change return type from int to void.
* count.c (count_syscall): Change return type from int to void.
* syscall.c (trace_syscall_exiting): Change code around call
to count_syscall accordingly.
Denys Vlasenko [Sun, 21 Aug 2011 15:26:55 +0000 (17:26 +0200)]
Conditionally optimize out unused code
* syscall.c (internal_syscall): Call internal_exec only if
SUNOS4 || (LINUX && TCB_WAITEXECVE).
* process.c (internal_exec): Define this function only if
SUNOS4 || (LINUX && TCB_WAITEXECVE).
(printwaitn): Don't check wordsize if SUPPORTED_PERSONALITIES == 1.
* signal.c (sys_kill): Likewise.
* syscall.c (is_negated_errno): Likewise.
(trace_syscall_exiting): Fold a tprintf into tprintfs which follow it.
Denys Vlasenko [Sat, 20 Aug 2011 00:12:33 +0000 (02:12 +0200)]
Small optimization in signal and ioctl tables
Trivial shuffling of data tables puts them all in one file,
allowing gcc to see their sizes and eliminate variables
which store these sizes.
Surprisingly, in C mode gcc does not optimize out static const int
variables. Help it by using enums instead.
* defs.h: Stop exporting ioctlent{0,1,2}, nioctlents{0,1,2},
signalent{0,1,2}, nsignals{0,1,2}.
* ioctl.c: Remove definitions of ioctlent{,0,1,2} and nioctlents{,0,1,2}.
* signal.c: Remove definitions of signalent{,0,1,2} and nsignals{,0,1,2}.
* syscall.c: Move above definitions to this file. Make them static const
or enums if suitable.
Denys Vlasenko [Fri, 19 Aug 2011 22:03:10 +0000 (00:03 +0200)]
Make needlessly static data local
* syscall.c (get_scno): For POWERPC64 and X86-64, variable currpers
is declared static. But its old data is never used. Convert it
to ordinary local variable.
Denys Vlasenko [Fri, 19 Aug 2011 15:41:28 +0000 (17:41 +0200)]
Set saner MAX_ARGS (6 or 8) for X86_64 and I386
I noticed that tcp->u_args[MAX_ARGS] array is way larger than
I'd expect: for all arches except HPPA it has 32 (!) elements.
I looked at the code and so far I spotted only one abuser of
this fact: sys_sigreturn. On several arches, it saves sigset_t
into tcp->u_args[1...N] on entry and prints it on exit, a-la
The problem here is that in glibc sigset_t is insanely large:
128 bytes, and using sizeof(sigset_t) in memcpy will overrun
&tcp->u_args[1] even with MAX_ARGS == 32:
On 32 bits, sizeof(tcp->u_args) == 32*4 == 128 bytes!
We may already have a bug there!
This commit changes the code to save NSIG / 8 bytes only.
NSIG can't ever be > 256, and in practice is <= 129,
thus NSIG / 8 is <= 16 bytes == 4 32-bit words,
and even MAX_ARGS == 5 should be enough for saving signal masks.
* defs.h: Reduce MAX_ARGS for X86_64 and I386 from 32 to 8
for FreeBSD and to 6 for everyone else. Add comment about current
state of needed MAX_ARGS.
* signal.c: Add comment about size of sigset_t.
(sprintsigmask): Reduce static string buffer from 8k to 2k.
(sys_sigreturn): Fix sigset saving to save only NSIG / 8 bytes,
not sizeof(sigset_t) bytes.
* linux/mips/syscallent.h: Reduce nargs of printargs-type syscall to 7.
* linux/arm/syscallent.h: Reduce nargs of printargs-type syscall to 6.
* linux/i386/syscallent.h: Likewise.
* linux/m68k/syscallent.h: Likewise.
* linux/powerpc/syscallent.h: Likewise.
* linux/s390/syscallent.h: Likewise.
* linux/s390x/syscallent.h: Likewise.
* linux/sh/syscallent.h: Likewise.
* linux/sh64/syscallent.h: Likewise.
* linux/sparc/syscallent.h: Likewise.
Denys Vlasenko [Fri, 19 Aug 2011 15:07:38 +0000 (17:07 +0200)]
Optimize sys_old_mmap
* mem.c (sys_old_mmap): For Ia64 and 32-bit personality of x86-64,
copy narrow parameters from userspace by single umove, not by six
separate ones; then assign them to long u_arg[i]. For SH[64],
avoid copying of tcp->u_arg.
(sys_mmap): Add FIXME comment - SH64 and i386 seem to be handled
differently for no apparent reason.
* test/mmap_offset_decode.c: New test program, illustrates FIXME.
Denys Vlasenko [Fri, 19 Aug 2011 14:11:07 +0000 (16:11 +0200)]
Untangle ifdef forest in sys_mmap64. No code changes
After careful analysis, it looks like !LINUX and ALPHA
pass all seven parameters in registers; and in all other cases
parameters are on stack (pointed to by tcp->u_arg[0]).
In light of this, reorganize ifdefs, making them simpler,
without changing any logic.
After this, it's apparent we use tcp->u_arg[4,5,6] and possibly
[7] without checking that it's valid to do so.
So far, just add a comment about this.
* mem.c (sys_mmap64): Rewrite ifdefs in a much simpler way.
Add comments about apparent bugs.
Denys Vlasenko [Fri, 19 Aug 2011 14:01:51 +0000 (16:01 +0200)]
Style and comment fixes, no code changes
* mem.c: Indent includes to show nesting better.
(addtileflags): Fix style of this function definition;
correct wrong endif comment, add another endif comment.
Denys Vlasenko [Thu, 18 Aug 2011 10:48:56 +0000 (12:48 +0200)]
Cosmetic fixes, no code changes
* defs.h: Add/reformat comments.
* signal.c: Remove wrong comment. Add warning directive
when we detect that NSIG is undefined. Add comment about
NSIG on ARM. Fix typo in comment.
(signame): Reformat code a bit without changes to logic.
Shorten static buffer.
(sys_rt_sigprocmask): Remove stray empty line.
* syscall.c: Add warning directive when we detect that
NSIG is undefined. Add comment about NSIG on ARM.
Dmitry V. Levin [Tue, 16 Aug 2011 18:57:29 +0000 (18:57 +0000)]
Fix PTRACE_SETOPTIONS tests
* strace.c [LINUX] (kill_save_errno): New function.
(test_ptrace_setoptions_followfork): Change return type to void.
Fix and harden error handling. Use kill_save_errno() to avoid errno
clobbering. Treat EIO from ptrace() the same way as EINVAL.
(test_ptrace_setoptions_for_all): Use kill_save_errno() to avoid errno
clobbering. Treat EIO from ptrace() the same way as EINVAL.
(main): Update use of test_ptrace_setoptions_followfork().
Denys Vlasenko [Wed, 17 Aug 2011 13:18:21 +0000 (15:18 +0200)]
Remove tcp->parent and TCB_CLONE_THREAD.
tcp->parent is used for only two things:
(1) to send signal on detach via tgkill (need to know tgid).
Solution: use tkill, it needs only tid.
(2) to optimize out ptrace options setting for new tracees.
Not a big deal if we drop this optimization: "set options" op is fast,
doing it just one extra time once per each tracee is hardly measurable.
TCB_CLONE_THREAD is a misnomer. It used only to flag sibling we attached to
in startup_attach. This is used to prevent infinite recursive rescanning
of /proc/PID/task.
Despite the name, there is no guarantee it is set only on non-leader:
if one would run "strace -f -p THREAD_ID" and THREAD_ID is *not*
a thread leader, strace will happily attach to it and all siblings
and will think that THREAD_ID is the leader! Which is a bug, but
since we no longer detach when we think tracee is going to die,
this bug no longer matters, because we do not use the knowledge
about thread group leaders for anything. (We used it to delay
leader's exit).
IOW: after this patch strace has no need to know about threads, parents
and children, and so on. Therefore it does not track that information.
It treats all tracees as independent entities. Overall,
this simplifies code a lot.
* defs.h: Add TCB_ATTACH_DONE flag, remove TCB_CLONE_THREAD flag
and struct tcb::parent field.
* process.c (internal_fork): Don't set tcpchild->parent.
* strace.c (startup_attach): Use TCB_ATTACH_DONE flag instead of
TCB_CLONE_THREAD to avoid attach attempts on already-attached threads.
Unlike TCB_CLONE_THREAD, TCB_ATTACH_DONE bit is used only temporarily,
and only in this function. We clear it on every tcb before we return.
(detach): Use tkill instead of tgkill.
(trace): Set ptrace options on new tracees unconditionally,
not only when tcp->parent == NULL.
Denys Vlasenko [Wed, 17 Aug 2011 09:30:56 +0000 (11:30 +0200)]
Remove TCB_SUSPENDED constant and related code.
Since we no longer suspend waitpid'ing tracees, we have only one case when
we suspend tracee: when we pick up a new tracee created by clone/fork/vfork.
Background: on some other OSes, attach to child is done this way:
get fork's result (pid), loop ptrace(PTRACE_ATTACH) until you hook up
new process/thread. This is ugly and not safe, but what matters for us
is that it doesn't require suspending. Suspending is required
on Linux only, because on Linux attach to child is done differently.
On Linux, we use two methods of catching new tracee:
adding CLONE_THREAD bit to syscall (if needed, we change
[v]fork into clone before that), or using ptrace options.
In both cases, it may be so that new tracee appears before one which
created it returns from syscall. In this case, current code
suspends new tracee until its creator returns. Only then
strace can determine who is its parent (it needs child's pid for this,
which is visible in parent's [v]fork/clone result).
This is inherently racy. For example, what if SIGKILL kills
creator after it succeeded creating child, but before it returns?
Looks like we will have child suspended forever.
But after previous commit, we DO NOT NEED parent<->child link for anything.
Therefore we do not need suspending too. Bingo!
This patch removes suspending code. Now new tracees will be continued
right away. Next patch will remove tcp->parent member.
* defs.h: Remove TCB_SUSPENDED constant
* process.c (handle_new_child): Delete this function.
(internal_fork): Do not call handle_new_child on syscall exit.
* strace.c (handle_ptrace_event): Delete this function.
(trace): Do not suspend new child; remove all handling
of now impossible TCB_SUSPENDED condition.
Denys Vlasenko [Wed, 17 Aug 2011 08:45:32 +0000 (10:45 +0200)]
Do not detach when we think tracee is going to die.
Current code plays some ungodly tricks, trying to not detach
thread group leader until all threads exit.
Also, it detaches from a tracee when signal delivery is detected
which will cause tracee to exit.
This operation is racy (not to mention the determination
whether signal is set to SIG_DFL is a horrible hack):
after we determined that this signal is indeed fatal
but before we detach and let process die,
*other thread* may set a handler to this signal, and
we will leak the process, falsely displaying it as killed!
I need to look in the past to figure out why we even do it.
First guess is that it's a workaround for old kernel bugs:
kernel used to deliver exit notifications to the tracer,
not to real parent. These workarounds are ancient
(internal_exit is from 1995).
The patch deletes the hacks. We no longer need tcp->nclone_threads,
TCB_EXITING and TCB_GROUP_EXITING. We also lose a few rather
ugly functions.
I also added a new message: "+++ exited with EXITCODE +++"
which shows exact moment strace got exit notification.
It is analogous to existing "+++ killed by SIG +++" message.
* defs.h: Delete struct tcb::nclone_threads field,
TCB_EXITING and TCB_GROUP_EXITING constants,
declarations of sigishandled() and internal_exit().
* process.c (internal_exit): Delete this function.
(handle_new_child): Don't ++tcp->nclone_threads.
* signal.c (parse_sigset_t): Delete this function.
(sigishandled): Delete this function.
* strace.c (startup_attach): Don't tcbtab[tcbi]->nclone_threads++.
(droptcb): Don't delay dropping if tcp->nclone_threads > 0,
don't drop parent if its nclone_threads reached 0:
just drop (only) this tcb unconditionally.
(detach): don't drop parent.
(handle_group_exit): Delete this function.
(handle_ptrace_event): Instead of handle_group_exit, just drop tcb;
do not panic if we see WIFEXITED from an attached pid;
print "+++ exited with EXITCODE +++" for every WIFEXITED pid.
* syscall.c (internal_syscall): Do not treat sys_exit specially -
don't call internal_exit on it.
Denys Vlasenko [Mon, 15 Aug 2011 10:24:14 +0000 (12:24 +0200)]
Slight optimization and cleanup in trace()
* strace.c (trace): Do not recalculate "cflag ? &ru : NULL"
again and again. Do not clear errno unnecessarily.
Consistently check wait errors as pid < 0, not pid == -1.
Indent ifdefs for better readability.
Remove comments after endif if ifdef/endif block is really tiny.
* configure.ac (AC_CHECK_DECLS): Add PTRACE_O_TRACESYSGOOD,
PTRACE_O_TRACEEXEC, PTRACE_O_TRACEEXIT, PTRACE_EVENT_EXEC,
PTRACE_EVENT_VFORK_DONE and PTRACE_EVENT_EXIT.
* defs.h [LINUX]: Define these PTRACE_* constants when they are not
provided by <sys/ptrace.h>.
Reported-by: Douglas Mencken <dougmencken@gmail.com> Reported-by: Steve Bennett <steveb@workware.net.au>
Denys Vlasenko [Fri, 24 Jun 2011 21:01:57 +0000 (23:01 +0200)]
Trivial cleanups
* strace.c (trace): Change ifdef LINUX to make a bit more sense,
remove wrong comment at its endif. Slightly optimize
"+++ killed by SIG +++" message for systems without WCOREDUMP macro.
Denys Vlasenko [Fri, 24 Jun 2011 14:41:35 +0000 (16:41 +0200)]
Better debug logging of allocations and waitpit results
* strace.c (alloc_tcb): Print number of allocated tcb's if -d.
(droptcb): Likewise.
(handle_ptrace_event): Remove PTRACE_EVENT_EXEC debug message.
(trace): Improve logging of waitpid: show WIFxxx, exitcode/signal,
ptrace event name, WCOREDUMP - all on one line.
Denys Vlasenko [Thu, 23 Jun 2011 20:22:34 +0000 (22:22 +0200)]
Optimize arrays of register indexes in syscall_enter
* syscall.c (syscall_enter) [BFIN]: Make register no array "static const".
[SH]: Make register no array "const", pre-multiply it by 4.
[SH64]: Make register no array "const".
[X86_64]: Make register no array "const", pre-multiply it by 8.
Denys Vlasenko [Thu, 23 Jun 2011 20:10:54 +0000 (22:10 +0200)]
Deindent syscall_enter by removing unnecessary braces. No code changes.
syscall_enter has many long (>80 columns) lines.
It is aggravated by the fact that it has a lot of {} blocks
which are not necessary (the code is the same without them).
This patch removes {}s and deindents affected lines.
While at it, it indents ifdefs so that nesting is easier to track,
and adds a few spaces in the expressions, such as
"tcp->u_nargs*sizeof..." -> "tcp->u_nargs * sizeof...".
There is no actual changes to the code here.
Denys Vlasenko [Thu, 23 Jun 2011 20:06:39 +0000 (22:06 +0200)]
Remove dead "ifndef CLONE_PTRACE" branch
process.c defines CLONE_PTRACE for Linux, so it can't be undefined.
Therefore ifndef CLONE_PTRACE code is dead (since at least 2004).
This patch removes it.
* process.c (handle_new_child): Remove ifdef CLONE_PTRACE/endif (but not
the code inside) and entire ifndef CLONE_PTRACE/endif block.
Denys Vlasenko [Thu, 23 Jun 2011 19:46:37 +0000 (21:46 +0200)]
Remove TCB_FOLLOWFORK
TCB_FOLLOWFORK flag seems to be unnecessary, because we either follow
all [v]forks/clones or don't follow any, therefore global variable
followfork is an already existing indicator of what we want to do.
This patch drops all setting/clearing of TCB_FOLLOWFORK bit,
and replaces checks for this bit by checks of followfork value.
In internal_fork, check is moved to in front of if(), since
the check is needed on both "entering" and "exiting" branch.
* defs.h: Remove TCB_FOLLOWFORK define.
* process.c (internal_fork): Do not set/clear TCB_FOLLOWFORK,
test followfork instead of tcp->flags & TCB_FOLLOWFORK.
(handle_new_child): Likewise.
* strace.c (startup_attach): Likewise.
Denys Vlasenko [Thu, 23 Jun 2011 11:10:28 +0000 (13:10 +0200)]
Do not allocate tiny cap_user_header/data structures, place them on stack.
This allows us to avoid having code to malloc them, and code to check
for malloc failure. Resulting code decrease:
text data bss dec hex filename
10175 0 16 10191 27cf system.o.old
9797 0 0 9797 2645 system.o
* system.c (sys_capget): Put cap_user_header_t and cap_user_data_t
on stack, rather than allocating them in heap. These structures
are very small (a few integer fields), stack is a better place
for them.
(sys_capset): Likewise.
Denys Vlasenko [Wed, 22 Jun 2011 11:17:16 +0000 (13:17 +0200)]
Make strace_fopen abort on error
Error from strace_fopen in main results in call to exit(1).
Error from strace_fopen in newoutf is propagated to newoutf
callers: startup_attach (where it results in exit(1))
and alloc_tcb (where error is ignored). In second case,
the behavior doesn't seem to be right: it means with -ff
on open error for new LOGFILE.PID the output will continue
to go into *the same file as the previous process* - which
would be confusing. Moreover, on droptcb outf may be closed
and the output of other, still running process outputting
to the same outf will be lost. I don't think this is sane.
IOW: in all cases, error in strace_fopen should be fatal.
* strace.c (strace_fopen): Abort on error instead of returning NULL.
(newoutf): Change return type to void.
(startup_attach): Remove error check on newoutf return value.
(main): Remove error check on strace_fopen return value.
Denys Vlasenko [Wed, 22 Jun 2011 11:11:23 +0000 (13:11 +0200)]
Make set_cloexec_flag abort on error
set_cloexec_flag() may fail only if we pass it a bad fd,
such as -1 or non-opened one. If we do, we have a bug
in the caller. It makes no sense to try to continue
running when we detect such a blatant bug in our own code.
* strace (set_cloexec_flag): Abort instead of returning error
indicator. Change function to return void.
(strace_fopen): Remove error check on set_cloexec_flag return value.
(proc_open): Likewise.
(proc_poll_open): Likewise.
Denys Vlasenko [Wed, 22 Jun 2011 11:03:56 +0000 (13:03 +0200)]
Make strace_popen abort on error
It makes no sense to postpone abort on strace_popen error
unti it returns. Moreover, out-of-memory error was exiting
without any message.
While at it, use 0 as "none" for popen_pid, as optimization.
* strace: Initialize popen_pid to 0 - this puts it in bss.
(trace): Reset popen_pid to 0 instead of -1.
(strace_popen): Never return NULL as error indicator,
abort with good error message instead.
(main): Remove NULL check of strace_popen result.
Denys Vlasenko [Wed, 22 Jun 2011 10:45:25 +0000 (12:45 +0200)]
Delete fork_tcb()
Get rid of fork_tcb() function. It used to do what the comment
above it says, but now it doesn't do much:
it only sets tcp->flags |= TCB_FOLLOWFORK and maybe calls
expand_tcbtab(). The second operation is not necessary, since
alloc_tcp() will do it itself when needed.
This patch deletes fork_tcb(), open-coding tcp->flags |= TCB_FOLLOWFORK
where it was formerly called. It also makes nprocs, tcbtabsize and
expand_tcbtab() static. (While at it, I nuked redundant
extern char **environ declaration: strace.c had *two* of them...)
* defs.h: Remove declarations of nprocs, tcbtabsize and
expand_tcbtab.
* process.c (fork_tcb): Remove this function.
(internal_fork): Open-code fork_tcb.
(handle_new_child): Likewise.
* strace.c: Remove redundant "extern char **environ". Declare
nprocs and tcbtabsize static.
(expand_tcbtab): Make it static.
Denys Vlasenko [Wed, 22 Jun 2011 10:41:57 +0000 (12:41 +0200)]
Simplify expand_tcbtab and alloc_tcb
Get rid of a few intermediate variables, simplifies a few expressions,
and uses error_msg_and_die instead of more verbose
fprintf+cleanup+exit sequence.
In alloc_tcp, I use memset to clear entire new tcp.
This not only saves a few bytes of code, but lowers the chances
of future bugs where some data "leaks out" into new tcb's
from old ones because we forgot to re-initialize it.
* strace.c (expand_tcbtab): Simplify this function. No logic changes.
(alloc_tcb): Likewise.
Denys Vlasenko [Tue, 21 Jun 2011 22:09:25 +0000 (00:09 +0200)]
Trivial fixes
* process.c (internal_fork): Remove conditionals which make no difference
(we return 0 on both branches of these ifs).
* util.c: Fix indentation of an ifdef.
Denys Vlasenko [Tue, 21 Jun 2011 12:34:10 +0000 (14:34 +0200)]
Fix regression introduced by "Properly handle real SIGTRAPs" change
Commit 3454e4b463e6c22c7ea8c5461ef5a077f4650a54
introduced a bug: sometimes, TRACECLONE/TRACE[V]FORK opts were not set.
The check (tcp->parent == NULL) in old code was meant to check
"if we are not a child created by auto-attach" - in this case,
options need to be set on the child; otherwise they are inherited
and do not need to be set.
I misunderstood the check and if tcp->parent is not NULL, I was
setting only ptrace_setoptions_for_all bits.
This change fixes the problem. Since the fixed logic makes it
unnecessary to keep two sets of options in separate variables,
I merge them back into one variable, ptrace_setoptions.
* defs.h: Merge ptrace_setoptions_followfork and ptrace_setoptions_for_all
into one variable, ptrace_setoptions.
* strace.c: Likewise.
(test_ptrace_setoptions_followfork): Use ptrace_setoptions variable.
(test_ptrace_setoptions_for_all): Likewise.
(main): Likewise.
* process.c (internal_fork): Likewise.
(internal_exec): Likewise.
* strace.c (trace): Fix the bug where different options were set
depending on "tcp->parent == NULL" condition. Add a comment
which makes it more clear why this condition is checked.
Denys Vlasenko [Sat, 18 Jun 2011 09:29:10 +0000 (11:29 +0200)]
Do not suspend waitpid.
strace used to suspend waitpid until there is a child
for waitpid'ing process to collect status from.
Apparently, it was done because in some very old kernels
(circa 2002 or even earlier) there were ptrace bugs which
were making waitpid in real parent to not see children.
This kernel bug is fixed long ago. This change removes the workaround.
test/wait_must_be_interruptible.c is a test program which
illustrates why without this change strace changes
programs's behavior.
* defs.h: Delete waitpid and nclone_waiting members from from struct tcb.
Remove declaration of internal_wait().
* process.c (internal_wait): Remove this function.
* strace.c (alloc_tcb): Do not set tcp->nclone_waiting.
(resume): Remove this function.
(resume_from_tcp): Remove this function.
(detach): Do not call resume_from_tcp().
(handle_group_exit): Do not call resume_from_tcp().
* syscall.c (internal_syscall): Do not call internal_wait().
Andi Kleen [Mon, 13 Jun 2011 22:05:44 +0000 (22:05 +0000)]
Enhance io_submit() decoding
strace didn't decode important fields in the iocb passed to io_submit.
This patch changes the code to dump them all. Also it prefixes the fields
with names to make it easier to read.
* desc.c (iocb_cmd_lookup, print_common_flags): New functions.
(sys_io_submit): New iocb decoder.
Denys Vlasenko [Wed, 8 Jun 2011 23:43:22 +0000 (01:43 +0200)]
Don't display bogus parameter for sigreturn syscall
* linux/*/syscallent.h: For those arches which use sys_sigreturn,
not printargs, to show [rt_]sigreturn syscall, change number of arguments
from 1 to 0: sys_sigreturn function doesn't use syscall parameters.
(I guess kernel doesn't actually _have_ any parameters for this syscall,
at least on these architectures). Do the same change for I386 and x86-64
even though they use printargs: I looked at kernel code and syscall
definitely doesn't have any parameters on these arches.
(I hesitate to change 1 to 0 params for arches I don't know -
it is remotely possible some of them do have a parameter for this syscall).
Denys Vlasenko [Wed, 8 Jun 2011 23:36:29 +0000 (01:36 +0200)]
Optimize sigreturn handling
* signal.c (sys_sigreturn): move stack pointer variables,
and for SPARC and MIPS, stack pointer and sigmask reading code
into "if (entering) ..." block, because it is only needed
in this branch; load tcp->u_arg[1] into sigmask for display
_after_ we know for sure u_arg[1] does contain valid sigmask
(IOW: perform operation only when we know we will need the result)
projects / strace / log