Packaging location of GDB script files

[topazus]

I got the comments from a reviewer [1], which I filed a ticket to make corrade package into Fedora.

corrade/src/debuggers/CMakeLists.txt

Lines 27 to 43 in f21fc1d

	# TODO: GDB also has an ability to auto-load script files from a system-wide
	# directory, which would make it possible to load them without having to
	# manually update `.gdbinit` files. Unfortunately, this requires GDB to be
	# compiled with `--with-system-gdbinit-dir`, which neither ArchLinux nor Ubuntu
	# does, both set only `--with-system-gdbinit` which would mean having to
	# *append* to that file on install. Not the way to go.
	#
	# TODO: there's also an option to install those next to the libraries (such as
	# having /usr/lib/CorradeContainers-gdb.py), or embedded in the binaries in the
	# .debug_gdb_scripts section. Both still require some whitelisting from the
	# user, though, so no automagic either:
	# https://sourceware.org/gdb/onlinedocs/gdb/Python-Auto_002dloading.html#Python-Auto_002dloading
	# https://sourceware.org/gdb/onlinedocs/gdb/dotdebug_005fgdb_005fscripts-section.html#dotdebug_005fgdb_005fscripts-section
	if(NOT CORRADE_TARGET_MSVC AND NOT CORRADE_TARGET_EMSCRIPTEN)
	install(DIRECTORY gdb DESTINATION ${CORRADE_DATA_INSTALL_DIR}/debuggers)
	endif()

I have little knowledge about GDB script files. The reviewer suggested putting the pretty printers related files of corrade for gdb into /usr/share/gdb/python/corrade, which the files go into /usr/share/corrade/debuggers/gdb currently. I quoted the reviewer's comment below:

It is great to have pretty printers for gdb. Have you considered placing them into /usr/share/gdb/python/corrade instead of custom location? What way they could be auto-loaded if devel package were present. Just "python import corrade" might activate them. With smart auto-load named script they could be activated just automatially when debugging corrade libraries.
I have found just matreshka-devel and gdb-exploitable provide code into it. Others place python scripts directly into /usr/share/gdb/auto-load directories. libstdc++ or mono-core packages provide something there, you may want to consult how to prepare auto-loaded pretty printers with your devel package.
More information about auto-loading: https://sourceware.org/gdb/current/onlinedocs/gdb.html/Auto_002dloading.html#Auto_002dloading

[1] https://bugzilla.redhat.com/show_bug.cgi?id=2226670#c3

[williamjcm]

When I contributed the pretty-printers to Corrade and Magnum, I didn't know there was a "standard" location for them.

I need to make a PR to update the Corrade printers (turns out something in Containers::String changed and the script can't print SSO'd strings anymore, I need to fix that and make other printers that rely on enum values more robust) and add printers for types that were added since my initial contribution, so I'll look into changing the path then.

[mosra]

Oh wow, RPM packages included directly in the distro! 🤩

Putting them into /usr/share/gdb/python/corrade makes sense for me, I remember we were trying to find the canonical location and failed, so we just put those somewhere :)

@williamjcm I think this change would fix the String SSO printing (similar as #165 for MSVC natvis). If you confirm it works, I'll commit that together with changing the install path, the remaining containers aren't as critical as this fix I think. Having an enum for that value isn't needed in my opinion, the ABI for the string classes is unlikely to change anymore as there are no more unused bits in their representation.

diff --git a/src/debuggers/gdb/printers.py b/src/debuggers/gdb/printers.py
index 69e9a275a..b66f5e013 100644
--- a/src/debuggers/gdb/printers.py
+++ b/src/debuggers/gdb/printers.py
@@ -386,7 +386,8 @@ class CorradeString(CorradeStringTypePrinter):
     """Prints a Containers::String"""
 
     def to_string(self):
-        if bool(self.val['_small']['size'] & 0x80) is True:
+        # 0x40 is Implementation::SmallStringBit
+        if bool(self.val['_small']['size'] & 0x40) is True:
             return self.val['_small']['data'].string(length=int(self.val['_small']['size'] & ~0xc0))
         return self.val['_large']['data'].string(length=int(self.val['_large']['size']))
 

[williamjcm]

I think this change would fix the String SSO printing

Yes, it does.

[mosra]

@williamjcm I made some changes to the directory layout to keep the usage consistent between the installed and in-source case, could you verify it still works as expected and the docs are reasonable? It's 8d14558 in the next branch.

Thanks again!

[williamjcm]

So, in that commit, things get installed in /usr/share/gdb/python/gdb (assuming CMAKE_PREFIX_PATH is set to /usr, of course). That folder already contains GDB's built-in gdb module, so the installation would need to drop the corrade folder in /usr/share/gdb/python. Also, since GDB's Python's sys.path already contains /usr/share/gdb/python as its first element, using an installed Corrade would lead to the gdbinit file being reduced to this:

python
from corrade import register_corrade_printers
register_corrade_printers(gdb.current_objfile())

Using the Python module from anywhere else than the standard location (such as a submodule) would still require adding the right folder to sys.path, obviously.

On another note, I noticed this from the OP's quote:

Others place python scripts directly into /usr/share/gdb/auto-load directories. libstdc++ or mono-core packages provide something there, you may want to consult how to prepare auto-loaded pretty printers with your devel package.

I checked on my machine, and did find a few Python scripts for libstdc++, GLib, GStreamer, and a few other libs, which pretty much did the same things as the gbdinit script in the Corrade docs (with or without sys.path manipulation depending on the lib). Those scripts only work for dynamic libraries (as they're named after the SO file), though, so static builds of Corrade must not lead to the generation of such a script.

[mosra]

Argh, I messed up the install() command. With 4922edf (next again) it should be hopefully correct? If you confirm, I'll do a similar change on the Magnum side as well. Thanks for the hint with implicit path as well, adapted the docs.

I also have a few libs in the auto-load directories, might be a good idea to add those in the next iteration. Putting that among TOODs, for now I only aim to get the path fixed :)

[williamjcm]

The path is now correct indeed.

[mosra]

Thanks, pushing 4922edf to master :)