From: Masatake YAMATO <yamato@redhat.com>
Date: Wed, 16 Apr 2014 06:33:09 +0000 (+0900)
Subject: unwind: report expected backtracing error
X-Git-Tag: v4.9~51
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b45b7faa1fc0373831fb00a3892de45b4df87232;p=strace

unwind: report expected backtracing error

When a file mmap'ed to the target process is unlink'ed, backtracing the
stack would fail.  Current implementation reports it as
"backtracing_error".  To avoid confusion, the message is changed to
"expected_backtracing_error".

Here is the reproducer:

  $ cat ./p-deleted.c
  #include <unistd.h>

  int main(int argc, char **argv) {
    return unlink(argv[0]) < 0;
  }

  $ strace -e unlink -k ./p-deleted
  unlink("./p-deleted")                   = 0
   > /usr/lib64/libc-2.18.so(unlink+0x7) [0xe7f17]
   > /home/yamato/var/strace/t_unwind/p-deleted (deleted)(+0x0) [0x575]
   > /usr/lib64/libc-2.18.so(__libc_start_main+0xf5) [0x21d65]
   > backtracing_error [0x7ffff1365590]
  +++ exited with 0 +++

p-deleted is deleted therefore backtracing_error is reported.  This
patch records the deleted marker when making mmap cache and refers the
recorded information in the case "backtracing_error" to switch the
message.

Here is the output of this patch:

  $ strace -e unlink -k ./p-deleted
  unlink("./p-deleted")                   = 0
   > /usr/lib64/libc-2.18.so(unlink+0x7) [0xe7f17]
   > /home/yamato/var/strace/t_unwind/p-deleted (deleted)(+0x0) [0x575]
   > /usr/lib64/libc-2.18.so(__libc_start_main+0xf5) [0x21d65]
   > expected_backtracing_error [0x7ffff1365590]
  +++ exited with 0 +++

This solution is not perfect: if a file is unlink'ed after making the
mmap cache and before unwinding, strace cannot have a chance to record
the deleted marker.

In this version of patch, hardcoded magic number used in comparing "(delete)"
string is replaced with strlen as suggested by Dmitry Levin.

In old version of patch, the deleted entry was thrown away from mmap
cache to avoid to report "backtracing_error".  In this patch I keep it,
and just switch the error message.
Inspired by the review comment from Dmitry Levin.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
---

diff --git a/unwind.c b/unwind.c
index ef5a404c..53191be4 100644
--- a/unwind.c
+++ b/unwind.c
@@ -48,6 +48,7 @@ struct mmap_cache_t {
 	unsigned long end_addr;
 	unsigned long mmap_offset;
 	char* binary_filename;
+	bool deleted;
 };
 
 /*
@@ -134,6 +135,10 @@ build_mmap_cache(struct tcb* tcp)
 	struct mmap_cache_t *cache_head;
 	FILE *fp;
 
+	const char *deleted = " (deleted)";
+	size_t blen;
+	size_t dlen;
+
 	sprintf(filename, "/proc/%d/maps", tcp->pid);
 	fp = fopen(filename, "r");
 	if (!fp) {
@@ -170,6 +175,13 @@ build_mmap_cache(struct tcb* tcp)
 		cur_entry->mmap_offset = mmap_offset;
 		cur_entry->binary_filename = strdup(binary_path);
 
+		dlen = strlen(deleted);
+		blen = strlen(binary_path);
+		if (blen >= dlen && strcmp(binary_path + blen - dlen, deleted) == 0)
+			cur_entry->deleted = true;
+		else
+			cur_entry->deleted = false;
+
 		/*
 		 * sanity check to make sure that we're storing
 		 * non-overlapping regions in ascending order
@@ -271,6 +283,7 @@ stacktrace_walk(struct tcb *tcp,
 	char * symbol_name;
 	struct mmap_cache_t* cur_mmap_cache;
 	unsigned long true_offset;
+	bool berror_expected = false;
 
 	if (!tcp->mmap_cache)
 		error_msg_and_die("bug: mmap_cache is NULL");
@@ -313,6 +326,9 @@ stacktrace_walk(struct tcb *tcp,
 						die_out_of_memory();
 				}
 
+				if (cur_mmap_cache->deleted)
+					berror_expected = true;
+
 				true_offset = ip - cur_mmap_cache->start_addr +
 					cur_mmap_cache->mmap_offset;
 				if (symbol_name[0]) {
@@ -349,7 +365,10 @@ stacktrace_walk(struct tcb *tcp,
 		}
 		if (lower > upper) {
 			error_action(data,
-				     "backtracing_error", ip);
+				     berror_expected
+				     ?"expected_backtracing_error"
+				     :"unexpected_backtracing_error",
+				     ip);
 			goto ret;
 		}