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

[BUG] segfault if a non-UTF-8 locale is being used #3

Closed
NexAdn opened this issue Sep 18, 2021 · 3 comments
Closed

[BUG] segfault if a non-UTF-8 locale is being used #3

NexAdn opened this issue Sep 18, 2021 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@NexAdn
Copy link
Contributor

NexAdn commented Sep 18, 2021

First of all: Nice work! I'm looking forward to using btop++ on my machines.

Describe the bug

When $LANG has a non-UTF-8 locale set (or a locale without the exact suffix UTF-8, e.g. ´de_DE.utf8`), the program terminates with a segfault.

To Reproduce

$ LANG=de_DE.utf8 ./bin/btop # for a build inside the repo

Expected behavior

The program should accept the locale and start as expected or don't accept the locale and exit with an error message.

Screenshots

No screenshots, but see this gdb output:

$ gdb --args ./bin/btop --debug                                                                                                                                       ✔  183
GNU gdb (Gentoo 10.2 vanilla) 10.2
Copyright (C) 2021 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://bugs.gentoo.org/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./bin/btop...
(gdb) r
Starting program: /home/nex/build/btop/bin/btop --debug
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7c5b15e in __pthread_clockjoin_ex () from /lib64/libpthread.so.0
(gdb) bt
#0  0x00007ffff7c5b15e in __pthread_clockjoin_ex () from /lib64/libpthread.so.0
#1  0x0000555555561c74 in clean_quit (sig=-1) at src/btop.cpp:187
#2  0x0000555555561f40 in _exit_handler () at src/btop.cpp:232
#3  0x00007ffff7ad2f67 in ?? () from /lib64/libc.so.6
#4  0x00007ffff7ad310a in exit () from /lib64/libc.so.6
#5  0x00005555555673b2 in main (argc=2, argv=0x7fffffffd908) at src/btop.cpp:728
(gdb) q
A debugging session is active.

	Inferior 1 [process 47362] will be killed.

Quit anyway? (y or n) y

Info (please complete the following information):

  • btop++ version: 1.0.0
  • Architecture: x86_64
  • Platform: Gentoo Linux
  • (Linux) Kernel: 5.4.80-gentoo-x86_64
  • Terminal used: x11-terms/terminator
  • Font used: Anonymous Pro Regular

Additional context

contents of ~/.config/btop/error.log: file non-existent since the program couldn't start properly

Since I have LANG=de_DE.utf8 set and thus a UTF-8-capable locale, starting btop with --utf-force helped as a workaround. However, the proper behavior should be that LANG=de_DE.utf8 should be recognized as UTF-8-capable locale and thus the program shouldn't even get to the point of trying to generate an error message.

I've already found the offending line:

exit(1);

with a bit of context:

btop/src/btop.cpp

Lines 705 to 728 in 480b421

//? Try to find and set a UTF-8 locale
if (bool found = false; not str_to_upper((string)std::setlocale(LC_ALL, NULL)).ends_with("UTF-8")) {
if (const string lang = (string)getenv("LANG"); str_to_upper(lang).ends_with("UTF-8")) {
found = true;
std::setlocale(LC_ALL, lang.c_str());
}
else if (const string loc = std::locale("").name(); not loc.empty()) {
try {
for (auto& l : ssplit(loc, ';')) {
if (str_to_upper(l).ends_with("UTF-8")) {
found = true;
std::setlocale(LC_ALL, l.substr(l.find('=') + 1).c_str());
break;
}
}
}
catch (const std::out_of_range&) { found = false; }
}
if (not found and Global::utf_force)
Logger::warning("No UTF-8 locale detected! Forcing start with --utf-force argument.");
else if (not found) {
Global::exit_error_msg = "No UTF-8 locale detected!\nUse --utf-force argument to force start if you're sure your terminal can handle it.";
exit(1);

So one issue is that the locale didn't get recognized as UTF-8 properly.
The more important issue here is that btop probably tries to join a pthread which it never created–more or less a use-before-malloc:

btop/src/btop.cpp

Lines 705 to 728 in 480b421

//? Try to find and set a UTF-8 locale
if (bool found = false; not str_to_upper((string)std::setlocale(LC_ALL, NULL)).ends_with("UTF-8")) {
if (const string lang = (string)getenv("LANG"); str_to_upper(lang).ends_with("UTF-8")) {
found = true;
std::setlocale(LC_ALL, lang.c_str());
}
else if (const string loc = std::locale("").name(); not loc.empty()) {
try {
for (auto& l : ssplit(loc, ';')) {
if (str_to_upper(l).ends_with("UTF-8")) {
found = true;
std::setlocale(LC_ALL, l.substr(l.find('=') + 1).c_str());
break;
}
}
}
catch (const std::out_of_range&) { found = false; }
}
if (not found and Global::utf_force)
Logger::warning("No UTF-8 locale detected! Forcing start with --utf-force argument.");
else if (not found) {
Global::exit_error_msg = "No UTF-8 locale detected!\nUse --utf-force argument to force start if you're sure your terminal can handle it.";
exit(1);

So the proper fix for the second issue is probably just checking if Runner::runner_id is a valid thread (i.e. has been created).
But on that notice, why bother using pthread directly and not std::thread?
If it's because an std::thread on the stack automatically starts a thread, just wrap it inside a std::unique_ptr.
A check using std::unique_ptr::operator bool then is a very convenient method to check if there is a thread possibly running.
Also, you don't have to take care of cleaning up after the thread since std::unique_ptr does that for you.
But that's just an idea.
I suppose you had a valid reason to use pthread directly.

@NexAdn NexAdn added the bug Something isn't working label Sep 18, 2021
@NexAdn NexAdn changed the title [BUG] [BUG] segfault if a non-UTF-8 locale is being used Sep 18, 2021
@aristocratos
Copy link
Owner

@NexAdn
Didn't know "utf8" was valid. But should be an easy fix.

So the proper fix for the second issue is probably just checking if Runner::runner_id is a valid thread (i.e. has been created).
But on that notice, why bother using pthread directly and not std::thread?

As I understood it, std::thread is not guaranteed to be using posix threads which I need to block the signal handlers inside the thread. Was using std::thread when I started writing, but changed since I didn't know if the blocking would work in every case.
And couldn't find functionality to block signals in std::thread.

Thanks for suggested fixes :)
Will release some bugfixes later today or tomorrow.

@aristocratos
Copy link
Owner

Fixed in commit fca1d8c

@aristocratos
Copy link
Owner

v1.0.1 "Bug fixes" released.

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants