Tarantool development patches archive
 help / color / mirror / Atom feed
From: Ilya Markov <imarkov@tarantool.org>
To: georgy@tarantool.org
Cc: tarantool-patches@freelists.org
Subject: [tarantool-patches] [error 3/3] error: Add C frames in error.traceback
Date: Fri,  4 May 2018 17:07:20 +0300	[thread overview]
Message-ID: <bcb4c91ba22b3f8f65a156b6173ffe647c1e2ae8.1525442633.git.imarkov@tarantool.org> (raw)
In-Reply-To: <cover.1525442633.git.imarkov@tarantool.org>
In-Reply-To: <cover.1525442633.git.imarkov@tarantool.org>

* Add backtrace generation in exception contructor.
* Add merge of lua and C traces.
* Modify C backtrace with support of current fiber stack.

Relates #677
---
 src/backtrace.cc                  | 10 ++++-
 src/exception.cc                  | 49 +++++++++++++++++++++
 src/lua/error.c                   | 92 +++++++++++++++++++++++++++++++++++++--
 test/app/traceback.result         | 45 ++++++++++++++++---
 test/app/traceback.test.lua       | 14 +++++-
 test/box/misc.result              | 26 ++++++++---
 test/replication/wal_off.result   |  2 +-
 test/replication/wal_off.test.lua |  2 +-
 8 files changed, 220 insertions(+), 20 deletions(-)

diff --git a/src/backtrace.cc b/src/backtrace.cc
index a86255e..c0c778e 100644
--- a/src/backtrace.cc
+++ b/src/backtrace.cc
@@ -368,7 +368,15 @@ backtrace_foreach(backtrace_cb cb, coro_context *coro_ctx, void *cb_ctx)
 {
 	unw_cursor_t unw_cur;
 	unw_context_t unw_ctx;
-	coro_unwcontext(&unw_ctx, coro_ctx);
+	if (coro_ctx == &fiber()->ctx) {
+		/*
+		 * We don't have to move over stack as we are inspecting
+		 * current fiber.
+		 */
+		unw_getcontext(&unw_ctx);
+	} else {
+		coro_unwcontext(&unw_ctx, coro_ctx);
+	}
 	unw_init_local(&unw_cur, &unw_ctx);
 	int frame_no = 0;
 	unw_word_t sp = 0, old_sp = 0, ip, offset;
diff --git a/src/exception.cc b/src/exception.cc
index 56077f7..d6b36ab 100644
--- a/src/exception.cc
+++ b/src/exception.cc
@@ -36,6 +36,7 @@
 
 #include "fiber.h"
 #include "reflection.h"
+#include "backtrace.h"
 
 extern "C" {
 
@@ -93,6 +94,7 @@ static const struct method_info exception_methods[] = {
 const struct type_info type_Exception = make_type("Exception", NULL,
 	exception_methods);
 
+
 void *
 Exception::operator new(size_t size)
 {
@@ -114,6 +116,44 @@ Exception::~Exception()
 	if (this != &out_of_memory) {
 		assert(refs == 0);
 	}
+	if (this->frames_count > 0) {
+		struct diag_frame *frame, *next;
+		/* Cleanup lua trace. */
+		for (frame = rlist_first_entry(&this->frames, typeof(*frame),
+					       link);
+		     &frame->link != &this->frames;) {
+			next = rlist_next_entry(frame, link);
+			free(frame);
+			frame = next;
+		}
+	}
+}
+
+static int
+error_backtrace_cb(int frameno, void *frameret, const char *func,
+		   size_t offset, void *cb_ctx)
+{
+	(void) frameno;
+	(void) frameret;
+	(void) offset;
+	Exception *e = (Exception *) cb_ctx;
+
+	struct diag_frame * frame =
+		(struct diag_frame *) malloc(sizeof(*frame));
+	if (frame == NULL) {
+		diag_set(OutOfMemory, sizeof(struct diag_frame),
+			 "malloc", "struct diag_frame");
+		return -1;
+	}
+	if (func)
+		snprintf(frame->func_name, sizeof(frame->func_name), "%s", func);
+	else
+		frame->func_name[0] = 0;
+	frame->filename[0] = 0;
+	frame->line = 0;
+	rlist_add_tail_entry(&e->frames, frame, link);
+	e->frames_count++;
+	return 0;
 }
 
 Exception::Exception(const struct type_info *type_arg, const char *file,
@@ -121,6 +161,15 @@ Exception::Exception(const struct type_info *type_arg, const char *file,
 {
 	error_create(this, exception_destroy, exception_raise,
 		     exception_log, type_arg, file, line);
+	int old_errno = errno;
+	if (cord()) {
+		backtrace_foreach(error_backtrace_cb, &fiber()->ctx, this);
+		if (this->frames_count > 0) {
+			rlist_shift_tail(&this->frames);
+			this->frames_count--;
+		}
+	}
+	errno = old_errno;
 }
 
 void
diff --git a/src/lua/error.c b/src/lua/error.c
index e6ccd32..3e667a5 100644
--- a/src/lua/error.c
+++ b/src/lua/error.c
@@ -31,6 +31,7 @@
 
 #include <diag.h>
 #include <fiber.h>
+#include <backtrace.h>
 #include "utils.h"
 #include "error.h"
 
@@ -104,15 +105,36 @@ luaT_pusherror(struct lua_State *L, struct error *e)
 	luaL_setcdatagc(L, -2);
 }
 
+static inline void
+copy_frame(struct diag_frame *dest, struct diag_frame *src)
+{
+	dest->line = src->line;
+	strcpy(dest->func_name, src->func_name);
+	strcpy(dest->filename, src->filename);
+}
+
 static int
 traceback_error(struct lua_State *L, struct error *e)
 {
 	lua_Debug ar;
 	int level = 0;
+	struct rlist lua_frames;
+	struct rlist *lua_framesp;
+	struct diag_frame *frame, *next;
+	if (e->frames_count <= 0) {
+		lua_framesp = &e->frames;
+	} else {
+		lua_framesp = &lua_frames;
+		rlist_create(lua_framesp);
+	}
+	/*
+	 * At this moment error object was created from exception so
+	 * traceback list is already created and filled with C trace.
+	 * Now we need to create lua trace and merge it with the existing one.
+	 */
 	while (lua_getstack(L, level++, &ar) > 0) {
 		lua_getinfo(L, "Sln", &ar);
-		struct diag_frame *frame =
-				(struct diag_frame *) malloc(sizeof(*frame));
+		frame = (struct diag_frame *) malloc(sizeof(*frame));
 		if (frame == NULL) {
 			luaT_pusherror(L, e);
 			return 1;
@@ -136,7 +158,67 @@ traceback_error(struct lua_State *L, struct error *e)
 			frame->line = 0;
 			e->frames_count++;
 		}
-		rlist_add_entry(&e->frames, frame, link);
+		rlist_add_entry(lua_framesp, frame, link);
+	}
+	if (e->frames_count > 0) {
+		struct diag_frame *lua_frame = rlist_first_entry(lua_framesp,
+							 typeof(*lua_frame),
+							 link);
+		rlist_foreach_entry(frame, &e->frames, link) {
+			/* We insert trace of lua user code in c trace,
+			 * where C calls lua.*/
+			if (strncmp(frame->func_name, "lj_BC_FUNCC",
+				    sizeof("lj_BC_FUNCC") - 1) == 0 &&
+			    frame->line != -1) {
+				/* We have to bypass internal error calls. */
+				next = rlist_next_entry(frame, link);
+				if (strncmp(next->func_name, "lj_err_run",
+					    sizeof("lj_err_run") - 1) == 0)
+					continue;
+				e->frames_count--;
+				for (;&lua_frame->link != lua_framesp;
+				      lua_frame = rlist_next_entry(lua_frame,
+								    link)) {
+					/* Skip empty frames. */
+					if (strncmp(lua_frame->filename,
+						    "[C]",
+						    sizeof("[C]") - 1) == 0 &&
+					    (*lua_frame->func_name) == '?')
+						continue;
+					break;
+				}
+
+				for (; &lua_frame->link != lua_framesp;
+				       lua_frame = rlist_next_entry(lua_frame,
+								    link)) {
+					if (lua_frame->filename[0]== 0 &&
+					    lua_frame->func_name[0] == 0)
+						break;
+					struct diag_frame *frame_copy =
+						(struct diag_frame *)
+						    malloc(sizeof(*frame_copy));
+					copy_frame(frame_copy, lua_frame);
+					rlist_add_entry(&frame->link,
+							frame_copy, link);
+					e->frames_count++;
+					/*
+					 * Mark the C frame that it was replaced
+					 * with lua.
+					 */
+					frame->line = -1;
+				}
+			}
+		}
+	}
+	if (&e->frames != lua_framesp) {
+		/* Cleanup lua trace. */
+		for (frame = rlist_first_entry(lua_framesp, typeof(*frame),
+					       link);
+		     &frame->link != lua_framesp;) {
+			next = rlist_next_entry(frame, link);
+			free(frame);
+			frame = next;
+		}
 	}
 	luaT_pusherror(L, e);
 	return 1;
@@ -174,6 +256,10 @@ lua_error_gettraceback(struct lua_State *L)
 	rlist_foreach_entry(frame, &e->frames, link) {
 		if (frame->func_name[0] != 0 || frame->line > 0 ||
 		    frame->filename[0] != 0) {
+			if (strncmp(frame->func_name, "lj_BC_FUNCC",
+				    sizeof("lj_BC_FUNCC") - 1) == 0 &&
+			    frame->line == -1)
+				continue;
 			/* push index */
 			lua_pushnumber(L, index++);
 			/* push value - table of filename and line */
diff --git a/test/app/traceback.result b/test/app/traceback.result
index bb54573..85576bf 100644
--- a/test/app/traceback.result
+++ b/test/app/traceback.result
@@ -5,17 +5,17 @@ err.type
 ---
 - ClientError
 ...
-err.trace[1].file
+err.trace[12].file
 ---
 - builtin/socket.lua
 ...
-err.trace[1].line
+err.trace[12].line
 ---
 - 997
 ...
 #err.trace
 ---
-- 8
+- 30
 ...
 s, err = pcall(error, "oh no" )
 ---
@@ -33,7 +33,7 @@ err.message
 ...
 #err.trace
 ---
-- 8
+- 33
 ...
 nil_var=nil
 ---
@@ -61,7 +61,7 @@ err.message
 ...
 #err.trace
 ---
-- 10
+- 36
 ...
 box.begin()
 ---
@@ -82,7 +82,38 @@ err.message
 ...
 #err.trace
 ---
-- 10
+- 39
+...
+s = box.schema.space.create("space")
+---
+...
+_ = s:create_index("prim")
+---
+...
+_ =s:on_replace(fail)
+---
+...
+st,err = pcall(s.insert, s, {1})
+---
+...
+err= err:unpack()
+---
+...
+err.type
+---
+- LuajitError
+...
+err.message
+---
+- '[string "function fail() return nil_var.b end "]:1: attempt to index global ''nil_var''
+  (a nil value)'
+...
+#err.trace
+---
+- 45
+...
+s:drop()
+---
 ...
 errinj = box.error.injection
 ---
@@ -117,7 +148,7 @@ err.message
 ...
 #err.trace
 ---
-- 8
+- 32
 ...
 space:drop()
 ---
diff --git a/test/app/traceback.test.lua b/test/app/traceback.test.lua
index 51af677..1034049 100644
--- a/test/app/traceback.test.lua
+++ b/test/app/traceback.test.lua
@@ -1,7 +1,7 @@
 s, err = pcall(box.error, 1, "err")
 err.type
-err.trace[1].file
-err.trace[1].line
+err.trace[12].file
+err.trace[12].line
 #err.trace
 
 s, err = pcall(error, "oh no" )
@@ -27,6 +27,16 @@ err.type
 err.message
 #err.trace
 
+s = box.schema.space.create("space")
+_ = s:create_index("prim")
+_ =s:on_replace(fail)
+st,err = pcall(s.insert, s, {1})
+err= err:unpack()
+err.type
+err.message
+#err.trace
+s:drop()
+
 errinj = box.error.injection
 space = box.schema.space.create('tweedledum')
 index = space:create_index('primary', { type = 'hash' })
diff --git a/test/box/misc.result b/test/box/misc.result
index a5cad07..0a3c445 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -130,42 +130,58 @@ e:unpack()
   code: 1
   message: Illegal parameters, bla bla
   trace:
-  - file: builtin/socket.lua
-    line: <number>
+  - function: ClientError::ClientError(char const*, unsigned int, unsigned int, ...)
+  - function: BuildClientError
+  - function: box_error_set
+  - function: luaT_error_raise(lua_State*)
   - function: pcall
   - file: builtin/box/console.lua
+    function: eval
     line: <number>
   - file: builtin/box/console.lua
     function: repl
     line: <number>
   - file: builtin/box/console.lua
-    function: eval
     line: <number>
   - function: pcall
   - file: builtin/socket.lua
     line: <number>
+  - function: lua_pcall
+  - function: luaT_call
+  - function: luaB_pcall
   - function: pcall
   - file: builtin/box/console.lua
+    function: eval
     line: <number>
   - file: builtin/box/console.lua
     function: repl
     line: <number>
   - file: builtin/box/console.lua
-    function: eval
     line: <number>
   - function: pcall
   - file: builtin/socket.lua
     line: <number>
+  - function: lua_pcall
+  - function: luaT_call
+  - function: luaB_pcall
   - function: pcall
   - file: builtin/box/console.lua
+    function: eval
     line: <number>
   - file: builtin/box/console.lua
     function: repl
     line: <number>
   - file: builtin/box/console.lua
-    function: eval
     line: <number>
   - function: pcall
+  - file: builtin/socket.lua
+    line: <number>
+  - function: lua_pcall
+  - function: luaT_call
+  - function: lua_fiber_run_f
+  - function: fiber_cxx_invoke(int (*)(__va_list_tag*), __va_list_tag*)
+  - function: fiber_loop
+  - function: coro_init
 ...
 e.type
 ---
diff --git a/test/replication/wal_off.result b/test/replication/wal_off.result
index f50bfc2..451c8de 100644
--- a/test/replication/wal_off.result
+++ b/test/replication/wal_off.result
@@ -84,7 +84,7 @@ box.cfg { replication = wal_off_uri }
 check = "Read access to universe"
 ---
 ...
-while string.find(box.info.replication[wal_off_id].upstream.message, check) == nil do fiber.sleep(0.01) end
+while box.info.replication[wal_off_id].upstream.message == nil or string.find(box.info.replication[wal_off_id].upstream.message, check) == nil do fiber.sleep(0.01) end
 ---
 ...
 box.cfg { replication = "" }
diff --git a/test/replication/wal_off.test.lua b/test/replication/wal_off.test.lua
index 43cd0ae..9a5e02d 100644
--- a/test/replication/wal_off.test.lua
+++ b/test/replication/wal_off.test.lua
@@ -29,7 +29,7 @@ test_run:cmd('switch default')
 
 box.cfg { replication = wal_off_uri }
 check = "Read access to universe"
-while string.find(box.info.replication[wal_off_id].upstream.message, check) == nil do fiber.sleep(0.01) end
+while box.info.replication[wal_off_id].upstream.message == nil or string.find(box.info.replication[wal_off_id].upstream.message, check) == nil do fiber.sleep(0.01) end
 box.cfg { replication = "" }
 
 test_run:cmd("stop server wal_off")
-- 
2.7.4

      parent reply	other threads:[~2018-05-04 14:07 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <14848871.eLyv8AkjAN@home.lan>
2018-05-04 14:07 ` [tarantool-patches] [error 0/3] Introduce error traceback Ilya Markov
2018-05-04 14:07   ` [tarantool-patches] [error 1/3] lua: moving lua error functions to separate file Ilya Markov
2018-05-04 14:07   ` [tarantool-patches] [error 2/3] error: Add lua traceback Ilya Markov
2018-05-04 14:07   ` Ilya Markov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bcb4c91ba22b3f8f65a156b6173ffe647c1e2ae8.1525442633.git.imarkov@tarantool.org \
    --to=imarkov@tarantool.org \
    --cc=georgy@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [error 3/3] error: Add C frames in error.traceback' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox