fix: multiple definitions for scap_engine_handle

[federico-sysdig]

What type of PR is this?
/kind cleanup

Any specific area of the project related to this PR?
/area build

Does this PR require a change in the driver versions?
No

What this PR does / why we need it:
Class scap_engine_handle is defined once in its header file, but has a single pointer data member SCAP_HANDLE_T* m_handle that contains a macro that changes depending on what source file is including it. This is by design, the idea being that it is a sort of hierarchy of classes that share the same functions. However this is not without problems as if one tries to enable link-time optimization (LTO) the linker presents an error that the one-definition rule (ODR) has been broken.
This PR solves the issue by using a generic void* for the data member and using a cast to the appropriate type each time is is used in a function. This is possible as manipulating functions are specialized for each type, so knowledge of the type is available.
The technique sacrifices somewhat the "beauty" of the previous approach: each cast is syntactically "ugly", but saves the ODR and overcomes linker errors when LTO is enabled.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

[github-actions]

Please double check driver/API_VERSION file. See versioning.

/hold

[FedeDP]

/milestone 0.18.0

[gnosek]

This PR is exactly the reason why I went with SCAP_HANDLE_T ;) The casts it introduces are ugly af, though I agree that if SCAP_HANDLE_T breaks LTO, we should get rid of it.

But please try to make the (inevitable) casts a bit easier on the eyes :)

[poiana]

LGTM label has been added.

Git tree hash: 23511d387de91315465b39b00e1fe4cc03e0dc0a

[incertum]

We likely need a rebase for the CI to pass.

[federico-sysdig]

We likely need a rebase for the CI to pass.

Thanks @incertum, I've rebased. Though I think there's some kind of issue that causes build problems in some scenarios. I will have to make time and sort them out.

[github-actions]

Perf diff from master - unit tests

Warning:
1 out of order events recorded.
     6.69%     -1.27%  [.] sinsp::next
     1.40%     +0.79%  [.] sinsp::fetch_next_event
     0.51%     +0.62%  [.] sinsp_evt::get_param
     1.40%     -0.59%  [.] std::_Hashtable<long, std::pair<long const, std::shared_ptr<sinsp_threadinfo> >, std::allocator<std::pair<long const, std::shared_ptr<sinsp_threadinfo> > >, std::__detail::_Select1st, std::equal_to<long>, std::hash<long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::find
     2.77%     +0.58%  [.] std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release
     5.39%     -0.50%  [.] sinsp_evt::get_type
     0.77%     +0.45%  [.] sinsp_utils::find_longest_matching_evt_param
     0.21%     +0.41%  [.] sinsp_parser::parse_clone_exit_child

Perf diff from master - scap file

    20.18%     -8.82%  [.] sinsp_evt_formatter::tostring_withformat
    11.22%     +4.61%  [.] sinsp_filter_check::tostring
     2.91%     +3.91%  [.] sinsp_filter_check::extract_nocache
    11.61%     -2.04%  [.] sinsp_filter_check_event::extract_single
     2.93%     +1.99%  [.] std::_Hashtable<long, std::pair<long const, std::shared_ptr<sinsp_threadinfo> >, std::allocator<std::pair<long const, std::shared_ptr<sinsp_threadinfo> > >, std::__detail::_Select1st, std::equal_to<long>, std::hash<long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_before_node
     5.89%     -1.74%  [.] sinsp_filter_check::apply_transformers
     5.80%     -1.69%  [.] sinsp_filter_check_thread::extract_single
     2.95%     +0.35%  [.] load_ppm_sc_table
     2.89%     +0.32%  [.] sinsp_thread_manager::get_thread_ref
     8.74%     +0.30%  [.] sinsp_filter_check::rawval_to_string

Heap diff from master - unit tests

total runtime: 0.00s.
calls to allocation functions: 70 (70/s)
temporary memory allocations: 266 (266/s)
peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

total runtime: -0.01s.
calls to allocation functions: 0 (0/s)
temporary memory allocations: -1 (166/s)
peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

[github-actions]

Perf diff from master - unit tests

     9.13%     +2.05%  [.] sinsp_parser::reset
     4.29%     -0.86%  [.] sinsp_evt::load_params
     0.89%     +0.74%  [.] libsinsp::sinsp_suppress::process_event
     4.66%     +0.67%  [.] next
     0.30%     +0.51%  [.] sinsp_split[abi:cxx11]
     0.11%     +0.48%  [.] sinsp_container_manager::resolve_container
     5.32%     -0.44%  [.] sinsp_parser::process_event
     1.20%     -0.39%  [.] sinsp_parser::parse_context_switch
     0.72%     +0.39%  [.] std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>
     1.07%     -0.38%  [.] std::_Hashtable<long, std::pair<long const, std::shared_ptr<sinsp_threadinfo> >, std::allocator<std::pair<long const, std::shared_ptr<sinsp_threadinfo> > >, std::__detail::_Select1st, std::equal_to<long>, std::hash<long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::find

Perf diff from master - scap file

     7.55%     +7.18%  [.] sinsp_filter_check::tostring
     3.75%     +6.78%  [.] std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>
    16.41%     -3.52%  [.] next
    11.13%     -3.13%  [.] std::_Hashtable<long, std::pair<long const, std::shared_ptr<sinsp_threadinfo> >, std::allocator<std::pair<long const, std::shared_ptr<sinsp_threadinfo> > >, std::__detail::_Select1st, std::equal_to<long>, std::hash<long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_before_node
     3.74%     +2.54%  [.] gzfile_read
     4.36%     +2.10%  [.] main
     3.27%     +0.57%  [.] sinsp_split[abi:cxx11]
    16.81%     +0.43%  [.] sinsp_evt_formatter::tostring_withformat
     3.77%     +0.37%  [.] rawstring_check::extract_single
     3.77%     +0.34%  [.] sinsp_thread_manager::get_thread_ref

Heap diff from master - unit tests

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

[codecov]

Codecov Report

Attention: Patch coverage is 13.51351% with 64 lines in your changes missing coverage. Please review.

Project coverage is 50.98%. Comparing base (2e87063) to head (f0346c0).
Report is 19 commits behind head on master.

Files	Patch %	Lines
userspace/libscap/engine/kmod/scap_kmod.c	13.88%	31 Missing ⚠️
userspace/libscap/engine/bpf/scap_bpf.c	0.00%	19 Missing ⚠️
userspace/libscap/engine/gvisor/gvisor.cpp	0.00%	9 Missing ⚠️
userspace/libscap/engine/savefile/scap_savefile.c	25.00%	3 Missing ⚠️
userspace/libscap/engine/noop/noop.c	0.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #1889    +/-   ##
========================================
  Coverage   50.98%   50.98%            
========================================
  Files         310      310            
  Lines       39612    39612            
  Branches    17455    17707   +252     
========================================
+ Hits        20197    20198     +1     
- Misses      14338    14349    +11     
+ Partials     5077     5065    -12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[federico-sysdig]

I've recently rebased and now the required CI stages are passing. There are still some broken stages, but I can't see how they could be related to this PR.

The round of approvals need to restart after the rebase. Could anyone please review?

cc @gnosek

[poiana]

LGTM label has been added.

Git tree hash: b5ffbecf0cb811faf57f2753fd8fd56060e667f4

[federico-sysdig]

Not sure whom to go to for soliciting the needed second review. Can anyone help, please?
cc @FedeDP @LucaGuerra @incertum

[federico-sysdig]

@hbrueckner
IIUC the system has elected you as one of the reviewer and your contribution is essential for moving the PR forward.

[incertum]

/unhold

[incertum]

Not sure if it will merge, @LucaGuerra mind checking as I am offline now...

[incertum]

Kind bump, why is this stuck and not merging? @falcosecurity/libs-maintainers thanks!

[github-actions]

Perf diff from master - unit tests

     4.96%     +1.80%  [.] sinsp_evt::get_type
     5.78%     +0.83%  [.] sinsp::next
    11.36%     -0.70%  [.] sinsp_parser::reset
     1.32%     -0.62%  [.] libsinsp::events::is_unknown_event
     1.38%     +0.60%  [.] libsinsp::sinsp_suppress::process_event
     4.14%     -0.50%  [.] gzfile_read
     0.99%     +0.48%  [.] scap_event_encode_params_v
     1.26%     +0.48%  [.] std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>
     0.33%     +0.46%  [.] sinsp_evt::get_direction
     5.20%     +0.45%  [.] next

Perf diff from master - scap file

    14.90%     -3.66%  [.] sinsp_filter_check_event::extract_single
    11.14%     -3.61%  [.] sinsp_evt::get_type
    18.58%     +3.58%  [.] sinsp_filter_check::tostring
     7.39%     +3.43%  [.] sinsp_evt_formatter::tostring_withformat
     7.23%     +1.95%  [.] sinsp::next
     7.40%     -1.78%  [.] sinsp_thread_manager::get_thread_ref
     3.67%     +1.65%  [.] std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>
     3.73%     +1.63%  [.] gzfile_read
     3.73%     -0.23%  [.] std::_Hashtable<long, std::pair<long const, std::shared_ptr<sinsp_threadinfo> >, std::allocator<std::pair<long const, std::shared_ptr<sinsp_threadinfo> > >, std::__detail::_Select1st, std::equal_to<long>, std::hash<long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_before_node
     7.46%     +0.05%  [.] sinsp_filter_check_thread::extract_single

Heap diff from master - unit tests

peak heap memory consumption: 818B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

peak heap memory consumption: -30B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

[gnosek]

/approve

pretty please?

[poiana]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: federico-sysdig, gnosek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [gnosek]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment