From dc9edd7daa1c4020b1f083e86b7913e4ceec0c45 Mon Sep 17 00:00:00 2001 From: Anuj Shukla Date: Fri, 16 Oct 2026 08:05:51 -0400 Subject: [PATCH 1/5] fix(rocprofiler-systems): exit cleanly instead of SIGABRT on invalid rocprof-sys-instrument input --- .../rocprof-sys-instrument.cpp | 33 ++++++- .../tests/pytest/test_binaries.py | 99 +++++++++++++++++++ 2 files changed, 128 insertions(+), 4 deletions(-) diff --git a/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.cpp b/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.cpp index 2ccce9bbb4b5..3b2938f7abf8 100644 --- a/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.cpp +++ b/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.cpp @@ -37,8 +37,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -287,12 +289,14 @@ auto _activate = //======================================================================================// // -// entry point +// instrument implementation // //======================================================================================// // +namespace +{ int -main(int argc, char** argv) +run(int argc, char** argv) { argv0 = argv[0]; @@ -1185,8 +1189,9 @@ main(int argc, char** argv) // mutname was resolved against the search paths in the argument pre-scan if(!path::is_regular_file(mutname)) { - fprintf(stderr, "Error! Application '%s' does not exist or is not a regular " - "file. Nothing to instrument.\n", + fprintf(stderr, + "Error! Application '%s' does not exist or is not a regular " + "file. Nothing to instrument.\n", _cmdv[0]); return EXIT_FAILURE; } @@ -2886,6 +2891,26 @@ main(int argc, char** argv) } return code; } +} // namespace + +//======================================================================================// +// +// entry point +// +//======================================================================================// +// +int +main(int argc, char** argv) +{ + try + { + return run(argc, argv); + } catch(const std::exception& ex) + { + std::cerr << "[rocprof-sys] error: " << ex.what() << '\n'; + return EXIT_FAILURE; + } +} //======================================================================================// // query_instr -- check whether there are one or more instrumentation points diff --git a/projects/rocprofiler-systems/tests/pytest/test_binaries.py b/projects/rocprofiler-systems/tests/pytest/test_binaries.py index eb39f9c5a7c6..293941b0c868 100644 --- a/projects/rocprofiler-systems/tests/pytest/test_binaries.py +++ b/projects/rocprofiler-systems/tests/pytest/test_binaries.py @@ -24,6 +24,14 @@ def get_sleep_cmd() -> str: return sleep_cmd +def get_true_cmd() -> str: + """Return the path to true, or skip the test if it isn't on PATH.""" + true_cmd = shutil.which("true") + if not true_cmd: + pytest.skip("true command not found") + return true_cmd + + # ============================================================================ # Avail format consistency data # ============================================================================ @@ -384,6 +392,97 @@ def test_max_library_functions(self): ) self.assert_regex(result, pass_regex=pass_regex) + def _assert_usage_error(self, result, pass_regex): + self.assert_regex( + result, + pass_regex=pass_regex, + fail_regex=[ + r"terminate called after throwing", + r"Signal 6 caught", + r"Killing process", + ], + use_abort_fail_regex=False, + ) + if result.returncode != 1: + pytest.fail(f"invalid input should exit 1, got {result.returncode}") + + @pytest.mark.timeout(60) + def test_invalid_input_non_elf(self, test_output_dir): + """A non-ELF text file is a usage error and must exit 1, not SIGABRT.""" + bad_file = test_output_dir / "not-an-elf.txt" + bad_file.write_text("This is not an ELF binary.\n") + + result = self.run_test( + "baseline", + target=self.target, + run_args=["--", str(bad_file)], + fail_on_not_found=True, + fail_on_pass=True, + ) + self._assert_usage_error( + result, + [r"is a text file", r"only supports instrumenting binary files"], + ) + + @pytest.mark.timeout(60) + def test_invalid_input_missing_target(self, test_output_dir): + """A missing target is a usage error and must exit 1, not SIGABRT.""" + missing = test_output_dir / "does-not-exist" + + result = self.run_test( + "baseline", + target=self.target, + run_args=["--", str(missing)], + fail_on_not_found=True, + fail_on_pass=True, + ) + self._assert_usage_error( + result, [r"does not exist or is not a regular file\. Nothing to instrument"] + ) + + @pytest.mark.timeout(180) + def test_invalid_input_reinstrument(self, test_output_dir): + """Reject a second rewrite, and leave the first instrumented binary runnable.""" + true_cmd = get_true_cmd() + first_inst = test_output_dir / "first.inst" + second_inst = test_output_dir / "second.inst" + + baseline = self.run_test( + "baseline", + target=self.target, + run_args=["-o", str(first_inst), "--", true_cmd], + working_directory=test_output_dir, + fail_on_not_found=True, + ) + self.assert_regex(baseline) + self.assert_file_exists(first_inst) + + rejected = self.run_test( + "baseline", + target=self.target, + run_args=["-o", str(second_inst), "--", str(first_inst)], + working_directory=test_output_dir, + fail_on_pass=True, + fail_on_not_found=True, + ) + self._assert_usage_error( + rejected, + [ + r"unable to open/reinstrument previously instrumented binary", + r"Failed to open binary", + ], + ) + if second_inst.exists(): + pytest.fail(f"rejected rewrite wrote {second_inst}") + + kept = self.run_test( + "baseline", + target="rocprof-sys-run", + run_args=["--", str(first_inst)], + fail_on_not_found=True, + ) + self.assert_regex(kept) + # ============================================================================ # rocprof-sys-avail tests From 711468c0a465c0d4763276ac5e12e95ac16d1c1f Mon Sep 17 00:00:00 2001 From: Anuj Shukla Date: Mon, 19 Oct 2026 07:00:47 -0400 Subject: [PATCH 2/5] Adressed test failure --- .../rocprof-sys-instrument.cpp | 8 +++--- .../tests/pytest/test_binaries.py | 27 ++++++++++--------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.cpp b/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.cpp index 3b2938f7abf8..f9b08b1fbf9b 100644 --- a/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.cpp +++ b/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.cpp @@ -296,7 +296,7 @@ auto _activate = namespace { int -run(int argc, char** argv) +run_instrumentation(int argc, char** argv) { argv0 = argv[0]; @@ -2904,10 +2904,10 @@ main(int argc, char** argv) { try { - return run(argc, argv); - } catch(const std::exception& ex) + return run_instrumentation(argc, argv); + } catch(const std::exception& err) { - std::cerr << "[rocprof-sys] error: " << ex.what() << '\n'; + std::cerr << "[rocprof-sys] error: " << err.what() << '\n'; return EXIT_FAILURE; } } diff --git a/projects/rocprofiler-systems/tests/pytest/test_binaries.py b/projects/rocprofiler-systems/tests/pytest/test_binaries.py index 293941b0c868..eb96da08ae32 100644 --- a/projects/rocprofiler-systems/tests/pytest/test_binaries.py +++ b/projects/rocprofiler-systems/tests/pytest/test_binaries.py @@ -24,14 +24,6 @@ def get_sleep_cmd() -> str: return sleep_cmd -def get_true_cmd() -> str: - """Return the path to true, or skip the test if it isn't on PATH.""" - true_cmd = shutil.which("true") - if not true_cmd: - pytest.skip("true command not found") - return true_cmd - - # ============================================================================ # Avail format consistency data # ============================================================================ @@ -441,16 +433,24 @@ def test_invalid_input_missing_target(self, test_output_dir): ) @pytest.mark.timeout(180) - def test_invalid_input_reinstrument(self, test_output_dir): - """Reject a second rewrite, and leave the first instrumented binary runnable.""" - true_cmd = get_true_cmd() + def test_invalid_input_reinstrument(self, test_output_dir, rocprof_config): + """Reject a second rewrite, and leave the first instrumented binary runnable. + + Use a built ELF example rather than /usr/bin/true: on RHEL that path is a + text coreutils wrapper, so instrumentation refuses it before rewrite. + """ + try: + source_bin = str(rocprof_config.get_target_executable("parallel-overhead")) + except FileNotFoundError: + pytest.skip("parallel-overhead example not built") + first_inst = test_output_dir / "first.inst" second_inst = test_output_dir / "second.inst" baseline = self.run_test( "baseline", target=self.target, - run_args=["-o", str(first_inst), "--", true_cmd], + run_args=["-o", str(first_inst), "--", source_bin], working_directory=test_output_dir, fail_on_not_found=True, ) @@ -475,10 +475,11 @@ def test_invalid_input_reinstrument(self, test_output_dir): if second_inst.exists(): pytest.fail(f"rejected rewrite wrote {second_inst}") + # Keep the instrumented binary runnable; tiny args so the check stays fast. kept = self.run_test( "baseline", target="rocprof-sys-run", - run_args=["--", str(first_inst)], + run_args=["--", str(first_inst), "1", "1", "1"], fail_on_not_found=True, ) self.assert_regex(kept) From 396c573e019bf9d9c2376373d99782fb5fffdffa Mon Sep 17 00:00:00 2001 From: Anuj Shukla Date: Tue, 20 Oct 2026 00:23:33 -0400 Subject: [PATCH 3/5] Adressed review comments --- .../rocprof-sys-instrument.cpp | 29 ++++--------------- .../rocprof-sys-instrument.hpp | 6 ++-- 2 files changed, 9 insertions(+), 26 deletions(-) diff --git a/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.cpp b/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.cpp index f9b08b1fbf9b..133939660c99 100644 --- a/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.cpp +++ b/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.cpp @@ -289,14 +289,13 @@ auto _activate = //======================================================================================// // -// instrument implementation +// entry point // //======================================================================================// // -namespace -{ int -run_instrumentation(int argc, char** argv) +main(int argc, char** argv) +try { argv0 = argv[0]; @@ -2890,26 +2889,10 @@ run_instrumentation(int argc, char** argv) log_ofs->close(); } return code; -} -} // namespace - -//======================================================================================// -// -// entry point -// -//======================================================================================// -// -int -main(int argc, char** argv) +} catch(const std::exception& err) { - try - { - return run_instrumentation(argc, argv); - } catch(const std::exception& err) - { - std::cerr << "[rocprof-sys] error: " << err.what() << '\n'; - return EXIT_FAILURE; - } + std::cerr << "[rocprof-sys] error: " << err.what() << '\n'; + return EXIT_FAILURE; } //======================================================================================// diff --git a/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.hpp b/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.hpp index c05cf8093d43..3422d1de3da3 100644 --- a/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.hpp +++ b/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.hpp @@ -162,7 +162,7 @@ rocprofsys_get_address_space(patch_pointer_t const& _bpatch, int _cmdc, char** _ { errprintf(-127, "'%s' is a text file. rocprof-sys only supports instrumenting " - "binary files", + "binary files\n", _name.c_str()); } @@ -214,13 +214,13 @@ rocprofsys_get_address_space(patch_pointer_t const& _bpatch, int _cmdc, char** _ verbprintf(4, "[env] %s=%s\n", itr.first.c_str(), itr.second.c_str()); } - if(_cmdc < 1) errprintf(-127, "No command provided"); + if(_cmdc < 1) errprintf(-127, "No command provided\n"); if(is_text_file(_cmdv[0])) { errprintf(-1, "'%s' is a text file. rocprof-sys only supports instrumenting " - "binary files", + "binary files\n", _cmdv[0]); } From a24df6ba30f2f844a699d2724b93bf1b179d67fe Mon Sep 17 00:00:00 2001 From: Anuj Shukla Date: Tue, 20 Oct 2026 00:23:33 -0400 Subject: [PATCH 4/5] Adressed review comments --- .../source/bin/rocprof-sys-instrument/fwd.hpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/fwd.hpp b/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/fwd.hpp index 4cba8ad40668..6d3bcd8d4dfb 100644 --- a/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/fwd.hpp +++ b/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/fwd.hpp @@ -250,11 +250,8 @@ extern std::unique_ptr log_ofs; "[rocprof-sys][exe] Error! " __VA_ARGS__); \ throw std::runtime_error(std::string{ _buff.data() }); \ } \ - else \ - { \ - if(debug_print || verbose_level >= LEVEL) \ - fprintf(stderr, "[rocprof-sys][exe] Warning! " __VA_ARGS__); \ - } \ + if(debug_print || verbose_level >= LEVEL) \ + fprintf(stderr, "[rocprof-sys][exe] Warning! " __VA_ARGS__); \ fflush(stderr); \ } From 794327987a3bd1abaef5fd8ac9663d0db354b76a Mon Sep 17 00:00:00 2001 From: Anuj Shukla Date: Wed, 21 Oct 2026 05:36:18 -0400 Subject: [PATCH 5/5] Adressed review comment part-2 --- projects/rocprofiler-systems/CHANGELOG.md | 5 +++++ .../bin/rocprof-sys-instrument/rocprof-sys-instrument.hpp | 3 +-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/projects/rocprofiler-systems/CHANGELOG.md b/projects/rocprofiler-systems/CHANGELOG.md index 7eecc0507411..0ed1d84754c1 100644 --- a/projects/rocprofiler-systems/CHANGELOG.md +++ b/projects/rocprofiler-systems/CHANGELOG.md @@ -29,6 +29,11 @@ Full documentation for ROCm Systems Profiler is available at [https://rocm.docs. ### Resolved issues +- Fixed `rocprof-sys-instrument` aborting with `SIGABRT` on invalid input (non-ELF text + file, missing target, or already-instrumented binary). Uncaught `std::runtime_error` + exceptions from instrumentation setup are now caught in `main`, reported as + `[rocprof-sys] error: ...`, and the process exits with status 1 instead of + terminating. - Fixed a crash (`SIGSEGV`) in `rocprof-sys-instrument` when instrumenting Fortran+HIP programs compiled with `gfortran -g` or `-g -O0`. gfortran encodes assumed-shape and allocatable array bounds as DWARF location expression blocks rather than integer diff --git a/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.hpp b/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.hpp index 3422d1de3da3..2c47db845f11 100644 --- a/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.hpp +++ b/projects/rocprofiler-systems/source/bin/rocprof-sys-instrument/rocprof-sys-instrument.hpp @@ -214,8 +214,7 @@ rocprofsys_get_address_space(patch_pointer_t const& _bpatch, int _cmdc, char** _ verbprintf(4, "[env] %s=%s\n", itr.first.c_str(), itr.second.c_str()); } - if(_cmdc < 1) errprintf(-127, "No command provided\n"); - + // Empty _cmdc is rejected in main before this helper is called. if(is_text_file(_cmdv[0])) { errprintf(-1,