[Cmake][Sys] print backtrace if POSIX signal is catched#1692
[Cmake][Sys] print backtrace if POSIX signal is catched#1692tdavidcl merged 21 commits intoShamrock-code:mainfrom
Conversation
|
Thanks @tdavidcl for opening this PR! You can do multiple things directly here: Once the workflow completes a message will appear displaying informations related to the run. Also the PR gets automatically reviewed by gemini, you can: |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the system's debugging capabilities by integrating the Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request integrates cpptrace to provide more detailed backtraces when a POSIX signal is caught, which is a valuable addition for debugging. However, the current implementation introduces a significant security risk by using non-async-signal-safe functions like fmt::format, std::string, and std::cout within the signal handler. This can lead to deadlocks, undefined behavior, and potential denial of service. Additionally, there's a minor typo in a CMake comment and duplicated code in SignalCatch.cpp that should be refactored for maintainability.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature to print a backtrace when a POSIX signal is caught, which is very helpful for debugging crashes. It integrates the cpptrace library as a submodule and updates the signal handling logic to generate and display/save a detailed crash report. However, the implementation has significant security concerns: the signal handler uses non-async-signal-safe functions, potentially leading to deadlocks and Denial of Service, and the crash report file creation is vulnerable to path traversal and symlink attacks due to an unsanitized environment variable for the output path. Additionally, the robustness of the static formatter object initialization should be improved to avoid potential issues during program startup. Addressing these points will enhance both the security and stability of the application's error handling.
|
AI will never shut up even when asked to about safety … |
|
this PR will wait for jeremy-rifkin/cpptrace#321 before changing the submodule to the resulting commit and merge |
Workflow reportworkflow report corresponding to commit bc59fe6 Pre-commit check reportPre-commit check: ✅ Test pipeline can run. Clang-tidy diff reportNo relevant changes found. You should now go back to your normal life and enjoy a hopefully sunny day while waiting for the review. Doxygen diff with
|
No description provided.