Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2024 Pyarelal Knowles, MIT License
# Copyright (c) 2024-2025 Pyarelal Knowles, MIT License

cmake_minimum_required(VERSION 3.20)

Expand All @@ -10,8 +10,22 @@ add_library(decodeless_offset_ptr INTERFACE)
target_include_directories(decodeless_offset_ptr INTERFACE include)

if(MSVC_IDE)
target_sources(decodeless_offset_ptr INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}/include/decodeless/offset_ptr.natvis
${CMAKE_CURRENT_SOURCE_DIR}/include/decodeless/offset_span.natvis)
# Natvis for debugging in visual studio
target_sources(
decodeless_offset_ptr
INTERFACE "${CMAKE_CURRENT_SOURCE_DIR}/debugging/offset_ptr.natvis"
"${CMAKE_CURRENT_SOURCE_DIR}/debugging/offset_span.natvis")
else()
# Python pretty printers for linux debugging
option(DECODELESS_EMBED_PRETTY_PRINTERS
"Embed python pretty printers in .debug_gdb_scripts sections" ON)
Comment on lines +20 to +21
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Add documentation for the CMake option.

Consider adding a more detailed description for the DECODELESS_EMBED_PRETTY_PRINTERS option to help users understand its purpose and impact.

 option(DECODELESS_EMBED_PRETTY_PRINTERS
-        "Embed python pretty printers in .debug_gdb_scripts sections" ON)
+        "Embed python pretty printers in .debug_gdb_scripts sections for enhanced GDB debugging experience. \
+         When enabled, the pretty printers will be embedded in the object files and automatically loaded by GDB." ON)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
option(DECODELESS_EMBED_PRETTY_PRINTERS
"Embed python pretty printers in .debug_gdb_scripts sections" ON)
option(DECODELESS_EMBED_PRETTY_PRINTERS
"Embed python pretty printers in .debug_gdb_scripts sections for enhanced GDB debugging experience. \
When enabled, the pretty printers will be embedded in the object files and automatically loaded by GDB." ON)

if(DECODELESS_EMBED_PRETTY_PRINTERS)
include(debugging/pretty_printer_embed.cmake)
decodeless_embed_pretty_printers(
decodeless_offset_ptr
"${CMAKE_CURRENT_SOURCE_DIR}/debugging/offset_ptr_pretty_printer.py"
"${CMAKE_CURRENT_SOURCE_DIR}/debugging/offset_span_pretty_printer.py")
endif()
endif()

add_library(decodeless::offset_ptr ALIAS decodeless_offset_ptr)
Expand Down
119 changes: 105 additions & 14 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,24 +1,31 @@
# decodeless::offset_ptr

Tiny header-only library for an address space independent data structure. The
humble offset pointer is a relative address from its own address in memory
rather than an absolute address. As long as the thing it points to is in the
same address space it will still work. Quite useful if you're...
`offset_ptr` is a self-relative pointer, encapsulated with C++:

```cpp
T* operator->() const { return (T*)((char*)this + m_offset); }
```

This is a tiny header-only C++ library. The humble offset pointer is a relative
address from its own address in memory rather than an absolute address. It is
address space independent. As long as the thing it points to is mapped or copied
within the same address range it will still work. Quite useful if you're...

- Saving a data structure to disk
- Better yet, memory mapping a file from disk
- Sharing memory between processes
- Memory mapping a file from disk, or even just reading big chunks
- Copying complex data structures to the GPU
- Duplicating the raw memory of a data structure, e.g. a low overhead deep copy

A tiny offset span class is provided too, which is just an offset pointer and
size. It `offset_span` is very similar to `std::span` but with a relative address.
size. `offset_span` is very similar to `std::span` but with a relative address.

Part of the [`decodeless`](https://github.com/decodeless) collection of utility
libraries for conveniently reading and writing files via memory mapping.

**Example: offset_ptr**

```
```cpp
#include <decodeless/offset_ptr.hpp>

using namespace decodeless;
Expand Down Expand Up @@ -51,7 +58,7 @@ EXPECT_TRUE(file.data == 42);

**Example: offset_span**

```
```cpp
#include <decodeless/offset_span.hpp>

using namespace decodeless;
Expand All @@ -72,19 +79,103 @@ EXPECT_TRUE(file.values.size() == std::size(file.data));

## Notes

- The offset pointer behaves like a regular C pointer
- Initializes to `nullptr` (perhaps controversial - the `glm` library does not)
- The offset pointer behaves like a regular C pointer - you can assign to it,
dereference etc., and it'll just work
- Initializes to `nullptr` (perhaps controversial - the `glm` library does not zero-initialize)
- The difference is internally it stores an address offset from itself/`this`
- `nullptr` is encoded as the value `1` to allow pointing to itself
- Does not
[propagate_const](https://en.cppreference.com/w/cpp/experimental/propagate_const)
and arguably should, for something designed for file mapping

## Natvis
## CMake Integration

This is a header-only library with no dependencies other than C++20. CMake
integration is provided. You don't have to use CMake, but debug visualizers are
integrated if you do. You can use one of the following:

**add_subdirectory**

Add the library as a submodule (`git add submodule
https://github.com/decodeless/offset_ptr.git`), then in cmake:

```cmake
add_subdirectory(path/to/offset_ptr)
target_link_libraries(myproject PRIVATE decodeless::offset_ptr)
```

**FetchContent**

CMake will download the library to the build directory at configure time.

```cmake
include(FetchContent)
FetchContent_Declare(
decodeless_offset_ptr
GIT_REPOSITORY https://github.com/decodeless/offset_ptr.git
GIT_TAG release_tag
GIT_SHALLOW TRUE
)
FetchContent_MakeAvailable(decodeless_offset_ptr)

target_link_libraries(myproject PRIVATE decodeless::offset_ptr)
```

**find_package**

For visual studio users (and somewhat vscode
https://github.com/microsoft/vscode-cpptools/issues/10917), natvis files are
included to allow debug inspection of pointers and spans.
If using in a library, a config file is provided for
`find_package(... CONFIG ...)`, which trivially
includes `CMakeLists.txt`. See
[decodeless_writer](https://github.com/decodeless/writer/blob/main/CMakeLists.txt)
for an example.

```cmake
find_package(decodeless_offset_ptr REQUIRED CONFIG PATHS ...)

target_link_libraries(myproject PRIVATE decodeless::offset_ptr)
```

## Debug Visualization

If you're using CMake, debugging should be seamless on windows without any setup
and near-seamless on linux (see `auto-load safe-path` below).

Without debug visualizers you'd just see `m_offset` in bytes when hovering over
an `offset_ptr`. You *could* add a watch or print
`*(MyType*)((char*)&offsetPtr + offsetPtr.m_offset)` ... lol. Instead, it'd
be nicer if a debugger showed `offset_ptr` like it was a regular pointer and
`offset_span` like a regular array. That's what the visualizers in
[./debugging](debugging/) do.

**Natvis**

For visual studio users (and somewhat
[vscode](https://github.com/microsoft/vscode-cpptools/issues/10917)), natvis
files are included to allow debug inspection of pointers and spans.

**Pretty Printing**

For gdb, python pretty printers are embedded in
[`.debug_gdb_scripts`](https://www.heuristic42.com/blog/64) sections during
compilation.

You may need to set an [auto-load
safe-path](https://sourceware.org/gdb/current/onlinedocs/gdb.html/dotdebug_005fgdb_005fscripts-section.html)
before gdb will read the inlined scripts. Add this to `~/.gdbinit`:
Comment on lines +162 to +164
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Fix compound word and improve sentence structure.

Improve readability:

-You may need to set an [auto-load
-safe-path](https://sourceware.org/gdb/current/onlinedocs/gdb.html/dotdebug_005fgdb_005fscripts-section.html)
-before gdb will read the inlined scripts. Add this to `~/.gdbinit`:
+You may need to set an [autoload
+safe-path](https://sourceware.org/gdb/current/onlinedocs/gdb.html/dotdebug_005fgdb_005fscripts-section.html)
+before GDB will read the inlined scripts. Add this to `~/.gdbinit`:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
You may need to set an [auto-load
safe-path](https://sourceware.org/gdb/current/onlinedocs/gdb.html/dotdebug_005fgdb_005fscripts-section.html)
before gdb will read the inlined scripts. Add this to `~/.gdbinit`:
You may need to set an [autoload
safe-path](https://sourceware.org/gdb/current/onlinedocs/gdb.html/dotdebug_005fgdb_005fscripts-section.html)
before GDB will read the inlined scripts. Add this to `~/.gdbinit`:
🧰 Tools
🪛 LanguageTool

[misspelling] ~162-~162: This word is normally spelled as one.
Context: ...g compilation. You may need to set an [auto-load safe-path](https://sourceware.org/gdb/c...

(EN_COMPOUNDS_AUTO_LOAD)


```set auto-load safe-path <binary directory or parent>```

Or add the following to vscode's `launch.json`:

```json
"setupCommands": [
{
"text": "set auto-load safe-path .",
"description": "enable loading pretty printers",
"ignoreFailures": false
}
],
```

## Contributing

Expand Down
File renamed without changes.
42 changes: 42 additions & 0 deletions debugging/offset_ptr_pretty_printer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import gdb
import re


class OffsetPtrPrinter:
"""Pretty printer for decodeless::offset_ptr<T>"""
def __init__(self, val):
self.val = val
self.T = self.val.type.strip_typedefs().unqualified().template_argument(0)
Comment on lines +5 to +9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Class documentation is concise.

The class docstring is clear. Consider adding short docstrings for each method to aid future maintainers.


def pointer(self):
base, offset = self.val.address, self.val["m_offset"]

# Handle nullptr magic offset value of '1'
if offset == 1:
base, offset = gdb.Value(0), gdb.Value(0)

return (base.cast(gdb.lookup_type("char").pointer()) + offset).cast(self.T.pointer())

def children(self):
# TODO: rather than a virtual '*' member, can this be made to behave
# more like a real pointer?
return [("operator*()", self.pointer().dereference())]

def to_string(self):
return f'({self.T} *) {self.pointer()}'


offset_ptr_lookup_pattern = re.compile("^decodeless::offset_ptr<.*>$")


def offset_ptr_lookup(val):
global offset_ptr_lookup_pattern
lookup_tag = val.type.strip_typedefs().unqualified().tag
if lookup_tag is None:
return None
if offset_ptr_lookup_pattern.match(lookup_tag):
return OffsetPtrPrinter(val)
return None


gdb.pretty_printers.append(offset_ptr_lookup)
File renamed without changes.
65 changes: 65 additions & 0 deletions debugging/offset_span_pretty_printer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import gdb
import re


# Encapsulates a list of children. Calls .child() on the pretty printer only
# when requested. This is a workaround for poor performance when inspecting
# large arrays in the debugger.
class LazyChildren:
def __init__(self, obj, length):
self._obj = obj
self._length = length

def __getitem__(self, index):
if not 0 <= index < self._length:
raise IndexError("Index out of range")
return self._obj.child(index)

def __len__(self):
return self._length


class OffsetSpanPrinter:
"""Pretty printer for decodeless::offset_span<T>"""
def __init__(self, val):
self.T = val.type.strip_typedefs().unqualified().template_argument(0)
self.length = val["m_size"]

ptr = val["m_data"]
base = ptr.address
offset = ptr["m_offset"]
if offset == 1:
base = gdb.Value(0)
offset = gdb.Value(0)
self.pointer = (base.cast(gdb.lookup_type("char").pointer()) + offset).cast(self.T.pointer())
Comment on lines +22 to +34
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

OffsetSpanPrinter constructor is readable.

The logic of extracting type and adjusting base/offset for a null-like representation is consistent with what we saw in offset_ptr. Possibly add a short comment explaining offset==1 is treated as NULL.


def child(self, i):
return (f"[{i}]", (self.pointer + i).dereference())

def children(self):
return LazyChildren(self, self.length)

def __len__(self):
return self.length

def to_string(self):
return f'offset_span<{self.T}> of length {self.length}'

def display_hint(self):
return 'array'


offset_span_lookup_pattern = re.compile("^decodeless::offset_span<.*>$")


def offset_span_lookup(val):
global offset_span_lookup_pattern
lookup_tag = val.type.strip_typedefs().unqualified().tag
if lookup_tag is None:
return None
if offset_span_lookup_pattern.match(lookup_tag):
return OffsetSpanPrinter(val)
return None


gdb.pretty_printers.append(offset_span_lookup)
32 changes: 32 additions & 0 deletions debugging/pretty_printer_embed.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Copyright (c) 2025 Pyarelal Knowles, MIT License

# Function to embed a python pretty printer in an object (.o) file and add it as
# a dependency of the given target. See https://www.heuristic42.com/blog/64/.
# Usage: decodeless_embed_pretty_printer(MyTarget printer_a.py printer_b.py)
function(decodeless_embed_pretty_printers target)
Comment on lines +3 to +6
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Enhance documentation with platform requirements.

The documentation is clear but could be more helpful by mentioning:

  • Platform limitations (Linux/GNU toolchain only)
  • Required tools (objcopy)
  • CMake option that controls this feature
 # Function to embed a python pretty printer in an object (.o) file and add it as
 # a dependency of the given target. See https://www.heuristic42.com/blog/64/.
+# Note: This functionality is only available on Linux/GNU platforms and requires
+# objcopy from binutils. It is controlled by DECODELESS_EMBED_PRETTY_PRINTERS option.
 # Usage: decodeless_embed_pretty_printer(MyTarget printer_a.py printer_b.py)

foreach(source IN LISTS ARGN)
# Generate a *.o filename
get_filename_component(source_name "${source}" NAME_WLE)
set(object "${CMAKE_CURRENT_BINARY_DIR}/${source_name}.o")

# Create a .bin file with a .debug_gdb_scripts section and generate a .o file from it
add_custom_command(
OUTPUT "${object}"
COMMAND
"$<TARGET_FILE:decodeless_pretty_printer_embed>" "${source}" "${object}.bin" &&
${CMAKE_OBJCOPY} -I binary -O elf64-x86-64 --rename-section .data=.debug_gdb_scripts "${object}.bin" "${object}"
DEPENDS decodeless_pretty_printer_embed "${source}"
COMMENT "Embedding GDB pretty-printer ${source}")
Comment on lines +13 to +19
Copy link

@coderabbitai coderabbitai bot Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Add error handling for missing tools and failed commands.

The custom command should fail gracefully if objcopy is not available or if the commands fail.

+find_program(CMAKE_OBJCOPY objcopy)
+if(NOT CMAKE_OBJCOPY)
+  message(WARNING "objcopy not found. GDB pretty printers will not be embedded.")
+  return()
+endif()

 add_custom_command(
   OUTPUT "${object}"
   COMMAND
     "$<TARGET_FILE:decodeless_pretty_printer_embed>" "${source}" "${object}.bin" &&
     ${CMAKE_OBJCOPY} -I binary -O elf64-x86-64 --rename-section .data=.debug_gdb_scripts "${object}.bin" "${object}"
+  COMMAND_ERROR_IS_FATAL ANY
   DEPENDS decodeless_pretty_printer_embed "${source}"
   COMMENT "Embedding GDB pretty-printer ${source}")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
add_custom_command(
OUTPUT "${object}"
COMMAND
"$<TARGET_FILE:decodeless_pretty_printer_embed>" "${source}" "${object}.bin" &&
${CMAKE_OBJCOPY} -I binary -O elf64-x86-64 --rename-section .data=.debug_gdb_scripts "${object}.bin" "${object}"
DEPENDS decodeless_pretty_printer_embed "${source}"
COMMENT "Embedding GDB pretty-printer ${source}")
find_program(CMAKE_OBJCOPY objcopy)
if(NOT CMAKE_OBJCOPY)
message(WARNING "objcopy not found. GDB pretty printers will not be embedded.")
return()
endif()
add_custom_command(
OUTPUT "${object}"
COMMAND "$<TARGET_FILE:decodeless_pretty_printer_embed>" "${source}" "${object}.bin" &&
${CMAKE_OBJCOPY} -I binary -O elf64-x86-64 --rename-section .data=.debug_gdb_scripts "${object}.bin" "${object}"
COMMAND_ERROR_IS_FATAL ANY
DEPENDS decodeless_pretty_printer_embed "${source}"
COMMENT "Embedding GDB pretty-printer ${source}")

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's safe to assume people compiling on linux will have this. && should cover failures

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pknowles Understood, that's a fair point. The && operator will indeed handle command failures, and objcopy being a standard Linux tool makes the explicit check unnecessary.

🐰 nods thoughtfully while taking notes


✏️ Learnings added
Learnt from: pknowles
PR: decodeless/offset_ptr#6
File: debugging/pretty_printer_embed.cmake:13-19
Timestamp: 2025-02-07T22:31:28.117Z
Learning: For Linux-specific functionality in CMake, it's acceptable to assume the availability of standard binary utilities (binutils) like `objcopy`. The `&&` operator in shell commands provides sufficient error handling by ensuring subsequent commands only run if the previous command succeeds.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


# Add the object file as a target dependency
# TODO: avoid the custom target dance
target_link_libraries(${target} INTERFACE "${object}")
add_custom_target("generate_${source_name}" DEPENDS "${object}")
add_dependencies(${target} "generate_${source_name}")
Comment on lines +22 to +25
Copy link

@coderabbitai coderabbitai bot Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider using object library to avoid custom targets.

The TODO comment suggests finding an alternative to custom targets. Consider using an OBJECT library type instead:

-    target_link_libraries(${target} INTERFACE "${object}")
-    add_custom_target("generate_${source_name}" DEPENDS "${object}")
-    add_dependencies(${target} "generate_${source_name}")
+    add_library("${source_name}_obj" OBJECT IMPORTED)
+    set_property(TARGET "${source_name}_obj" PROPERTY IMPORTED_OBJECTS "${object}")
+    target_link_libraries(${target} INTERFACE "${source_name}_obj")

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I investigated OBJECT libraries and couldn't get around the need for extra targets. One day I might return to this.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pknowles Understood, thanks for investigating that approach. I'll retract my suggestion about using OBJECT libraries.


✏️ Learnings added
Learnt from: pknowles
PR: decodeless/offset_ptr#6
File: debugging/pretty_printer_embed.cmake:22-25
Timestamp: 2025-02-07T22:33:13.665Z
Learning: Using OBJECT libraries in CMake for embedding GDB pretty printers still requires extra targets, making it not significantly better than using custom targets directly.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

endforeach()
endfunction()
Comment on lines +6 to +27
Copy link

@coderabbitai coderabbitai bot Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider potential portability on non-ELF platforms and streamline custom target usage.

The approach of invoking objcopy with -I binary -O elf64-x86-64 and then renaming .data to .debug_gdb_scripts may not be portable to non-ELF platforms (e.g., Windows). Additionally, the TODO comment indicates there's an interest in avoiding the extra custom targets. Investigate dynamically generating these object files in a more cross-platform manner and simplifying or automating the custom target logic.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

objcopy is already gnu binutils thing. I.e. we're already in the linux part of the cross platform support and this file is not used with MSVC. I already dislike the seemingly architecture dependent nature of this command, but again I haven't found an alternative way to add to an ELF section without specifying the architecture. Maybe by getting gcc or ld to first create an empty .o with the existing architecture but that's getting really long winded for something that should be simple.

Yep. If you could suggest a way around the custom_target that works I'll totally add it, but I've spent a long time on this already and have come up empty.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pknowles Thank you for the detailed explanation. I understand now that this is specifically for Linux/GNU toolchain support, and the architecture-specific nature is an inherent limitation of the ELF tooling. I agree that alternatives would likely introduce more complexity. I'll withdraw my concern about portability, and I appreciate you considering the custom target suggestion despite the current limitations.


✏️ Learnings added
Learnt from: pknowles
PR: decodeless/offset_ptr#6
File: debugging/pretty_printer_embed.cmake:6-27
Timestamp: 2025-02-01T01:52:50.022Z
Learning: The pretty printer embedding in `debugging/pretty_printer_embed.cmake` is specifically for Linux/GNU toolchain support and is not used with MSVC. The architecture-specific nature of the objcopy command is a known limitation when working with ELF sections.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment on lines +21 to +27
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Improve TODO comment with investigation results.

The TODO comment could be more descriptive about the investigation results.

Apply this diff to improve the comment:

-    # TODO: avoid the custom target dance
+    # TODO: avoid the custom target dance. OBJECT libraries were investigated but
+    # still required extra targets. Alternative approaches welcome.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Add the object file as a target dependency
# TODO: avoid the custom target dance
target_link_libraries(${target} INTERFACE "${object}")
add_custom_target("generate_${source_name}" DEPENDS "${object}")
add_dependencies(${target} "generate_${source_name}")
endforeach()
endfunction()
# Add the object file as a target dependency
# TODO: avoid the custom target dance. OBJECT libraries were investigated but
# still required extra targets. Alternative approaches welcome.
target_link_libraries(${target} INTERFACE "${object}")
add_custom_target("generate_${source_name}" DEPENDS "${object}")
add_dependencies(${target} "generate_${source_name}")


# Adding the necessary headers to the .debug_gdb_scripts section is not easy
# with cmake. Much nicer to build a little C++ executable to do the generation
# with no other dependencies.
add_executable(decodeless_pretty_printer_embed "${CMAKE_CURRENT_LIST_DIR}/pretty_printer_embed.cpp")
Comment on lines +29 to +32
Copy link

@coderabbitai coderabbitai bot Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Provide an option to disable adding this executable if not needed.

Some projects may not require pretty-printer embedding on certain platforms or build configurations. Consider wrapping this target in an option guard to reduce build overhead when no GDB integration is needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already done - this file doesn't get included if the option is disabled.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pknowles I apologize for the oversight. You're absolutely right - I can see the option DECODELESS_EMBED_PRETTY_PRINTERS and the conditional inclusion in the main CMakeLists.txt.

🔗 Analysis chain
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Find and display the relevant parts of CMakeLists.txt that show the option and inclusion
rg -A 5 -B 5 "pretty_printer" CMakeLists.txt

Length of output: 605


✏️ Learnings added
Learnt from: pknowles
PR: decodeless/offset_ptr#6
File: debugging/pretty_printer_embed.cmake:29-32
Timestamp: 2025-02-01T01:53:30.925Z
Learning: The pretty printer embedding functionality in offset_ptr is controlled by the CMake option DECODELESS_EMBED_PRETTY_PRINTERS (default: ON) and is only available on non-MSVC platforms.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

39 changes: 39 additions & 0 deletions debugging/pretty_printer_embed.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#include <filesystem>
#include <fstream>
#include <iostream>
#include <ostream>

// Tiny program to dump python source code for a .debug_gdb_scripts section into
// a binary file that can then be consume by objcopy. It just adds some header
// bits and a null terminator.
// Usage: ./exe <python_script.py> <output.bin>
// See https://sourceware.org/gdb/current/onlinedocs/gdb.html/dotdebug_005fgdb_005fscripts-section.html
// SECTION_SCRIPT_ID_PYTHON_FILE = 1
// SECTION_SCRIPT_ID_SCHEME_FILE = 3
// SECTION_SCRIPT_ID_PYTHON_TEXT = 4 <--- source code
// SECTION_SCRIPT_ID_SCHEME_TEXT = 6
int main(int argc, char** argv) {
if (argc != 3) {
std::cerr << "Usage: ./embed <input.py> <output.bin>\n";
return EXIT_FAILURE;
}

std::ifstream src(argv[1]);
if (!src.good()) {
std::cerr << "Error: could not open input file '" << argv[1] << "'\n";
return EXIT_FAILURE;
}

std::ofstream dst(argv[2], std::ios::binary);
if (!dst.good()) {
std::cerr << "Error: could not open output file '" << argv[2] << "'\n";
return EXIT_FAILURE;
}

std::string name = std::filesystem::path(argv[1]).filename().string();
dst.write("\x04", 1); // "python text" identifier byte
dst << name << "\n"; // first line is the name of the script
dst << src.rdbuf(); // inline the script
dst.write("\0", 1); // null terminated
Comment on lines +33 to +37
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider validating write success.
If writing fails (e.g., filesystem is full), the program won’t catch it. Checking for dst.fail() or adding more robust error handling may prevent incomplete binary files.

return EXIT_SUCCESS;
}
2 changes: 0 additions & 2 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ target_link_libraries(
decodeless::offset_ptr
gtest_main)

# TODO: presets?
# https://stackoverflow.com/questions/45955272/modern-way-to-set-compiler-flags-in-cross-platform-cmake-project
if(MSVC)
target_compile_options(${PROJECT_NAME}_tests PRIVATE /W4 /WX)
else()
Expand Down