Segmentation Fault in stepcompress.c

[gitdacat]

I am getting a segmentation fault in stepcompress.c:

Using host libthread_db library "/lib/aarch64-linux-gnu/libthread_db.so.1".
Core was generated by `/home/orangepi/klippy-env/bin/python /home/orangepi/klipper/klippy/klippy.py /h'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  stepcompress_find_past_position (sc=0x3410e9c0, clock=120143323148) at /home/orangepi/klipper/klippy/chelper/stepcompress.c:592
592	        if (clock < hs->first_clock) {
[Current thread is 1 (Thread 0x7f8c59f1a0 (LWP 3606))]
(gdb) p hs
$1 = (struct history_steps *) 0x335a4
(gdb) po hs
Cannot access memory at address 0x335a4
(gdb) bt
#0  stepcompress_find_past_position (sc=0x3410e9c0, clock=120143323148) at /home/orangepi/klipper/klippy/chelper/stepcompress.c:592
#1  0x0000007f9c6763e0 in ?? () from /lib/aarch64-linux-gnu/libffi.so.8
#2  0x0000007f9c675b24 in ?? () from /lib/aarch64-linux-gnu/libffi.so.8
#3  0x0000007f9b19cd84 in cdata_call (cd=0x34155d10, args=<optimized out>, kwds=<optimized out>) at c/_cffi_backend.c:3186
#4  0x00000000004946f8 in _PyObject_MakeTpCall ()
#5  0x00000000004aa3f0 in _PyEval_EvalFrameDefault ()
#6  0x0000000000512514 in ?? ()
#7  0x00000000005115a8 in ?? ()
#8  0x00000000004ae3f4 in _PyEval_EvalFrameDefault ()
#9  0x0000000000512514 in ?? ()
#10 0x00000000005115a8 in ?? ()
#11 0x00000000006566b0 in ?? ()
#12 0x000000000060aeec in ?? ()
#13 0x0000007fa1be2030 in ?? () from /lib/aarch64-linux-gnu/libc.so.6
#14 0x0000007fa1c4bf1c in ?? () from /lib/aarch64-linux-gnu/libc.so.6
(gdb) p clock
$2 = 120143323148
(gdb)

[gitdacat]

The start of this linked list does look ok :

(gdb) list
587	stepcompress_find_past_position(struct stepcompress *sc, uint64_t clock)
588	{
589	    int64_t last_position = sc->last_position;
590	    struct history_steps *hs;
591	    list_for_each_entry(hs, &sc->history_list, node) {
592	        if (clock < hs->first_clock) {
593	            last_position = hs->start_position;
594	            continue;
595	        }
596	        if (clock >= hs->last_clock)
(gdb) po sc
evaluation of this expression requires the target program to be active
(gdb) p sc->history_list
$1 = {root = {next = 0x34580fc0, prev = 0x34b438e0}}
(gdb) p hs
$2 = (struct history_steps *) 0x335a4
(gdb) p sc->history_list
$3 = {root = {next = 0x34580fc0, prev = 0x34b438e0}}
(gdb) p sc->history_list->next
There is no member named next.
(gdb) p sc->history_list->root
$4 = {next = 0x34580fc0, prev = 0x34b438e0}
(gdb) p sc->history_list->root->next
$5 = (struct list_node *) 0x34580fc0
(gdb) p *(sc->history_list->root->next)
$6 = {next = 0x33059160, prev = 0x3410ea48}
(gdb) p *(sc->history_list->root->next)->next
$7 = {next = 0x34e383f0, prev = 0x34580fc0}
(gdb) p $7->next
$8 = (struct list_node *) 0x34e383f0
(gdb) p *($7->next)->next
$9 = {next = 0x34e2c9f0, prev = 0x34e383f0}
(gdb) p *($9->next)->next
$10 = {next = 0x34b26e60, prev = 0x34e2c9f0}
(gdb) p *($10->next)->next
$11 = {next = 0x34d94910, prev = 0x34b26e60}
(gdb) p *($11->next)->next
$12 = {next = 0x34d42af0, prev = 0x34d94910}
(gdb) p *($12->next)->next
$13 = {next = 0x34a5d7a0, prev = 0x34d42af0}
...
(gdb) p *($->next)->next
$14 = {next = 0x343f5650, prev = 0x34a5d7a0}
(gdb) p *($->next)->next
$15 = {next = 0x34afab70, prev = 0x343f5650}
(gdb) p *($->next)->next
$16 = {next = 0x34ce54d0, prev = 0x34afab70}
(gdb) p *($->next)->next
$17 = {next = 0x335a4cc0, prev = 0x34ce54d0}
(gdb) p *($->next)->next
$18 = {next = 0x34d54090, prev = 0x335a4cc0}
(gdb) p *($->next)->next
$19 = {next = 0x33cdc060, prev = 0x34d54090}
(gdb) p *($->next)->next
$20 = {next = 0x34da9370, prev = 0x33cdc060}
(gdb) p *($->next)->next
$21 = {next = 0x32eca970, prev = 0x34da9370}
(gdb) p *($->next)->next
$22 = {next = 0x34efd2f0, prev = 0x32eca970}
(gdb) p *($->next)->next
$23 = {next = 0x34b24180, prev = 0x34efd2f0}
(gdb) p *($->next)->next
$24 = {next = 0x32ecb7f0, prev = 0x34b24180}
(gdb) p *($->next)->next
$25 = {next = 0x34cf32d0, prev = 0x32ecb7f0}
(gdb) p *($->next)->next
$26 = {next = 0x34f186e0, prev = 0x34cf32d0}
(gdb) p *($->next)->next
$27 = {next = 0x34ccdb50, prev = 0x34f186e0}
(gdb) p *($->next)->next
$28 = {next = 0x34bacb30, prev = 0x34ccdb50}
(gdb) p *($->next)->next
$29 = {next = 0x34b293e0, prev = 0x34bacb30}
(gdb) p *($->next)->next
$30 = {next = 0x34b4d7a0, prev = 0x34b293e0}
(gdb) p *($->next)->next
$31 = {next = 0x34e24820, prev = 0x34b4d7a0}
(gdb) p *($->next)->next
$32 = {next = 0x34095f00, prev = 0x34e24820}
(gdb) p *($->next)->next
$33 = {next = 0x34e58140, prev = 0x34095f00}
(gdb) p *($->next)->next
$34 = {next = 0x34aeac80, prev = 0x34e58140}
(gdb) p *($->next)->next
$35 = {next = 0x34e19ae0, prev = 0x34aeac80}
(gdb) p *($->next)->next
$36 = {next = 0x347057c0, prev = 0x34e19ae0}
(gdb) p *($->next)->next
$37 = {next = 0x3414d070, prev = 0x347057c0}

[gitdacat]

I have a gcode reproduced the same problem twice.

The stepcompress implementation in kalico is actually quite outdated compare to klipper.
Any chance we can have the latest klipper stepcompress?

[gitdacat]

This patch work for me.

WARNING: I do NOT think this is the correct patch. but this help me bandaids the problem.

diff --git a/klippy/chelper/__init__.py b/klippy/chelper/__init__.py
index f8cf2e588..73f694259 100644
--- a/klippy/chelper/__init__.py
+++ b/klippy/chelper/__init__.py
@@ -13,7 +13,7 @@ import cffi

 GCC_CMD = "gcc"
 COMPILE_ARGS = (
-    "-Wall -g -O2 -shared -fPIC"
+    "-Wall -g -O1 -shared -fPIC"
     " -flto -fwhole-program -fno-use-linker-plugin"
     " -o %s %s"
 )
diff --git a/klippy/chelper/list.h b/klippy/chelper/list.h
index 12fe2b038..227f80731 100644
--- a/klippy/chelper/list.h
+++ b/klippy/chelper/list.h
@@ -11,7 +11,7 @@
  ****************************************************************/

 struct list_node {
-    struct list_node *next, *prev;
+    volatile struct list_node *next, *prev;
 };

 struct list_head {
@@ -47,6 +47,7 @@ list_del(struct list_node *n)
 {
     struct list_node *prev = n->prev;
     struct list_node *next = n->next;
+    n->next = (void*)0xdeadbeef;
     next->prev = prev;
     prev->next = next;
 }
@@ -54,9 +55,9 @@ list_del(struct list_node *n)
 static inline void
 __list_add(struct list_node *n, struct list_node *prev, struct list_node *next)
 {
-    next->prev = n;
     n->next = next;
     n->prev = prev;
+    next->prev = n;
     prev->next = n;
 }

diff --git a/klippy/chelper/stepcompress.c b/klippy/chelper/stepcompress.c
index 310f2bf31..6fddc6ec5 100644
--- a/klippy/chelper/stepcompress.c
+++ b/klippy/chelper/stepcompress.c
@@ -588,7 +588,10 @@ stepcompress_find_past_position(struct stepcompress *sc, uint64_t clock)
 {
     int64_t last_position = sc->last_position;
     struct history_steps *hs;
-    list_for_each_entry(hs, &sc->history_list, node) {
+    struct history_steps *v;
+    int i = 0;
+    list_for_each_entry_safe(hs, v, &sc->history_list, node) {
+	i++;
         if (clock < hs->first_clock) {
             last_position = hs->start_position;
             continue;

[rcopra]

I've just experienced this issue twice in a row - I didn't have core dumps enabled at the time though, so I'm not able to trace the issue to anything specific. Just a printer shutdown with a segfault error:

Nov 05 23:12:35 trident systemd[1]: klipper.service: Main process exited, code=killed, status=11/SEGV

[rcopra]

This patch work for me.

WARNING: I do NOT think this is the correct patch. but this help me bandaids the problem.

diff --git a/klippy/chelper/__init__.py b/klippy/chelper/__init__.py
index f8cf2e588..73f694259 100644
--- a/klippy/chelper/__init__.py
+++ b/klippy/chelper/__init__.py
@@ -13,7 +13,7 @@ import cffi

 GCC_CMD = "gcc"
 COMPILE_ARGS = (
-    "-Wall -g -O2 -shared -fPIC"
+    "-Wall -g -O1 -shared -fPIC"
     " -flto -fwhole-program -fno-use-linker-plugin"
     " -o %s %s"
 )
diff --git a/klippy/chelper/list.h b/klippy/chelper/list.h
index 12fe2b038..227f80731 100644
--- a/klippy/chelper/list.h
+++ b/klippy/chelper/list.h
@@ -11,7 +11,7 @@
  ****************************************************************/

 struct list_node {
-    struct list_node *next, *prev;
+    volatile struct list_node *next, *prev;
 };

 struct list_head {
@@ -47,6 +47,7 @@ list_del(struct list_node *n)
 {
     struct list_node *prev = n->prev;
     struct list_node *next = n->next;
+    n->next = (void*)0xdeadbeef;
     next->prev = prev;
     prev->next = next;
 }
@@ -54,9 +55,9 @@ list_del(struct list_node *n)
 static inline void
 __list_add(struct list_node *n, struct list_node *prev, struct list_node *next)
 {
-    next->prev = n;
     n->next = next;
     n->prev = prev;
+    next->prev = n;
     prev->next = n;
 }

diff --git a/klippy/chelper/stepcompress.c b/klippy/chelper/stepcompress.c
index 310f2bf31..6fddc6ec5 100644
--- a/klippy/chelper/stepcompress.c
+++ b/klippy/chelper/stepcompress.c
@@ -588,7 +588,10 @@ stepcompress_find_past_position(struct stepcompress *sc, uint64_t clock)
 {
     int64_t last_position = sc->last_position;
     struct history_steps *hs;
-    list_for_each_entry(hs, &sc->history_list, node) {
+    struct history_steps *v;
+    int i = 0;
+    list_for_each_entry_safe(hs, v, &sc->history_list, node) {
+	i++;
         if (clock < hs->first_clock) {
             last_position = hs->start_position;
             continue;

This fix worked for me, thanks for sharing your findings.