Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reproducible SIGSEGV with st.font_fallback #159

Closed
maybebyte opened this issue Jul 21, 2023 · 5 comments · Fixed by #164
Closed

Reproducible SIGSEGV with st.font_fallback #159

maybebyte opened this issue Jul 21, 2023 · 5 comments · Fixed by #164

Comments

@maybebyte
Copy link

Hey, I've been working on writing an OpenBSD port for this and noticed this compiler warning (please bear in mind throughout this issue that I'm not a C developer):

In file included from x.c:243:
./xst.c:76:34: warning: expression which evaluates to zero treated as a null pointer constant of type 'char *' [-Wnon-literal-null-conversion]
                                font2[endchar + count + 1] = '\0';

I looked at the relevant code:

config.def.h line 9:

static char *font2[] = {
/*	"Inconsolata for Powerline:pixelsize=12:antialias=true:autohint=true", */
/*	"Hack Nerd Font Mono:pixelsize=11:antialias=true:autohint=true", */
};

xst.c line 65:

		XRESOURCE_LOAD_META("font_fallback") {
			int count = 0, endchar = fonts_count = sizeof(font2) / sizeof(*font2);
			for (int i = 0; ret.addr[i]; i++) if (ret.addr[i] == ',') count++;
			if (count > 0)
			{
				for (int i = 0; i <= count; i++)
				{
					if (i == 0) font2[endchar + i] = strtok(ret.addr, ",");
					else				font2[endchar + i] = strtok(NULL, ",");
					fonts_count++;
				}
				font2[endchar + count + 1] = '\0';
			} else if (ret.addr) {
				font2[endchar] = ret.addr;
				fonts_count++;
			}
		}

What I realized is that you can reliably cause xst to dump a core file by passing a comma (either alone or as a trailing comma) to st.font_fallback because it increments count:

$ grep st.font_fallback ~/.Xresources
st.font_fallback: ,

$ xst
Segmentation fault (core dumped)

$ egdb -q xst xst.core
Reading symbols from xst...
[New process 195719]
Core was generated by `xst'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00000ec5e2d55653 in xloadsparefonts () at x.c:1173
1173			if (**fp == '-')
(gdb) bt
#0  0x00000ec5e2d55653 in xloadsparefonts () at x.c:1173
#1  0x00000ec5e2d5480d in xinit (cols=80, rows=24) at x.c:1333
#2  0x00000ec5e2d542d4 in main (argc=<optimized out>, argv=<optimized out>) at x.c:2759
(gdb) quit

It seems like it leads into undefined behavior. I'm unsure what the best way to address this would be, but wanted to bring it to your attention all the same.

@actionless
Copy link
Collaborator

actionless commented Sep 28, 2024

i've just got same crash even when specified font name correctly (and without extra , etc)

@actionless
Copy link
Collaborator

actionless commented Sep 28, 2024

hmm, for some reason i could reproduce it only with arch default build flags, but not when building locally

in aur i could temporary work it around with this:

diff --git a/PKGBUILD b/PKGBUILD
index 14e847e..5ec78ea 100644
--- a/PKGBUILD
+++ b/PKGBUILD
@@ -13,7 +13,11 @@ license=('MIT')
 depends=('libxft')
 makedepends=('ncurses' 'libxext' 'git')
 source=("git+https://github.com/gnotclub/${_pkgname}.git")
 sha1sums=('SKIP')
+options=('!buildflags')

 provides=(
        ${_pkgname}

mb playing with build flags could help in your situations as well?

later on i'll try to look into it

@actionless
Copy link
Collaborator

arch flags

CFLAGS  = -I/usr/X11R6/include  -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/harfbuzz -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-6 -pthread  -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/harfbuzz -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-6 -pthread -DVERSION="0.9" -D_XOPEN_SOURCE=600 
       -march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions
       -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security         -fstack-clash-protection -fcf-protection         -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -flto=auto
LDFLAGS = -L/usr/X11R6/lib -lm -lrt -lX11 -lutil -lXft -lXrender -lfontconfig -lfreetype  -lfreetype -Wl,-O1 -Wl,--sort-common -Wl,--as-needed -Wl,-z,relro -Wl,-z,now          -Wl,-z,pack-relative-relocs -flto=auto
CC      = c99

st default flags:

CFLAGS  = -I/usr/X11R6/include  -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/harfbuzz -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-6 -pthread  -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/harfbuzz -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-6 -pthread -DVERSION="0.9" -D_XOPEN_SOURCE=600 
		 -O1
LDFLAGS = -L/usr/X11R6/lib -lm -lrt -lX11 -lutil -lXft -lXrender -lfontconfig -lfreetype  -lfreetype
CC      = c99

@actionless
Copy link
Collaborator

i found a moment to debug it - apparently it's caused by -flto=auto flag

@actionless
Copy link
Collaborator

see #164

actionless added a commit that referenced this issue Nov 18, 2024
, Fixes: #144, Fixes: #165) (#164)

* fix building with -flto=auto

* fix: rename back default_fallback_fonts to font2 for backwards-compatibility with local users' config.h

* fix(x: zoomabs): fix fallback fonts' handling when zooming (Fixes: #144)

* fix(xst: xtdb_load: font_fallback): add missing return statements on font loading error

* chore: fix GH CI 

* fix(xst: reload): init and load spare fonts (aka font2), call xhints after resize (fixes: #165)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants