[dev] [st] Use after free in font cache
Hi, got some crashes. Looks like st is calling XftFontClose in the last member of the font cache array when it runs out of space in it, but that xft font is still used somewhere else. To reproduce: 1. Make the font cache array smaller, in x.c "static Fontcache frc[1];" 2. wget https://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-demo.txt 3. Start st in valgrind or similar 4. less UTF-8-demo.txt 5. Scroll down a few pages Valgrind: Invalid read of size 8 at 0x589E028: XftDrawGlyphFontSpec (xftdraw.c:728) by 0x116C7D: xdrawglyphfontspecs (x.c:1234) by 0x1178C9: drawregion (x.c:1437) by 0x1175E2: draw (x.c:1389) by 0x11842C: run (x.c:1674) by 0x118CC2: main (x.c:1764) Address 0xa9a75c0 is 208 bytes inside a block of size 18,376 free'd at 0x4C2D16B: free (in ...) by 0x58A09DB: XftFontManageMemory (xftfreetype.c:1142) by 0x116401: xmakeglyphfontspecs (x.c:1089) by 0x11777F: drawregion (x.c:1415) by 0x1175E2: draw (x.c:1389) by 0x11842C: run (x.c:1674) by 0x118CC2: main (x.c:1764) Block was alloc'd at at 0x4C2BE7F: malloc (in ...) by 0x58A047B: XftFontOpenInfo (xftfreetype.c:892) by 0x58A14BA: XftFontOpenPattern (xftfreetype.c:1034) by 0x11643A: xmakeglyphfontspecs (x.c:1093) by 0x11777F: drawregion (x.c:1415) by 0x1175E2: draw (x.c:1389) by 0x11842C: run (x.c:1674) by 0x118CC2: main (x.c:1764) Line numbers for st git master, 7f990328 I've run into this in a real world situation (some chinese characters on irc) and as a workaround I've increased the font cache array size from 16 to 32.
[dev] [st] [PATCH] Fix extra bracketed paste markers when pasting >8kb
Before this patch, when pasting over BUFSIZE (8192 bytes here), st would
do the following:
\e[200~...8192 bytes...\e[201~\e[200~...remaining bytes...\e[201~
With this patch, the start marker is only sent when the offset is 0 (at
the beginning of selnotify) and the end marker is only sent when the
remaining bytes to read are 0 (at the end).
For short pastes, both conditions are true in the same iteration.
For long pastes, it removes the extra markers in the middle, keeping the
intended wrapping:
\e[200~...8192 bytes..remaining bytes...\e[201~
---
st.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/st.c b/st.c
index bcf74b3..9de30ec 100644
--- a/st.c
+++ b/st.c
@@ -1135,10 +1135,10 @@ selnotify(XEvent *e)
*repl++ = '\r';
}
- if (IS_SET(MODE_BRCKTPASTE))
+ if (IS_SET(MODE_BRCKTPASTE) && ofs == 0)
ttywrite("\033[200~", 6);
ttysend((char *)data, nitems * format / 8);
- if (IS_SET(MODE_BRCKTPASTE))
+ if (IS_SET(MODE_BRCKTPASTE) && rem == 0)
ttywrite("\033[201~", 6);
XFree(data);
/* number of 32-bit chunks returned */
--
2.0.0+fc2
Re: [dev] [PATCH] Replace character with U+FFFD if wcwidth() is -1
On 28 October 2014 08:54, Dmitrij D. Czarkoff wrote: > Helpful when new Unicode codepoints are not recognized by libc. In practice, this only makes things worse by not displaying characters that are perfectly supported by fontconfig and xft, but not by libc. But... whatever.
Re: [dev] [PATCH] [st] Use inverted defaultbg/fg for selection when bg/fg are the same
On 27 October 2014 11:54, Martti Kühne wrote: > On Mon, Oct 27, 2014 at 3:01 AM, dequis wrote: >> The background/foreground of selected text is currently set by setting >> ATTR_REVERSE, which flips its normal bg/fg. When the text being >> selected has the same bg and fg, it won't be readable after selecting >> it, either. >> > > > This may sound trivial, but. > How about you paste it somewhere else? I've done that - annoying to do constantly, but it's an okayish workaround. I've also done that by using the same input box of the irc client (which is the one that happens to be the most convenient one), which is even more annoying since you have to stop the selection within the range of the 10 pixels of width of the last character of the line, if done carelessly the newline gets selected which results in the copied line getting sent to irc. Of course I've already learnt (through many mistakes) not to do that. Call me nitpicky, I've just done it hundreds (if not thousands) of times and the annoyance stacks up. But I already have a perfectly good and trivial patch that fixes the issue, so why would I bother with a workaround like that?
[dev] [PATCH] [st] Use inverted defaultbg/fg for selection when bg/fg are the same
The background/foreground of selected text is currently set by setting
ATTR_REVERSE, which flips its normal bg/fg. When the text being
selected has the same bg and fg, it won't be readable after selecting
it, either.
This may sound silly - my main use case is IRC channels using color
codes for black-on-black to mark 'spoilers'.
This patch allows that text to be read by selecting it, turning it
into text with white bg and black fg (given default values for
defaultbg/fg), just like most normal unformatted text when selected.
From 380f8ba874976a8392d9c2a6639775b45fce5d91 Mon Sep 17 00:00:00 2001
From: dequis
Date: Sun, 26 Oct 2014 22:39:46 -0300
Subject: [PATCH] Use inverted defaultbg/fg for selection when bg/fg are the
same
The background/foreground of selected text is currently set by setting
ATTR_REVERSE, which flips its normal bg/fg. When the text being selected
has the same bg and fg, it won't be readable after selecting it, either.
This may sound silly - my main use case is IRC channels using color
codes for black-on-black to mark 'spoilers'.
This patch allows that text to be read by selecting it, turning it into
text with white bg and black fg (given default values for defaultbg/fg),
just like most normal unformatted text when selected.
---
st.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/st.c b/st.c
index 12af1ab..e2b7003 100644
--- a/st.c
+++ b/st.c
@@ -3296,9 +3296,14 @@ xdraws(char *s, Glyph base, int x, int y, int charlen, int bytelen) {
}
if(base.mode & ATTR_REVERSE) {
- temp = fg;
- fg = bg;
- bg = temp;
+ if (bg == fg) {
+ bg = &dc.col[defaultfg];
+ fg = &dc.col[defaultbg];
+ } else {
+ temp = fg;
+ fg = bg;
+ bg = temp;
+ }
}
if(base.mode & ATTR_FAINT && !(base.mode & ATTR_BOLD)) {
--
2.1.2
Re: [dev] [PATCH] [st] Fix issues with wcwidth() returning -1 for unsupported unicode chars
On 26 October 2014 19:36, Dmitrij D. Czarkoff wrote: > Apparently no workaround is needed - just use sane libc or avoid using > new codepoints until glibc fixes that. Or, if you won't feel dirty > after that, send a patch to glibc instead. 'Just using musl' doesn't seem feasible given a glibc-based system - it's easier for smaller programs, but in the case of st i'd have to rebuild xlib, xcb, xft, freetype, fontconfig, etc (and their dependencies) against musl. Not to mention that musl doesn't implement unicode 7.0 either, only up to 6.2. Avoiding using new codepoints is not really an option either, they just appear in messages written by other people and result in unreadable/invisible lines. They offset columns by -1, so with enough characters, they can make a whole line appear as empty. My workaround until now was to open a different terminal emulator just to read the 'missing' lines. I'd say that knowingly allowing that kind of rendering glitch is a bad idea.
[dev] [PATCH] [st] Fix issues with wcwidth() returning -1 for unsupported unicode chars
Hi suckless! First, thanks for st. Been using it for a long while,
still impressed at how it gets a lot of stuff right - stuff that urxvt
failed miserably at. There's only one issue that has been bothering me
particularly.
The issue itself:
Unicode characters added since unicode 5.2 (released in 2009, the
latest revision[1] is 7.0) are not supported by the wcwidth()
implementation of glibc, and as a result, they behave weirdly in st.
The man page of wcwidth() specifies that -1 is returned for invalid
unicode characters. I found a stack overflow question[2] about this
same issue.
How st handles it:
I made a gif[3] showing its behavior.
It just offsets the columns by the value returned by wcwidth,
expecting either 1 or 2, not -1. So each unsupported unicode character
behaves like a printable backspace.
Picked U+0524[4] for the tests. The st on the top shows the current
behavior, the st on the bottom is my patched version. The first two
lines typed in the gif are spaces followed by that character. Third
line is the letter 'a' just to show how it overlaps.
Then I used a tmux keybinding that is supposed to scan for URLs, but
the main effect here is refreshing the terminal contents, which makes
those characters vanish. That z^H is a typo, ignore that.
My patch:
Just wcwidth(...) -> abs(wcwidth(...))
In other words: if wcwidth returns -1, interpret that as a column
width of 1. It's a bit dirty and lazy, but it works wonderfully for
most characters.
I'm not sure what the "correct" solution would be, but it's definitely
not something as simple as this - would mean fixing the libc to
support unicode up to 7.0, or implementing our own version of it.
Opinions?
[1]: http://www.fileformat.info/info/unicode/version/index.htm
[2]:
http://stackoverflow.com/questions/16371418/why-does-wcwidth-return-1-with-a-sign-that-i-can-print-on-the-terminal
[3]: http://i.imgur.com/MDzMJJH.gif
[4]: http://www.fileformat.info/info/unicode/char/0524/index.htm
From 810937df2c8a693deb26ac278c73f0147353079b Mon Sep 17 00:00:00 2001
From: dequis
Date: Sat, 25 Oct 2014 23:07:38 -0300
Subject: [PATCH] Fix issues with wcwidth() returning -1 for unsupported
unicode chars
Unicode characters added since unicode 5.2 are not supported by the
wcwidth() implementation of glibc, and as a result, they behave weirdly
in st. The man page of wcwidth() specifies that -1 is returned for
invalid unicode characters.
This patch wraps the wcwidth() calls with abs() to ensure that a column
size of 1 is used for unknown unicode characters.
---
st.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/st.c b/st.c
index 23dd7f1..12af1ab 100644
--- a/st.c
+++ b/st.c
@@ -2576,7 +2576,7 @@ tputc(char *c, int len) {
unicodep = ascii = *c;
} else {
utf8decode(c, &unicodep, UTF_SIZ);
- width = wcwidth(unicodep);
+ width = abs(wcwidth(unicodep));
control = ISCONTROLC1(unicodep);
ascii = unicodep;
}
@@ -3440,7 +3440,7 @@ xdraws(char *s, Glyph base, int x, int y, int charlen, int bytelen) {
xp, winy + frc[i].font->ascent,
(FcChar8 *)u8c, u8cblen);
- xp += xw.cw * wcwidth(unicodep);
+ xp += xw.cw * abs(wcwidth(unicodep));
}
/*
--
2.1.2
