Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wrap rr macros restart and jump in python #3867

Merged
merged 1 commit into from
Nov 3, 2024

Conversation

GitMensch
Copy link
Contributor

fixes #3859 warnings during startup

@GitMensch
Copy link
Contributor Author

Looks like it is working - CI fails with an unrelated error during build:

/tmp/actions-runner/_work/rr/rr/src/test/open_null.c: In function ‘main’:
/tmp/actions-runner/_work/rr/rr/src/test/open_null.c:8:20: error: ‘SYS_open’ undeclared (first use in this function); did you mean ‘SYS_fsopen’?
    8 |   int fd = syscall(SYS_open, NULL, O_RDONLY);
      |                    ^~~~~~~~
      |                    SYS_fsopen
/tmp/actions-runner/_work/rr/rr/src/test/open_null.c:8:20: note: each undeclared identifier is reported only once for each function it appears in

@rocallahan rocallahan merged commit 663476c into rr-debugger:master Nov 3, 2024
3 of 5 checks passed
@sidkshatriya
Copy link
Contributor

This commit causes a regression on both Fedora 40 (has gdb 15.2-1.fc40) and Fedora 41 (has gdb 15.2-1.fc41) .

A few tests start failing. e.g. ctest -R checkpoint_mmap_shared. Reverting this specific commit causes the tests to pass again.

@GitMensch
Copy link
Contributor Author

Can you please give some details on the failure? "Doesn't work" is hardly something I can work with :-/

@user202729
Copy link

user202729 commented Nov 4, 2024

I think the problem is you're having \n in C++ code, which passes the literal newline to gdb, which of course will break because then the python command will just be the first line.

Replace \n with \\n might work (or split the whole thing to multiple lines and use triple-quote, see e.g. the Python block below), but I haven't tested locally.

Maybe you need a trailing \n on each python command too.

(On another note, does GitHub Actions not run on Fedora?)

p/s: sorry, didn't have time to send a PR myself.

@sidkshatriya
Copy link
Contributor

sidkshatriya commented Nov 4, 2024

@GitMensch

Can you please give some details on the failure? "Doesn't work" is hardly something I can work with :-/

Sorry -- I didn't have time to debug the issue. I simply reverted the commit and saw the tests working again. Are you able to run the test checkpoint_mmap_shared on your local machine ? If the test fails for you also I think you will be able to then debug the error.

@sidkshatriya
Copy link
Contributor

sidkshatriya commented Nov 4, 2024

@user202729

(On another note, does GitHub Actions not run on Fedora?)

Looking at the "Details" button where this commit was merged it does not seem the CI tests were run at all.

@GitMensch
Here is the error taken from the gdb log (for ctest -R checkpoint_mmap_shared)

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /tmp/rr-test-checkpoint_mmap_shared-VbXYvN2mk/mmap_shared-VbXYvN2mk-0/mmap_hardlink_3_mmap_shared-VbXYvN2mk..
  File "<string>", line 1
    gdb.execute('define jump
                ^
SyntaxError: unterminated string literal (detected at line 1)
/proc/4133/fd/5:180: Error in sourced command file:
Error occurred in Python: unterminated string literal (detected at line 1) (<string>, line 1)
...
...

@GitMensch
Copy link
Contributor Author

Ah, yes, that makes sense: executing python gdb.execute('define restart\nrun c$arg0\nend') in GDB works fine - but we need to escape the \n for the C++ literal. I'm working on that now.

@sidkshatriya
Copy link
Contributor

@user202729 was correct in their analysis. The following diff causes the test to pass again.

--- a/src/launch_debugger.cc
+++ b/src/launch_debugger.cc
@@ -50,8 +50,8 @@ static const string& gdb_rr_macros() {
     ss << DebuggerExtensionCommandHandler::gdb_macros()
        // gdb warns about redefining inbuilt commands, silence that by
        // wrapping it in python code
-       << "python gdb.execute('define jump\nrr-denied jump\nend')"
-       << "python gdb.execute('define restart\nrun c$arg0\nend')"
+       << "python gdb.execute('define jump\\nrr-denied jump\\nend')\n"
+       << "python gdb.execute('define restart\\nrun c$arg0\\nend')\n"
        << "document restart\n"
        << "restart at checkpoint N\n"
        << "checkpoints are created with the 'checkpoint' command\n"

GitMensch added a commit to GitMensch/rr that referenced this pull request Nov 4, 2024
missing CXX string escape for \n
GitMensch added a commit to GitMensch/rr that referenced this pull request Nov 4, 2024
missing CXX string escape for \n
GitMensch added a commit to GitMensch/rr that referenced this pull request Nov 4, 2024
missing CXX string escape for \n
@sidkshatriya
Copy link
Contributor

sidkshatriya commented Nov 4, 2024

@GitMensch The commit that you added to GitMensch@9f7c2fc will not work as you need a \n at the end of the C++ string. Please see the diff that I posted above.


Nevermind -- I see you added that in GitMensch@fae42c2 -- Cool !

@GitMensch
Copy link
Contributor Author

Should be fixed with #3869 ... which in the end ended with exactly the diff from above.

rocallahan pushed a commit that referenced this pull request Nov 4, 2024
missing CXX string escape for \n
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spurious "Really redefine built-in command" logged on screen
4 participants