[Openmp-commits] [openmp] 22558c8 - [OpenMP] libomp: Fix possible NULL dereferences
via Openmp-commits
openmp-commits at lists.llvm.org
Mon Dec 7 08:14:16 PST 2020
Author: AndreyChurbanov
Date: 2020-12-07T19:09:07+03:00
New Revision: 22558c8501eaf5e7547ee13fa5a009efdec6dc90
URL: https://github.com/llvm/llvm-project/commit/22558c8501eaf5e7547ee13fa5a009efdec6dc90
DIFF: https://github.com/llvm/llvm-project/commit/22558c8501eaf5e7547ee13fa5a009efdec6dc90.diff
LOG: [OpenMP] libomp: Fix possible NULL dereferences
Check pointer returned by strchr, as it can be NULL in case of broken
format of input string. Introduced new function __kmp_str_loc_numbers
for fast parsing of numbers only in the location string.
Also made some cleanup of __kmp_str_loc_init declaration and usage:
- changed type of init_fname parameter to bool;
- changed input from true to false in places where fname is not used.
Differential Revision: https://reviews.llvm.org/D90962
Added:
Modified:
openmp/runtime/src/kmp_debugger.cpp
openmp/runtime/src/kmp_itt.inl
openmp/runtime/src/kmp_lock.cpp
openmp/runtime/src/kmp_str.cpp
openmp/runtime/src/kmp_str.h
Removed:
################################################################################
diff --git a/openmp/runtime/src/kmp_debugger.cpp b/openmp/runtime/src/kmp_debugger.cpp
index 490300f9b207..2a1f633c49c1 100644
--- a/openmp/runtime/src/kmp_debugger.cpp
+++ b/openmp/runtime/src/kmp_debugger.cpp
@@ -269,7 +269,7 @@ int __kmp_omp_num_threads(ident_t const *ident) {
if (info->num > 0 && info->array != 0) {
kmp_omp_nthr_item_t *items =
(kmp_omp_nthr_item_t *)__kmp_convert_to_ptr(info->array);
- kmp_str_loc_t loc = __kmp_str_loc_init(ident->psource, 1);
+ kmp_str_loc_t loc = __kmp_str_loc_init(ident->psource, true);
int i;
for (i = 0; i < info->num; ++i) {
if (kmp_location_match(&loc, &items[i])) {
diff --git a/openmp/runtime/src/kmp_itt.inl b/openmp/runtime/src/kmp_itt.inl
index 6257cea4faf3..09d5480284e0 100644
--- a/openmp/runtime/src/kmp_itt.inl
+++ b/openmp/runtime/src/kmp_itt.inl
@@ -115,7 +115,8 @@ LINKAGE void __kmp_itt_region_forking(int gtid, int team_size, int barriers) {
// that the tools more or less standardized on:
// "<func>$omp$parallel@[file:]<line>[:<col>]"
char *buff = NULL;
- kmp_str_loc_t str_loc = __kmp_str_loc_init(loc->psource, 1);
+ kmp_str_loc_t str_loc =
+ __kmp_str_loc_init(loc->psource, /* init_fname */ false);
buff = __kmp_str_format("%s$omp$parallel:%d@%s:%d:%d", str_loc.func,
team_size, str_loc.file, str_loc.line,
str_loc.col);
@@ -155,7 +156,8 @@ LINKAGE void __kmp_itt_region_forking(int gtid, int team_size, int barriers) {
if ((frm < KMP_MAX_FRAME_DOMAINS) &&
(__kmp_itt_region_team_size[frm] != team_size)) {
char *buff = NULL;
- kmp_str_loc_t str_loc = __kmp_str_loc_init(loc->psource, 1);
+ kmp_str_loc_t str_loc =
+ __kmp_str_loc_init(loc->psource, /* init_fname */ false);
buff = __kmp_str_format("%s$omp$parallel:%d@%s:%d:%d", str_loc.func,
team_size, str_loc.file, str_loc.line,
str_loc.col);
@@ -212,7 +214,8 @@ LINKAGE void __kmp_itt_frame_submit(int gtid, __itt_timestamp begin,
// that the tools more or less standardized on:
// "<func>$omp$parallel:team_size@[file:]<line>[:<col>]"
char *buff = NULL;
- kmp_str_loc_t str_loc = __kmp_str_loc_init(loc->psource, 1);
+ kmp_str_loc_t str_loc =
+ __kmp_str_loc_init(loc->psource, /* init_fname */ false);
buff = __kmp_str_format("%s$omp$parallel:%d@%s:%d:%d", str_loc.func,
team_size, str_loc.file, str_loc.line,
str_loc.col);
@@ -234,7 +237,8 @@ LINKAGE void __kmp_itt_frame_submit(int gtid, __itt_timestamp begin,
return; // something's gone wrong, returning
if (__kmp_itt_region_team_size[frm] != team_size) {
char *buff = NULL;
- kmp_str_loc_t str_loc = __kmp_str_loc_init(loc->psource, 1);
+ kmp_str_loc_t str_loc =
+ __kmp_str_loc_init(loc->psource, /* init_fname */ false);
buff = __kmp_str_format("%s$omp$parallel:%d@%s:%d:%d", str_loc.func,
team_size, str_loc.file, str_loc.line,
str_loc.col);
@@ -273,7 +277,8 @@ LINKAGE void __kmp_itt_frame_submit(int gtid, __itt_timestamp begin,
// Transform compiler-generated region location into the format
// that the tools more or less standardized on:
// "<func>$omp$frame@[file:]<line>[:<col>]"
- kmp_str_loc_t str_loc = __kmp_str_loc_init(loc->psource, 1);
+ kmp_str_loc_t str_loc =
+ __kmp_str_loc_init(loc->psource, /* init_fname */ false);
if (imbalance) {
char *buff_imb = NULL;
buff_imb = __kmp_str_format("%s$omp$barrier-imbalance:%d@%s:%d",
@@ -365,25 +370,12 @@ LINKAGE void __kmp_itt_metadata_loop(ident_t *loc, kmp_uint64 sched_type,
}
// Parse line and column from psource string: ";file;func;line;col;;"
- char *s_line;
- char *s_col;
KMP_DEBUG_ASSERT(loc->psource);
-#ifdef __cplusplus
- s_line = strchr(CCAST(char *, loc->psource), ';');
-#else
- s_line = strchr(loc->psource, ';');
-#endif
- KMP_DEBUG_ASSERT(s_line);
- s_line = strchr(s_line + 1, ';'); // 2-nd semicolon
- KMP_DEBUG_ASSERT(s_line);
- s_line = strchr(s_line + 1, ';'); // 3-rd semicolon
- KMP_DEBUG_ASSERT(s_line);
- s_col = strchr(s_line + 1, ';'); // 4-th semicolon
- KMP_DEBUG_ASSERT(s_col);
-
kmp_uint64 loop_data[5];
- loop_data[0] = atoi(s_line + 1); // read line
- loop_data[1] = atoi(s_col + 1); // read column
+ int line, col;
+ __kmp_str_loc_numbers(loc->psource, &line, &col);
+ loop_data[0] = line;
+ loop_data[1] = col;
loop_data[2] = sched_type;
loop_data[3] = iterations;
loop_data[4] = chunk;
@@ -409,12 +401,11 @@ LINKAGE void __kmp_itt_metadata_single(ident_t *loc) {
__kmp_release_bootstrap_lock(&metadata_lock);
}
- kmp_str_loc_t str_loc = __kmp_str_loc_init(loc->psource, 1);
+ int line, col;
+ __kmp_str_loc_numbers(loc->psource, &line, &col);
kmp_uint64 single_data[2];
- single_data[0] = str_loc.line;
- single_data[1] = str_loc.col;
-
- __kmp_str_loc_free(&str_loc);
+ single_data[0] = line;
+ single_data[1] = col;
__itt_metadata_add(metadata_domain, __itt_null, string_handle_sngl,
__itt_metadata_u64, 2, single_data);
diff --git a/openmp/runtime/src/kmp_lock.cpp b/openmp/runtime/src/kmp_lock.cpp
index ee72a8610359..6fa6bf060673 100644
--- a/openmp/runtime/src/kmp_lock.cpp
+++ b/openmp/runtime/src/kmp_lock.cpp
@@ -3889,7 +3889,7 @@ void __kmp_cleanup_user_locks(void) {
if (__kmp_env_consistency_check && (!IS_CRITICAL(lck)) &&
((loc = __kmp_get_user_lock_location(lck)) != NULL) &&
(loc->psource != NULL)) {
- kmp_str_loc_t str_loc = __kmp_str_loc_init(loc->psource, 0);
+ kmp_str_loc_t str_loc = __kmp_str_loc_init(loc->psource, false);
KMP_WARNING(CnsLockNotDestroyed, str_loc.file, str_loc.line);
__kmp_str_loc_free(&str_loc);
}
diff --git a/openmp/runtime/src/kmp_str.cpp b/openmp/runtime/src/kmp_str.cpp
index 75fd1e25f347..24526e587412 100644
--- a/openmp/runtime/src/kmp_str.cpp
+++ b/openmp/runtime/src/kmp_str.cpp
@@ -295,7 +295,54 @@ int __kmp_str_fname_match(kmp_str_fname_t const *fname, char const *pattern) {
return dir_match && base_match;
} // __kmp_str_fname_match
-kmp_str_loc_t __kmp_str_loc_init(char const *psource, int init_fname) {
+// Get the numeric fields from source location string.
+// For clang these fields are Line/Col of the start of the construct.
+// For icc these are LineBegin/LineEnd of the construct.
+// Function is fast as it does not duplicate string (which involves memory
+// allocation), and parses the string in place.
+void __kmp_str_loc_numbers(char const *Psource, int *LineBeg,
+ int *LineEndOrCol) {
+ char *Str;
+ KMP_DEBUG_ASSERT(LineBeg);
+ KMP_DEBUG_ASSERT(LineEndOrCol);
+ // Parse Psource string ";file;func;line;line_end_or_column;;" to get
+ // numbers only, skipping string fields "file" and "func".
+
+ // Find 1-st semicolon.
+ KMP_DEBUG_ASSERT(Psource);
+#ifdef __cplusplus
+ Str = strchr(CCAST(char *, Psource), ';');
+#else
+ Str = strchr(Psource, ';');
+#endif
+ // Check returned pointer to see if the format of Psource is broken.
+ if (Str) {
+ // Find 2-nd semicolon.
+ Str = strchr(Str + 1, ';');
+ }
+ if (Str) {
+ // Find 3-rd semicolon.
+ Str = strchr(Str + 1, ';');
+ }
+ if (Str) {
+ // Read begin line number.
+ *LineBeg = atoi(Str + 1);
+ // Find 4-th semicolon.
+ Str = strchr(Str + 1, ';');
+ } else {
+ // Broken format of input string, cannot read the number.
+ *LineBeg = 0;
+ }
+ if (Str) {
+ // Read end line number.
+ *LineEndOrCol = atoi(Str + 1);
+ } else {
+ // Broken format of input string, cannot read the number.
+ *LineEndOrCol = 0;
+ }
+}
+
+kmp_str_loc_t __kmp_str_loc_init(char const *psource, bool init_fname) {
kmp_str_loc_t loc;
loc._bulk = NULL;
diff --git a/openmp/runtime/src/kmp_str.h b/openmp/runtime/src/kmp_str.h
index 9e669bbe4742..98e2f4a457fe 100644
--- a/openmp/runtime/src/kmp_str.h
+++ b/openmp/runtime/src/kmp_str.h
@@ -81,7 +81,7 @@ int __kmp_str_fname_match(kmp_str_fname_t const *fname, char const *pattern);
structure keeps source location in more convenient form.
Usage:
- kmp_str_loc_t loc = __kmp_str_loc_init( ident->psource, 0 );
+ kmp_str_loc_t loc = __kmp_str_loc_init(ident->psource, false);
// use loc.file, loc.func, loc.line, loc.col.
// loc.fname is available if second argument of __kmp_str_loc_init is true.
__kmp_str_loc_free( & loc );
@@ -98,7 +98,8 @@ struct kmp_str_loc {
int col;
}; // struct kmp_str_loc
typedef struct kmp_str_loc kmp_str_loc_t;
-kmp_str_loc_t __kmp_str_loc_init(char const *psource, int init_fname);
+kmp_str_loc_t __kmp_str_loc_init(char const *psource, bool init_fname);
+void __kmp_str_loc_numbers(char const *Psource, int *Line, int *Col);
void __kmp_str_loc_free(kmp_str_loc_t *loc);
int __kmp_str_eqf(char const *lhs, char const *rhs);
More information about the Openmp-commits
mailing list