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/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); \ } 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 6afb673266e7..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 @@ -37,8 +37,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -293,6 +295,7 @@ auto _activate = // int main(int argc, char** argv) +try { argv0 = argv[0]; @@ -2886,6 +2889,10 @@ main(int argc, char** argv) log_ofs->close(); } return code; +} catch(const std::exception& err) +{ + 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..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 @@ -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,12 @@ 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"); - + // Empty _cmdc is rejected in main before this helper is called. 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]); } diff --git a/projects/rocprofiler-systems/tests/pytest/test_binaries.py b/projects/rocprofiler-systems/tests/pytest/test_binaries.py index eb39f9c5a7c6..eb96da08ae32 100644 --- a/projects/rocprofiler-systems/tests/pytest/test_binaries.py +++ b/projects/rocprofiler-systems/tests/pytest/test_binaries.py @@ -384,6 +384,106 @@ 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, 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), "--", source_bin], + 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}") + + # 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), "1", "1", "1"], + fail_on_not_found=True, + ) + self.assert_regex(kept) + # ============================================================================ # rocprof-sys-avail tests