From 42218a1dc3dd7c241284d4c5c0507348f2ef3294 Mon Sep 17 00:00:00 2001 From: Zhang Qinghua Date: Fri, 14 Aug 2020 17:20:40 +0800 Subject: [PATCH] Fix the codedex issues and other issues. --- .../backend/session/kernel_build_client.h | 27 ++++++++++++++----- mindspore/ccsrc/common/duplex_pipe.cc | 12 ++++----- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/mindspore/ccsrc/backend/session/kernel_build_client.h b/mindspore/ccsrc/backend/session/kernel_build_client.h index f442c8846d1..f1d5cf2f19c 100644 --- a/mindspore/ccsrc/backend/session/kernel_build_client.h +++ b/mindspore/ccsrc/backend/session/kernel_build_client.h @@ -90,11 +90,18 @@ class KernelBuildClient { std::string res; *dp_ >> res; // Filter out the interference + if (res.empty()) { + MS_LOG(EXCEPTION) << "Response is empty"; + } auto start = res.find(kTag); if (start == std::string::npos) { MS_LOG(EXCEPTION) << "Response seems incorrect, res: " << res; } - res = res.substr(start + std::strlen(kTag), res.size() - start); + auto pos = start + std::strlen(kTag); + if (pos > res.size()) { // Safe check for codedex + MS_LOG(EXCEPTION) << "Response seems incorrect, res(" << res.size() << "): {" << res << "}, start: " << start; + } + res = res.substr(pos); // Revert the line feed and space if (res != kSuccess && res != kAck && res != kErr && res != kTrue) { ReplaceStr(&res, kLF, '\n'); @@ -113,25 +120,30 @@ class KernelBuildClient { std::shared_ptr dp_; }; -static inline std::string GetScriptFilePath(const std::string cmd_env, const std::string &cmd_script) { +static std::string GetScriptFilePath(const std::string cmd_env, const std::string &cmd_script) { std::string cmd = cmd_env; (void)cmd.append(1, ' ').append(cmd_script); FILE *fpipe = popen(cmd.c_str(), "r"); if (fpipe == nullptr) { - MS_LOG(EXCEPTION) << "popen failed, " << strerror(errno) << "(" << errno << ")"; + MS_LOG(EXCEPTION) << "popen failed, errno: " << errno; } bool start = false; std::string result; char buf[kBufferSize]; while (std::fgets(buf, sizeof(buf), fpipe) != nullptr) { + auto len = std::strlen(buf); + if (len == 0 || len >= kBufferSize) { + // Safe check for codedex + // Should never reach here + MS_LOG(EXCEPTION) << "fgets() failed, len: " << len << ", errno: " << errno; + } if (std::strncmp(buf, kTag, std::strlen(kTag)) == 0) { start = true; } // Filter with 'kTAG' and '\n' if (start) { - auto size = std::strlen(buf); - bool line_end = buf[size - 1] == '\n'; - result.append(buf, line_end ? size - 1 : size); + bool line_end = buf[len - 1] == '\n'; + result.append(buf, line_end ? len - 1 : len); if (line_end) { break; } @@ -142,6 +154,9 @@ static inline std::string GetScriptFilePath(const std::string cmd_env, const std if (result.empty() || result.rfind(py_suffix) != (result.length() - py_suffix.length())) { MS_LOG(EXCEPTION) << "py file seems incorrect, result: {" << result << "}"; } + if (strlen(kTag) > result.size()) { // Safe check for codedex + MS_LOG(EXCEPTION) << "result size seems incorrect, result(" << result.size() << "): {" << result << "}"; + } result = result.substr(strlen(kTag)); MS_LOG(DEBUG) << "result: " << result; return result; diff --git a/mindspore/ccsrc/common/duplex_pipe.cc b/mindspore/ccsrc/common/duplex_pipe.cc index b41be6e573e..ce2e3d0c8d9 100644 --- a/mindspore/ccsrc/common/duplex_pipe.cc +++ b/mindspore/ccsrc/common/duplex_pipe.cc @@ -24,12 +24,12 @@ namespace mindspore { int DuplexPipe::Open(std::initializer_list arg_list, bool append_fds) { if (pipe(fd1_) == -1) { - DP_EXCEPTION << "pipe 1 failed, " << strerror(errno) << "(" << errno << ")"; + DP_EXCEPTION << "pipe 1 failed, errno: " << errno; } if (pipe(fd2_) == -1) { close(fd1_[0]); close(fd1_[1]); - DP_EXCEPTION << "pipe 2 failed, " << strerror(errno) << "(" << errno << ")"; + DP_EXCEPTION << "pipe 2 failed, errno: " << errno; } pid_ = fork(); @@ -38,7 +38,7 @@ int DuplexPipe::Open(std::initializer_list arg_list, bool append_fd close(fd1_[1]); close(fd2_[0]); close(fd2_[1]); - DP_EXCEPTION << "fork failed, " << strerror(errno) << "(" << errno << ")"; + DP_EXCEPTION << "fork failed, errno: " << errno; } else if (pid_ == 0) { // Remote process DP_INFO << "Remote process, pid: " << getpid() << ", " << fd1_[0] << "/" << fd2_[1]; remote_stdout_ = dup(STDOUT_FILENO); @@ -61,7 +61,7 @@ int DuplexPipe::Open(std::initializer_list arg_list, bool append_fd } args.emplace_back(nullptr); if (execvp(args[0], const_cast(&args[0])) == -1) { - DP_EXCEPTION << "execute " << args[0] << " failed, " << strerror(errno) << "(" << errno << ")"; + DP_EXCEPTION << "execute " << args[0] << " failed, errno: " << errno; } } else { // Local process DP_INFO << "Local process, id: " << getpid() << ", " << fd2_[0] << "/" << fd1_[1]; @@ -77,13 +77,13 @@ int DuplexPipe::Open(std::initializer_list arg_list, bool append_fd void DuplexPipe::Write(const std::string &buf, bool flush) { // Write the string into pipe if (write(fd1_[1], buf.data(), buf.size()) == -1) { - DP_ERROR << "write failed, error: " << strerror(errno) << "(" << errno << ")"; + DP_ERROR << "write failed, errno: " << errno; return; } if (flush) { // Flush into the pipe if (write(fd1_[1], "\n", 1) == -1) { - DP_ERROR << "write failed, error: " << strerror(errno) << "(" << errno << ")"; + DP_ERROR << "write failed, errno: " << errno; return; } }