runtime failure with -flto

[stefan11111]

when building dfbterm with -flto, dfbterm segfaults when ran. this is fixed when also passing -fno-strict-aliasing to gcc, which leads me to think this is caused by some ub. I think this may be from one of the functions with void* args which are casted to other types, and the gdb backtraces seem to indicate the same.

from gdb:

(gdb) r
Starting program: /usr/bin/dfbterm
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[New Thread 0x7ffff77e36c0 (LWP 2977)]
(*) Direct/Thread: Started 'SigHandler' (2977) [CRITICAL - OTHER/0] <8388608>...

   ~~~~~~~~~~~~~~~~~~~~~~~~~| DirectFB 1.7.7 |~~~~~~~~~~~~~~~~~~~~~~~~~~
        (c) 2012-2015  DirectFB integrated media GmbH
        (c) 2001-2015  The world wide DirectFB Open Source Community
        (c) 2000-2004  Convergence (integrated media) GmbH
      ----------------------------------------------------------------

(*) DirectFB/Core: Single Application Core. (2024-06-19 16:20)
(*) Direct/Memcpy: Using Generic 64bit memcpy()
[New Thread 0x7ffff6dc86c0 (LWP 2978)]
(*) Direct/Thread: Started 'Fusion Dispatch' (2978) [MESSAGING - OTHER/0] <8388608>...
[New Thread 0x7ffff65c76c0 (LWP 2979)]
(*) Direct/Thread: Started 'X11 Input' (2979) [INPUT - OTHER/0] <8388608>...
(*) DirectFB/Input: X11 Input 0.1 (directfb.org)
(*) DirectFB/Genefx: MMX detected and enabled
(*) DirectFB/Graphics: MMX Software Rasterizer 0.7 (directfb.org)
(*) DirectFB/Core/WM: Default 0.3 (directfb.org)
[New Thread 0x7ffff5dc66c0 (LWP 2980)]
(*) Direct/Thread: Started 'Genefx' (2980) [DEFAULT - OTHER/0] <8388608>...
(*) X11/Display: Not using XShm.
(*) Direct/Interface: Loaded 'DGIFF' implementation of 'IDirectFBFont'.
(*) Direct/Interface: Loaded 'DFIFF' implementation of 'IDirectFBImageProvider'.
 (!!!)  *** UNIMPLEMENTED [fusion_get_fusionee_pid] *** [fusion.c:4147]
[Detaching after fork from child process 2981]
[Detaching after fork from child process 2982]
[New Thread 0x7fffeffff6c0 (LWP 2983)]
(*) Direct/Thread: Started 'Term Update' (2983) [DEFAULT - OTHER/0] <8388608>...

Thread 6 "Term Update" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffeffff6c0 (LWP 2983)]
0x0000555555561d14 in vt_parse_vt (vt=0x55555573ab20, ptr=<optimized out>, length=<optimized out>) at ../dfbterm-9999/src/libzvt/vt.c:1590
warning: 1590   ../dfbterm-9999/src/libzvt/vt.c: No such file or directory
(gdb) bt
#0  0x0000555555561d14 in vt_parse_vt (vt=0x55555573ab20, ptr=<optimized out>, length=<optimized out>) at ../dfbterm-9999/src/libzvt/vt.c:1590
#1  term_update (thread=<optimized out>, arg=0x555555609680) at ../dfbterm-9999/src/dfbterm.c:673
#2  0x00007ffff7dbc0d4 in ?? () from /usr/lib64/libdirect-1.7.so.7
#3  0x00007ffff7c3c0aa in ?? () from /lib64/libc.so.6
#4  0x00007ffff7cc14f8 in ?? () from /lib64/libc.so.6
(gdb) q
A debugging session is active.

        Inferior 1 [process 2974] will be killed.

Quit anyway? (y or n) y

(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /usr/bin/dfbterm
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[New Thread 0x7ffff77e36c0 (LWP 3537)]
(*) Direct/Thread: Started 'SigHandler' (3537) [CRITICAL - OTHER/0] <8388608>...

   ~~~~~~~~~~~~~~~~~~~~~~~~~| DirectFB 1.7.7 |~~~~~~~~~~~~~~~~~~~~~~~~~~
        (c) 2012-2015  DirectFB integrated media GmbH
        (c) 2001-2015  The world wide DirectFB Open Source Community
        (c) 2000-2004  Convergence (integrated media) GmbH
      ----------------------------------------------------------------

(*) DirectFB/Core: Single Application Core. (2024-06-19 16:20)
(*) Direct/Memcpy: Using Generic 64bit memcpy()
[New Thread 0x7ffff6dc86c0 (LWP 3538)]
(*) Direct/Thread: Started 'Fusion Dispatch' (3538) [MESSAGING - OTHER/0] <8388608>...
[New Thread 0x7ffff65c76c0 (LWP 3539)]
(*) Direct/Thread: Started 'X11 Input' (3539) [INPUT - OTHER/0] <8388608>...
(*) DirectFB/Input: X11 Input 0.1 (directfb.org)
(*) DirectFB/Genefx: MMX detected and enabled
(*) DirectFB/Graphics: MMX Software Rasterizer 0.7 (directfb.org)
(*) DirectFB/Core/WM: Default 0.3 (directfb.org)
[New Thread 0x7ffff5dc66c0 (LWP 3540)]
(*) Direct/Thread: Started 'Genefx' (3540) [DEFAULT - OTHER/0] <8388608>...
(*) X11/Display: Not using XShm.
(*) Direct/Interface: Loaded 'DGIFF' implementation of 'IDirectFBFont'.
(*) Direct/Interface: Loaded 'DFIFF' implementation of 'IDirectFBImageProvider'.
 (!!!)  *** UNIMPLEMENTED [fusion_get_fusionee_pid] *** [fusion.c:4147]
[Detaching after fork from child process 3541]
[Detaching after fork from child process 3542]
[New Thread 0x7fffeffff6c0 (LWP 3543)]
(*) Direct/Thread: Started 'Term Update' (3543) [DEFAULT - OTHER/0] <8388608>...

Thread 6 "Term Update" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffeffff6c0 (LWP 3543)]
0x0000555555558302 in vt_cursor_state (user_data=0x40b1002040b10020, state=0) at ../dfbterm-9999/src/dfbterm.c:229
warning: 229    ../dfbterm-9999/src/dfbterm.c: No such file or directory
(gdb) bt
#0  0x0000555555558302 in vt_cursor_state (user_data=0x40b1002040b10020, state=0) at ../dfbterm-9999/src/dfbterm.c:229
#1  0x000055555555fa38 in vt_update (vx=0x55555573a770, update_state=0) at ../dfbterm-9999/src/libzvt/update.c:474
#2  0x00005555555620aa in term_update (thread=<optimized out>, arg=0x5555556092d0) at ../dfbterm-9999/src/dfbterm.c:679
#3  0x00007ffff7dbc0d4 in ?? () from /usr/lib64/libdirect-1.7.so.7
#4  0x00007ffff7c3c0aa in ?? () from /lib64/libc.so.6
#5  0x00007ffff7cc14f8 in ?? () from /lib64/libc.so.6
(gdb) q
A debugging session is active.

        Inferior 1 [process 3536] will be killed.

Quit anyway? (y or n) y

[stefan11111]

Also, a patch which fixes some code that made the compiler throw warnings and which seemed suspicious:

+++ b/src/glib.c        2024-06-19 22:03:08.689574762 +0300
@@ -60,7 +60,8 @@
 g_realloc( void          *mem,
            unsigned long  size )
 {
-     return g_realloc( mem, size );
+     void *tmp = realloc ( mem, size );
+     return tmp ? tmp : mem;
 }

 GSList *
@@ -140,10 +141,10 @@
 g_string_new( const char *str )
 {
      GString *string = malloc( sizeof(GString) );
-     int      len    = strlen( str );
+     int      len    = strlen( str ) + 1;

      string->str = malloc( len );
-     string->len = len;
+     string->len = len - 1;

      strcpy( string->str, str );

[caramelli]

Nice find!

[stefan11111]

Nice find!

I saw the latest commit, and I still think the g_realloc implementation might be now what is desired.

On failure, realloc returns NULL and leaves the original chunk untouched. In this case, it might be preferable to return the original memory chunk, to not cause a memory leak. However, this depends on how g_realloc in used in the code. if it's checked for failure, it's not a problem. If it's simply supposed that g_realloc never fails, then it can be a problem.

From looking at the code, it seems that the latter is the case.

As a side note, glib is notoriously bad at handling memory allocation errors, and simply calls abort() if the allocation fails.

any ideas on the lto/strict aliasing thing?

[caramelli]

I agree on realloc(), I note the point. For information, the internal libzvt/vt.c which calls g_realloc() comes directly from the old archive https://download.gnome.org/sources/libzvt/2.0/libzvt-2.0.1.tar.gz

I'm not used to enabling LTO (I should). No idea yet, but this strict aliasing issue is something to fix.

[stefan11111]

I made a pr to address the strict aliasing issue: https://github.com/directfb2/DFBTerm/pull/7 I will post the patch here too:

diff --git a/src/libzvt/vt.c b/src/libzvt/vt.c
index 1083968..0119689 100644
--- a/src/libzvt/vt.c
+++ b/src/libzvt/vt.c
@@ -1785,7 +1785,7 @@ vt_init(struct vt_em *vt, int width, int height)
   vt->childpid = -1;
   vt->keyfd = -1;

-  vt->this_line = (struct vt_line *)vt->lines.head;
+  memcpy(&vt->this_line, &vt->lines.head, sizeof(struct vt_line*));

   vt->scrollbacklines=0;
   vt->scrollbackoffset=0;

The issue can also be fixed by adding more volatiles, but I like the memcpy approach more.

diff --git a/src/libzvt/vt.c b/src/libzvt/vt.c
index 1083968..008e448 100644
--- a/src/libzvt/vt.c
+++ b/src/libzvt/vt.c
@@ -1785,7 +1785,7 @@ vt_init(struct vt_em *vt, int width, int height)
   vt->childpid = -1;
   vt->keyfd = -1;

-  vt->this_line = (struct vt_line *)vt->lines.head;
+  ((volatile struct vt_em *)vt)->this_line = (struct vt_line *)((volatile struct vt_em *)vt)->lines.head;

   vt->scrollbacklines=0;
   vt->scrollbackoffset=0;

[stefan11111]

I also made a patch to fix the -Wunused-result warnings. Should I add it to the pr?

diff --git a/src/libzvt/gnome-pty-helper.c b/src/libzvt/gnome-pty-helper.c
index 9601a68..3df433b 100644
--- a/src/libzvt/gnome-pty-helper.c
+++ b/src/libzvt/gnome-pty-helper.c
@@ -489,11 +489,11 @@ open_ptys (int utmp, int wtmp, int lastlog)

    /* drop privileges to the user level */
 #if defined(HAVE_SETEUID)
-   seteuid (pwent->pw_uid);
-   setegid (pwent->pw_gid);
+   (void)!seteuid (pwent->pw_uid);
+   (void)!setegid (pwent->pw_gid);
 #elif defined(HAVE_SETREUID)
-   setreuid (savedUid, pwent->pw_uid);
-   setregid (savedGid, pwent->pw_gid);
+   (void)!setreuid (savedUid, pwent->pw_uid);
+   (void)!setregid (savedGid, pwent->pw_gid);
 #else
 #error "No means to drop privileges! Huge security risk! Won't compile."
 #endif
@@ -502,25 +502,25 @@ open_ptys (int utmp, int wtmp, int lastlog)

    /* Restore saved priveleges to root */
 #ifdef HAVE_SETEUID
-   seteuid (savedUid);
-   setegid (savedGid);
+   (void)!seteuid (savedUid);
+   (void)!setegid (savedGid);
 #elif defined(HAVE_SETREUID)
-   setreuid (pwent->pw_uid, savedUid);
-   setregid (pwent->pw_gid, savedGid);
+   (void)!setreuid (pwent->pw_uid, savedUid);
+   (void)!setregid (pwent->pw_gid, savedGid);
 #else
 #error "No means to raise privileges! Huge security risk! Won't compile."
 #endif
    /* openpty() failed, reject request */
    if (status == -1){
        result = 0;
-       write (STDIN_FILENO, &result, sizeof (result));
+       (void)!write (STDIN_FILENO, &result, sizeof (result));
        return 0;
    }

    /* a bit tricky, we re-do the part of the openpty()  */
    /* that required root priveleges, and, hence, failed */
    group_info = getgrnam ("tty");
-   fchown (slave_pty, getuid (), group_info ? group_info->gr_gid : -1);
+   (void)!fchown (slave_pty, getuid (), group_info ? group_info->gr_gid : -1);
    fchmod (slave_pty, S_IRUSR | S_IWUSR | S_IWGRP);
    /* It's too late to call revoke at this time... */
    /* revoke(term_name); */
diff --git a/src/libzvt/subshell.c b/src/libzvt/subshell.c
index 0378e98..db84669 100644
--- a/src/libzvt/subshell.c
+++ b/src/libzvt/subshell.c
@@ -75,7 +75,7 @@ sigchld_handler (int signo)
        if (waitpid (child->pid, &status, WNOHANG) == child->pid){
            child->exit_status = status;
            child->dead = 1;
-           write (child->fd, "D", 1);
+           (void)!write (child->fd, "D", 1);
            return;
        }
    }
@@ -373,7 +373,7 @@ zvt_init_subshell (struct vt_em *vt, char *pty_name, int log)
    } else {
        close (slave_pty);

-       pipe(p);
+       (void)!pipe(p);

        vt->msgfd = p [0];

@@ -391,7 +391,7 @@ zvt_init_subshell (struct vt_em *vt, char *pty_name, int log)
        pid = waitpid (vt->childpid, &status, WUNTRACED | WNOHANG);
        if (pid == vt->childpid && child->pid >= 0){
            child->pid = 0;
-           write (child->fd, "D", 1);
+           (void)!write (child->fd, "D", 1);
            return -1;
        }

@@ -435,8 +435,8 @@ zvt_shutdown_subshell (struct vt_em *vt)
    /* shutdown pty through helper */
    if (vt->pty_tag) {
        op = GNOME_PTY_CLOSE_PTY;
-       write (helper_socket_protocol [0], &op, sizeof (op));
-       write (helper_socket_protocol [0], &vt->pty_tag, sizeof (vt->pty_tag));
+       (void)!write (helper_socket_protocol [0], &op, sizeof (op));
+       (void)!write (helper_socket_protocol [0], &vt->pty_tag, sizeof (vt->pty_tag));
        vt->pty_tag = NULL;
    }

[caramelli]

Good to see you fixed this runtime failure with -flto

Your patch regarding warnings in libzvt looks good, feel free to open a PR about it.

[stefan11111]

Good to see you fixed this runtime failure with -flto

Your patch regarding warnings in libzvt looks good, feel free to open a PR about it.

I opened a pr. also put a question in the message. https://github.com/directfb2/DFBTerm/pull/8