Re: [Tarantool-patches] [PATCH v5 4/4] crash: report crash data to the feedback server

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

On 24.12.2020 14:16, Cyrill Gorcunov wrote:
> On Thu, Dec 24, 2020 at 12:22:53AM +0300, Cyrill Gorcunov wrote:
>> On Wed, Dec 23, 2020 at 07:47:44PM +0100, Vladislav Shpilevoy wrote:
>>> Hi! Thanks for the patch!
>>>
>>> See 3 comments and fixes below, and top of the branch in a
>>> separate commit.
>>>
>>
>> Vlad, since we can use json_escape I dropped usage of base64
>> encoder for backtrace. The final interdiff I pushed into
>> gorcunov/gh-5261-crash-report-6 is the following
> 
> FWIW gorcunov/gh-5261-crash-report-6 passes all tests
> https://gitlab.com/tarantool/tarantool/-/pipelines/234153923

The branch didn't work on my machine when I tried to crash Tarantool.
Because uname.version even being json-escaped contained a substring
which is not a valid Lua string. This was the broken string:

	"Darwin Kernel Version 19.5.0: Tue May 26 20:41:44 PDT 2020; root:xnu-6153.121.2~2\/RELEASE_X86_64"

Lua didn't understand symbol sequence `\/`.

To workaround this I made the child process accept the dump data and
the feedback host via command line arguments. Now seems to be working.

I pushed my fixes on top of the branch, and pasted them below.

====================
diff --git a/src/lib/core/crash.c b/src/lib/core/crash.c
index e62af6836..ee4991b52 100644
--- a/src/lib/core/crash.c
+++ b/src/lib/core/crash.c
@@ -258,10 +258,10 @@ crash_report_feedback_daemon(struct crash_info *cinfo)
 	 * modifications in future.
 	 */
 	size = (char *)uname_ptr - p;
-	snprintf_safe("require(\'http.client\').post(\'%s\',"
-		      "'{\"crashdump\":{\"version\":\"%d\","
-		      "\"data\":", feedback_host,
-		      crashinfo_version);
+	snprintf_safe("{");
+	snprintf_safe("\"crashdump\":{");
+	snprintf_safe("\"version\":\"%d\",", crashinfo_version);
+	snprintf_safe("\"data\":");
 
 	/* The "data" key value */
 	snprintf_safe("{");
@@ -334,15 +334,20 @@ crash_report_feedback_daemon(struct crash_info *cinfo)
 	 * main feedback daemon uses.
 	 */
 	size = e - p;
-	snprintf_safe("}}',{timeout=1});os.exit(1);");
+	snprintf_safe("}");
+	snprintf_safe("}");
 
 	pr_debug("crashinfo script: %s", head);
 
-	char *exec_argv[4] = {
+	char *exec_argv[7] = {
 		[0] = tarantool_path,
 		[1] = "-e",
-		[2] = head,
-		[3] = NULL,
+		[2] = "require('http.client').post(arg[1],arg[2],{timeout=1});"
+		      "os.exit(1);",
+		[3] = "-",
+		[4] = feedback_host,
+		[5] = head,
+		[6] = NULL,
 	};
 
 	/*
@@ -360,9 +365,11 @@ crash_report_feedback_daemon(struct crash_info *cinfo)
 		 * is running fine.
 		 */
 		execve(exec_argv[0], exec_argv, environ);
-		pr_crit("exec(%s,[%s,%s,%s]) failed",
+		pr_crit("exec(%s,[%s,%s,%s,%s,%s,%s,%s]) failed",
 			exec_argv[0], exec_argv[0],
-			exec_argv[1], exec_argv[2]);
+			exec_argv[1], exec_argv[2],
+			exec_argv[3], exec_argv[4],
+			exec_argv[5], exec_argv[6]);
 		_exit(1);
 	} else if (pid < 0) {
 		pr_crit("unable to vfork (errno %d)", errno);