diff --git a/.github/workflows/continuous_integration.yml b/.github/workflows/continuous_integration.yml index 9ab5824359..ace70fd5db 100644 --- a/.github/workflows/continuous_integration.yml +++ b/.github/workflows/continuous_integration.yml @@ -6,11 +6,13 @@ on: branches: [ amd-staging, amd-mainline ] paths-ignore: - '*.md' + - '**/README.md' - 'source/docs/**' - 'CODEOWNERS' pull_request: paths-ignore: - '*.md' + - '**/README.md' - 'source/docs/**' - 'CODEOWNERS' diff --git a/.github/workflows/formatting.yml b/.github/workflows/formatting.yml index a721226e2e..02a0d56f92 100644 --- a/.github/workflows/formatting.yml +++ b/.github/workflows/formatting.yml @@ -8,6 +8,8 @@ on: - '.github/workflows/pull_*.yml' - '.github/workflows/linting.yml' - '.github/workflows/markdown_lint.yml' + - '*.md' + - '**/README.md' concurrency: group: ${{ github.workflow }}-${{ github.ref }} diff --git a/.github/workflows/restrictions.yml b/.github/workflows/restrictions.yml new file mode 100644 index 0000000000..21e92cf4db --- /dev/null +++ b/.github/workflows/restrictions.yml @@ -0,0 +1,53 @@ + +name: Code Restrictions +permissions: + contents: read + +# This workflow ensures that certain code restrictions are applied. +# For examples, rocprofiler-sdk cannot use std::regex because of issues +# when loaded into an application compiled with C++ dual ABI support because +# while std::regex itself (and std::regex_traits) being ABI-tagged, +# std::__detail::_Scanner is not. Applications compiled with dual ABI support +# will either throw an exception or produce a segfault and thus, this workflow +# attempts to catch usage of std::regex during CI. + +on: + workflow_dispatch: + pull_request: + paths-ignore: + - '.github/workflows/pull_*.yml' + - '.github/workflows/linting.yml' + - '.github/workflows/markdown_lint.yml' + - '*.md' + - '**/README.md' + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + regex: + runs-on: ubuntu-latest + env: + FOLDERS: "source/lib/common source/lib/rocprofiler-sdk source/lib/rocprofiler-sdk-roctx" + + steps: + - uses: actions/checkout@v4 + + - name: Install dependencies + run: | + sudo apt-get update + sudo apt-get install -y python3-pip + python3 -m pip install -U cmake-format + + - name: Apply restriction + run: | + set +e + FILES="$(find ${FOLDERS} -type f)" + GREP="$(grep -E -n 'std::regex|' ${FILES})" + if [ "${GREP}" != "" ]; then + echo -e "\nError! std::regex is not allowed in ${FOLDERS}...\n" + echo -e "\nResults:\n" + echo "${GREP}" + exit 1 + fi diff --git a/CHANGELOG.md b/CHANGELOG.md index fe3eaf6c77..e0f61b2378 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -187,6 +187,7 @@ Full documentation for ROCprofiler-SDK is available at [rocm.docs.amd.com/projec - Fixed missing callbacks around internal thread creation within counter collection service - Fixed potential data race in rocprofiler-sdk double buffering scheme +- Usage of std::regex in core rocprofiler-sdk library which causes segfaults/exceptions when used under dual ABI ### Removed diff --git a/source/lib/common/elf_utils.cpp b/source/lib/common/elf_utils.cpp index e4d4a8c7f0..6361d96ee3 100644 --- a/source/lib/common/elf_utils.cpp +++ b/source/lib/common/elf_utils.cpp @@ -34,7 +34,6 @@ #include #include #include -#include #include #include #include @@ -78,16 +77,16 @@ ElfInfo::ElfInfo(std::string _fname) {} bool -ElfInfo::has_symbol(std::regex&& _re) const +ElfInfo::has_symbol(const std::function& _checker) const { for(const auto& itr : symbol_entries) { - if(!itr.name.empty() && std::regex_search(itr.name, _re)) return true; + if(!itr.name.empty() && _checker(itr.name)) return true; } // For stripped binaries for(const auto& itr : dynamic_symbol_entries) { - if(!itr.name.empty() && std::regex_search(itr.name, _re)) return true; + if(!itr.name.empty() && _checker(itr.name)) return true; } return false; diff --git a/source/lib/common/elf_utils.hpp b/source/lib/common/elf_utils.hpp index 4cc606b85b..ead9498def 100644 --- a/source/lib/common/elf_utils.hpp +++ b/source/lib/common/elf_utils.hpp @@ -25,9 +25,9 @@ #include #include +#include #include #include -#include #include #include #include @@ -94,7 +94,7 @@ struct ElfInfo std::vector dynamic_entries = {}; std::vector reloc_entries = {}; - bool has_symbol(std::regex&&) const; + bool has_symbol(const std::function&) const; friend bool operator==(const ElfInfo& lhs, const ElfInfo& rhs) { diff --git a/source/lib/rocprofiler-sdk/agent.cpp b/source/lib/rocprofiler-sdk/agent.cpp index bf4373c597..793cea5717 100644 --- a/source/lib/rocprofiler-sdk/agent.cpp +++ b/source/lib/rocprofiler-sdk/agent.cpp @@ -46,11 +46,12 @@ #include #include #include -#include #include #include #include +#include #include +#include #include #include #include @@ -112,6 +113,7 @@ parse_cpu_info() break; } + line = sdk::parse::strip(std::move(line), " \t\n\v\f\r"); if(line.empty()) { if(!current_block.empty()) blocks.emplace_back(std::move(current_block)); @@ -134,46 +136,68 @@ parse_cpu_info() auto info_v = cpu_info{}; for(const auto& itr : bitr) { - auto match = std::smatch{}; - const std::regex re{".*: (.*)$"}; - if(std::regex_match(itr, match, re)) + auto match = sdk::parse::tokenize(itr, std::vector{": "}); + if(match.size() == 2) { - if(match.size() == 2) - { - std::ssub_match value = match[1]; - - if(itr.find("vendor_id") == 0) - info_v.vendor_id = value.str(); - else if(itr.find("model name") == 0) + auto get_stol = [_label = std::string_view{itr}](const auto& _value) -> long { + try { - info_v.model_name = value.str(); - size_t first_colon_pos = match.str().find(':'); - // This handles the case where the model name has multiple colons - // Example "model name : AMD EPYC : 100-000000248" - if(first_colon_pos != std::string::npos) - { - // Extract the model name after the first colon - info_v.model_name = match.str().substr(first_colon_pos + 1); - // Remove leading and trailing whitespaces - info_v.model_name = std::regex_replace( - info_v.model_name, std::regex("^\\s+|\\s+$"), ""); - } + return std::stol(_value); + } catch(std::exception& e) + { + ROCP_CI_LOG(WARNING) << fmt::format("rocprofiler-sdk agent encountered " + "error while parsing CPU info '{}': {}", + _label, + e.what()); + } + return 0; + }; + + auto value = match.back(); + + if(itr.find("vendor_id") == 0) + info_v.vendor_id = value; + else if(itr.find("model name") == 0) + { + info_v.model_name = value; + size_t first_colon_pos = value.find(':'); + // This handles the case where the model name has multiple colons + // Example "model name : AMD EPYC : 100-000000248" + if(first_colon_pos != std::string::npos) + { + // Extract the model name after the first colon + info_v.model_name = value.substr(first_colon_pos + 1); + // Remove leading and trailing whitespaces + info_v.model_name = + sdk::parse::strip(std::string{info_v.model_name}, " \t\n\v\f\r"); } - else if(itr.find("processor") == 0) - info_v.processor = std::stol(value.str()); - else if(itr.find("cpu family") == 0) - info_v.family = std::stol(value.str()); - else if(itr.find("model") == 0 && itr.find("model name") != 0) - info_v.model = std::stol(value.str()); - else if(itr.find("physical id") == 0) - info_v.physical_id = std::stol(value.str()); - else if(itr.find("core id") == 0) - info_v.core_id = std::stol(value.str()); - else if(itr.find("apicid") == 0) - info_v.apicid = std::stol(value.str()); } + else if(itr.find("processor") == 0) + info_v.processor = get_stol(value); + else if(itr.find("cpu family") == 0) + info_v.family = get_stol(value); + else if(itr.find("model") == 0 && itr.find("model name") != 0) + info_v.model = get_stol(value); + else if(itr.find("physical id") == 0) + info_v.physical_id = get_stol(value); + else if(itr.find("core id") == 0) + info_v.core_id = get_stol(value); + else if(itr.find("apicid") == 0) + info_v.apicid = get_stol(value); + } + else + { + // Each processor_block is grouped by the presence of an empty line in /proc/cpuinfo + // so no checks for empty lines are performed inside this loop. If an empty line is + // found, that should be considered an error. Entries like "power management:" with + // no info (i.e. where the ":" is the last character on the line) can be ignored + auto last_colon_pos = itr.find_last_of(':'); + ROCP_CI_LOG_IF( + INFO, last_colon_pos < itr.length() && (last_colon_pos + 1) != itr.length()) + << fmt::format("Encountered unexpected /proc/cpuinfo line format: '{}'", itr); } } + if(info_v.is_valid()) processor_info.emplace_back(info_v); else @@ -633,7 +657,19 @@ read_topology() else agent_info.model_name = ""; - if(!gpu_id_prop.empty()) agent_info.gpu_id = std::stoull(gpu_id_prop.front()); + if(!gpu_id_prop.empty()) + { + try + { + agent_info.gpu_id = std::stoull(gpu_id_prop.front()); + } catch(std::exception& e) + { + ROCP_CI_LOG(WARNING) << fmt::format("rocprofiler-sdk agent encountered error while " + "parsing gpu id property '{}': {}", + gpu_id_prop.front(), + e.what()); + } + } read_property(properties, "cpu_cores_count", agent_info.cpu_cores_count); read_property(properties, "simd_count", agent_info.simd_count); diff --git a/source/lib/rocprofiler-sdk/code_object/hsa/code_object.cpp b/source/lib/rocprofiler-sdk/code_object/hsa/code_object.cpp index 2b333c333f..2c32e5d0c6 100644 --- a/source/lib/rocprofiler-sdk/code_object/hsa/code_object.cpp +++ b/source/lib/rocprofiler-sdk/code_object/hsa/code_object.cpp @@ -33,7 +33,6 @@ #include #include #include -#include #include #include diff --git a/source/lib/rocprofiler-sdk/code_object/hsa/kernel_symbol.cpp b/source/lib/rocprofiler-sdk/code_object/hsa/kernel_symbol.cpp index 4c86368f84..6d28ed6419 100644 --- a/source/lib/rocprofiler-sdk/code_object/hsa/kernel_symbol.cpp +++ b/source/lib/rocprofiler-sdk/code_object/hsa/kernel_symbol.cpp @@ -33,7 +33,6 @@ #include #include #include -#include #include #include diff --git a/source/lib/rocprofiler-sdk/registration.cpp b/source/lib/rocprofiler-sdk/registration.cpp index 22b899e452..e02f14c502 100644 --- a/source/lib/rocprofiler-sdk/registration.cpp +++ b/source/lib/rocprofiler-sdk/registration.cpp @@ -310,7 +310,9 @@ find_clients() if(fs::exists(itr) && resolved_exists(itr)) { auto elfinfo = common::elf_utils::read(itr); - if(!elfinfo.has_symbol(std::regex{"^rocprofiler_configure$"})) + if(!elfinfo.has_symbol([](std::string_view symname) { + return (symname == "rocprofiler_configure"); + })) { ROCP_CI_LOG(WARNING) << fmt::format( "[ROCP_TOOL_LIBRARIES] rocprofiler-sdk tool library '{}' did not " @@ -379,7 +381,9 @@ find_clients() if(fs::exists(itr) && resolved_exists(itr)) { auto elfinfo = common::elf_utils::read(itr); - if(!elfinfo.has_symbol(std::regex{"^rocprofiler_configure$"})) + if(!elfinfo.has_symbol([](std::string_view symname) { + return (symname == "rocprofiler_configure"); + })) { ROCP_INFO << fmt::format( "Shared library '{}' did not contain the 'rocprofiler_configure' symbol " diff --git a/source/lib/tests/common/CMakeLists.txt b/source/lib/tests/common/CMakeLists.txt index 199da21699..b7fbd1b34b 100644 --- a/source/lib/tests/common/CMakeLists.txt +++ b/source/lib/tests/common/CMakeLists.txt @@ -5,7 +5,7 @@ project(rocprofiler-sdk-tests-common LANGUAGES C CXX) include(GoogleTest) -set(common_sources demangling.cpp environment.cpp md5sum.cpp mpl.cpp) +set(common_sources demangling.cpp environment.cpp md5sum.cpp mpl.cpp parse.cpp) add_executable(common-tests) target_sources(common-tests PRIVATE ${common_sources}) diff --git a/source/lib/tests/common/parse.cpp b/source/lib/tests/common/parse.cpp new file mode 100644 index 0000000000..5e3b63c85f --- /dev/null +++ b/source/lib/tests/common/parse.cpp @@ -0,0 +1,70 @@ +// MIT License +// +// Copyright (c) 2023-2025 Advanced Micro Devices, Inc. All rights reserved. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +#include + +#include +#include + +#include +#include +#include +#include + +namespace sdk = ::rocprofiler::sdk; + +TEST(parse, tokenize_characters) +{ + auto tokenized = sdk::parse::tokenize(std::string_view{"0, GPU-DEADBEEFDEADBEEF,, -1"}, ", "); + + ASSERT_EQ(tokenized.size(), 3) + << fmt::format("TOKENIZED: '{}'", fmt::join(tokenized.begin(), tokenized.end(), "' | '")); + EXPECT_EQ(tokenized.at(0), "0"); + EXPECT_EQ(tokenized.at(1), "GPU-DEADBEEFDEADBEEF"); + EXPECT_EQ(tokenized.at(2), "-1"); +} + +TEST(parse, tokenize_strings) +{ + auto tokenized = sdk::parse::tokenize(std::string_view{"0, GPU-DEADBEEFDEADBEEF, -1, 8"}, + std::vector{", "}); + + ASSERT_EQ(tokenized.size(), 4) + << fmt::format("TOKENIZED: '{}'", fmt::join(tokenized.begin(), tokenized.end(), "' | '")); + EXPECT_EQ(tokenized.at(0), "0"); + EXPECT_EQ(tokenized.at(1), "GPU-DEADBEEFDEADBEEF"); + EXPECT_EQ(tokenized.at(2), "-1"); + EXPECT_EQ(tokenized.at(3), "8"); +} + +TEST(parse, strip) +{ + auto test_message = std::string{" \t\v\f TEST MESSAGE\n\r\n "}; + auto stripped_message = sdk::parse::strip(std::string{test_message}, " \t\n\v\f\r"); + auto expected_message = std::string{"TEST MESSAGE"}; + + EXPECT_EQ(stripped_message, expected_message) + << fmt::format("Expected '{}' after stripping '{}'\nResult: '{}'", + expected_message, + test_message, + stripped_message); +}