From eb325bd388743a6c1e7f5208d753a859240724e3 Mon Sep 17 00:00:00 2001 From: Albert Ziegenhagel Date: Wed, 1 May 2024 18:33:42 +0200 Subject: [PATCH 1/8] Rename "apps" subdirectory to "tools" --- .github/workflows/clang-format.yml | 2 +- CMakeLists.txt | 2 +- {apps => tools}/CMakeLists.txt | 0 {apps => tools}/analysis.cpp | 0 {apps => tools}/etl_file.cpp | 0 {apps => tools}/perf_data_file.cpp | 0 {apps => tools}/snail-profiler.manifest.xml | 0 {apps => tools}/snail-profiler.ps1 | 0 8 files changed, 2 insertions(+), 2 deletions(-) rename {apps => tools}/CMakeLists.txt (100%) rename {apps => tools}/analysis.cpp (100%) rename {apps => tools}/etl_file.cpp (100%) rename {apps => tools}/perf_data_file.cpp (100%) rename {apps => tools}/snail-profiler.manifest.xml (100%) rename {apps => tools}/snail-profiler.ps1 (100%) diff --git a/.github/workflows/clang-format.yml b/.github/workflows/clang-format.yml index e5d8d40..d42d30d 100644 --- a/.github/workflows/clang-format.yml +++ b/.github/workflows/clang-format.yml @@ -16,7 +16,7 @@ jobs: - name: Run clang-format uses: DoozyX/clang-format-lint-action@v0.16.2 with: - source: './snail ./tests ./apps' + source: './snail ./tests ./tools' exclude: './snail/common/third_party' extensions: 'cpp,hpp' clangFormatVersion: 16 diff --git a/CMakeLists.txt b/CMakeLists.txt index c391e3b..7f84ee5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -88,7 +88,7 @@ add_subdirectory(snail) if(SNAIL_BUILD_TOOLS) # Just for testing/development - add_subdirectory(apps) + add_subdirectory(tools) endif() # ======= diff --git a/apps/CMakeLists.txt b/tools/CMakeLists.txt similarity index 100% rename from apps/CMakeLists.txt rename to tools/CMakeLists.txt diff --git a/apps/analysis.cpp b/tools/analysis.cpp similarity index 100% rename from apps/analysis.cpp rename to tools/analysis.cpp diff --git a/apps/etl_file.cpp b/tools/etl_file.cpp similarity index 100% rename from apps/etl_file.cpp rename to tools/etl_file.cpp diff --git a/apps/perf_data_file.cpp b/tools/perf_data_file.cpp similarity index 100% rename from apps/perf_data_file.cpp rename to tools/perf_data_file.cpp diff --git a/apps/snail-profiler.manifest.xml b/tools/snail-profiler.manifest.xml similarity index 100% rename from apps/snail-profiler.manifest.xml rename to tools/snail-profiler.manifest.xml diff --git a/apps/snail-profiler.ps1 b/tools/snail-profiler.ps1 similarity index 100% rename from apps/snail-profiler.ps1 rename to tools/snail-profiler.ps1 From d2e2c51e453c88bef25f7d914fd7fc41d8529753 Mon Sep 17 00:00:00 2001 From: Albert Ziegenhagel Date: Wed, 1 May 2024 18:49:20 +0200 Subject: [PATCH 2/8] Rename tool executables --- .github/workflows/ci.yml | 2 +- tools/CMakeLists.txt | 21 ++++++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 60e6b4a..e9c2822 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -187,7 +187,7 @@ jobs: with: name: snail-tools-${{ matrix.arch }}-${{ matrix.platform }} path: | - ${{ github.workspace }}/install/tools/bin/app_* + ${{ github.workspace }}/install/tools/bin/snail-tool-* ${{ github.workspace }}/install/tools/bin/info.txt deploy-head: diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt index 32b9066..856170c 100644 --- a/tools/CMakeLists.txt +++ b/tools/CMakeLists.txt @@ -1,19 +1,22 @@ -add_executable(app_etl_file etl_file.cpp) -target_link_libraries(app_etl_file PRIVATE compile_options etl utf8cpp) +add_executable(snail_tool_etl etl_file.cpp) +target_link_libraries(snail_tool_etl PRIVATE compile_options etl utf8cpp) +set_target_properties(snail_tool_etl PROPERTIES OUTPUT_NAME "snail-tool-etl") -add_executable(app_perf_data_file perf_data_file.cpp) -target_link_libraries(app_perf_data_file PRIVATE compile_options perf_data) +add_executable(snail_tool_perf_data perf_data_file.cpp) +target_link_libraries(snail_tool_perf_data PRIVATE compile_options perf_data) +set_target_properties(snail_tool_perf_data PROPERTIES OUTPUT_NAME "snail-tool-perfdata") -add_executable(app_analysis analysis.cpp) -target_link_libraries(app_analysis PRIVATE compile_options analysis) +add_executable(snail_tool_analysis analysis.cpp) +target_link_libraries(snail_tool_analysis PRIVATE compile_options analysis) +set_target_properties(snail_tool_analysis PROPERTIES OUTPUT_NAME "snail-tool-analysis") install( TARGETS - app_etl_file - app_perf_data_file - app_analysis + snail_tool_etl + snail_tool_perf_data + snail_tool_analysis COMPONENT tools RUNTIME DESTINATION bin From e2280f2b0b5a224bf82ed02415f5c47ec446d8f6 Mon Sep 17 00:00:00 2001 From: Albert Ziegenhagel Date: Wed, 1 May 2024 19:39:07 +0200 Subject: [PATCH 3/8] Improve detecting invalid ETL files Change some assertions to throwing exceptions, which allows to react to invalid ETL files. --- snail/etl/etl_file.cpp | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/snail/etl/etl_file.cpp b/snail/etl/etl_file.cpp index f41b68e..25885d8 100644 --- a/snail/etl/etl_file.cpp +++ b/snail/etl/etl_file.cpp @@ -303,25 +303,37 @@ void etl_file::open(const std::filesystem::path& file_path) const auto buffer_header = parser::wmi_buffer_header_view(file_buffer); - assert(buffer_header.buffer_type() == parser::etw_buffer_type::header); + if(buffer_header.buffer_type() != parser::etw_buffer_type::header) + { + throw std::runtime_error("Invalid ETL file: invalid buffer header type"); + } if(read_bytes < buffer_header.wnode().saved_offset()) { throw std::runtime_error("Invalid ETL file: insufficient size for buffer"); } - [[maybe_unused]] const auto marker = parser::generic_trace_marker_view(file_buffer.subspan(parser::wmi_buffer_header_view::static_size)); - assert(marker.is_trace_header() && marker.is_trace_header_event_trace() && !marker.is_trace_message()); - assert(marker.header_type() == parser::trace_header_type::system32 || - marker.header_type() == parser::trace_header_type::system64); + const auto marker = parser::generic_trace_marker_view(file_buffer.subspan(parser::wmi_buffer_header_view::static_size)); + if(!marker.is_trace_header() || !marker.is_trace_header_event_trace() || marker.is_trace_message()) + { + throw std::runtime_error("Invalid ETL file: invalid trace marker"); + } + if(marker.header_type() != parser::trace_header_type::system32 && + marker.header_type() != parser::trace_header_type::system64) + { + throw std::runtime_error("Invalid ETL file: invalid trace header type"); + } const auto system_trace_header = parser::system_trace_header_view(file_buffer.subspan( parser::wmi_buffer_header_view::static_size)); // the first record needs to be a event-trace-header - assert(system_trace_header.packet().group() == parser::event_trace_group::header); - assert(system_trace_header.packet().type() == 0); - assert(system_trace_header.version() == 2); + if(system_trace_header.packet().group() != parser::event_trace_group::header || + system_trace_header.packet().type() != 0 || + system_trace_header.version() != 2) + { + throw std::runtime_error("Invalid ETL file: invalid initial event trace header record."); + } const auto header_event = parser::event_trace_v2_header_event_view(file_buffer.subspan( parser::wmi_buffer_header_view::static_size + From 549ccb9d86074eb361543022058f2893018c2f66 Mon Sep 17 00:00:00 2001 From: Albert Ziegenhagel Date: Wed, 1 May 2024 19:40:05 +0200 Subject: [PATCH 4/8] Print main header information from ETL file --- tools/etl_file.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/etl_file.cpp b/tools/etl_file.cpp index 1eefe6d..3d8d484 100644 --- a/tools/etl_file.cpp +++ b/tools/etl_file.cpp @@ -443,6 +443,14 @@ int main(int argc, char* argv[]) return EXIT_FAILURE; } + std::cout << std::format(" start_time: {}\n", file.header().start_time); + std::cout << std::format(" end_time: {}\n", file.header().end_time); + std::cout << std::format(" start_time_qpc_ticks: {}\n", file.header().start_time_qpc_ticks); + std::cout << std::format(" qpc_frequency: {}\n", file.header().qpc_frequency); + std::cout << std::format(" pointer_size: {}\n", file.header().pointer_size); + std::cout << std::format(" number_of_processors: {}\n", file.header().number_of_processors); + std::cout << std::format(" number_of_buffers: {}\n", file.header().number_of_buffers); + counting_event_observer observer; std::size_t sample_count = 0; From d10548ba2df9840af1882ab88f22ce7af10bf4de Mon Sep 17 00:00:00 2001 From: Albert Ziegenhagel Date: Wed, 1 May 2024 19:40:22 +0200 Subject: [PATCH 5/8] Catch errors during ETL file processing --- tools/etl_file.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/etl_file.cpp b/tools/etl_file.cpp index 3d8d484..eaa359a 100644 --- a/tools/etl_file.cpp +++ b/tools/etl_file.cpp @@ -1062,7 +1062,15 @@ int main(int argc, char* argv[]) } std::cout << "\n"; - file.process(observer); + try + { + file.process(observer); + } + catch(const std::exception& e) + { + std::cerr << std::format("Failed to process ETL file: {}\n", e.what()); + return EXIT_FAILURE; + } std::cout << "\n"; std::cout << "Number of samples:\n"; From b07c7241cfff49201240cc0341511f6bb159cd88 Mon Sep 17 00:00:00 2001 From: Albert Ziegenhagel Date: Wed, 1 May 2024 19:41:31 +0200 Subject: [PATCH 6/8] Invoke buffer header callback before, not after the buffer is being processed --- snail/etl/etl_file.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/snail/etl/etl_file.cpp b/snail/etl/etl_file.cpp index 25885d8..1f124cb 100644 --- a/snail/etl/etl_file.cpp +++ b/snail/etl/etl_file.cpp @@ -67,7 +67,9 @@ struct next_event_priority_info buffer_info read_buffer(std::ifstream& file_stream, std::streampos buffer_start_pos, - std::array& buffer_data) + std::array& buffer_data, + const etl_file::header_data& file_header_, + event_observer& callbacks) { file_stream.seekg(buffer_start_pos); file_stream.read(reinterpret_cast(buffer_data.data()), common::narrow_cast(buffer_data.size())); @@ -86,6 +88,8 @@ buffer_info read_buffer(std::ifstream& file_stream, const auto header = parser::wmi_buffer_header_view(header_buffer); + callbacks.handle_buffer(file_header_, header); + if(header.wnode().buffer_size() > total_buffer.size()) { throw std::runtime_error(std::format( @@ -431,7 +435,10 @@ void etl_file::process(event_observer& callbacks) processor_data.current_buffer_info = read_buffer(file_stream_, remaining_buffers.back().start_pos, - processor_data.current_buffer_data); + processor_data.current_buffer_data, + header_, + callbacks); + remaining_buffers.pop_back(); event_queue.push(next_event_priority_info{ @@ -466,8 +473,6 @@ void etl_file::process(event_observer& callbacks) const auto buffer_exhausted = buffer_info.current_payload_offset >= buffer_info.payload_buffer.size(); if(buffer_exhausted) { - callbacks.handle_buffer(header_, parser::wmi_buffer_header_view(buffer_info.header_buffer)); - auto& remaining_buffers = processor_data.remaining_buffers; // If there are no remaining buffers for this processor, stop extracting events for it. @@ -477,7 +482,10 @@ void etl_file::process(event_observer& callbacks) // Keep in mind, that `remaining_buffers` is sorted. buffer_info = read_buffer(file_stream_, remaining_buffers.back().start_pos, - processor_data.current_buffer_data); + processor_data.current_buffer_data, + header_, + callbacks); + remaining_buffers.pop_back(); } From 85d7f2c5e4d57ccaeb5678b6b861996861a1d294 Mon Sep 17 00:00:00 2001 From: Albert Ziegenhagel Date: Wed, 1 May 2024 19:42:03 +0200 Subject: [PATCH 7/8] Error out on compressed ETL buffers They are not currently supported --- snail/etl/etl_file.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/snail/etl/etl_file.cpp b/snail/etl/etl_file.cpp index 1f124cb..47fa5c9 100644 --- a/snail/etl/etl_file.cpp +++ b/snail/etl/etl_file.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include @@ -105,6 +106,13 @@ buffer_info read_buffer(std::ifstream& file_stream, read_bytes)); } + if((header.buffer_flag() & std::to_underlying(parser::etw_buffer_flag::compressed)) != 0) + { + throw std::runtime_error(std::format( + "Unsupported ETL file: Buffer {} is marked as compressed, but compressed buffers are not yet supported.", + header.wnode().sequence_number())); + } + const auto payload_size = header.wnode().saved_offset() - parser::wmi_buffer_header_view::static_size; const auto payload_buffer = total_buffer.subspan(parser::wmi_buffer_header_view::static_size, payload_size); From b1cc22f5c5eeb22355e776c7476dabc4aebc4092 Mon Sep 17 00:00:00 2001 From: Albert Ziegenhagel Date: Wed, 1 May 2024 19:51:40 +0200 Subject: [PATCH 8/8] Upgrade googltest to 1.14.0 Mainly to fix some warnings emitted by clang. --- third-party/gtest/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third-party/gtest/CMakeLists.txt b/third-party/gtest/CMakeLists.txt index efbe16f..a6ff2f7 100644 --- a/third-party/gtest/CMakeLists.txt +++ b/third-party/gtest/CMakeLists.txt @@ -6,7 +6,7 @@ include(FetchContent) FetchContent_Declare( googletest GIT_REPOSITORY https://github.com/google/googletest.git - GIT_TAG b796f7d44681514f58a683a3a71ff17c94edb0c1 # v1.13.0 + GIT_TAG f8d7d77c06936315286eb55f8de22cd23c188571 # v1.14.0 SYSTEM )