From facff39ff876c79add7c160b3ebdc07b17331d79 Mon Sep 17 00:00:00 2001 From: luopengting Date: Wed, 31 Mar 2021 11:18:00 +0800 Subject: [PATCH] fix codedex in RDR 1. check whether path is empty in IsEveryFilenameValid(). 2. do not print the string in IsStrLengthValid(), it's not safe. 3. make the constants about hash in recorder_manager.h. 4. initialize some variables. --- mindspore/ccsrc/debug/common.cc | 22 ++++++++----------- mindspore/ccsrc/debug/common.h | 6 ++--- mindspore/ccsrc/debug/env_config_parser.cc | 4 ++-- mindspore/ccsrc/debug/rdr/graph_recorder.cc | 2 +- mindspore/ccsrc/debug/rdr/graph_recorder.h | 2 +- mindspore/ccsrc/debug/rdr/recorder_manager.h | 12 +++++----- .../debug/rdr/stream_exec_order_recorder.h | 2 +- 7 files changed, 22 insertions(+), 28 deletions(-) diff --git a/mindspore/ccsrc/debug/common.cc b/mindspore/ccsrc/debug/common.cc index 3d544b088f..a3141f8961 100644 --- a/mindspore/ccsrc/debug/common.cc +++ b/mindspore/ccsrc/debug/common.cc @@ -172,18 +172,11 @@ std::optional Common::GetEnvConfigFile() { return config_file; } -bool Common::IsStrLengthValid(const std::string &str, const int &length_limit, const std::string &error_message, - const bool &print_str) { +bool Common::IsStrLengthValid(const std::string &str, const int &length_limit, const std::string &error_message) { const int len_str = str.length(); if (len_str > length_limit) { - std::ostringstream msg; - if (print_str) { - msg << error_message << "The string is " << str << ", its length is " << str.length(); - } else { - msg << error_message << "The length is " << str.length(); - } - msg << ", exceeding the limit of " << length_limit << "."; - MS_LOG(WARNING) << msg.str(); + MS_LOG(WARNING) << error_message << "The length is " << str.length() << ", exceeding the limit of " << length_limit + << "."; return false; } return true; @@ -191,6 +184,10 @@ bool Common::IsStrLengthValid(const std::string &str, const int &length_limit, c bool Common::IsEveryFilenameValid(const std::string &path, const int &length_limit, const std::string &error_message) { int left_pos = 0; + if (path.empty()) { + MS_LOG(WARNING) << error_message << "The path is empty."; + return false; + } int len_path = path.length(); for (int i = 0; i < len_path; i++) { if (i != 0) { @@ -216,8 +213,7 @@ bool Common::IsEveryFilenameValid(const std::string &path, const int &length_lim return true; } -bool Common::IsPathValid(const std::string &path, const int &length_limit, const std::string &error_message, - const bool &print_str) { +bool Common::IsPathValid(const std::string &path, const int &length_limit, const std::string &error_message) { std::string err_msg = "Detail: "; if (!error_message.empty()) { err_msg = error_message + " " + err_msg; @@ -228,7 +224,7 @@ bool Common::IsPathValid(const std::string &path, const int &length_limit, const return false; } - if (!IsStrLengthValid(path, length_limit, err_msg, print_str)) { + if (!IsStrLengthValid(path, length_limit, err_msg)) { return false; } diff --git a/mindspore/ccsrc/debug/common.h b/mindspore/ccsrc/debug/common.h index c8a1ec1f1c..a553254cf2 100644 --- a/mindspore/ccsrc/debug/common.h +++ b/mindspore/ccsrc/debug/common.h @@ -32,10 +32,8 @@ class Common { static std::optional GetRealPath(const std::string &input_path); static std::optional GetConfigFile(const std::string &env); static std::optional GetEnvConfigFile(); - static bool IsStrLengthValid(const std::string &str, const int &length_limit, const std::string &error_message = "", - const bool &print_str = true); - static bool IsPathValid(const std::string &path, const int &length_limit, const std::string &error_message = "", - const bool &print_str = true); + static bool IsStrLengthValid(const std::string &str, const int &length_limit, const std::string &error_message = ""); + static bool IsPathValid(const std::string &path, const int &length_limit, const std::string &error_message = ""); static bool IsFilenameValid(const std::string &filename, const int &length_limit, const std::string &error_message = ""); static bool CreateNotExistDirs(const std::string &path); diff --git a/mindspore/ccsrc/debug/env_config_parser.cc b/mindspore/ccsrc/debug/env_config_parser.cc index 77a4d7e696..50247dcc6a 100644 --- a/mindspore/ccsrc/debug/env_config_parser.cc +++ b/mindspore/ccsrc/debug/env_config_parser.cc @@ -55,7 +55,7 @@ std::optional GetRdrPathFromEnv() { std::string err_msg = "RDR path parse from environment variable failed. Please check the settings about '" + std::string(kPathEnv) + "' in environment variables."; std::string path = path_char; - if (!Common::IsPathValid(path, maxDirectoryLength, err_msg, false)) { + if (!Common::IsPathValid(path, maxDirectoryLength, err_msg)) { return std::string(""); } return path; @@ -186,7 +186,7 @@ void EnvConfigParser::ParseRdrPath(const nlohmann::json &content) { } std::string path = content; - if (!Common::IsPathValid(path, maxDirectoryLength, err_msg, false)) { + if (!Common::IsPathValid(path, maxDirectoryLength, err_msg)) { return; } diff --git a/mindspore/ccsrc/debug/rdr/graph_recorder.cc b/mindspore/ccsrc/debug/rdr/graph_recorder.cc index cd8a2d72d4..a33e27dc94 100644 --- a/mindspore/ccsrc/debug/rdr/graph_recorder.cc +++ b/mindspore/ccsrc/debug/rdr/graph_recorder.cc @@ -79,7 +79,7 @@ void GraphRecorder::Export() { LocDumpMode dump_mode = LocDumpMode(dump_graph_info_.dump_mode); DumpIRForRDR(realpath_ir, func_graph_, dump_graph_info_.dump_full_name, dump_mode); } else { - MS_LOG(WARNING) << "Unknown save graph LocDumoMode: " << dump_graph_info_.dump_mode + MS_LOG(WARNING) << "Unknown save graph LocDumpMode: " << dump_graph_info_.dump_mode << ", it must be in the range [0,2]."; } } diff --git a/mindspore/ccsrc/debug/rdr/graph_recorder.h b/mindspore/ccsrc/debug/rdr/graph_recorder.h index 0e546c6d26..0f06dee0b9 100644 --- a/mindspore/ccsrc/debug/rdr/graph_recorder.h +++ b/mindspore/ccsrc/debug/rdr/graph_recorder.h @@ -44,7 +44,7 @@ class GraphRecorder : public BaseRecorder { private: FuncGraphPtr func_graph_; std::string graph_type_; - DumpGraphParams dump_graph_info_; + DumpGraphParams dump_graph_info_{false, 0}; }; using GraphRecorderPtr = std::shared_ptr; } // namespace mindspore diff --git a/mindspore/ccsrc/debug/rdr/recorder_manager.h b/mindspore/ccsrc/debug/rdr/recorder_manager.h index 2d8b24a06f..363ceed96c 100644 --- a/mindspore/ccsrc/debug/rdr/recorder_manager.h +++ b/mindspore/ccsrc/debug/rdr/recorder_manager.h @@ -25,14 +25,14 @@ #include namespace mindspore { +// The number is the reciprocal of the golden ratio. +const unsigned int magicConstant = 0x9e3779b9; +const unsigned int hashShiftLeft = 6; +const unsigned int hashShiftRight = 2; + template inline void hash_combine(std::size_t *seed, const T &val) { - // The number is the reciprocal of the golden ratio. - unsigned int magic_constant = 0x9e3779b9; - - int shift_left = 6; - int shift_right = 2; - *seed ^= std::hash()(val) + magic_constant + (*seed << shift_left) + (*seed >> shift_right); + *seed ^= std::hash()(val) + magicConstant + (*seed << hashShiftLeft) + (*seed >> hashShiftRight); } template diff --git a/mindspore/ccsrc/debug/rdr/stream_exec_order_recorder.h b/mindspore/ccsrc/debug/rdr/stream_exec_order_recorder.h index 36f35e46b8..7e692ac787 100644 --- a/mindspore/ccsrc/debug/rdr/stream_exec_order_recorder.h +++ b/mindspore/ccsrc/debug/rdr/stream_exec_order_recorder.h @@ -50,7 +50,7 @@ class ExecNode { uint32_t logic_id_; uint32_t stream_id_; std::string node_info_; - uint32_t event_id_; + uint32_t event_id_{0}; std::vector label_ids_; std::vector active_stream_ids_; };