From d7019a85ffb7518d84f20723d61f3c3b240908d2 Mon Sep 17 00:00:00 2001 From: Luca Guerra Date: Fri, 10 Jan 2025 11:34:59 +0000 Subject: [PATCH 1/6] fix(libsinsp): make proc.pX behave like proc.aX[1] Signed-off-by: Luca Guerra --- .../libsinsp/sinsp_filtercheck_thread.cpp | 246 ++++-------------- userspace/libsinsp/threadinfo.cpp | 17 ++ userspace/libsinsp/threadinfo.h | 12 +- 3 files changed, 82 insertions(+), 193 deletions(-) diff --git a/userspace/libsinsp/sinsp_filtercheck_thread.cpp b/userspace/libsinsp/sinsp_filtercheck_thread.cpp index a634c4361b..4144f16940 100644 --- a/userspace/libsinsp/sinsp_filtercheck_thread.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_thread.cpp @@ -1264,89 +1264,48 @@ uint8_t* sinsp_filter_check_thread::extract_single(sinsp_evt* evt, return NULL; } } - case TYPE_PPID: - if(tinfo->is_main_thread()) { - if(!should_extract_xid(tinfo->m_ptid)) { - return NULL; - } - RETURN_EXTRACT_VAR(tinfo->m_ptid); - } else { - sinsp_threadinfo* mt = tinfo->get_main_thread(); + case TYPE_PPID: { + sinsp_threadinfo* mt = tinfo->get_ancestor_process(); + if(!mt) { + return NULL; + } - if(mt != NULL) { - if(!should_extract_xid(mt->m_ptid)) { - return NULL; - } - RETURN_EXTRACT_VAR(mt->m_ptid); - } else { - return NULL; - } + if(!should_extract_xid(mt->m_pid)) { + return NULL; } + RETURN_EXTRACT_VAR(mt->m_pid); + } case TYPE_PNAME: { - sinsp_threadinfo* ptinfo = m_inspector->get_thread_ref(tinfo->m_ptid, false, true).get(); - - if(ptinfo != NULL) { - m_tstr = ptinfo->get_comm(); - RETURN_EXTRACT_STRING(m_tstr); - } else { + sinsp_threadinfo* ptinfo = tinfo->get_ancestor_process(); + if(!ptinfo) { return NULL; } + + m_tstr = ptinfo->get_comm(); + RETURN_EXTRACT_STRING(m_tstr); } case TYPE_PCMDLINE: { - sinsp_threadinfo* ptinfo = m_inspector->get_thread_ref(tinfo->m_ptid, false, true).get(); - - if(ptinfo != NULL) { - sinsp_threadinfo::populate_cmdline(m_tstr, ptinfo); - RETURN_EXTRACT_STRING(m_tstr); - } else { + sinsp_threadinfo* ptinfo = tinfo->get_ancestor_process(); + if(!ptinfo) { return NULL; } + + sinsp_threadinfo::populate_cmdline(m_tstr, ptinfo); + RETURN_EXTRACT_STRING(m_tstr); } case TYPE_ACMDLINE: { - sinsp_threadinfo* mt = NULL; - - if(tinfo->is_main_thread()) { - mt = tinfo; - } else { - mt = tinfo->get_main_thread(); - - if(mt == NULL) { - return NULL; - } + sinsp_threadinfo* mt = tinfo->get_ancestor_process(m_argid); + if(!mt) { + return NULL; } - for(int32_t j = 0; j < m_argid; j++) { - mt = mt->get_parent_thread(); - - if(mt == NULL) { - return NULL; - } - } sinsp_threadinfo::populate_cmdline(m_tstr, mt); RETURN_EXTRACT_STRING(m_tstr); } case TYPE_APID: { - sinsp_threadinfo* mt = NULL; - - if(tinfo->is_main_thread()) { - mt = tinfo; - } else { - mt = tinfo->get_main_thread(); - - if(mt == NULL) { - return NULL; - } - } - - // - // Search for a specific ancestors - // - for(int32_t j = 0; j < m_argid; j++) { - mt = mt->get_parent_thread(); - - if(mt == NULL) { - return NULL; - } + sinsp_threadinfo* mt = tinfo->get_ancestor_process(m_argid); + if(!mt) { + return NULL; } if(!should_extract_xid(mt->m_pid)) { @@ -1355,92 +1314,45 @@ uint8_t* sinsp_filter_check_thread::extract_single(sinsp_evt* evt, RETURN_EXTRACT_VAR(mt->m_pid); } case TYPE_ANAME: { - sinsp_threadinfo* mt = NULL; - - if(tinfo->is_main_thread()) { - mt = tinfo; - } else { - mt = tinfo->get_main_thread(); - - if(mt == NULL) { - return NULL; - } - } - - for(int32_t j = 0; j < m_argid; j++) { - mt = mt->get_parent_thread(); - - if(mt == NULL) { - return NULL; - } + sinsp_threadinfo* mt = tinfo->get_ancestor_process(m_argid); + if(!mt) { + return NULL; } m_tstr = mt->get_comm(); RETURN_EXTRACT_STRING(m_tstr); } case TYPE_PEXE: { - sinsp_threadinfo* ptinfo = m_inspector->get_thread_ref(tinfo->m_ptid, false, true).get(); - - if(ptinfo != NULL) { - m_tstr = ptinfo->get_exe(); - RETURN_EXTRACT_STRING(m_tstr); - } else { + sinsp_threadinfo* ptinfo = tinfo->get_ancestor_process(); + if(!ptinfo) { return NULL; } + + m_tstr = ptinfo->get_exe(); + RETURN_EXTRACT_STRING(m_tstr); } case TYPE_AEXE: { - sinsp_threadinfo* mt = NULL; - - if(tinfo->is_main_thread()) { - mt = tinfo; - } else { - mt = tinfo->get_main_thread(); - - if(mt == NULL) { - return NULL; - } - } - - for(int32_t j = 0; j < m_argid; j++) { - mt = mt->get_parent_thread(); - - if(mt == NULL) { - return NULL; - } + sinsp_threadinfo* mt = tinfo->get_ancestor_process(m_argid); + if(!mt) { + return NULL; } m_tstr = mt->get_exe(); RETURN_EXTRACT_STRING(m_tstr); } case TYPE_PEXEPATH: { - sinsp_threadinfo* ptinfo = m_inspector->get_thread_ref(tinfo->m_ptid, false, true).get(); - - if(ptinfo != NULL) { - m_tstr = ptinfo->get_exepath(); - RETURN_EXTRACT_STRING(m_tstr); - } else { + sinsp_threadinfo* ptinfo = tinfo->get_ancestor_process(); + if(!ptinfo) { return NULL; } + + m_tstr = ptinfo->get_exepath(); + RETURN_EXTRACT_STRING(m_tstr); } case TYPE_AEXEPATH: { - sinsp_threadinfo* mt = NULL; - - if(tinfo->is_main_thread()) { - mt = tinfo; - } else { - mt = tinfo->get_main_thread(); - - if(mt == NULL) { - return NULL; - } - } - - for(int32_t j = 0; j < m_argid; j++) { - mt = mt->get_parent_thread(); - - if(mt == NULL) { - return NULL; - } + sinsp_threadinfo* mt = tinfo->get_ancestor_process(m_argid); + if(!mt) { + return NULL; } m_tstr = mt->get_exepath(); @@ -1478,7 +1390,7 @@ uint8_t* sinsp_filter_check_thread::extract_single(sinsp_evt* evt, RETURN_EXTRACT_PTR(res); } - case TYPE_DURATION: + case TYPE_DURATION: { if(tinfo->m_clone_ts != 0) { m_val.s64 = evt->get_ts() - tinfo->m_clone_ts; ASSERT(m_val.s64 > 0); @@ -1486,18 +1398,18 @@ uint8_t* sinsp_filter_check_thread::extract_single(sinsp_evt* evt, } else { return NULL; } + } case TYPE_PPID_DURATION: { - sinsp_threadinfo* ptinfo = m_inspector->get_thread_ref(tinfo->m_ptid, false, true).get(); - - if(ptinfo != NULL) { - if(ptinfo->m_clone_ts != 0) { - m_val.s64 = evt->get_ts() - ptinfo->m_clone_ts; - ASSERT(m_val.s64 > 0); - RETURN_EXTRACT_VAR(m_val.s64); - } - } else { + sinsp_threadinfo* ptinfo = tinfo->get_ancestor_process(); + if(!ptinfo) { return NULL; } + + if(ptinfo->m_clone_ts != 0) { + m_val.s64 = evt->get_ts() - ptinfo->m_clone_ts; + ASSERT(m_val.s64 > 0); + RETURN_EXTRACT_VAR(m_val.s64); + } } case TYPE_FDOPENCOUNT: m_val.u64 = tinfo->get_fd_opencount(); @@ -1596,56 +1508,6 @@ uint8_t* sinsp_filter_check_thread::extract_single(sinsp_evt* evt, m_val.u64 = tinfo->m_vpid; RETURN_EXTRACT_VAR(m_val.u64); - /* - case TYPE_PROC_CPU: - { - uint16_t etype = evt->get_type(); - - if(etype == PPME_PROCINFO_E) - { - double thval; - uint64_t tcpu; - - sinsp_evt_param* parinfo = evt->get_param(0); - tcpu = *(uint64_t*)parinfo->m_val; - - parinfo = evt->get_param(1); - tcpu += *(uint64_t*)parinfo->m_val; - - if(tinfo->m_last_t_tot_cpu != 0) - { - uint64_t deltaval = tcpu - tinfo->m_last_t_tot_cpu; - thval = (double)deltaval;// / (ONE_SECOND_IN_NS / 100); - if(thval > 100) - { - thval = 100; - } - } - else - { - thval = 0; - } - - tinfo->m_last_t_tot_cpu = tcpu; - - uint64_t ets = evt->get_ts(); - sinsp_threadinfo* mt = tinfo->get_main_thread(); - - if(ets != mt->m_last_mt_cpu_ts) - { - mt->m_last_mt_tot_cpu = 0; - mt->m_last_mt_cpu_ts = ets; - } - - mt->m_last_mt_tot_cpu += thval; - m_val.d = mt->m_last_mt_tot_cpu; - - RETURN_EXTRACT_VAR(m_val.d); - } - - return NULL; - } - */ case TYPE_THREAD_CPU: { return extract_thread_cpu(evt, len, tinfo, true, true); } diff --git a/userspace/libsinsp/threadinfo.cpp b/userspace/libsinsp/threadinfo.cpp index 4011baf1be..1f03279a40 100644 --- a/userspace/libsinsp/threadinfo.cpp +++ b/userspace/libsinsp/threadinfo.cpp @@ -772,6 +772,23 @@ sinsp_threadinfo* sinsp_threadinfo::get_parent_thread() { return m_inspector->get_thread_ref(m_ptid, false).get(); } +sinsp_threadinfo* sinsp_threadinfo::get_ancestor_process(uint32_t n) { + sinsp_threadinfo* mt = get_main_thread(); + + for(uint32_t i = 0; i < n; i++) { + if(mt == nullptr) { + return nullptr; + } + mt = mt->get_parent_thread(); + if(mt == nullptr) { + return nullptr; + } + mt = mt->get_main_thread(); + } + + return mt; +} + sinsp_fdinfo* sinsp_threadinfo::add_fd(int64_t fd, std::unique_ptr fdinfo) { sinsp_fdtable* fd_table_ptr = get_fd_table(); if(fd_table_ptr == NULL) { diff --git a/userspace/libsinsp/threadinfo.h b/userspace/libsinsp/threadinfo.h index aed22fff92..985cb99f1d 100644 --- a/userspace/libsinsp/threadinfo.h +++ b/userspace/libsinsp/threadinfo.h @@ -235,10 +235,20 @@ class SINSP_PUBLIC sinsp_threadinfo : public libsinsp::state::table_entry { } /*! - \brief Get the process that launched this thread's process. + \brief Get the thread that launched this thread's process. */ sinsp_threadinfo* get_parent_thread(); + /*! + \brief Get the process that launched this thread's process (its parent) or any of its + ancestors. + + \param n when 1 it will look for the parent process, when 2 the grandparent and so forth. + + \return Pointer to the threadinfo or NULL if it doesn't exist + */ + sinsp_threadinfo* get_ancestor_process(uint32_t n = 1); + /*! \brief Retrieve information about one of this thread/process FDs. From 2a7b31cb4d65b1296d7ce1a75c0c39dd564bab37 Mon Sep 17 00:00:00 2001 From: Luca Guerra Date: Fri, 10 Jan 2025 16:57:18 +0000 Subject: [PATCH 2/6] fix(libsinsp/tests): remove incorrect tests Signed-off-by: Luca Guerra --- userspace/libsinsp/test/events_proc.ut.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/userspace/libsinsp/test/events_proc.ut.cpp b/userspace/libsinsp/test/events_proc.ut.cpp index e2f7588411..f4fa3e6a9d 100644 --- a/userspace/libsinsp/test/events_proc.ut.cpp +++ b/userspace/libsinsp/test/events_proc.ut.cpp @@ -719,23 +719,19 @@ TEST_F(sinsp_with_test_input, spawn_process) { // check that the name is updated ASSERT_EQ(get_field_as_string(evt, "proc.name"), "test-exe"); ASSERT_EQ(get_field_as_string(evt, "proc.aname[0]"), "test-exe"); - ASSERT_EQ(get_field_as_string(evt, "proc.aname"), "test-exe"); // check that the pid is updated ASSERT_EQ(get_field_as_string(evt, "proc.pid"), "20"); ASSERT_EQ(get_field_as_string(evt, "proc.vpid"), "20"); ASSERT_EQ(get_field_as_string(evt, "proc.apid[0]"), "20"); - ASSERT_EQ(get_field_as_string(evt, "proc.apid"), "20"); // check that the exe is updated (first arg given in this test setup is same as full exepath) ASSERT_EQ(get_field_as_string(evt, "proc.exe"), "/bin/test-exe"); ASSERT_EQ(get_field_as_string(evt, "proc.aexe[0]"), "/bin/test-exe"); - ASSERT_EQ(get_field_as_string(evt, "proc.aexe"), "/bin/test-exe"); // check that the exepath is updated ASSERT_EQ(get_field_as_string(evt, "proc.exepath"), "/bin/test-exe"); ASSERT_EQ(get_field_as_string(evt, "proc.aexepath[0]"), "/bin/test-exe"); - ASSERT_EQ(get_field_as_string(evt, "proc.aexepath"), "/bin/test-exe"); // check session leader (sid) related fields ASSERT_EQ(get_field_as_string(evt, "proc.sid"), "0"); @@ -774,7 +770,6 @@ TEST_F(sinsp_with_test_input, spawn_process) { ASSERT_EQ(get_field_as_string(evt, "proc.pcmdline"), "init"); ASSERT_EQ(get_field_as_string(evt, "proc.acmdline[0]"), "test-exe -c 'echo aGVsbG8K | base64 -d'"); - ASSERT_EQ(get_field_as_string(evt, "proc.acmdline"), "test-exe -c 'echo aGVsbG8K | base64 -d'"); ASSERT_EQ(get_field_as_string(evt, "proc.acmdline[1]"), "init"); ASSERT_FALSE(field_has_value(evt, "proc.acmdline[2]")); From 8db1e64718835f39cdab3e2129dd405b65a7360a Mon Sep 17 00:00:00 2001 From: Luca Guerra Date: Fri, 10 Jan 2025 16:57:34 +0000 Subject: [PATCH 3/6] fix(build): update build flags Signed-off-by: Luca Guerra --- cmake/modules/CompilerFlags.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/modules/CompilerFlags.cmake b/cmake/modules/CompilerFlags.cmake index 2da4999f58..7ee8719b94 100644 --- a/cmake/modules/CompilerFlags.cmake +++ b/cmake/modules/CompilerFlags.cmake @@ -45,7 +45,7 @@ if(NOT MSVC) if(BUILD_WARNINGS_AS_ERRORS) set(CMAKE_COMPILE_WARNING_AS_ERROR ON) set(CMAKE_SUPPRESSED_WARNINGS - "-Wno-unused-parameter -Wno-sign-compare -Wno-implicit-fallthrough -Wno-format-truncation" + "-Wno-unused-parameter -Wno-sign-compare -Wno-implicit-fallthrough -Wno-format-truncation -Wno-deprecated-declarations" ) if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") # Clang needs these for suppressing these warnings: - C++20 array designators used with From 41de1368a49e4121191c91b41854dc355c3af047 Mon Sep 17 00:00:00 2001 From: Luca Guerra Date: Fri, 10 Jan 2025 17:08:14 +0000 Subject: [PATCH 4/6] fix(libsinsp): update field descriptions Signed-off-by: Luca Guerra --- userspace/libsinsp/sinsp_filtercheck_thread.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/userspace/libsinsp/sinsp_filtercheck_thread.cpp b/userspace/libsinsp/sinsp_filtercheck_thread.cpp index 4144f16940..e3c7f79432 100644 --- a/userspace/libsinsp/sinsp_filtercheck_thread.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_thread.cpp @@ -120,7 +120,7 @@ static const filtercheck_field_info sinsp_filter_check_thread_fields[] = { PF_NA, "proc.pname", "Parent Name", - "The proc.name truncated after 16 characters) of the process generating the event."}, + "The proc.name (truncated after 16 characters) of the parent process."}, {PT_CHARBUF, EPF_ARG_ALLOWED | EPF_NO_RHS | EPF_NO_TRANSFORMER, PF_NA, @@ -153,8 +153,7 @@ static const filtercheck_field_info sinsp_filter_check_thread_fields[] = { PF_NA, "proc.pcmdline", "Parent Command Line", - "The proc.cmdline (full command line (proc.name + proc.args)) of the parent of the " - "process generating the event."}, + "The proc.cmdline (full command line (proc.name + proc.args)) of the parent process."}, {PT_CHARBUF, EPF_ARG_ALLOWED | EPF_NO_RHS | EPF_NO_TRANSFORMER, PF_NA, From d6ffd396066b7961637ad6af4c744b770522e150 Mon Sep 17 00:00:00 2001 From: Lorenzo Susini Date: Mon, 13 Jan 2025 11:45:14 +0000 Subject: [PATCH 5/6] test(userspace/libsinsp): test new threadinfo api get_ancestor_process Signed-off-by: Lorenzo Susini --- userspace/libsinsp/test/events_proc.ut.cpp | 1 + userspace/libsinsp/test/thread_table.ut.cpp | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/userspace/libsinsp/test/events_proc.ut.cpp b/userspace/libsinsp/test/events_proc.ut.cpp index f4fa3e6a9d..fd3df39ec9 100644 --- a/userspace/libsinsp/test/events_proc.ut.cpp +++ b/userspace/libsinsp/test/events_proc.ut.cpp @@ -19,6 +19,7 @@ limitations under the License. #include #include +#include #include "test_utils.h" /* Assert if the thread `exepath` is set to the right value diff --git a/userspace/libsinsp/test/thread_table.ut.cpp b/userspace/libsinsp/test/thread_table.ut.cpp index 810e8a4889..e1f3cbba22 100644 --- a/userspace/libsinsp/test/thread_table.ut.cpp +++ b/userspace/libsinsp/test/thread_table.ut.cpp @@ -617,3 +617,20 @@ TEST_F(sinsp_with_test_input, THRD_TABLE_add_and_remove_many_children) { /* Only init process */ ASSERT_EQ(m_inspector.m_thread_manager->get_thread_count(), 1); } + +TEST_F(sinsp_with_test_input, THRD_TABLE_proc_apid_ppid) { + DEFAULT_TREE + + // Thread p5_t1_ptid is generated by p4_t2_tid, which is a thread + // Retrieve it to check apid and ppid + auto p5_t1_info = m_inspector.get_thread_ref(p5_t1_tid, false).get(); + ASSERT_EQ(p5_t1_info->m_ptid, p4_t2_tid); + + auto p4_t1_info = m_inspector.get_thread_ref(p4_t1_tid, false).get(); + auto p5_t1_parent_info = p5_t1_info->get_ancestor_process(); + ASSERT_EQ(p4_t1_info->m_pid, p5_t1_parent_info->m_pid); + + auto p3_t1_info = m_inspector.get_thread_ref(p3_t1_tid, false).get(); + auto p5_t1_gparent_info = p5_t1_info->get_ancestor_process(2); + ASSERT_EQ(p3_t1_info->m_pid, p5_t1_gparent_info->m_pid); +} From a143296e5fef957cec196512816f66c9fc5d83d9 Mon Sep 17 00:00:00 2001 From: Lorenzo Susini Date: Mon, 13 Jan 2025 12:14:12 +0000 Subject: [PATCH 6/6] test(userspace/libsinsp): test proc.ppid and proc.apid[1] when parent is thread Signed-off-by: Lorenzo Susini --- userspace/libsinsp/test/events_proc.ut.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/userspace/libsinsp/test/events_proc.ut.cpp b/userspace/libsinsp/test/events_proc.ut.cpp index fd3df39ec9..0b8696bb35 100644 --- a/userspace/libsinsp/test/events_proc.ut.cpp +++ b/userspace/libsinsp/test/events_proc.ut.cpp @@ -1352,3 +1352,20 @@ TEST_F(sinsp_with_test_input, last_exec_ts) { // Check we execed after the last clone ASSERT_GT(evt->get_thread_info()->m_lastexec_ts, evt->get_thread_info()->m_clone_ts); } + +TEST_F(sinsp_with_test_input, proc_ppid_apid) { + /* Instantiate the default tree */ + DEFAULT_TREE + + /* Create a child for `p2_t3` */ + int64_t p7_t1_tid = 100; + [[maybe_unused]] int64_t p7_t1_pid = 100; + [[maybe_unused]] int64_t p7_t1_ptid = p2_t3_tid; + + auto evt = generate_clone_x_event(p7_t1_tid, p2_t3_tid, p2_t3_pid, p2_t3_ptid); + ASSERT_THREAD_CHILDREN(p2_t3_tid, 1, 1, p7_t1_tid); + + /* Check that proc.ppid and proc.apid[1] are the same and that this holds even in the case + a thread performed a clone */ + ASSERT_EQ(get_field_as_string(evt, "proc.ppid"), get_field_as_string(evt, "proc.apid[1]")); +}