[Lldb-commits] [PATCH] D158237: Change LLGSTest.cpp to run environment_check inferior with stdin/stdout disabled, to work around ASAN CI bot issue
Jason Molenda via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Thu Aug 17 17:22:02 PDT 2023
jasonmolenda created this revision.
jasonmolenda added a reviewer: labath.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.
We have an ASAN+UBSAN bot for lldb with swift supported added on swift.org - https://ci.swift.org/view/LLDB/job/oss-lldb-asan-macos/ - and it has been failing for a few months, it seems. The failing test is the LLDBServerTests. There is a test that is setting an environment variable and running lldb/unittests/tools/lldb-server/inferior/environment_check.cpp to confirm (via the exit value) that the env var was set. The test in lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp launches the app, continues and expects it to exit:
ASSERT_THAT_ERROR(Client.ContinueAll(), Succeeded());
ASSERT_THAT_EXPECTED(
Client.GetLatestStopReplyAs<StopReplyExit>(),
HasValue(testing::Property(&StopReply::getKind,
WaitStatus{WaitStatus::Exit, 0})));
This ContinueAll ends up in unittests/tools/lldb-server/tests/TestClient.cpp `TestClient::Continue()` which sends a continue packet, and expects to see either a stop reply packet or a process-exited response,
[...]
auto StopReplyOr = SendMessage<StopReply>(
message, m_process_info->GetEndian(), m_register_infos);
if (!StopReplyOr)
return StopReplyOr.takeError();
m_stop_reply = std::move(*StopReplyOr);
if (!isa<StopReplyStop>(m_stop_reply)) {
StringExtractorGDBRemote R;
PacketResult result = ReadPacket(R, GetPacketTimeout(), false);
[...]
When run on the UBSan bot, the inferior (environment_check) is also built ASAN and while it is running, asan or a malloc library prints a note to stderr, `environment_check(41292,0x113e7a600) malloc: nano zone abandoned due to inability to preallocate reserved vm space.`. When we look at the bot logging, we see the problem:
[ INFO ] /Users/ec2-user/jenkins/workspace/oss-lldb-asan-macos/llvm-project/lldb/unittests/tools/lldb-server/tests/TestClient.cpp:201:: Send Packet: vCont;c
[ INFO ] /Users/ec2-user/jenkins/workspace/oss-lldb-asan-macos/llvm-project/lldb/unittests/tools/lldb-server/tests/TestClient.cpp:204:: Read Packet: O656e7669726f6e6d656e745f636865636b2833323338352c307831313535623336303029206d616c6c6f633a206e616e6f207a6f6e65206162616e646f6e65642064756520746f20696e6162696c69747920746f20707265616c6c6f6361746520726573657276656420766d2073706163652e0d0a
/Users/ec2-user/jenkins/workspace/oss-lldb-asan-macos/llvm-project/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp:50: Failure
Value of: llvm::detail::TakeError(Client.ContinueAll())
Expected: succeeded
Actual: failed (Unable to parse StopReply: Invalid packet)
We sent a `vCont;c` to continue, and some stdout/stderr text was received while it was running. And the next packet we receive is almost certainly the "process has exited successfully" result, and the check in `TestClient::Continue()` would have succeeded.
There's no stdin/stdout/stderr needed with environment_check, so the easiest fix I'd like to try is to have it launched with these channels not sent up to lldb at all. We should see the vCont;c followed by the "process exited successfully" packet and everyone will be happy.
This test is originally @labath but I don't want to assume he'll have time to look at this. Let's see if anyone has an opinion, Pavel don't feel obligated to weigh in unless you have time/want to.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D158237
Files:
lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
lldb/unittests/tools/lldb-server/tests/TestClient.cpp
lldb/unittests/tools/lldb-server/tests/TestClient.h
Index: lldb/unittests/tools/lldb-server/tests/TestClient.h
===================================================================
--- lldb/unittests/tools/lldb-server/tests/TestClient.h
+++ lldb/unittests/tools/lldb-server/tests/TestClient.h
@@ -48,8 +48,9 @@
/// using this for generic tests, as the two stubs have different
/// command-line interfaces.
static llvm::Expected<std::unique_ptr<TestClient>>
- launchCustom(llvm::StringRef Log, llvm::ArrayRef<llvm::StringRef> ServerArgs, llvm::ArrayRef<llvm::StringRef> InferiorArgs);
-
+ launchCustom(llvm::StringRef Log, bool disable_stdio,
+ llvm::ArrayRef<llvm::StringRef> ServerArgs,
+ llvm::ArrayRef<llvm::StringRef> InferiorArgs);
~TestClient() override;
llvm::Error SetInferior(llvm::ArrayRef<std::string> inferior_args);
Index: lldb/unittests/tools/lldb-server/tests/TestClient.cpp
===================================================================
--- lldb/unittests/tools/lldb-server/tests/TestClient.cpp
+++ lldb/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -59,10 +59,13 @@
}
Expected<std::unique_ptr<TestClient>> TestClient::launch(StringRef Log, ArrayRef<StringRef> InferiorArgs) {
- return launchCustom(Log, {}, InferiorArgs);
+ return launchCustom(Log, false, {}, InferiorArgs);
}
-Expected<std::unique_ptr<TestClient>> TestClient::launchCustom(StringRef Log, ArrayRef<StringRef> ServerArgs, ArrayRef<StringRef> InferiorArgs) {
+Expected<std::unique_ptr<TestClient>>
+TestClient::launchCustom(StringRef Log, bool disable_stdio,
+ ArrayRef<StringRef> ServerArgs,
+ ArrayRef<StringRef> InferiorArgs) {
const ArchSpec &arch_spec = HostInfo::GetArchitecture();
Args args;
args.AppendArgument(LLDB_SERVER);
@@ -111,6 +114,8 @@
// Accept().
Info.SetMonitorProcessCallback(&ProcessLaunchInfo::NoOpMonitorCallback);
+ if (disable_stdio)
+ Info.GetFlags().Set(lldb::eLaunchFlagDisableSTDIO);
status = Host::LaunchProcess(Info);
if (status.Fail())
return status.ToError();
Index: lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
===================================================================
--- lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
+++ lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
@@ -41,9 +41,10 @@
// Test that --env takes precedence over inherited environment variables.
putenv(const_cast<char *>("LLDB_TEST_MAGIC_VARIABLE=foobar"));
- auto ClientOr = TestClient::launchCustom(getLogFileName(),
- { "--env", "LLDB_TEST_MAGIC_VARIABLE=LLDB_TEST_MAGIC_VALUE" },
- {getInferiorPath("environment_check")});
+ auto ClientOr = TestClient::launchCustom(
+ getLogFileName(), /* disable_stdio */ true,
+ {"--env", "LLDB_TEST_MAGIC_VARIABLE=LLDB_TEST_MAGIC_VALUE"},
+ {getInferiorPath("environment_check")});
ASSERT_THAT_EXPECTED(ClientOr, Succeeded());
auto &Client = **ClientOr;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D158237.551330.patch
Type: text/x-patch
Size: 2975 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20230818/723dafaa/attachment.bin>
More information about the lldb-commits
mailing list