diff --git a/src/runtime_src/core/common/api/aie/xrt_graph.cpp b/src/runtime_src/core/common/api/aie/xrt_graph.cpp index ce0a0b9b219..56b30391a06 100644 --- a/src/runtime_src/core/common/api/aie/xrt_graph.cpp +++ b/src/runtime_src/core/common/api/aie/xrt_graph.cpp @@ -1,6 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright (C) 2020-2021 Xilinx, Inc. All rights reserved. -// Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved. +// Copyright (C) 2023-2026 Advanced Micro Devices, Inc. All rights reserved. // This file implements XRT APIs as declared in // core/include/experimental/xrt_aie.h -- end user APIs @@ -18,6 +18,7 @@ #include "core/include/xcl_graph.h" #include "core/common/api/device_int.h" +#include "core/common/api/handle.h" #include "core/common/api/native_profile.h" #include "core/common/device.h" #include "core/common/error.h" @@ -257,7 +258,7 @@ namespace { // C-API Graph handles are inserted to this map. // Note: xrtGraphClose must be explicitly called before xclClose. -static std::map> graph_cache; +static xrt_core::handle_map> graph_cache; static std::shared_ptr open_graph(xrtDeviceHandle dhdl, const xuid_t xclbin_uuid, const char* graph_name, xrt::graph::access_mode am) @@ -272,17 +273,13 @@ open_graph(xrtDeviceHandle dhdl, const xuid_t xclbin_uuid, const char* graph_nam static std::shared_ptr get_graph_hdl(xrtGraphHandle graph_handle) { - auto itr = graph_cache.find(graph_handle); - if (itr == graph_cache.end()) - throw xrt_core::error(-EINVAL, "No such graph handle"); - return (*itr).second; + return graph_cache.get_or_error(graph_handle); } static void close_graph(xrtGraphHandle hdl) { - if (graph_cache.erase(hdl) == 0) - throw std::runtime_error("Unexpected internal error"); + graph_cache.remove_or_error(hdl); } static void @@ -323,7 +320,7 @@ wait_gmio(xrtDeviceHandle dhdl, const char *gmio_name) } // C-API Profiling handles are inserted to this map. -static std::map> profiling_cache; +static xrt_core::handle_map> profiling_cache; static std::shared_ptr create_profiling_event(xrtDeviceHandle dhdl) @@ -592,8 +589,9 @@ xrtGraphOpen(xrtDeviceHandle dev_handle, const xuid_t xclbin_uuid, const char* g { try { auto hdl = open_graph(dev_handle, xclbin_uuid, graph_name, xrt::graph::access_mode::primary); - graph_cache[hdl.get()] = hdl; - return hdl.get(); + auto key = hdl.get(); + graph_cache.add(key, std::move(hdl)); + return key; } catch (const xrt_core::error& ex) { xrt_core::send_exception_message(ex.what()); @@ -610,8 +608,9 @@ xrtGraphOpenExclusive(xrtDeviceHandle dev_handle, const xuid_t xclbin_uuid, cons { try { auto hdl = open_graph(dev_handle, xclbin_uuid, graph_name, xrt::graph::access_mode::exclusive); - graph_cache[hdl.get()] = hdl; - return hdl.get(); + auto key = hdl.get(); + graph_cache.add(key, std::move(hdl)); + return key; } catch (const xrt_core::error& ex) { xrt_core::send_exception_message(ex.what()); @@ -628,8 +627,9 @@ xrtGraphOpenShared(xrtDeviceHandle dev_handle, const xuid_t xclbin_uuid, const c { try { auto hdl = open_graph(dev_handle, xclbin_uuid, graph_name, xrt::graph::access_mode::shared); - graph_cache[hdl.get()] = hdl; - return hdl.get(); + auto key = hdl.get(); + graph_cache.add(key, std::move(hdl)); + return key; } catch (const xrt_core::error& ex) { xrt_core::send_exception_message(ex.what()); @@ -1024,7 +1024,7 @@ xrtAIEStartProfiling(xrtDeviceHandle handle, int option, const char *port1Name, const std::string port2 = port2Name ? port2Name : ""; auto hdl = event->start(option, port1, port2, value); if (hdl != xrt::aie::profiling_impl::invalid_handle) { - profiling_cache[hdl] = std::move(event); + profiling_cache.add(hdl, std::move(event)); return hdl; } else @@ -1053,11 +1053,7 @@ uint64_t xrtAIEReadProfiling(xrtDeviceHandle /*handle*/, int pHandle) { try { - auto it = profiling_cache.find(pHandle); - if (it != profiling_cache.end()) - return it->second->read(); - else - throw xrt_core::error(-EINVAL, "No such profiling handle"); + return profiling_cache.get_or_error(pHandle)->read(); } catch (const xrt_core::error& ex) { xrt_core::send_exception_message(ex.what()); @@ -1083,13 +1079,8 @@ void xrtAIEStopProfiling(xrtDeviceHandle /*handle*/, int pHandle) { try { - auto it = profiling_cache.find(pHandle); - if (it != profiling_cache.end()) { - it->second->stop(); - profiling_cache.erase(pHandle); - } - else - throw xrt_core::error(-EINVAL, "No such profiling handle"); + profiling_cache.get_or_error(pHandle)->stop(); + profiling_cache.remove_or_error(pHandle); } catch (const xrt_core::error& ex) { xrt_core::send_exception_message(ex.what()); diff --git a/src/runtime_src/core/common/api/handle.h b/src/runtime_src/core/common/api/handle.h index ac5f1413612..c151151cfc7 100644 --- a/src/runtime_src/core/common/api/handle.h +++ b/src/runtime_src/core/common/api/handle.h @@ -46,7 +46,7 @@ class handle_map> return handles; } - const std::shared_ptr& + std::shared_ptr get_or_error(HandleType handle) const { std::lock_guard lk(mutex); diff --git a/src/runtime_src/core/common/api/hw_queue.cpp b/src/runtime_src/core/common/api/hw_queue.cpp index fe26c44d608..c3f75e2c849 100644 --- a/src/runtime_src/core/common/api/hw_queue.cpp +++ b/src/runtime_src/core/common/api/hw_queue.cpp @@ -119,6 +119,7 @@ class command_manager std::mutex work_mutex; std::condition_variable work_cond; command_queue_type submitted_cmds; + std::vector running_cmds; // guarded by work_mutex bool stop = false; // thread can be constructed only after data members are initialized @@ -135,7 +136,6 @@ class command_manager monitor_loop() { std::vector busy_cmds; - std::vector running_cmds; while (true) { @@ -271,11 +271,20 @@ class command_manager m_impl->submit(cmd); } catch (...) { - // Remove the pending command + // Remove the specific command to avoid two races: + // 1. monitor may have already drained submitted_cmds into running_cmds + // 2. a concurrent launch() may have pushed another command, making + // pop_back() remove the wrong entry std::lock_guard lk(work_mutex); - assert(get_command_state(cmd)==ERT_CMD_STATE_NEW); - if (!submitted_cmds.empty()) - submitted_cmds.pop_back(); + assert(get_command_state(cmd) == ERT_CMD_STATE_NEW); + auto it = std::find(submitted_cmds.begin(), submitted_cmds.end(), cmd); + if (it != submitted_cmds.end()) + submitted_cmds.erase(it); + else { + auto rit = std::find(running_cmds.begin(), running_cmds.end(), cmd); + if (rit != running_cmds.end()) + running_cmds.erase(rit); + } throw; } diff --git a/src/runtime_src/core/common/api/xrt_bo.cpp b/src/runtime_src/core/common/api/xrt_bo.cpp index d3df7ec5d6b..6cd046cfdac 100755 --- a/src/runtime_src/core/common/api/xrt_bo.cpp +++ b/src/runtime_src/core/common/api/xrt_bo.cpp @@ -1073,7 +1073,7 @@ namespace { // deleted if no other shared ptrs exists for this buffer static xrt_core::handle_map> bo_cache; -static const std::shared_ptr& +static std::shared_ptr get_boh(xrtBufferHandle bhdl) { return bo_cache.get_or_error(bhdl); diff --git a/src/runtime_src/core/common/api/xrt_error.cpp b/src/runtime_src/core/common/api/xrt_error.cpp index 00e353fdeb3..df69135bfe0 100644 --- a/src/runtime_src/core/common/api/xrt_error.cpp +++ b/src/runtime_src/core/common/api/xrt_error.cpp @@ -336,6 +336,9 @@ xrtErrorGetString(xrtDeviceHandle, xrtErrorCode error, char* out, size_t len, si if (!out) return 0; + if (len == 0) + return -EINVAL; + auto cp_len = std::min(len - 1, str.size()); std::memcpy(out, str.c_str(), cp_len); out[cp_len] = 0; diff --git a/src/runtime_src/core/common/api/xrt_kernel.cpp b/src/runtime_src/core/common/api/xrt_kernel.cpp index ae4bdfbfdf1..b695de09c51 100644 --- a/src/runtime_src/core/common/api/xrt_kernel.cpp +++ b/src/runtime_src/core/common/api/xrt_kernel.cpp @@ -5169,7 +5169,16 @@ aie_error_message_v1(const ert_packet* epkt, const std::string& msg, << "\nnumber of uC reported = " << std::dec << ctx_health->aie4.num_uc; - for (uint32_t i = 0; i < ctx_health->aie4.num_uc; ++i) { + // clamp num_uc to entries that fit in the packet payload; fixed overhead + // is version+npu_gen+ctx_state+num_uc+ctx_error_type = 5 uint32_t words + constexpr uint32_t aie4_fixed_words = 5; + constexpr uint32_t uc_entry_words = sizeof(ert_uc_health_info) / sizeof(uint32_t); + const uint32_t max_uc = (epkt->count > aie4_fixed_words) + ? (epkt->count - aie4_fixed_words) / uc_entry_words + : 0; + const uint32_t num_uc = std::min(max_uc, ctx_health->aie4.num_uc); + + for (uint32_t i = 0; i < num_uc; ++i) { oss << "\nuc_info[" << i << "]: " << "\nuc_idx=0x" << std::setw(indent8) << std::hex << ctx_health->aie4.uc_info[i].uc_idx diff --git a/src/runtime_src/core/common/api/xrt_xclbin.cpp b/src/runtime_src/core/common/api/xrt_xclbin.cpp index e3ac7a40749..7672b14dc71 100644 --- a/src/runtime_src/core/common/api/xrt_xclbin.cpp +++ b/src/runtime_src/core/common/api/xrt_xclbin.cpp @@ -234,26 +234,35 @@ class xclbin::ip_impl void add_mem_at_idx(int32_t argidx, const xclbin::mem& mem) { - resize_args(argidx + 1); + // cast before +1 to avoid signed overflow when argidx == INT32_MAX + resize_args(static_cast(argidx) + 1); if (!m_args[argidx]) m_args[argidx] = xclbin::arg{std::make_shared()}; m_args[argidx].get_handle()->add_mem(mem); } public: - ip_impl(const ::connectivity* conn, + ip_impl(const ::connectivity* conn, size_t conn_size, const std::vector& mems, const ::ip_data* ip, int32_t ipidx) : m_ip(ip), m_ip_layout_idx(ipidx) { - if (!conn) + // m_count (int32_t) is the only non-array member, so sizeof(int32_t) + // is the minimum section size needed to read from struct connectivity + if (!conn || conn_size < sizeof(int32_t) || conn->m_count < 0) return; - for (int32_t idx = 0; idx < conn->m_count; ++idx) { + auto max_count = (conn_size - sizeof(int32_t)) / sizeof(::connection); + auto count = std::min(static_cast(conn->m_count), max_count); + for (int32_t idx = 0; idx < static_cast(count); ++idx) { auto& cxn = conn->m_connection[idx]; if (cxn.m_ip_layout_index != m_ip_layout_idx) continue; + if (cxn.arg_index < 0 || cxn.mem_data_index < 0 || + static_cast(cxn.mem_data_index) >= mems.size()) + throw std::runtime_error("xclbin: invalid connectivity index"); + add_mem_at_idx(cxn.arg_index, mems.at(cxn.mem_data_index)); } } @@ -412,12 +421,23 @@ class xclbin_impl init_mems(const xclbin_impl* ximpl) { std::vector mems; - if (auto mem_topology = ximpl->get_section(ASK_GROUP_TOPOLOGY)) { - mems.reserve(mem_topology->m_count); - for (int32_t idx = 0; idx < mem_topology->m_count; ++idx) { - auto mem = mem_topology->m_mem_data + idx; - mems.emplace_back(std::make_shared(mem, idx)); - } + auto [data, size] = ximpl->get_axlf_section(ASK_GROUP_TOPOLOGY); + // m_count is the only non-array member, so sizeof(int32_t) is both the + // minimum section size needed to read from struct mem_topology and also + // the byte offset to m_mem_data. + if (!data || size < sizeof(int32_t)) + return mems; + + auto mem_topology = reinterpret_cast(data); + if (mem_topology->m_count < 0) + return mems; + + auto max_count = (size - sizeof(int32_t)) / sizeof(::mem_data); + auto count = std::min(static_cast(mem_topology->m_count), max_count); + mems.reserve(count); + for (int32_t idx = 0; idx < static_cast(count); ++idx) { + auto mem = mem_topology->m_mem_data + idx; + mems.emplace_back(std::make_shared(mem, idx)); } return mems; } @@ -462,18 +482,29 @@ class xclbin_impl static std::vector init_ips(const xclbin_impl* ximpl, const std::vector& mems) { - auto ip_layout = ximpl->get_section(IP_LAYOUT); - if (!ip_layout) + auto [ip_data, ip_size] = ximpl->get_axlf_section(IP_LAYOUT); + // m_count is the only non-array member, so sizeof(int32_t) is both the + // minimum section size needed to read from struct ip_layout and also + // the byte offset to m_ip_data. + if (!ip_data || ip_size < sizeof(int32_t)) return {}; - auto conn = ximpl->get_section(ASK_GROUP_CONNECTIVITY); + auto ip_layout = reinterpret_cast(ip_data); + if (ip_layout->m_count < 0) + return {}; + + auto max_count = (ip_size - sizeof(int32_t)) / sizeof(::ip_data); + auto count = std::min(static_cast(ip_layout->m_count), max_count); + + auto [conn_data, conn_size] = ximpl->get_axlf_section(ASK_GROUP_CONNECTIVITY); + auto conn = reinterpret_cast(conn_data); std::vector ips; - ips.reserve(ip_layout->m_count); - for (int32_t idx = 0; idx < ip_layout->m_count; ++idx) + ips.reserve(count); + for (int32_t idx = 0; idx < static_cast(count); ++idx) ips.emplace_back (std::make_shared - (conn, mems, ip_layout->m_ip_data + idx, idx)); + (conn, conn_size, mems, ip_layout->m_ip_data + idx, idx)); return ips; } diff --git a/src/runtime_src/core/edge/user/shim.cpp b/src/runtime_src/core/edge/user/shim.cpp index 2f83d868591..9af13db9fd2 100644 --- a/src/runtime_src/core/edge/user/shim.cpp +++ b/src/runtime_src/core/edge/user/shim.cpp @@ -434,7 +434,7 @@ xclGetDeviceInfo2(xclDeviceInfo2 *info) mVBNV >> deviceName; } mVBNV.close(); - std::size_t length = deviceName.copy(info->mName, deviceName.length(),0); + std::size_t length = deviceName.copy(info->mName, sizeof(info->mName) - 1, 0); info->mName[length] = '\0'; return 0; } diff --git a/src/runtime_src/hip/core/graph.h b/src/runtime_src/hip/core/graph.h index c467b0beed1..4ab8e009801 100644 --- a/src/runtime_src/hip/core/graph.h +++ b/src/runtime_src/hip/core/graph.h @@ -91,7 +91,7 @@ class graph return m_node_list; } - const std::shared_ptr& + std::shared_ptr get_node(node_handle node_handle) const { return m_node_cache.get_or_error(node_handle);