[RFC] ci: catch coredumps

Message ID 20210111100146.3485-1-david.marchand@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series [RFC] ci: catch coredumps |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK

Commit Message

David Marchand Jan. 11, 2021, 10:01 a.m. UTC
  Parts of the unit tests code rely on forked/secondary processes
(expectedly) failing.
A crash in those situations could be missed so add a check on coredumps
presence after unit tests have run.

In some situations (like explicitly call rte_panic), coredump generation
must be disabled to avoid false positives.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Sending as a RFC, as this is a "nice to have" patch I had in store for
some time, but I did not see the actual need so far.

We could attach the coredumps in GHA result, but it would be hardly usable
without attaching all generated binaries...
Opinions?

---
 .ci/linux-build.sh    |  8 ++++++++
 app/test/test_debug.c | 11 +++++++++--
 app/test/test_mbuf.c  |  9 ++++++++-
 3 files changed, 25 insertions(+), 3 deletions(-)
  

Comments

Aaron Conole Jan. 11, 2021, 1:17 p.m. UTC | #1
David Marchand <david.marchand@redhat.com> writes:

> Parts of the unit tests code rely on forked/secondary processes
> (expectedly) failing.
> A crash in those situations could be missed so add a check on coredumps
> presence after unit tests have run.
>
> In some situations (like explicitly call rte_panic), coredump generation
> must be disabled to avoid false positives.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Sending as a RFC, as this is a "nice to have" patch I had in store for
> some time, but I did not see the actual need so far.
>
> We could attach the coredumps in GHA result, but it would be hardly usable
> without attaching all generated binaries...
> Opinions?

I think it's a good start.  We may not need to attach the binaries at
all - if we have access to gdb, we can run a script to just run simple
commands like 'thread apply all bt' and maybe some info commands.  That
can all be stuffed into the logs.  Usually a source level backtrace is
enough to work through what happened.

> ---
>  .ci/linux-build.sh    |  8 ++++++++
>  app/test/test_debug.c | 11 +++++++++--
>  app/test/test_mbuf.c  |  9 ++++++++-
>  3 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index d2c821adf3..d00a5804b4 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -57,7 +57,11 @@ meson build --werror $OPTS
>  ninja -C build
>  
>  if [ "$AARCH64" != "true" ]; then
> +    ulimit -c unlimited
> +    sudo sysctl -w kernel.core_pattern=/tmp/dpdk-core.%e.%p
> +
>      devtools/test-null.sh
> +    ! ls /tmp/dpdk-core.*.* 2>/dev/null
>  fi
>  
>  if [ "$ABI_CHECKS" = "true" ]; then
> @@ -102,5 +106,9 @@ if [ "$ABI_CHECKS" = "true" ]; then
>  fi
>  
>  if [ "$RUN_TESTS" = "true" ]; then
> +    ulimit -c unlimited
> +    sudo sysctl -w kernel.core_pattern=/tmp/dpdk-core.%e.%p
> +
>      sudo meson test -C build --suite fast-tests -t 3
> +    ! ls /tmp/dpdk-core.*.* 2>/dev/null
>  fi
> diff --git a/app/test/test_debug.c b/app/test/test_debug.c
> index 834a7386f5..23b24db177 100644
> --- a/app/test/test_debug.c
> +++ b/app/test/test_debug.c
> @@ -4,6 +4,8 @@
>  
>  #include <stdio.h>
>  #include <stdint.h>
> +#include <sys/resource.h>
> +#include <sys/time.h>
>  #include <sys/wait.h>
>  #include <unistd.h>
>  
> @@ -28,9 +30,14 @@ test_panic(void)
>  
>  	pid = fork();
>  
> -	if (pid == 0)
> +	if (pid == 0) {
> +		struct rlimit rl;
> +
> +		/* No need to generate a coredump when panicking. */
> +		rl.rlim_cur = rl.rlim_max = 0;
> +		setrlimit(RLIMIT_CORE, &rl);
>  		rte_panic("Test Debug\n");
> -	else if (pid < 0){
> +	} else if (pid < 0) {
>  		printf("Fork Failed\n");
>  		return -1;
>  	}
> diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
> index a40f7d4883..47a7b197d7 100644
> --- a/app/test/test_mbuf.c
> +++ b/app/test/test_mbuf.c
> @@ -1174,6 +1174,8 @@ test_refcnt_mbuf(void)
>  }
>  
>  #include <unistd.h>
> +#include <sys/resource.h>
> +#include <sys/time.h>
>  #include <sys/wait.h>
>  
>  /* use fork() to test mbuf errors panic */
> @@ -1186,9 +1188,14 @@ verify_mbuf_check_panics(struct rte_mbuf *buf)
>  	pid = fork();
>  
>  	if (pid == 0) {
> +		struct rlimit rl;
> +
> +		/* No need to generate a coredump when panicking. */
> +		rl.rlim_cur = rl.rlim_max = 0;
> +		setrlimit(RLIMIT_CORE, &rl);
>  		rte_mbuf_sanity_check(buf, 1); /* should panic */
>  		exit(0);  /* return normally if it doesn't panic */
> -	} else if (pid < 0){
> +	} else if (pid < 0) {
>  		printf("Fork Failed\n");
>  		return -1;
>  	}
  

Patch

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index d2c821adf3..d00a5804b4 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -57,7 +57,11 @@  meson build --werror $OPTS
 ninja -C build
 
 if [ "$AARCH64" != "true" ]; then
+    ulimit -c unlimited
+    sudo sysctl -w kernel.core_pattern=/tmp/dpdk-core.%e.%p
+
     devtools/test-null.sh
+    ! ls /tmp/dpdk-core.*.* 2>/dev/null
 fi
 
 if [ "$ABI_CHECKS" = "true" ]; then
@@ -102,5 +106,9 @@  if [ "$ABI_CHECKS" = "true" ]; then
 fi
 
 if [ "$RUN_TESTS" = "true" ]; then
+    ulimit -c unlimited
+    sudo sysctl -w kernel.core_pattern=/tmp/dpdk-core.%e.%p
+
     sudo meson test -C build --suite fast-tests -t 3
+    ! ls /tmp/dpdk-core.*.* 2>/dev/null
 fi
diff --git a/app/test/test_debug.c b/app/test/test_debug.c
index 834a7386f5..23b24db177 100644
--- a/app/test/test_debug.c
+++ b/app/test/test_debug.c
@@ -4,6 +4,8 @@ 
 
 #include <stdio.h>
 #include <stdint.h>
+#include <sys/resource.h>
+#include <sys/time.h>
 #include <sys/wait.h>
 #include <unistd.h>
 
@@ -28,9 +30,14 @@  test_panic(void)
 
 	pid = fork();
 
-	if (pid == 0)
+	if (pid == 0) {
+		struct rlimit rl;
+
+		/* No need to generate a coredump when panicking. */
+		rl.rlim_cur = rl.rlim_max = 0;
+		setrlimit(RLIMIT_CORE, &rl);
 		rte_panic("Test Debug\n");
-	else if (pid < 0){
+	} else if (pid < 0) {
 		printf("Fork Failed\n");
 		return -1;
 	}
diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
index a40f7d4883..47a7b197d7 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -1174,6 +1174,8 @@  test_refcnt_mbuf(void)
 }
 
 #include <unistd.h>
+#include <sys/resource.h>
+#include <sys/time.h>
 #include <sys/wait.h>
 
 /* use fork() to test mbuf errors panic */
@@ -1186,9 +1188,14 @@  verify_mbuf_check_panics(struct rte_mbuf *buf)
 	pid = fork();
 
 	if (pid == 0) {
+		struct rlimit rl;
+
+		/* No need to generate a coredump when panicking. */
+		rl.rlim_cur = rl.rlim_max = 0;
+		setrlimit(RLIMIT_CORE, &rl);
 		rte_mbuf_sanity_check(buf, 1); /* should panic */
 		exit(0);  /* return normally if it doesn't panic */
-	} else if (pid < 0){
+	} else if (pid < 0) {
 		printf("Fork Failed\n");
 		return -1;
 	}