From ceaf3457b8e00cccffe0cbba783414c74af29b54 Mon Sep 17 00:00:00 2001 From: leiguangyu Date: Mon, 24 Feb 2025 15:00:08 +0800 Subject: [PATCH] =?UTF-8?q?AI=E5=91=8A=E8=AD=A6=E9=9D=92=E6=9E=97=20Signed?= =?UTF-8?q?-off-by:leiguangyu=20?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: leiguangyu --- src/perf_events.cpp | 13 ++- src/perf_file_reader.cpp | 5 +- src/perf_file_writer.cpp | 2 +- src/ring_buffer.cpp | 5 +- src/subcommand_list.cpp | 2 +- src/subcommand_record.cpp | 25 +++--- src/subcommand_stat.cpp | 2 - src/utilities.cpp | 2 +- .../common/native/subcommand_record_test.cpp | 89 ++++++++++--------- 9 files changed, 79 insertions(+), 66 deletions(-) diff --git a/src/perf_events.cpp b/src/perf_events.cpp index 39ca41b..c1fa334 100644 --- a/src/perf_events.cpp +++ b/src/perf_events.cpp @@ -180,10 +180,16 @@ PerfEvents::~PerfEvents() for (auto it = cpuMmap_.begin(); it != cpuMmap_.end();) { const MmapFd &mmapItem = it->second; if (!isSpe_) { - munmap(mmapItem.mmapPage, (1 + mmapPages_) * pageSize_); + if (munmap(mmapItem.mmapPage, (1 + mmapPages_) * pageSize_) == -1) { + HLOGW("munmap failed."); + } } else { - munmap(mmapItem.mmapPage, (1 + auxMmapPages_) * pageSize_); - munmap(mmapItem.auxBuf, auxMmapPages_ * pageSize_); + if (munmap(mmapItem.mmapPage, (1 + auxMmapPages_) * pageSize_) == -1) { + HLOGW("munmap failed."); + } + if (munmap(mmapItem.auxBuf, auxMmapPages_ * pageSize_) == -1) { + HLOGW("munmap failed."); + } } it = cpuMmap_.erase(it); } @@ -670,6 +676,7 @@ bool PerfEvents::StartTracking(bool immediately) if (recordCallBack_) { if (!PrepareRecordThread()) { + HLOGW("PrepareRecordThread failed."); return false; } } diff --git a/src/perf_file_reader.cpp b/src/perf_file_reader.cpp index 34fa4a5..8307193 100644 --- a/src/perf_file_reader.cpp +++ b/src/perf_file_reader.cpp @@ -264,10 +264,7 @@ bool PerfFileReader::ReadRecord(ProcessRecordCB &callback) return false; } else { perf_event_header *header = reinterpret_cast(buf); - if (header == nullptr) { - HLOGE("read record header is null"); - return false; - } else if (header->size > RECORD_SIZE_LIMIT) { + if (header->size > RECORD_SIZE_LIMIT) { HLOGE("read record header size error %hu", header->size); return false; } diff --git a/src/perf_file_writer.cpp b/src/perf_file_writer.cpp index 4ee08e0..8b3e3b6 100644 --- a/src/perf_file_writer.cpp +++ b/src/perf_file_writer.cpp @@ -51,7 +51,7 @@ bool PerfFileWriter::Open(const std::string &fileName, bool compressData) } } std::string resolvedPath = CanonicalizeSpecPath(fileName.c_str()); - fp_ = fopen(resolvedPath.c_str(), "web+"); + fp_ = fopen(resolvedPath.c_str(), "wb+"); if (fp_ == nullptr) { char errInfo[ERRINFOLEN] = { 0 }; strerror_r(errno, errInfo, ERRINFOLEN); diff --git a/src/ring_buffer.cpp b/src/ring_buffer.cpp index 59ce443..67878e6 100644 --- a/src/ring_buffer.cpp +++ b/src/ring_buffer.cpp @@ -31,7 +31,10 @@ RingBuffer::RingBuffer(size_t size) : size_(size) } } -RingBuffer::~RingBuffer() {} +RingBuffer::~RingBuffer() +{ + buf_.reset(); +} // get size of the writable space size_t RingBuffer::GetFreeSize() const diff --git a/src/subcommand_list.cpp b/src/subcommand_list.cpp index 7b86c36..afb5c44 100644 --- a/src/subcommand_list.cpp +++ b/src/subcommand_list.cpp @@ -31,7 +31,7 @@ HiperfError SubCommandList::OnSubCommand(std::vector& args) requestEventTypes.push_back(it.first); } } else { - std::string requestEventType = args.front().c_str(); + std::string requestEventType = args.front(); auto it = SUPPORT_NAME_OPTIONS.find(requestEventType); if (it == SUPPORT_NAME_OPTIONS.end()) { printf("not support option: '%s'\n", requestEventType.c_str()); diff --git a/src/subcommand_record.cpp b/src/subcommand_record.cpp index ef5f6b6..69f6d74 100644 --- a/src/subcommand_record.cpp +++ b/src/subcommand_record.cpp @@ -21,7 +21,6 @@ #include #include #include -#include #if defined(CONFIG_HAS_SYSPARA) #include #endif @@ -64,8 +63,8 @@ const std::string PROC_VERSION = "/proc/version"; const std::string SAVED_CMDLINES_SIZE = "/sys/kernel/tracing/saved_cmdlines_size"; // when there are many events, start record will take more time. -const std::chrono::milliseconds CONTROL_WAITREPY_TOMEOUT = 2000ms; -const std::chrono::milliseconds CONTROL_WAITREPY_TOMEOUT_CHECK = 1000ms; +const std::chrono::milliseconds CONTROL_WAITREPY_TIMEOUT = 2000ms; +const std::chrono::milliseconds CONTROL_WAITREPY_TIMEOUT_CHECK = 1000ms; constexpr uint64_t MASK_ALIGNED_8 = 7; constexpr size_t MAX_DWARF_CALL_CHAIN = 2; @@ -1198,7 +1197,7 @@ void SubCommandRecord::ClientCommandHandle() struct pollfd pollFd { clientPipeInput_, POLLIN, 0 }; - int polled = poll(&pollFd, 1, CONTROL_WAITREPY_TOMEOUT.count()); + int polled = poll(&pollFd, 1, CONTROL_WAITREPY_TIMEOUT.count()); if (polled <= 0) { hasRead = false; continue; @@ -1250,19 +1249,19 @@ bool SubCommandRecord::ProcessControl() isFifoClient_ = true; bool ret = false; if (controlCmd_ == CONTROL_CMD_START) { - ret = SendFifoAndWaitReply(HiperfClient::ReplyStart, CONTROL_WAITREPY_TOMEOUT); + ret = SendFifoAndWaitReply(HiperfClient::ReplyStart, CONTROL_WAITREPY_TIMEOUT); } else if (controlCmd_ == CONTROL_CMD_RESUME) { - ret = SendFifoAndWaitReply(HiperfClient::ReplyResume, CONTROL_WAITREPY_TOMEOUT); + ret = SendFifoAndWaitReply(HiperfClient::ReplyResume, CONTROL_WAITREPY_TIMEOUT); } else if (controlCmd_ == CONTROL_CMD_PAUSE) { - ret = SendFifoAndWaitReply(HiperfClient::ReplyPause, CONTROL_WAITREPY_TOMEOUT); + ret = SendFifoAndWaitReply(HiperfClient::ReplyPause, CONTROL_WAITREPY_TIMEOUT); } else if (controlCmd_ == CONTROL_CMD_STOP) { - ret = SendFifoAndWaitReply(HiperfClient::ReplyStop, CONTROL_WAITREPY_TOMEOUT); + ret = SendFifoAndWaitReply(HiperfClient::ReplyStop, CONTROL_WAITREPY_TIMEOUT); if (!ret) { - ret = SendFifoAndWaitReply(HiperfClient::ReplyStop, CONTROL_WAITREPY_TOMEOUT); + ret = SendFifoAndWaitReply(HiperfClient::ReplyStop, CONTROL_WAITREPY_TIMEOUT); } ProcessStopCommand(ret); } else if (controlCmd_ == CONTROL_CMD_OUTPUT) { - ret = SendFifoAndWaitReply(HiperfClient::ReplyOutput, CONTROL_WAITREPY_TOMEOUT); + ret = SendFifoAndWaitReply(HiperfClient::ReplyOutput, CONTROL_WAITREPY_TIMEOUT); ProcessOutputCommand(ret); } @@ -1282,7 +1281,7 @@ void SubCommandRecord::ProcessStopCommand(bool ret) // wait sampling process exit really static constexpr uint64_t waitCheckSleepMs = 200; std::this_thread::sleep_for(milliseconds(waitCheckSleepMs)); - while (SendFifoAndWaitReply(HiperfClient::ReplyCheck, CONTROL_WAITREPY_TOMEOUT_CHECK)) { + while (SendFifoAndWaitReply(HiperfClient::ReplyCheck, CONTROL_WAITREPY_TIMEOUT_CHECK)) { std::this_thread::sleep_for(milliseconds(waitCheckSleepMs)); } HLOGI("wait reply check end."); @@ -1306,7 +1305,7 @@ void SubCommandRecord::ProcessOutputCommand(bool ret) std::this_thread::sleep_for(milliseconds(CHECK_WAIT_TIME_MS)); uint32_t outputFailCount = 0; while (!outputEnd_) { - ret = SendFifoAndWaitReply(HiperfClient::ReplyOutputCheck, CONTROL_WAITREPY_TOMEOUT_CHECK); + ret = SendFifoAndWaitReply(HiperfClient::ReplyOutputCheck, CONTROL_WAITREPY_TIMEOUT_CHECK); if (outputFailCount++ > MAX_CLIENT_OUTPUT_WAIT_COUNT || ret) { break; } @@ -1361,7 +1360,7 @@ bool SubCommandRecord::CreateFifoServer() int fd = open(CONTROL_FIFO_FILE_S2C.c_str(), O_RDONLY | O_NONBLOCK); std::string reply = ""; if (fd != -1) { - WaitFifoReply(fd, CONTROL_WAITREPY_TOMEOUT, reply); + WaitFifoReply(fd, CONTROL_WAITREPY_TIMEOUT, reply); } if (fd == -1 || reply != HiperfClient::ReplyOK) { if (reply != HiperfClient::ReplyOK) { diff --git a/src/subcommand_stat.cpp b/src/subcommand_stat.cpp index 296767c..00fd3df 100644 --- a/src/subcommand_stat.cpp +++ b/src/subcommand_stat.cpp @@ -421,8 +421,6 @@ std::string SubCommandStat::GetDetailComments(const std::unique_ptr= PATH_MAX) { + } else if (strlen(src) + 1 >= PATH_MAX) { HLOGE("Error: CanonicalizeSpecPath %s failed", src); return ""; } diff --git a/test/unittest/common/native/subcommand_record_test.cpp b/test/unittest/common/native/subcommand_record_test.cpp index 419ae54..18f20fc 100644 --- a/test/unittest/common/native/subcommand_record_test.cpp +++ b/test/unittest/common/native/subcommand_record_test.cpp @@ -2112,51 +2112,60 @@ HWTEST_F(SubCommandRecordTest, CheckThreadName, TestSize.Level1) HWTEST_F(SubCommandRecordTest, CheckDevhostMapOffset, TestSize.Level1) { - bool checkRet = false; - SubCommandRecord cmd; - cmd.SetHM(); - VirtualThread &kthread = cmd.virtualRuntime_.GetThread(cmd.virtualRuntime_.devhostPid_, - cmd.virtualRuntime_.devhostPid_); - kthread.ParseDevhostMap(cmd.virtualRuntime_.devhostPid_); - TestRecordCommand("-d 5 -s dwarf -o /data/local/tmp/test_maps.data", true, true); - StdoutRecord stdoutRecord; - stdoutRecord.Start(); - EXPECT_EQ(Command::DispatchCommand("dump -i /data/local/tmp/test_maps.data"), true); - std::string stringOut = stdoutRecord.Stop(); - std::istringstream stream(stringOut); - - std::string line; - bool isMmapRecord = false; - bool isMmapFirstLine = false; - uint64_t mapOffset = 0; - while (getline(stream, line)) { - if (strstr(line.c_str(), "record mmap:") != nullptr) { - isMmapRecord = true; - continue; - } - if (strstr(line.c_str(), "record sample:") != nullptr) { - break; - } - if (isMmapFirstLine) { - isMmapFirstLine = false; - uint64_t pgoff = 0; - int ret = sscanf_s(line.c_str(), " %*s 0x%" PRIx64 ", %*s %*s", &pgoff); - constexpr int numSlices {1}; - if (ret != numSlices) { - printf("unknown line %d: '%s' \n", ret, line.c_str()); + utsname unameBuf; + bool isHM = true; + if ((uname(&unameBuf)) == 0) { + std::string osrelease = unameBuf.release; + std::string sysname = unameBuf.sysname; + isHM = osrelease.find(HMKERNEL) != std::string::npos; + } + if (isHM) { + bool checkRet = false; + SubCommandRecord cmd; + cmd.SetHM(); + VirtualThread &kthread = cmd.virtualRuntime_.GetThread(cmd.virtualRuntime_.devhostPid_, + cmd.virtualRuntime_.devhostPid_); + kthread.ParseDevhostMap(cmd.virtualRuntime_.devhostPid_); + TestRecordCommand("-d 5 -s dwarf -o /data/local/tmp/test_maps.data", true, true); + StdoutRecord stdoutRecord; + stdoutRecord.Start(); + EXPECT_EQ(Command::DispatchCommand("dump -i /data/local/tmp/test_maps.data"), true); + std::string stringOut = stdoutRecord.Stop(); + std::istringstream stream(stringOut); + + std::string line; + bool isMmapRecord = false; + bool isMmapFirstLine = false; + uint64_t mapOffset = 0; + while (getline(stream, line)) { + if (strstr(line.c_str(), "record mmap:") != nullptr) { + isMmapRecord = true; + continue; + } + if (strstr(line.c_str(), "record sample:") != nullptr) { + break; + } + if (isMmapFirstLine) { + isMmapFirstLine = false; + uint64_t pgoff = 0; + int ret = sscanf_s(line.c_str(), " %*s 0x%" PRIx64 ", %*s %*s", &pgoff); + constexpr int numSlices {1}; + if (ret != numSlices) { + printf("unknown line %d: '%s' \n", ret, line.c_str()); + continue; + } + EXPECT_EQ(mapOffset, pgoff); + checkRet = true; continue; } - EXPECT_EQ(mapOffset, pgoff); - checkRet = true; - continue; - } - if (isMmapRecord) { - isMmapRecord = false; - isMmapFirstLine = GetMemMapOffset(cmd.virtualRuntime_.devhostPid_, mapOffset, kthread.memMaps_, line); + if (isMmapRecord) { + isMmapRecord = false; + isMmapFirstLine = GetMemMapOffset(cmd.virtualRuntime_.devhostPid_, mapOffset, kthread.memMaps_, line); + } } + EXPECT_EQ(checkRet, true); } - EXPECT_EQ(checkRet, true); } } // namespace HiPerf } // namespace Developtools -- Gitee