fix: Rewrite the Linux module finder

src/modulefinder/sentry_modulefinder_linux.h

@@ -4,26 +4,45 @@
 #include "sentry_boot.h"
 #include "sentry_slice.h"
 
+#define SENTRY_MAX_MAPPINGS 5
+
 typedef struct {
-    void *start;
-    void *end;
+    uint64_t start;
+    uint64_t end;
+    uint64_t offset;
+    char permissions[5];
+    uint64_t inode;
     sentry_slice_t file;
-} sentry_module_t;
+} sentry_parsed_module_t;
 
 typedef struct {
-    void *ptr;
-    size_t len;
-    int fd;
-} sentry_mmap_t;
+    uint64_t offset; // the offset in the mapped file
+    uint64_t size; // size of this mapping
+    uint64_t addr; // addr in memory of the mapping
+} sentry_mapped_region_t;
 
-#if SENTRY_UNITTEST
-bool sentry__mmap_file(sentry_mmap_t *mapping, const char *path);
-void sentry__mmap_close(sentry_mmap_t *mapping);
+typedef struct {
+    sentry_slice_t file;
+    sentry_mapped_region_t mappings[5];
+    uint64_t offset_in_inode;
+    uint64_t mappings_inode;
+    uint8_t num_mappings;
+} sentry_module_t;
 
-bool sentry__procmaps_read_ids_from_elf(sentry_value_t value, void *elf_ptr);
+#if SENTRY_UNITTEST
+bool sentry__procmaps_read_ids_from_elf(
+    sentry_value_t value, const sentry_module_t *module);
 
 int sentry__procmaps_parse_module_line(
-    const char *line, sentry_module_t *module);
+    const char *line, sentry_parsed_module_t *module);
+
+/**
+ * Checks that `start_offset` + `size` is a valid contiguous mapping in the
+ * mapped regions, and returns the translated pointer corresponding to
+ * `start_offset`.
+ */
+void *sentry__module_get_addr(
+    const sentry_module_t *module, uint64_t start_offset, uint64_t size);
 #endif
 
 #endif

src/sentry_sync.c

@@ -328,6 +328,7 @@ sentry__bgworker_shutdown(sentry_bgworker_t *bgw, uint64_t timeout)
             sentry__mutex_unlock(&bgw->task_lock);
             SENTRY_WARN(
                 "background thread failed to shut down cleanly within timeout");
+            sentry__thread_detach(bgw->thread_id);
             return 1;
         }
 

src/sentry_sync.h

@@ -163,6 +163,7 @@ typedef struct sentry__winmutex_s sentry_mutex_t;
             *ThreadId == INVALID_HANDLE_VALUE ? 1 : 0)
 #    define sentry__thread_join(ThreadId)                                      \
         WaitForSingleObject(ThreadId, INFINITE)
+#    define sentry__thread_detach(ThreadId) (void)ThreadId
 #    define sentry__thread_free(ThreadId)                                      \
         do {                                                                   \
             if (*ThreadId != INVALID_HANDLE_VALUE) {                           \
@@ -257,6 +258,7 @@ typedef pthread_cond_t sentry_cond_t;
 #    define sentry__thread_spawn(ThreadId, Func, Data)                         \
         (pthread_create(ThreadId, NULL, Func, Data) == 0 ? 0 : 1)
 #    define sentry__thread_join(ThreadId) pthread_join(ThreadId, NULL)
+#    define sentry__thread_detach(ThreadId) pthread_detach(ThreadId)
 #    define sentry__thread_free sentry__thread_init
 #    define sentry__threadid_equal pthread_equal
 #    define sentry__current_thread pthread_self

tests/__init__.py

@@ -80,7 +80,15 @@ def run(cwd, exe, args, env=dict(os.environ), **kwargs):
             *cmd,
         ]
     if "valgrind" in os.environ.get("RUN_ANALYZER", ""):
-        cmd = ["valgrind", "--leak-check=yes", *cmd]
+        cmd = [
+            "valgrind",
+            "--suppressions={}".format(
+                os.path.join(sourcedir, "tests", "valgrind.txt")
+            ),
+            "--error-exitcode=33",
+            "--leak-check=yes",
+            *cmd,
+        ]
     try:
         return subprocess.run([*cmd, *args], cwd=cwd, env=env, **kwargs)
     except subprocess.CalledProcessError:

tests/unit/test_modulefinder.c

@@ -37,12 +37,48 @@ SENTRY_TEST(module_finder)
     sentry_clear_modulecache();
 }
 
+SENTRY_TEST(module_addr)
+{
+#if !defined(SENTRY_PLATFORM_LINUX)
+    SKIP_TEST();
+#else
+    sentry_module_t module = { 0 };
+    module.num_mappings = 3;
+    // |    |    |    |    |    |    |    |
+    //           00000          1111122222
+    module.mappings[0].offset = 0;
+    module.mappings[0].size = 5;
+    module.mappings[0].addr = 10;
+    // here is a gap in the address space of size 10
+    module.mappings[1].offset = 5;
+    module.mappings[1].size = 5;
+    module.mappings[1].addr = 25;
+    module.mappings[2].offset = 10;
+    module.mappings[2].size = 5;
+    module.mappings[2].addr = 30;
+
+    void *ptr;
+
+    ptr = sentry__module_get_addr(&module, 0, 5);
+    TEST_CHECK(ptr == (void *)10);
+
+    ptr = sentry__module_get_addr(&module, 0, 6);
+    TEST_CHECK(ptr == NULL); // not contiguous
+
+    ptr = sentry__module_get_addr(&module, 7, 8);
+    TEST_CHECK(ptr == (void *)27);
+
+    ptr = sentry__module_get_addr(&module, 7, 9);
+    TEST_CHECK(ptr == NULL); // too big
+#endif
+}
+
 SENTRY_TEST(procmaps_parser)
 {
 #if !defined(SENTRY_PLATFORM_LINUX) || __SIZEOF_POINTER__ != 8
     SKIP_TEST();
 #else
-    sentry_module_t mod;
+    sentry_parsed_module_t mod;
     char contents[]
         = "7fdb549ce000-7fdb54bb5000 r-xp 00000000 08:01 3803938       "
           "             /lib/x86_64-linux-gnu/libc-2.27.so\n"
@@ -57,30 +93,30 @@ SENTRY_TEST(procmaps_parser)
     read = sentry__procmaps_parse_module_line(lines, &mod);
     lines += read;
     TEST_CHECK(read);
-    TEST_CHECK(mod.start == (void *)0x7fdb549ce000);
-    TEST_CHECK(mod.end == (void *)0x7fdb54bb5000);
+    TEST_CHECK(mod.start == 0x7fdb549ce000);
+    TEST_CHECK(mod.end == 0x7fdb54bb5000);
     TEST_CHECK(strncmp(mod.file.ptr, "/lib/x86_64-linux-gnu/libc-2.27.so",
                    mod.file.len)
         == 0);
 
     read = sentry__procmaps_parse_module_line(lines, &mod);
     lines += read;
     TEST_CHECK(read);
-    TEST_CHECK(mod.start == (void *)0x7f14753de000);
-    TEST_CHECK(mod.end == (void *)0x7f14755de000);
+    TEST_CHECK(mod.start == 0x7f14753de000);
+    TEST_CHECK(mod.end == 0x7f14755de000);
 
     read = sentry__procmaps_parse_module_line(lines, &mod);
     lines += read;
     TEST_CHECK(read);
-    TEST_CHECK(mod.start == (void *)0x7fe714493000);
-    TEST_CHECK(mod.end == (void *)0x7fe714494000);
+    TEST_CHECK(mod.start == 0x7fe714493000);
+    TEST_CHECK(mod.end == 0x7fe714494000);
     TEST_CHECK(mod.file.ptr == NULL);
 
     read = sentry__procmaps_parse_module_line(lines, &mod);
     lines += read;
     TEST_CHECK(read);
-    TEST_CHECK(mod.start == (void *)0x7fff8ca67000);
-    TEST_CHECK(mod.end == (void *)0x7fff8ca88000);
+    TEST_CHECK(mod.start == 0x7fff8ca67000);
+    TEST_CHECK(mod.end == 0x7fff8ca88000);
     TEST_CHECK(strncmp(mod.file.ptr, "[vdso]", mod.file.len) == 0);
 
     read = sentry__procmaps_parse_module_line(lines, &mod);
@@ -98,16 +134,19 @@ SENTRY_TEST(buildid_fallback)
     sentry_path_t *dir = sentry__path_dir(path);
     sentry__path_free(path);
 
+    sentry_module_t module = { 0 };
+    module.num_mappings = 1;
+    size_t *file_size = &module.mappings[0].size;
+    char **buf = (char **)&module.mappings[0].addr;
+
     sentry_value_t with_id_val = sentry_value_new_object();
-    sentry_mmap_t with_id_map;
     sentry_path_t *with_id_path
         = sentry__path_join_str(dir, "../fixtures/with-buildid.so");
-    TEST_CHECK(sentry__mmap_file(&with_id_map, with_id_path->path));
+    *buf = sentry__path_read_to_buffer(with_id_path, file_size);
     sentry__path_free(with_id_path);
 
-    TEST_CHECK(
-        sentry__procmaps_read_ids_from_elf(with_id_val, with_id_map.ptr));
-    sentry__mmap_close(&with_id_map);
+    TEST_CHECK(sentry__procmaps_read_ids_from_elf(with_id_val, &module));
+    sentry_free(*buf);
 
     TEST_CHECK_STRING_EQUAL(
         sentry_value_as_string(sentry_value_get_by_key(with_id_val, "code_id")),
@@ -118,15 +157,13 @@ SENTRY_TEST(buildid_fallback)
     sentry_value_decref(with_id_val);
 
     sentry_value_t x86_exe_val = sentry_value_new_object();
-    sentry_mmap_t x86_exe_map;
     sentry_path_t *x86_exe_path
         = sentry__path_join_str(dir, "../fixtures/sentry_example");
-    TEST_CHECK(sentry__mmap_file(&x86_exe_map, x86_exe_path->path));
+    *buf = sentry__path_read_to_buffer(x86_exe_path, file_size);
     sentry__path_free(x86_exe_path);
 
-    TEST_CHECK(
-        sentry__procmaps_read_ids_from_elf(x86_exe_val, x86_exe_map.ptr));
-    sentry__mmap_close(&x86_exe_map);
+    TEST_CHECK(sentry__procmaps_read_ids_from_elf(x86_exe_val, &module));
+    sentry_free(*buf);
 
     TEST_CHECK_STRING_EQUAL(
         sentry_value_as_string(sentry_value_get_by_key(x86_exe_val, "code_id")),
@@ -137,15 +174,13 @@ SENTRY_TEST(buildid_fallback)
     sentry_value_decref(x86_exe_val);
 
     sentry_value_t without_id_val = sentry_value_new_object();
-    sentry_mmap_t without_id_map;
     sentry_path_t *without_id_path
         = sentry__path_join_str(dir, "../fixtures/without-buildid.so");
-    TEST_CHECK(sentry__mmap_file(&without_id_map, without_id_path->path));
+    *buf = sentry__path_read_to_buffer(without_id_path, file_size);
     sentry__path_free(without_id_path);
 
-    TEST_CHECK(
-        sentry__procmaps_read_ids_from_elf(without_id_val, without_id_map.ptr));
-    sentry__mmap_close(&without_id_map);
+    TEST_CHECK(sentry__procmaps_read_ids_from_elf(without_id_val, &module));
+    sentry_free(*buf);
 
     TEST_CHECK(sentry_value_is_null(
         sentry_value_get_by_key(without_id_val, "code_id")));
@@ -155,15 +190,13 @@ SENTRY_TEST(buildid_fallback)
     sentry_value_decref(without_id_val);
 
     sentry_value_t x86_lib_val = sentry_value_new_object();
-    sentry_mmap_t x86_lib_map;
     sentry_path_t *x86_lib_path
         = sentry__path_join_str(dir, "../fixtures/libstdc++.so");
-    TEST_CHECK(sentry__mmap_file(&x86_lib_map, x86_lib_path->path));
+    *buf = sentry__path_read_to_buffer(x86_lib_path, file_size);
     sentry__path_free(x86_lib_path);
 
-    TEST_CHECK(
-        sentry__procmaps_read_ids_from_elf(x86_lib_val, x86_lib_map.ptr));
-    sentry__mmap_close(&x86_lib_map);
+    TEST_CHECK(sentry__procmaps_read_ids_from_elf(x86_lib_val, &module));
+    sentry_free(*buf);
 
     TEST_CHECK(
         sentry_value_is_null(sentry_value_get_by_key(x86_lib_val, "code_id")));

tests/unit/tests.inc

@@ -18,6 +18,7 @@ XX(invalid_dsn)
 XX(invalid_proxy)
 XX(iso_time)
 XX(lazy_attachments)
+XX(module_addr)
 XX(module_finder)
 XX(mpack_newlines)
 XX(mpack_removed_tags)

tests/valgrind.txt

@@ -0,0 +1,29 @@
+{
+    Checking the ELF Header of readable mapped memory
+    Memcheck:Addr1
+    fun:is_valid_elf_header
+}
+{
+    Reading Debug-Ids from readable mapped memory
+    Memcheck:Addr1
+    ...
+    fun:sentry__procmaps_read_ids_from_elf
+}
+{
+    Reading Debug-Ids from readable mapped memory
+    Memcheck:Addr2
+    ...
+    fun:sentry__procmaps_read_ids_from_elf
+}
+{
+    Reading Debug-Ids from readable mapped memory
+    Memcheck:Addr4
+    ...
+    fun:sentry__procmaps_read_ids_from_elf
+}
+{
+    Reading Debug-Ids from readable mapped memory
+    Memcheck:Addr8
+    ...
+    fun:sentry__procmaps_read_ids_from_elf
+}