Re: [LTP] [PATCH v5] coredump01: New core_pattern specifiers test

[Li Wang <li.wang@linux.dev>]

Hi Andrea,

> +static void verify_file_pattern(void)
> +{
> +	char dump[PATH_MAX + 32];
> +	pid_t pid;
> +
> +	tst_res(TINFO, "Testing file core_pattern");
> +
> +	set_pattern("%s/core.%%e.%%p.%%s", cwd);
> +
> +	pid = crash_child();
> +
> +	snprintf(dump, sizeof(dump), "%s/core.coredump01.%d.%d", cwd, pid, SIGABRT);
> +
> +	TST_EXP_PASS(access(dump, F_OK), "core.%%e.%%p.%%s expanded to core.coredump01.%d.%d",
> +		     pid, SIGABRT);

Generally this checks is really nice, but it'd be better to
add more checks for the core dump file?

e.g. file size > 0, ELF magic valid, and type == ET_CORE.

FYI:

+static int verify_core_dump(const char *path, long long expected_size)
+{
+       int fd;
+       struct stat st;
+       unsigned char magic[SELFMAG];
+       Elf64_Ehdr ehdr;
+
+       if (access(path, F_OK) != 0) {
+               tst_res(TFAIL, "Core dump file %s does not exist", path);
+               return -1;
+       }
+       tst_res(TPASS, "Core dump file %s exists", path);
+
+       SAFE_STAT(path, &st);
+       TST_EXP_EXPR(st.st_size > 0, "core file size = %ld bytes", st.st_size);
+
+       if (expected_size > 0 && st.st_size != expected_size) {
+               tst_res(TFAIL, "Core dump size mismatch: expected %lld bytes, got %ld",
+                       expected_size, st.st_size);
+               return -1;
+       }
+
+       fd = SAFE_OPEN(path, O_RDONLY, 0);
+       SAFE_READ(0, fd, magic, SELFMAG);
+       SAFE_CLOSE(fd);
+
+       TST_EXP_EXPR(!memcmp(magic, ELFMAG, 4),
+                       "core file has valid ELF magic");
+
+       fd = SAFE_OPEN(path, O_RDONLY, 0);
+       SAFE_READ(0, fd, &ehdr, sizeof(ehdr));
+       SAFE_READ(0, fd, &ehdr, sizeof(ehdr));
+       SAFE_CLOSE(fd);
+
+       TST_EXP_EXPR(ehdr.e_type == ET_CORE,
+                       "ELF type is ET_CORE (got %d)", ehdr.e_type);
+
+       return 0;
+}
+
 static void verify_file_pattern(void)
 {
        char dump[PATH_MAX + 32];
@@ -100,11 +140,14 @@ static void verify_file_pattern(void)

        TST_EXP_PASS(access(dump, F_OK), "core.%%e.%%p.%%s expanded to core.coredump01.%d.%d",
                     pid, SIGABRT);
+
+       verify_core_dump(dump, -1);
 }

 static void verify_pipe_pattern(void)
 {
        char res[PATH_MAX + 32], exe[PATH_MAX];
+       char dump[PATH_MAX + 32];
        int pid_seen, sig_seen, elf;
        long long bytes;
        pid_t pid;
@@ -121,6 +164,7 @@ static void verify_pipe_pattern(void)
        pid = crash_child();

        snprintf(res, sizeof(res), "%s/res.%d", cwd, pid);
+       snprintf(dump, sizeof(dump), "%s/res.%d.core", cwd, pid);

        /* the kernel spawns the helper asynchronously */
        if (TST_RETRY_FN_EXP_BACKOFF(access(res, F_OK), TST_RETVAL_EQ0, HELPER_TIMEOUT)) {
@@ -135,6 +179,8 @@ static void verify_pipe_pattern(void)
        TST_EXP_EQ_LI(pid_seen, pid);
        TST_EXP_EQ_LI(sig_seen, SIGABRT);
        TST_EXP_EXPR(elf && bytes > 0, "%s read %lli bytes of ELF core dump", HELPER, bytes);
+
+       verify_core_dump(dump, bytes);
 }

> +static void verify_pipe_pattern(void)
> +{
> +	char res[PATH_MAX + 32], exe[PATH_MAX];
> +	int pid_seen, sig_seen, elf;
> +	long long bytes;
> +	pid_t pid;
> +
> +	if (static_usermodehelper) {
> +		tst_res(TCONF, "CONFIG_STATIC_USERMODEHELPER is enabled, skipping pipe core_pattern");
> +		return;
> +	}
> +
> +	tst_res(TINFO, "Testing pipe core_pattern");
> +
> +	set_pattern("|%s %%e %%p %%s %s/res.%%p", helper_path, cwd);
> +
> +	pid = crash_child();
> +
> +	snprintf(res, sizeof(res), "%s/res.%d", cwd, pid);
> +
> +	/* the kernel spawns the helper asynchronously */
> +	if (TST_RETRY_FN_EXP_BACKOFF(access(res, F_OK), TST_RETVAL_EQ0, HELPER_TIMEOUT)) {
> +		tst_res(TFAIL, "%s did not report any core dump", HELPER);
> +		return;
> +	}
> +
> +	SAFE_FILE_SCANF(res, "exe=%15s pid=%d sig=%d bytes=%lld elf=%d",
> +			exe, &pid_seen, &sig_seen, &bytes, &elf);
> +
> +	TST_EXP_EQ_STR(exe, "coredump01");
> +	TST_EXP_EQ_LI(pid_seen, pid);
> +	TST_EXP_EQ_LI(sig_seen, SIGABRT);
> +	TST_EXP_EXPR(elf && bytes > 0, "%s read %lli bytes of ELF core dump", HELPER, bytes);

Here as well.

Also we need do something in the helper to save dump file.

> +}

-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp