From 14139232d53c33679296f14c63ad86aa83b29977 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Thu, 25 Feb 2021 13:43:09 -0800 Subject: [PATCH] capture: rename PidRoot to Overlay and add src/dst Really what we want to deal with here is tracking an overlay that we may need to be able to decode after the fact (in case processes exit or we need to do post-processing symbol resolution). For the podman case, that is $some_path mapped to root (/), generally speaking. For flatpak though, that would have two mappings, one for /app and another for /usr (possibly more). --- .../sysprof-capture-cursor.c | 4 +- .../sysprof-capture-reader.c | 34 ++++++++---- .../sysprof-capture-reader.h | 2 +- .../sysprof-capture-types.h | 13 +++-- .../sysprof-capture-writer-cat.c | 27 ++++++---- .../sysprof-capture-writer.c | 44 +++++++++------ .../sysprof-capture-writer.h | 7 +-- src/libsysprof/sysprof-elf-symbol-resolver.c | 14 ++--- src/libsysprof/sysprof-proc-source.c | 9 ++-- src/tests/test-capture.c | 53 +++++++++++++++++++ src/tools/sysprof-dump.c | 8 +-- 11 files changed, 153 insertions(+), 62 deletions(-) diff --git a/src/libsysprof-capture/sysprof-capture-cursor.c b/src/libsysprof-capture/sysprof-capture-cursor.c index eab3acae..7697ad48 100644 --- a/src/libsysprof-capture/sysprof-capture-cursor.c +++ b/src/libsysprof-capture/sysprof-capture-cursor.c @@ -223,8 +223,8 @@ sysprof_capture_cursor_foreach (SysprofCaptureCursor *self, delegate = READ_DELEGATE (sysprof_capture_reader_read_allocation); break; - case SYSPROF_CAPTURE_FRAME_PID_ROOT: - delegate = READ_DELEGATE (sysprof_capture_reader_read_pid_root); + case SYSPROF_CAPTURE_FRAME_OVERLAY: + delegate = READ_DELEGATE (sysprof_capture_reader_read_overlay); break; default: diff --git a/src/libsysprof-capture/sysprof-capture-reader.c b/src/libsysprof-capture/sysprof-capture-reader.c index 5ba52730..8f5e02e9 100644 --- a/src/libsysprof-capture/sysprof-capture-reader.c +++ b/src/libsysprof-capture/sysprof-capture-reader.c @@ -354,14 +354,18 @@ sysprof_capture_reader_bswap_mark (SysprofCaptureReader *self, } static inline void -sysprof_capture_reader_bswap_pid_root (SysprofCaptureReader *self, - SysprofCapturePidRoot *pr) +sysprof_capture_reader_bswap_overlay (SysprofCaptureReader *self, + SysprofCaptureOverlay *pr) { assert (self != NULL); assert (pr != NULL); if (SYSPROF_UNLIKELY (self->endian != __BYTE_ORDER)) - pr->layer = bswap_32 (pr->layer); + { + pr->layer = bswap_32 (pr->layer); + pr->src_len = bswap_32 (pr->src_len); + pr->dst_len = bswap_32 (pr->dst_len); + } } static inline void @@ -690,10 +694,10 @@ sysprof_capture_reader_read_mark (SysprofCaptureReader *self) return mark; } -const SysprofCapturePidRoot * -sysprof_capture_reader_read_pid_root (SysprofCaptureReader *self) +const SysprofCaptureOverlay * +sysprof_capture_reader_read_overlay (SysprofCaptureReader *self) { - SysprofCapturePidRoot *pr; + SysprofCaptureOverlay *pr; assert (self != NULL); assert ((self->pos % SYSPROF_CAPTURE_ALIGN) == 0); @@ -702,22 +706,30 @@ sysprof_capture_reader_read_pid_root (SysprofCaptureReader *self) if (!sysprof_capture_reader_ensure_space_for (self, sizeof *pr + 1)) return NULL; - pr = (SysprofCapturePidRoot *)(void *)&self->buf[self->pos]; + pr = (SysprofCaptureOverlay *)(void *)&self->buf[self->pos]; sysprof_capture_reader_bswap_frame (self, &pr->frame); - if (pr->frame.type != SYSPROF_CAPTURE_FRAME_PID_ROOT) + if (pr->frame.type != SYSPROF_CAPTURE_FRAME_OVERLAY) return NULL; - if (pr->frame.len < (sizeof *pr + 1)) + if (pr->frame.len < (sizeof *pr + 2)) return NULL; if (!sysprof_capture_reader_ensure_space_for (self, pr->frame.len)) return NULL; - pr = (SysprofCapturePidRoot *)(void *)&self->buf[self->pos]; + pr = (SysprofCaptureOverlay *)(void *)&self->buf[self->pos]; - sysprof_capture_reader_bswap_pid_root (self, pr); + sysprof_capture_reader_bswap_overlay (self, pr); + + /* Make sure there is enough space for paths and trailing \0 */ + if (((size_t)pr->src_len + (size_t)pr->dst_len) > (pr->frame.len - sizeof *pr - 2)) + return NULL; + + /* Enforce trailing \0 */ + pr->data[pr->src_len] = 0; + pr->data[pr->src_len+1+pr->dst_len] = 0; self->pos += pr->frame.len; diff --git a/src/libsysprof-capture/sysprof-capture-reader.h b/src/libsysprof-capture/sysprof-capture-reader.h index 1d7b72a2..9d10d700 100644 --- a/src/libsysprof-capture/sysprof-capture-reader.h +++ b/src/libsysprof-capture/sysprof-capture-reader.h @@ -121,7 +121,7 @@ const SysprofCaptureFileChunk *sysprof_capture_reader_read_file ( SYSPROF_AVAILABLE_IN_3_36 const SysprofCaptureAllocation *sysprof_capture_reader_read_allocation (SysprofCaptureReader *self); SYSPROF_AVAILABLE_IN_3_40 -const SysprofCapturePidRoot *sysprof_capture_reader_read_pid_root (SysprofCaptureReader *self); +const SysprofCaptureOverlay *sysprof_capture_reader_read_overlay (SysprofCaptureReader *self); SYSPROF_AVAILABLE_IN_ALL bool sysprof_capture_reader_reset (SysprofCaptureReader *self); SYSPROF_AVAILABLE_IN_ALL diff --git a/src/libsysprof-capture/sysprof-capture-types.h b/src/libsysprof-capture/sysprof-capture-types.h index 1fbcbec1..20295a2e 100644 --- a/src/libsysprof-capture/sysprof-capture-types.h +++ b/src/libsysprof-capture/sysprof-capture-types.h @@ -139,7 +139,7 @@ typedef enum SYSPROF_CAPTURE_FRAME_LOG = 12, SYSPROF_CAPTURE_FRAME_FILE_CHUNK = 13, SYSPROF_CAPTURE_FRAME_ALLOCATION = 14, - SYSPROF_CAPTURE_FRAME_PID_ROOT = 15, + SYSPROF_CAPTURE_FRAME_OVERLAY = 15, } SysprofCaptureFrameType; /* Not part of ABI */ @@ -177,9 +177,12 @@ SYSPROF_ALIGNED_BEGIN(1) typedef struct { SysprofCaptureFrame frame; - uint32_t layer; - char path[0]; -} SysprofCapturePidRoot + uint32_t layer : 8; + uint32_t padding : 24; + uint32_t src_len : 16; + uint32_t dst_len : 16; + char data[0]; +} SysprofCaptureOverlay SYSPROF_ALIGNED_END(1); SYSPROF_ALIGNED_BEGIN(1) @@ -366,7 +369,7 @@ SYSPROF_STATIC_ASSERT (sizeof (SysprofCaptureMetadata) == 64, "SysprofCaptureMet SYSPROF_STATIC_ASSERT (sizeof (SysprofCaptureLog) == 64, "SysprofCaptureLog changed size"); SYSPROF_STATIC_ASSERT (sizeof (SysprofCaptureFileChunk) == 284, "SysprofCaptureFileChunk changed size"); SYSPROF_STATIC_ASSERT (sizeof (SysprofCaptureAllocation) == 48, "SysprofCaptureAllocation changed size"); -SYSPROF_STATIC_ASSERT (sizeof (SysprofCapturePidRoot) == 28, "SysprofCapturePidRoot changed size"); +SYSPROF_STATIC_ASSERT (sizeof (SysprofCaptureOverlay) == 32, "SysprofCaptureOverlay changed size"); SYSPROF_STATIC_ASSERT ((offsetof (SysprofCaptureAllocation, addrs) % SYSPROF_CAPTURE_ALIGN) == 0, "SysprofCaptureAllocation.addrs is not aligned"); SYSPROF_STATIC_ASSERT ((offsetof (SysprofCaptureSample, addrs) % SYSPROF_CAPTURE_ALIGN) == 0, "SysprofCaptureSample.addrs is not aligned"); diff --git a/src/libsysprof-capture/sysprof-capture-writer-cat.c b/src/libsysprof-capture/sysprof-capture-writer-cat.c index 0cb69df5..35de0620 100644 --- a/src/libsysprof-capture/sysprof-capture-writer-cat.c +++ b/src/libsysprof-capture/sysprof-capture-writer-cat.c @@ -532,19 +532,28 @@ sysprof_capture_writer_cat (SysprofCaptureWriter *self, break; } - case SYSPROF_CAPTURE_FRAME_PID_ROOT: + case SYSPROF_CAPTURE_FRAME_OVERLAY: { - const SysprofCapturePidRoot *frame; + const SysprofCaptureOverlay *frame; + const char *src; + const char *dst; - if (!(frame = sysprof_capture_reader_read_pid_root (reader))) + if (!(frame = sysprof_capture_reader_read_overlay (reader))) goto panic; - sysprof_capture_writer_add_pid_root (self, - frame->frame.time, - frame->frame.cpu, - frame->frame.pid, - frame->path, - frame->layer); + /* This should have been verified alrady when decoding */ + assert (frame->frame.len >= (sizeof *frame + frame->src_len + 1 + frame->dst_len + 1)); + + src = &frame->data[0]; + dst = &frame->data[frame->src_len+1]; + + sysprof_capture_writer_add_overlay (self, + frame->frame.time, + frame->frame.cpu, + frame->frame.pid, + frame->layer, + src, + dst); break; } diff --git a/src/libsysprof-capture/sysprof-capture-writer.c b/src/libsysprof-capture/sysprof-capture-writer.c index 5a09b6a9..d05981c2 100644 --- a/src/libsysprof-capture/sysprof-capture-writer.c +++ b/src/libsysprof-capture/sysprof-capture-writer.c @@ -828,20 +828,27 @@ sysprof_capture_writer_add_fork (SysprofCaptureWriter *self, } bool -sysprof_capture_writer_add_pid_root (SysprofCaptureWriter *self, - int64_t time, - int cpu, - int32_t pid, - const char *path, - uint32_t layer) +sysprof_capture_writer_add_overlay (SysprofCaptureWriter *self, + int64_t time, + int cpu, + int32_t pid, + uint32_t layer, + const char *src, + const char *dst) { - SysprofCapturePidRoot *ev; - size_t strl = strlen (path); - size_t len = sizeof *ev + strl + 1; + SysprofCaptureOverlay *ev; + size_t srclen = strlen (src); + size_t dstlen = strlen (dst); + size_t len = sizeof *ev + srclen + 1 + dstlen + 1; assert (self != NULL); + assert (src != NULL); + assert (dst != NULL); - ev = (SysprofCapturePidRoot *)sysprof_capture_writer_allocate (self, &len); + if (srclen > INT16_MAX || dstlen > INT16_MAX) + return false; + + ev = (SysprofCaptureOverlay *)sysprof_capture_writer_allocate (self, &len); if (!ev) return false; @@ -850,12 +857,19 @@ sysprof_capture_writer_add_pid_root (SysprofCaptureWriter *self, cpu, pid, time, - SYSPROF_CAPTURE_FRAME_PID_ROOT); - ev->layer = layer; - memcpy (ev->path, path, strl); - ev->path[strl] = 0; + SYSPROF_CAPTURE_FRAME_OVERLAY); - self->stat.frame_count[SYSPROF_CAPTURE_FRAME_PID_ROOT]++; + ev->layer = layer; + ev->src_len = srclen; + ev->dst_len = dstlen; + + memcpy (&ev->data[0], src, srclen); + memcpy (&ev->data[srclen+1], dst, dstlen); + + ev->data[srclen] = 0; + ev->data[srclen+1+dstlen] = 0; + + self->stat.frame_count[SYSPROF_CAPTURE_FRAME_OVERLAY]++; return true; } diff --git a/src/libsysprof-capture/sysprof-capture-writer.h b/src/libsysprof-capture/sysprof-capture-writer.h index 2432aee2..4893c226 100644 --- a/src/libsysprof-capture/sysprof-capture-writer.h +++ b/src/libsysprof-capture/sysprof-capture-writer.h @@ -202,12 +202,13 @@ bool sysprof_capture_writer_add_allocation_copy (Sy const SysprofCaptureAddress *addrs, unsigned int n_addrs); SYSPROF_AVAILABLE_IN_3_40 -bool sysprof_capture_writer_add_pid_root (SysprofCaptureWriter *self, +bool sysprof_capture_writer_add_overlay (SysprofCaptureWriter *self, int64_t time, int cpu, int32_t pid, - const char *path, - uint32_t layer); + uint32_t layer, + const char *src, + const char *dst); SYSPROF_AVAILABLE_IN_ALL bool sysprof_capture_writer_flush (SysprofCaptureWriter *self); SYSPROF_AVAILABLE_IN_ALL diff --git a/src/libsysprof/sysprof-elf-symbol-resolver.c b/src/libsysprof/sysprof-elf-symbol-resolver.c index 4cbc8d58..e2ae1eba 100644 --- a/src/libsysprof/sysprof-elf-symbol-resolver.c +++ b/src/libsysprof/sysprof-elf-symbol-resolver.c @@ -162,10 +162,12 @@ sysprof_elf_symbol_resolver_load (SysprofSymbolResolver *resolver, sysprof_map_lookaside_insert (lookaside, &map); } - else if (type == SYSPROF_CAPTURE_FRAME_PID_ROOT) + else if (type == SYSPROF_CAPTURE_FRAME_OVERLAY) { - const SysprofCapturePidRoot *ev = sysprof_capture_reader_read_pid_root (reader); + const SysprofCaptureOverlay *ev = sysprof_capture_reader_read_overlay (reader); SysprofMapLookaside *lookaside = g_hash_table_lookup (self->lookasides, GINT_TO_POINTER (ev->frame.pid)); + const char *src = ev->data; + const char *dst = &ev->data[ev->src_len+1]; if (lookaside == NULL) { @@ -173,11 +175,9 @@ sysprof_elf_symbol_resolver_load (SysprofSymbolResolver *resolver, g_hash_table_insert (self->lookasides, GINT_TO_POINTER (ev->frame.pid), lookaside); } - /* Someday we might need to support more layers, but currently - * only the base layer (0) is used. - */ - if (ev->layer == 0) - sysprof_map_lookaside_set_root (lookaside, ev->path); + /* FIXME: use dst to map to things other than / */ + if (ev->dst_len == 1 && *dst == '/') + sysprof_map_lookaside_set_root (lookaside, src); } else { diff --git a/src/libsysprof/sysprof-proc-source.c b/src/libsysprof/sysprof-proc-source.c index 1a3b2125..2b8f790f 100644 --- a/src/libsysprof/sysprof-proc-source.c +++ b/src/libsysprof/sysprof-proc-source.c @@ -259,12 +259,9 @@ sysprof_proc_source_populate_pid_podman (SysprofProcSource *self, info->layers[i], "diff", NULL); - sysprof_capture_writer_add_pid_root (self->writer, - SYSPROF_CAPTURE_CURRENT_TIME, - -1, - pid, - path, - i); + sysprof_capture_writer_add_overlay (self->writer, + SYSPROF_CAPTURE_CURRENT_TIME, + -1, pid, i, path, "/"); } } } diff --git a/src/tests/test-capture.c b/src/tests/test-capture.c index 762d2ca9..c06ad209 100644 --- a/src/tests/test-capture.c +++ b/src/tests/test-capture.c @@ -947,6 +947,58 @@ test_writer_memory_alloc_free (void) g_unlink ("memory.syscap"); } +static void +test_reader_writer_overlay (void) +{ + SysprofCaptureWriter *writer; + SysprofCaptureReader *reader; + const SysprofCaptureOverlay *ev; + SysprofCaptureFrameType type; + gint r; + + writer = sysprof_capture_writer_new ("overlay1.syscap", 0); + + sysprof_capture_writer_add_overlay (writer, SYSPROF_CAPTURE_CURRENT_TIME, -1, -1, 123, "/foo", "/bar"); + sysprof_capture_writer_add_overlay (writer, SYSPROF_CAPTURE_CURRENT_TIME, -1, -1, 0, "/app", "/bin"); + sysprof_capture_writer_add_overlay (writer, SYSPROF_CAPTURE_CURRENT_TIME, -1, -1, 7, "/home/user/.local/share/containers/storage/overlay/1111111111111111111111111111111111111111111111111111111111111111/diff", "/"); + + g_clear_pointer (&writer, sysprof_capture_writer_unref); + + reader = sysprof_capture_reader_new ("overlay1.syscap"); + g_assert_nonnull (reader); + + ev = sysprof_capture_reader_read_overlay (reader); + g_assert_nonnull (ev); + g_assert_cmpint (ev->layer, ==, 123); + g_assert_cmpint (ev->src_len, ==, 4); + g_assert_cmpint (ev->dst_len, ==, 4); + g_assert_cmpstr (ev->data, ==, "/foo"); + g_assert_cmpstr (ev->data+ev->src_len+1, ==, "/bar"); + + ev = sysprof_capture_reader_read_overlay (reader); + g_assert_nonnull (ev); + g_assert_cmpint (ev->layer, ==, 0); + g_assert_cmpint (ev->src_len, ==, 4); + g_assert_cmpint (ev->dst_len, ==, 4); + g_assert_cmpstr (ev->data, ==, "/app"); + g_assert_cmpstr (ev->data+ev->src_len+1, ==, "/bin"); + + ev = sysprof_capture_reader_read_overlay (reader); + g_assert_nonnull (ev); + g_assert_cmpint (ev->layer, ==, 7); + g_assert_cmpint (ev->src_len, ==, 120); + g_assert_cmpint (ev->dst_len, ==, 1); + g_assert_cmpstr (ev->data, ==, "/home/user/.local/share/containers/storage/overlay/1111111111111111111111111111111111111111111111111111111111111111/diff"); + g_assert_cmpstr (ev->data+ev->src_len+1, ==, "/"); + + r = sysprof_capture_reader_peek_type (reader, &type); + g_assert_cmpint (r, ==, FALSE); + + g_clear_pointer (&reader, sysprof_capture_reader_unref); + + g_unlink ("overlay1.syscap"); +} + int main (int argc, char *argv[]) @@ -962,5 +1014,6 @@ main (int argc, g_test_add_func ("/SysprofCapture/ReaderWriter/metadata", test_reader_writer_metadata); g_test_add_func ("/SysprofCapture/ReaderWriter/file", test_reader_writer_file); g_test_add_func ("/SysprofCapture/ReaderWriter/cat-jitmap", test_reader_writer_cat_jitmap); + g_test_add_func ("/SysprofCapture/ReaderWriter/overlay", test_reader_writer_overlay); return g_test_run (); } diff --git a/src/tools/sysprof-dump.c b/src/tools/sysprof-dump.c index 4875b697..9bc7a82a 100644 --- a/src/tools/sysprof-dump.c +++ b/src/tools/sysprof-dump.c @@ -115,14 +115,16 @@ main (gint argc, break; } - case SYSPROF_CAPTURE_FRAME_PID_ROOT: + case SYSPROF_CAPTURE_FRAME_OVERLAY: { - const SysprofCapturePidRoot *pr = sysprof_capture_reader_read_pid_root (reader); + const SysprofCaptureOverlay *pr = sysprof_capture_reader_read_overlay (reader); + const char *src = pr->data; + const char *dst = &pr->data[pr->src_len+1]; if (pr == NULL) return EXIT_FAILURE; - g_print ("PID ROOT: pid=%d layer=%u path=%s\n", pr->frame.pid, pr->layer, pr->path); + g_print ("OVERLAY: pid=%d layer=%u src=%s dst=%s\n", pr->frame.pid, pr->layer, src, dst); break; }