This is an archive of the discontinued LLVM Phabricator instance.

[lldb] [gdb-remote] Include PID in vCont packets if multiprocess
ClosedPublic

Authored by mgorny on Aug 12 2022, 3:41 AM.

Details

Summary

Try to always send vCont packets and include the PID in them if running
multiprocess. This is necessary to ensure that with the upcoming full
multiprocess support always resumes the correct process without having
to resort to the legacy Hc packets.

Sponsored by: The FreeBSD Foundation

Diff Detail

Event Timeline

mgorny created this revision.Aug 12 2022, 3:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2022, 3:41 AM
Herald added a subscriber: arichardson. · View Herald Transcript
mgorny requested review of this revision.Aug 12 2022, 3:41 AM
labath accepted this revision.Aug 18 2022, 11:47 AM
labath added inline comments.
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
1189–1192

If we're going to have these in more places, we should add a helper function to format a pid.tid pair.

1190

Is this really necessary. If I recall correctly, we go through a lot of trouble to come up with some pid value (resort to hardcoding pid to 1 if everything fails)..

This revision is now accepted and ready to land.Aug 18 2022, 11:47 AM
mgorny marked 2 inline comments as done.Aug 18 2022, 11:08 PM
mgorny added inline comments.
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
1189–1192

I'll bear that in mind.

1190

Probably not. I've guessed maybe it could happen somewhat early in the process but I couldn't really reproduce it ever.

This revision was automatically updated to reflect the committed changes.
mgorny marked 2 inline comments as done.
Herald added a project: Restricted Project. · View Herald TranscriptAug 19 2022, 12:02 AM

This seems to cause all API tests to time out.

See LLDB Incremental buildbot: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46215/execution/node/70/log/

Can we revert this until we know what the root cause is?

This seems to cause all API tests to time out.

See LLDB Incremental buildbot: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46215/execution/node/70/log/

Can we revert this until we know what the root cause is?

I reverted this and the follow-up commit that disables a test on Windows for now. Please let us know if you need any help in diagnosing the problem!

This seems to cause all API tests to time out.

See LLDB Incremental buildbot: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46215/execution/node/70/log/

Can we revert this until we know what the root cause is?

I reverted this and the follow-up commit that disables a test on Windows for now. Please let us know if you need any help in diagnosing the problem!

Help would be most welcome. Unless my grep skills are failing me, the log doesn't contain anything but timeouts. I'd use at least some gdb-remote log. My only guess right now would be that debugserver doesn't support vCont;c:-1.

Alternatively, could you try reapplying the original commit but changing:

continue_packet.Format("vCont;c:{0}-1", pid_prefix);

back to:

continue_packet.PutCString("c");
Michael137 added a comment.EditedAug 19 2022, 9:58 AM

This seems to cause all API tests to time out.

See LLDB Incremental buildbot: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46215/execution/node/70/log/

Can we revert this until we know what the root cause is?

I reverted this and the follow-up commit that disables a test on Windows for now. Please let us know if you need any help in diagnosing the problem!

Help would be most welcome. Unless my grep skills are failing me, the log doesn't contain anything but timeouts. I'd use at least some gdb-remote log. My only guess right now would be that debugserver doesn't support vCont;c:-1.

Alternatively, could you try reapplying the original commit but changing:

continue_packet.Format("vCont;c:{0}-1", pid_prefix);

back to:

continue_packet.PutCString("c");

Can confirm this fixes the timeouts.
I'll revert for now and let you push the fix.

This seems to cause all API tests to time out.

See LLDB Incremental buildbot: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46215/execution/node/70/log/

Can we revert this until we know what the root cause is?

I reverted this and the follow-up commit that disables a test on Windows for now. Please let us know if you need any help in diagnosing the problem!

Help would be most welcome. Unless my grep skills are failing me, the log doesn't contain anything but timeouts. I'd use at least some gdb-remote log. My only guess right now would be that debugserver doesn't support vCont;c:-1.

Alternatively, could you try reapplying the original commit but changing:

continue_packet.Format("vCont;c:{0}-1", pid_prefix);

back to:

continue_packet.PutCString("c");

@mgorny

Snippet of test log with timeout:

********************
UNSUPPORTED: lldb-api :: commands/expression/call-restarts/TestCallThatRestarts.py (64 of 1755)
TIMEOUT: lldb-api :: sample_test/TestSampleInlineTest.py (65 of 1755)
******************** TEST 'lldb-api :: sample_test/TestSampleInlineTest.py' FAILED ********************
Script:
--
/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.8/bin/python3.8 /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --codesign-identity lldb_codesign --arch x86_64 --build-dir /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex -t --env TERM=vt100 --env LLVM_LIBS_DIR=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./lib --env LLVM_INCLUDE_DIR=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/include --env LLVM_TOOLS_DIR=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./bin --hermetic-libcxx --arch x86_64 --build-dir /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex --lldb-module-cache-dir /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./bin/lldb --compiler /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./bin/clang --dsymutil /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./bin/dsymutil --llvm-tools-dir /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./bin --lldb-libs-dir /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./lib /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/API/sample_test -p TestSampleInlineTest.py
--
Exit Code: -9
Timeout: Reached timeout of 600 seconds
mgorny reopened this revision.Aug 19 2022, 10:36 AM
This revision is now accepted and ready to land.Aug 19 2022, 10:36 AM
mgorny updated this revision to Diff 454058.Aug 19 2022, 10:37 AM

@aprantl, @Michael137, could you confirm that this version works correctly?

Michael137 accepted this revision.Aug 19 2022, 11:33 AM

@aprantl, @Michael137, could you confirm that this version works correctly?

ninja check-lldb-api runs fine with your patch

This revision was landed with ongoing or failed builds.Aug 19 2022, 11:21 PM
This revision was automatically updated to reflect the committed changes.

@aprantl, why did you revert d38985a36be8b0165787f8893b3d2b0f831d1e83? This change had nothing to do with this patch, and you broke Windows buildbot again. If you really can't wait a few hours for me to able to show up, I'd really appreciate if you took responsibility for the things you've broken.

Michael137 added a comment.EditedAug 20 2022, 1:06 AM

@aprantl, why did you revert d38985a36be8b0165787f8893b3d2b0f831d1e83? This change had nothing to do with this patch, and you broke Windows buildbot again. If you really can't wait a few hours for me to able to show up, I'd really appreciate if you took responsibility for the things you've broken.

Apologies, there was some miscommunication around which commit to revert. I should’ve checked the buildbot again