Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update: collection of minor cleanups across the codebase #1430

Merged
merged 7 commits into from
Oct 20, 2023
3 changes: 2 additions & 1 deletion cmake/modules/libscap.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ else()
endif()

get_filename_component(LIBSCAP_INCLUDE_DIR ${LIBSCAP_DIR}/userspace/libscap ABSOLUTE)
set(LIBSCAP_INCLUDE_DIRS ${LIBSCAP_INCLUDE_DIR} ${DRIVER_CONFIG_DIR})
get_filename_component(LIBSCAP_COMMON_INCLUDE_DIR ${LIBSCAP_DIR}/userspace/common ABSOLUTE)
set(LIBSCAP_INCLUDE_DIRS ${LIBSCAP_INCLUDE_DIR} ${LIBSCAP_COMMON_INCLUDE_DIR} ${DRIVER_CONFIG_DIR})

function(set_scap_target_properties target)
set_target_properties(${target} PROPERTIES
Expand Down
8 changes: 6 additions & 2 deletions driver/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1465,7 +1465,9 @@ static inline void record_drop_e(struct ppm_consumer_t *consumer,
consumer->need_to_insert_drop_e = 1;
} else {
if (consumer->need_to_insert_drop_e == 1 && !(drop_flags & UF_ATOMIC)) {
pr_err("drop enter event delayed insert\n");
if (verbose) {
pr_err("consumer:%p drop enter event delayed insert\n", consumer->consumer_id);
}
}

consumer->need_to_insert_drop_e = 0;
Expand Down Expand Up @@ -1613,7 +1615,9 @@ static inline void record_drop_x(struct ppm_consumer_t *consumer,
consumer->need_to_insert_drop_x = 1;
} else {
if (consumer->need_to_insert_drop_x == 1 && !(drop_flags & UF_ATOMIC)) {
pr_err("drop exit event delayed insert\n");
if (verbose) {
pr_err("consumer:%p drop exit event delayed insert\n", consumer->consumer_id);
}
}

consumer->need_to_insert_drop_x = 0;
Expand Down
1 change: 1 addition & 0 deletions userspace/libscap/scap.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ limitations under the License.

#include <stdio.h>

#include "compat/misc.h"
#include "scap.h"
#include "strerror.h"
#include "strl.h"
Expand Down
1 change: 1 addition & 0 deletions userspace/libsinsp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ include_directories(../common)
include_directories(${LIBSCAP_INCLUDE_DIR})
include_directories(../async)
include_directories(./include)
include_directories(../plugin)

option(USE_BUNDLED_DEPS "Enable bundled dependencies instead of using the system ones" ON)

Expand Down
8 changes: 4 additions & 4 deletions userspace/libsinsp/events/sinsp_events_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class set
}

public:
struct iterator
struct iterator
{
using iterator_category = std::forward_iterator_tag;
using difference_type = std::ptrdiff_t;
Expand All @@ -83,7 +83,7 @@ class set
}
reference operator*() { return m_val; }
pointer operator->() { return &m_val; }
iterator& operator++() { m_index++; set_val(); return *this; }
iterator& operator++() { m_index++; set_val(); return *this; }
iterator operator++(int) { iterator i = *this; ++(*this); return i; }
friend bool operator== (const iterator& a, const iterator& b)
{
Expand All @@ -110,7 +110,7 @@ class set
set(const set&) = default;
set& operator=(set&&) noexcept = default;
set& operator=(const set&) = default;
set<T>() = delete;
set() = delete;

template<typename InputIterator>
static set<T> from(InputIterator first, InputIterator last)
Expand All @@ -128,7 +128,7 @@ class set
{
return from(v.begin(), v.end());
}

template<typename Iterable>
set(const Iterable& v): set(from(v)) { }

Expand Down
4 changes: 2 additions & 2 deletions userspace/libsinsp/sinsp_filtercheck_event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,7 @@ uint8_t* sinsp_filter_check_event::extract(sinsp_evt *evt, OUT uint32_t* len, bo
uint16_t ppm_sc = *(uint16_t *)parinfo->m_val;

// Only generic enter event has the nativeID as second param
if(m_inspector->is_capture() && ppm_sc == PPM_SC_UNKNOWN && etype == PPME_GENERIC_E)
if(m_inspector && m_inspector->is_capture() && ppm_sc == PPM_SC_UNKNOWN && etype == PPME_GENERIC_E)
{
// try to enforce a forward compatibility for syscalls added
// after a scap file was generated,
Expand Down Expand Up @@ -981,7 +981,7 @@ uint8_t* sinsp_filter_check_event::extract(sinsp_evt *evt, OUT uint32_t* len, bo
uint16_t ppm_sc = *(uint16_t *)parinfo->m_val;

// Only generic enter event has the nativeID as second param
if (m_inspector->is_capture() && ppm_sc == PPM_SC_UNKNOWN && etype == PPME_GENERIC_E)
if (m_inspector && m_inspector->is_capture() && ppm_sc == PPM_SC_UNKNOWN && etype == PPME_GENERIC_E)
{
// try to enforce a forward compatibility for syscalls added
// after a scap file was generated,
Expand Down
9 changes: 6 additions & 3 deletions userspace/libsinsp/sinsp_filtercheck_evtin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,8 @@ inline uint8_t* sinsp_filter_check_evtin::extract_tracer(sinsp_evt *evt, sinsp_p
//
// If this is a thread-related field, reject anything that doesn't come from the same thread
//
if(static_cast<int64_t>(pae->m_tid) != evt->get_thread_info()->m_tid)
auto* tinfo = evt->get_thread_info();
if(!tinfo || static_cast<int64_t>(pae->m_tid) != tinfo->m_tid)
{
return NULL;
}
Expand All @@ -262,7 +263,8 @@ inline uint8_t* sinsp_filter_check_evtin::extract_tracer(sinsp_evt *evt, sinsp_p

if(tinfo)
{
if(tinfo->m_tid != evt->get_thread_info()->m_tid)
auto* evtinfo = evt->get_thread_info();
if(!evtinfo || tinfo->m_tid != evtinfo->m_tid)
{
return NULL;
}
Expand All @@ -283,7 +285,8 @@ inline uint8_t* sinsp_filter_check_evtin::extract_tracer(sinsp_evt *evt, sinsp_p

if(tinfo)
{
if(tinfo->m_pid != evt->get_thread_info()->m_ptid)
auto* evtinfo = evt->get_thread_info();
if(!evtinfo || tinfo->m_pid != evtinfo->m_ptid)
{
return NULL;
}
Expand Down
4 changes: 4 additions & 0 deletions userspace/libsinsp/sinsp_filtercheck_fdlist.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ uint8_t* sinsp_filter_check_fdlist::extract(sinsp_evt *evt, OUT uint32_t* len, b
uint16_t nfds = *(uint16_t *)payload;
uint32_t pos = 2;
sinsp_threadinfo* tinfo = evt->get_thread_info();
if(!tinfo)
{
return NULL;
}

m_strval.clear();

Expand Down
1 change: 1 addition & 0 deletions userspace/libsinsp/test/classes/sinsp_thread_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ TEST_F(sinsp_with_test_input, THRD_MANAGER_find_reaper_in_the_tree)
DEFAULT_TREE

auto p6_t1_tinfo = m_inspector.get_thread_ref(p6_t1_tid, false).get();
ASSERT_TRUE(p6_t1_tinfo);

/* Call the find reaper method, the reaper for p6_t1 should be p4_t1 */
auto reaper = m_inspector.m_thread_manager->find_new_reaper(p6_t1_tinfo);
Expand Down
3 changes: 3 additions & 0 deletions userspace/libsinsp/test/classes/sinsp_threadinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ TEST_F(sinsp_with_test_input, THRD_INFO_assign_children_to_reaper)
DEFAULT_TREE

auto p3_t1_tinfo = m_inspector.get_thread_ref(p3_t1_tid, false).get();
ASSERT_NE(p3_t1_tinfo, nullptr);

/* The reaper cannot be the current process */
EXPECT_THROW(p3_t1_tinfo->assign_children_to_reaper(p3_t1_tinfo), sinsp_exception);
Expand All @@ -98,6 +99,7 @@ TEST_F(sinsp_with_test_input, THRD_INFO_assign_children_to_reaper)
ASSERT_THREAD_CHILDREN(p1_t1_tid, 0, 0);

auto p1_t1_tinfo = m_inspector.get_thread_ref(p1_t1_tid, false).get();
ASSERT_NE(p1_t1_tinfo, nullptr);
p3_t1_tinfo->assign_children_to_reaper(p1_t1_tinfo);

/* all p3_t1 children should be removed */
Expand All @@ -120,6 +122,7 @@ TEST_F(sinsp_with_test_input, THRD_INFO_assign_children_to_a_nullptr)
DEFAULT_TREE

auto p2_t1_tinfo = m_inspector.get_thread_ref(p2_t1_tid, false).get();
ASSERT_NE(p2_t1_tinfo, nullptr);
/* This call should change the parent of all children of p2_t1 to `0` */
p2_t1_tinfo->assign_children_to_reaper(nullptr);

Expand Down
1 change: 1 addition & 0 deletions userspace/libsinsp/test/events_proc.ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,7 @@ TEST_F(sinsp_with_test_input, last_exec_ts)
std::string argsv = test_utils::to_null_delimited(args);
evt = add_event_advance_ts(increasing_ts(), parent_tid, PPME_SYSCALL_CLONE_20_X, 20, child_tid, "bash", empty_bytebuf, parent_pid, parent_tid, 0, "", 1024, 0, 68633, 12088, 7208, 0, "bash", scap_const_sized_buffer{cgroupsv.data(), cgroupsv.size()}, PPM_CL_CLONE_CHILD_CLEARTID | PPM_CL_CLONE_CHILD_SETTID, 1000, 1000, parent_pid, parent_tid);

ASSERT_TRUE(evt->get_thread_info());
// Check we initialize lastexec time to zero
ASSERT_EQ(evt->get_thread_info()->m_lastexec_ts, 0);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ TEST(scap_file_kexec_arm64, tail_lineage)
std::vector<int64_t> expected_traverse_parents_after_execve = {tid_sh, tid_containerd_shim1, tid_systemd1,
tid_containerd_shim2, tid_systemd2};
traverse_parents.clear();
ASSERT_TRUE(evt->get_thread_info());
evt->get_thread_info()->traverse_parent_state(visitor);
ASSERT_EQ(traverse_parents, expected_traverse_parents_after_execve);

Expand Down
2 changes: 2 additions & 0 deletions userspace/libsinsp/test/scap_files/kexec_x86/kexec_x86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ TEST(scap_file_kexec_x86, tail_lineage)
std::vector<int64_t> expected_traverse_parents_after_execve = {
tid_sh, tid_runc, tid_containerd_shim1, tid_systemd1, tid_containerd_shim2, tid_systemd2};
traverse_parents.clear();
ASSERT_TRUE(evt->get_thread_info());
evt->get_thread_info()->traverse_parent_state(visitor);
ASSERT_EQ(traverse_parents, expected_traverse_parents_after_execve);

Expand All @@ -99,6 +100,7 @@ TEST(scap_file_kexec_x86, tail_lineage)
std::vector<int64_t> expected_traverse_parents_after_remove = {tid_sh, tid_containerd_shim1, tid_systemd1,
tid_containerd_shim2, tid_systemd2};
traverse_parents.clear();
ASSERT_TRUE(evt->get_thread_info());
evt->get_thread_info()->traverse_parent_state(visitor);
ASSERT_EQ(traverse_parents, expected_traverse_parents_after_remove);

Expand Down
3 changes: 3 additions & 0 deletions userspace/libsinsp/test/thread_table.ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ TEST_F(sinsp_with_test_input, THRD_TABLE_traverse_default_tree)
/*=============================== p4_t1 traverse ===========================*/

sinsp_threadinfo* tinfo = m_inspector.get_thread_ref(p4_t1_tid, false, true).get();
ASSERT_TRUE(tinfo);

std::vector<int64_t> expected_p4_traverse_parents = {p4_t1_ptid, p3_t1_ptid, p2_t1_ptid};

Expand All @@ -264,6 +265,7 @@ TEST_F(sinsp_with_test_input, THRD_TABLE_traverse_default_tree)
/*=============================== p5_t2 traverse ===========================*/

tinfo = m_inspector.get_thread_ref(p5_t2_tid, false).get();
ASSERT_TRUE(tinfo);

std::vector<int64_t> expected_p5_traverse_parents = {p5_t2_ptid, p4_t2_ptid, p3_t1_ptid, p2_t1_ptid};

Expand Down Expand Up @@ -321,6 +323,7 @@ TEST_F(sinsp_with_test_input, THRD_TABLE_traverse_default_tree)
/*=============================== p6_t1 traverse ===========================*/

tinfo = m_inspector.get_thread_ref(p6_t1_tid, false).get();
ASSERT_TRUE(tinfo);

std::vector<int64_t> expected_p6_traverse_parents = {p4_t1_tid, p2_t3_tid, INIT_TID};

Expand Down
6 changes: 3 additions & 3 deletions userspace/libsinsp/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -950,16 +950,16 @@ void sinsp_utils::split_container_image(const std::string &image,
}
}

void sinsp_utils::parse_suppressed_types(const std::vector<std::string> &supp_strs,
std::vector<uint16_t> *supp_ids)
void sinsp_utils::parse_suppressed_types(const std::vector<std::string>& supp_strs,
std::vector<ppm_event_code>* supp_ids)
{
for (auto ii = 0; ii < PPM_EVENT_MAX; ii++)
{
auto iter = std::find(supp_strs.begin(), supp_strs.end(),
event_name_by_id(ii));
if (iter != supp_strs.end())
{
supp_ids->push_back(ii);
supp_ids->push_back(static_cast<ppm_event_code>(ii));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions userspace/libsinsp/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ class sinsp_utils
std::string &digest,
bool split_repo = true);

static void parse_suppressed_types(const std::vector<std::string> &supp_strs,
std::vector<uint16_t> *supp_ids);
static void parse_suppressed_types(const std::vector<std::string>& supp_strs,
std::vector<ppm_event_code>* supp_ids);

static const char* event_name_by_id(uint16_t id);

Expand Down
Loading