Backtrace screen (improvements over #1270)#1821
Conversation
|
Can you note, which commits for the first vs. second part? I assume the first commit ("Rename *" for the first and the remaining 4 later? I think we can also do this all at once. But I'm ope on that front. |
Yes, the first commit is the option rename, and the remaining 4 commits are the new feature / configure options. The reason I suggest the rename is that |
|
Do we have alternative backends for backtracing/demangling besides unwind-ptrace and iberty? Given there's both an unwind implementation in LLVM and GCC, what else are sensible alternatives that warrent implementation of the |
Well, I don't know what other libraries are possible for the backtrace feature, but I leave room for those new backends. For the demangling function, I can see libdemangle from Solaris / Sun Studio as a possible alternative, but I didn't check that yet. It's better for @MrCirdo 's code (#1270) get merged first before considering alternatives. |
|
NP. No need to decide on all the possible backends and variants now. That's something we can leave open for the first implementation. Priority is getting things work; make them flexible and fast is the next step. No need to over-engineer things from the get-go. I just wanted to evaluate if there's a reasonable need for allowing the backends for backtraces and demangling to be replaced. |
a964363 to
798b603
Compare
798b603 to
9ced4f4
Compare
Great! Perfect! Thank you very much!
With pleasure! Wait, let’s get on the same page @Explorer09. My primary concern about supporting the BSD platform (FreeBSD, NetBSD, macOS, etc) is that I don't know if NB : Sorry if I take time to answer because I need to do some preparation for a coding interview. |
I didn't test running the code on BSD yet. The only thing I've tested is building htop with libunwind-ptrace and libiberty on FreeBSD on a CI server. I've read the man page of My aim is to make it buildable in Linux and FreeBSD; other OSes are too difficult and I would expect libunwind port to those platforms first before we try making our backtrace feature work there. |
I.e. no further changes from @Explorer09 unless needed/requested. And possibly rebases onto main maybe.
I think this PR will likely stay as is; i.e. no major changes.
You can ask @fraggerfox regarding NetBSD details if necessary.
NP. |
The libunwind README says its supported operating systems are Linux, FreeBSD, Solaris, QNX and HP-UX (partially). NetBSD is not included. For Solaris, there's no pre-built binary package for libunwind so testing on it would be difficult. QNX and HP-UX are out of htop's scope. So there remains Linux and FreeBSD, which are what I tried to make htop's backtrace work for. |
This question depends on the backend. There could a hypothetical backend that can trace the debug file (DWARF format?) of the program and print the source file names. For the purpose of the UI, I don't think "FILE" poses ambiguity. |
3496ddf to
ef82218
Compare
ef82218 to
3ffafb7
Compare
3ffafb7 to
f648dcc
Compare
f648dcc to
a0d1cc4
Compare
|
Apart from the "FILE"->"MODULE" column rename any other open issues for this PR? |
a0d1cc4 to
0529f5b
Compare
I just pushed the modification to the parameters of the Apart from that point you mentioned, I don't have any issues with this PR. |
0529f5b to
1751ac0
Compare
|
I would say this branch is all ready to be merged.
The consensus from the previous discussions is to use "FILE" for the column, not "MODULE". |
* Explain that "/usr/include/libunwind/libunwind.h" is a Debian package thing. (LLVM libunwind upstream doesn't install the headers in that strange location.) (See also 6388033 and issue htop-dev#894) * Clarifications on why we define UNW_LOCAL_ONLY during redection of libunwind functions. No change in configure script behavior. Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
The configure option '--enable-unwind' doesn't actually enable a feature in htop but selects a library dependency for a feature (printing local backtrace for this case). Use a '--with-' naming to reflect the option's purpose better. For backward compatibility (with automated build scripts), using '--enable-unwind' will still work but it will print a warning. Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
For the remote backtrace feature that would be introduced in the next commit. For now the feature supports only one backend (libunwind-ptrace). This commit adds checks to libunwind-ptrace. Co-authored-by: Odric Roux-Paris <odric@roux-paris.fr> Co-authored-by: Benny Baumann <BenBE@geshi.org> Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
1751ac0 to
2c893ed
Compare
Add a new feature to display backtrace of any process. The new backtrace screen can be started by pressing the key 'b' when a process is selected. This commit contains only the very basic information of the backtrace screen (frame indices, addresses and symbol names). Additional information like library names and demangling support will be added in subsequent commits. The code in this commit was primarily written by Odric. Kang-Che and Benny provided code improvements and bug fixes that were incorporated into this main commit. Co-authored-by: Odric Roux-Paris <odric@roux-paris.fr> Co-authored-by: Kang-Che Sung <explorer09@gmail.com> Co-authored-by: Benny Baumann <BenBE@geshi.org>
This covers Linux and FreeBSD build jobs Co-authored-by: Odric Roux-Paris <odric@roux-paris.fr> Co-authored-by: Kang-Che Sung <explorer09@gmail.com>
Extend the backtrace screen feature to show library filenames as a new column in the backtrace screen. This feature depend on libunwind 1.8 or later (unw_get_elf_filename()) Co-authored-by: Benny Baumann <BenBE@geshi.org> Co-authored-by: Kang-Che Sung <explorer09@gmail.com>
For demangling support in the backtrace screen, which will be added in the next commit. Two backends will be supported: libiberty (GNU) and libdemangle (Solaris). This commit adds checks for both of them. Co-authored-by: Odric Roux-Paris <odric@roux-paris.fr> Co-authored-by: Benny Baumann <BenBE@geshi.org> Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
Add support of demangling symbol names in the backtrace screen. Two backends are supported: libiberty (GNU) and libdemangle (Solaris). [The initial code for demangling support with libiberty was written by Odric. Kang-Che expanded the code to support libdemangle. Benny provided code reviewes and minor fixes.] Co-authored-by: Kang-Che Sung <explorer09@gmail.com> Co-authored-by: Benny Baumann <BenBE@geshi.org>
For Linux and FreeBSD build jobs
2c893ed to
349519f
Compare
|
Did a few minor adjustments to fix minor issues and to streamline the commit messages somewhat. |
This branch is experimental and related to #1270 (the backtrace screen feature).
I've written and improved the configure script so that it can check
libunwind-ptraceandlibibertyfor the backtrace feature proposed in #1270.The changes come with two parts:
The first part can be reviewed and cherry-picked into
mainearly. It renames the configure option--enable-unwindto--with-libunwindto better reflect it's purpose. The old option name is preserved for compatibility but will trigger warnings when used.The second part consists of several commits that add the new configure options
--enable-backtraceand--enable-demanglingand the checking code oflibunwind-ptraceandlibiberty. The code might look large and complicated, but I've tried a lot on the usability and correctness of the check. For example thislibunwind-ptracecheck can work with and withoutpkg-config, and I tried to ensure thecplus_demangle()function has a correct prototype.The
libunwind-ptraceandlibibertylibraries can be detected both in Linux and in FreeBSD. However, I haven't ported the backtrace screen code from @MrCirdo to ensure they really build, especially in FreeBSD.