Skip to content

Commit

Permalink
Fix a number of issues from the latest coverity scan.
Browse files Browse the repository at this point in the history
  • Loading branch information
@aarongreig
aarongreig committed Nov 22, 2024
1 parent e3247c2 commit a284d1f
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 45 deletions.
12 changes: 9 additions & 3 deletions source/adapters/cuda/common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,19 @@ void detail::ur::assertion(bool Condition, const char *Message) {

// Global variables for ZER_EXT_RESULT_ADAPTER_SPECIFIC_ERROR
thread_local ur_result_t ErrorMessageCode = UR_RESULT_SUCCESS;
thread_local char ErrorMessage[MaxMessageSize];
thread_local char ErrorMessage[MaxMessageSize]{};

// Utility function for setting a message and warning
[[maybe_unused]] void setErrorMessage(const char *pMessage,
ur_result_t ErrorCode) {
assert(strlen(pMessage) <= MaxMessageSize);
strcpy(ErrorMessage, pMessage);
assert(strlen(pMessage) < MaxMessageSize);
// Copy at most MaxMessageSize - 1 bytes to ensure the resultant string is
// always null terminated.
#if defined(_WIN32)
strncpy_s(ErrorMessage, MaxMessageSize - 1, pMessage, strlen(pMessage));
#else
strncpy(ErrorMessage, pMessage, MaxMessageSize - 1);
#endif
ErrorMessageCode = ErrorCode;
}

Expand Down
8 changes: 7 additions & 1 deletion source/adapters/hip/common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,19 @@ void detail::ur::assertion(bool Condition, const char *pMessage) {

// Global variables for UR_RESULT_ADAPTER_SPECIFIC_ERROR
thread_local ur_result_t ErrorMessageCode = UR_RESULT_SUCCESS;
thread_local char ErrorMessage[MaxMessageSize];
thread_local char ErrorMessage[MaxMessageSize]{};

// Utility function for setting a message and warning
[[maybe_unused]] void setErrorMessage(const char *pMessage,
ur_result_t ErrorCode) {
assert(strlen(pMessage) < MaxMessageSize);
// Copy at most MaxMessageSize - 1 bytes to ensure the resultant string is
// always null terminated.
#if defined(_WIN32)
strncpy_s(ErrorMessage, MaxMessageSize - 1, pMessage, strlen(pMessage));
#else
strncpy(ErrorMessage, pMessage, MaxMessageSize - 1);
#endif
ErrorMessageCode = ErrorCode;
}

Expand Down
12 changes: 9 additions & 3 deletions source/adapters/level_zero/common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,15 +332,21 @@ template <> zes_structure_type_t getZesStructureType<zes_mem_properties_t>() {

// Global variables for ZER_EXT_RESULT_ADAPTER_SPECIFIC_ERROR
thread_local ur_result_t ErrorMessageCode = UR_RESULT_SUCCESS;
thread_local char ErrorMessage[MaxMessageSize];
thread_local char ErrorMessage[MaxMessageSize]{};
thread_local int32_t ErrorAdapterNativeCode;

// Utility function for setting a message and warning
[[maybe_unused]] void setErrorMessage(const char *pMessage,
ur_result_t ErrorCode,
int32_t AdapterErrorCode) {
assert(strlen(pMessage) <= MaxMessageSize);
strcpy(ErrorMessage, pMessage);
assert(strlen(pMessage) < MaxMessageSize);
// Copy at most MaxMessageSize - 1 bytes to ensure the resultant string is
// always null terminated.
#if defined(_WIN32)
strncpy_s(ErrorMessage, MaxMessageSize - 1, pMessage, strlen(pMessage));
#else
strncpy(ErrorMessage, pMessage, MaxMessageSize - 1);
#endif
ErrorMessageCode = ErrorCode;
ErrorAdapterNativeCode = AdapterErrorCode;
}
Expand Down
12 changes: 9 additions & 3 deletions source/adapters/native_cpu/common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,19 @@
// Global variables for UR_RESULT_ADAPTER_SPECIFIC_ERROR
// See urGetLastResult
thread_local ur_result_t ErrorMessageCode = UR_RESULT_SUCCESS;
thread_local char ErrorMessage[MaxMessageSize];
thread_local char ErrorMessage[MaxMessageSize]{};

// Utility function for setting a message and warning
[[maybe_unused]] void setErrorMessage(const char *pMessage,
ur_result_t ErrorCode) {
assert(strlen(pMessage) <= MaxMessageSize);
strcpy(ErrorMessage, pMessage);
assert(strlen(pMessage) < MaxMessageSize);
// Copy at most MaxMessageSize - 1 bytes to ensure the resultant string is
// always null terminated.
#if defined(_WIN32)
strncpy_s(ErrorMessage, MaxMessageSize - 1, pMessage, strlen(pMessage));
#else
strncpy(ErrorMessage, pMessage, MaxMessageSize - 1);
#endif
ErrorMessageCode = ErrorCode;
}

Expand Down
14 changes: 11 additions & 3 deletions source/adapters/opencl/common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,19 @@ namespace cl_adapter {

/* Global variables for urAdapterGetLastError() */
thread_local int32_t ErrorMessageCode = 0;
thread_local char ErrorMessage[MaxMessageSize];
thread_local char ErrorMessage[MaxMessageSize]{};

[[maybe_unused]] void setErrorMessage(const char *Message, int32_t ErrorCode) {
assert(strlen(Message) <= cl_adapter::MaxMessageSize);
strcpy(cl_adapter::ErrorMessage, Message);
assert(strlen(Message) < cl_adapter::MaxMessageSize);
// Copy at most MaxMessageSize - 1 bytes to ensure the resultant string is
// always null terminated.
#if defined(_WIN32)
strncpy_s(cl_adapter::ErrorMessage, MaxMessageSize - 1, Message,
strlen(Message));
#else
strncpy(cl_adapter::ErrorMessage, Message, MaxMessageSize - 1);
#endif

ErrorMessageCode = ErrorCode;
}
} // namespace cl_adapter
Expand Down
7 changes: 3 additions & 4 deletions source/loader/layers/sanitizer/asan_report.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,10 @@ void ReportBadFree(uptr Addr, const StackTrace &stack,
if (!AI) {
getContext()->logger.always("{} may be allocated on Host Memory",
(void *)Addr);
} else {
assert(!AI->IsReleased && "Chunk must be not released");
PrintAllocateInfo(Addr, AI.get());
}

assert(AI && !AI->IsReleased && "Chunk must be not released");

PrintAllocateInfo(Addr, AI.get());
}

void ReportBadContext(uptr Addr, const StackTrace &stack,
Expand Down
2 changes: 1 addition & 1 deletion source/loader/layers/sanitizer/stacktrace.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ bool Contains(const std::string &s, const char *p) {

// Parse back trace information in the following formats:
// <module_name>([function_name]+function_offset) [offset]
void ParseBacktraceInfo(BacktraceInfo BI, std::string &ModuleName,
void ParseBacktraceInfo(const BacktraceInfo &BI, std::string &ModuleName,
uptr &Offset) {
// Parse module name
size_t End = BI.find_first_of('(');
Expand Down
4 changes: 2 additions & 2 deletions test/conformance/source/environment.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ void uur::PlatformEnvironment::selectPlatformFromOptions() {
std::stringstream errstr;
errstr << "Multiple possible platforms found; please select one of the "
"following or set --platforms_count=1:\n";
for (auto p : platforms_filtered) {
for (const auto &p : platforms_filtered) {
errstr << " --backend=" << backend_to_str(p.backend)
<< " --platform=\"" << p.name << "\"\n";
}
Expand Down Expand Up @@ -287,7 +287,7 @@ PlatformEnvironment::parsePlatformOptions(int argc, char **argv) {
} else if (std::strncmp(arg, "--backend=", sizeof("--backend=") - 1) ==
0) {
std::string backend_string{&arg[std::strlen("--backend=")]};
if (!parse_backend(backend_string)) {
if (!parse_backend(std::move(backend_string))) {
return options;
}
} else if (std::strncmp(arg, "--platforms_count=",
Expand Down
18 changes: 9 additions & 9 deletions test/conformance/usm/urUSMFree.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,16 @@ TEST_P(urUSMFreeTest, SuccessDeviceAlloc) {
size_t allocation_size = sizeof(int);
ASSERT_SUCCESS(urUSMDeviceAlloc(context, device, nullptr, nullptr,
allocation_size, &ptr));
ASSERT_NE(ptr, nullptr);

ur_event_handle_t event = nullptr;

uint8_t pattern = 0;
ASSERT_SUCCESS(urEnqueueUSMFill(queue, ptr, sizeof(pattern), &pattern,
EXPECT_SUCCESS(urEnqueueUSMFill(queue, ptr, sizeof(pattern), &pattern,
allocation_size, 0, nullptr, &event));
EXPECT_SUCCESS(urQueueFlush(queue));
ASSERT_SUCCESS(urEventWait(1, &event));
EXPECT_SUCCESS(urEventWait(1, &event));

ASSERT_NE(ptr, nullptr);
ASSERT_SUCCESS(urUSMFree(context, ptr));
ASSERT_SUCCESS(urEventRelease(event));
}
Expand All @@ -43,15 +43,15 @@ TEST_P(urUSMFreeTest, SuccessHostAlloc) {
size_t allocation_size = sizeof(int);
ASSERT_SUCCESS(
urUSMHostAlloc(context, nullptr, nullptr, allocation_size, &ptr));
ASSERT_NE(ptr, nullptr);

ur_event_handle_t event = nullptr;
uint8_t pattern = 0;
ASSERT_SUCCESS(urEnqueueUSMFill(queue, ptr, sizeof(pattern), &pattern,
EXPECT_SUCCESS(urEnqueueUSMFill(queue, ptr, sizeof(pattern), &pattern,
allocation_size, 0, nullptr, &event));
EXPECT_SUCCESS(urQueueFlush(queue));
ASSERT_SUCCESS(urEventWait(1, &event));
EXPECT_SUCCESS(urEventWait(1, &event));

ASSERT_NE(ptr, nullptr);
ASSERT_SUCCESS(urUSMFree(context, ptr));
ASSERT_SUCCESS(urEventRelease(event));
}
Expand All @@ -73,15 +73,15 @@ TEST_P(urUSMFreeTest, SuccessSharedAlloc) {
size_t allocation_size = sizeof(int);
ASSERT_SUCCESS(urUSMSharedAlloc(context, device, nullptr, nullptr,
allocation_size, &ptr));
ASSERT_NE(ptr, nullptr);

ur_event_handle_t event = nullptr;
uint8_t pattern = 0;
ASSERT_SUCCESS(urEnqueueUSMFill(queue, ptr, sizeof(pattern), &pattern,
EXPECT_SUCCESS(urEnqueueUSMFill(queue, ptr, sizeof(pattern), &pattern,
allocation_size, 0, nullptr, &event));
EXPECT_SUCCESS(urQueueFlush(queue));
ASSERT_SUCCESS(urEventWait(1, &event));
EXPECT_SUCCESS(urEventWait(1, &event));

ASSERT_NE(ptr, nullptr);
ASSERT_SUCCESS(urUSMFree(context, ptr));
ASSERT_SUCCESS(urEventRelease(event));
}
Expand Down
25 changes: 9 additions & 16 deletions test/conformance/usm/urUSMSharedAlloc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ struct urUSMSharedAllocTest

ur_usm_pool_handle_t pool = nullptr;
bool usePool = std::get<0>(getParam()).value;
void *ptr = nullptr;
};

// The 0 value parameters are not relevant for urUSMSharedAllocTest tests, they
Expand All @@ -57,17 +58,16 @@ UUR_TEST_SUITE_P(
uur::printUSMAllocTestString<urUSMSharedAllocTest>);

TEST_P(urUSMSharedAllocTest, Success) {
void *ptr = nullptr;
size_t allocation_size = sizeof(int);
ASSERT_SUCCESS(urUSMSharedAlloc(context, device, nullptr, pool,
allocation_size, &ptr));
ASSERT_NE(ptr, nullptr);

ur_event_handle_t event = nullptr;
uint8_t pattern = 0;
ASSERT_SUCCESS(urEnqueueUSMFill(queue, ptr, sizeof(pattern), &pattern,
EXPECT_SUCCESS(urEnqueueUSMFill(queue, ptr, sizeof(pattern), &pattern,
allocation_size, 0, nullptr, &event));
ASSERT_SUCCESS(urEventWait(1, &event));
EXPECT_SUCCESS(urEventWait(1, &event));

ASSERT_SUCCESS(urUSMFree(context, ptr));
EXPECT_SUCCESS(urEventRelease(event));
Expand All @@ -85,17 +85,16 @@ TEST_P(urUSMSharedAllocTest, SuccessWithDescriptors) {
ur_usm_desc_t usm_desc{UR_STRUCTURE_TYPE_USM_DESC, &usm_host_desc,
/* mem advice flags */ UR_USM_ADVICE_FLAG_DEFAULT,
/* alignment */ 0};
void *ptr = nullptr;
size_t allocation_size = sizeof(int);
ASSERT_SUCCESS(urUSMSharedAlloc(context, device, &usm_desc, pool,
allocation_size, &ptr));
ASSERT_NE(ptr, nullptr);

ur_event_handle_t event = nullptr;
uint8_t pattern = 0;
ASSERT_SUCCESS(urEnqueueUSMFill(queue, ptr, sizeof(pattern), &pattern,
EXPECT_SUCCESS(urEnqueueUSMFill(queue, ptr, sizeof(pattern), &pattern,
allocation_size, 0, nullptr, &event));
ASSERT_SUCCESS(urEventWait(1, &event));
EXPECT_SUCCESS(urEventWait(1, &event));

ASSERT_SUCCESS(urUSMFree(context, ptr));
EXPECT_SUCCESS(urEventRelease(event));
Expand All @@ -107,31 +106,28 @@ TEST_P(urUSMSharedAllocTest, SuccessWithMultipleAdvices) {
/* mem advice flags */ UR_USM_ADVICE_FLAG_SET_READ_MOSTLY |
UR_USM_ADVICE_FLAG_BIAS_CACHED,
/* alignment */ 0};
void *ptr = nullptr;
size_t allocation_size = sizeof(int);
ASSERT_SUCCESS(urUSMSharedAlloc(context, device, &usm_desc, pool,
allocation_size, &ptr));
ASSERT_NE(ptr, nullptr);

ur_event_handle_t event = nullptr;
uint8_t pattern = 0;
ASSERT_SUCCESS(urEnqueueUSMFill(queue, ptr, sizeof(pattern), &pattern,
EXPECT_SUCCESS(urEnqueueUSMFill(queue, ptr, sizeof(pattern), &pattern,
allocation_size, 0, nullptr, &event));
ASSERT_SUCCESS(urEventWait(1, &event));
EXPECT_SUCCESS(urEventWait(1, &event));

ASSERT_SUCCESS(urUSMFree(context, ptr));
EXPECT_SUCCESS(urEventRelease(event));
}

TEST_P(urUSMSharedAllocTest, InvalidNullHandleContext) {
void *ptr = nullptr;
ASSERT_EQ_RESULT(
UR_RESULT_ERROR_INVALID_NULL_HANDLE,
urUSMSharedAlloc(nullptr, device, nullptr, pool, sizeof(int), &ptr));
}

TEST_P(urUSMSharedAllocTest, InvalidNullHandleDevice) {
void *ptr = nullptr;
ASSERT_EQ_RESULT(
UR_RESULT_ERROR_INVALID_NULL_HANDLE,
urUSMSharedAlloc(context, nullptr, nullptr, pool, sizeof(int), &ptr));
Expand All @@ -144,14 +140,12 @@ TEST_P(urUSMSharedAllocTest, InvalidNullPtrMem) {
}

TEST_P(urUSMSharedAllocTest, InvalidUSMSize) {
void *ptr = nullptr;
ASSERT_EQ_RESULT(
UR_RESULT_ERROR_INVALID_USM_SIZE,
urUSMSharedAlloc(context, device, nullptr, pool, -1, &ptr));
}

TEST_P(urUSMSharedAllocTest, InvalidValueAlignPowerOfTwo) {
void *ptr = nullptr;
ur_usm_desc_t desc = {};
desc.stype = UR_STRUCTURE_TYPE_USM_DESC;
desc.align = 5;
Expand Down Expand Up @@ -185,16 +179,15 @@ TEST_P(urUSMSharedAllocAlignmentTest, SuccessAlignedAllocations) {
/* mem advice flags */ UR_USM_ADVICE_FLAG_DEFAULT,
alignment};

void *ptr = nullptr;
ASSERT_SUCCESS(urUSMSharedAlloc(context, device, &usm_desc, pool,
allocation_size, &ptr));
ASSERT_NE(ptr, nullptr);

ur_event_handle_t event = nullptr;
uint8_t pattern = 0;
ASSERT_SUCCESS(urEnqueueUSMFill(queue, ptr, sizeof(pattern), &pattern,
EXPECT_SUCCESS(urEnqueueUSMFill(queue, ptr, sizeof(pattern), &pattern,
allocation_size, 0, nullptr, &event));
ASSERT_SUCCESS(urEventWait(1, &event));
EXPECT_SUCCESS(urEventWait(1, &event));

ASSERT_SUCCESS(urUSMFree(context, ptr));
EXPECT_SUCCESS(urEventRelease(event));
Expand Down

0 comments on commit a284d1f

Please sign in to comment.