[OpenMP] Fix possible NULL dereference

[AndreyChurbanov]

Summary

According to dlsym description, the value of symbol could be NULL,
and there is no error in this case. Thus dlerror will also return NULL in
this case. We need to check the value returned by dlerror before printing it.

[protze.joachim]

start_tool = (ompt_start_tool_t)dlsym(h, "ompt_start_tool");

I find the dlsym description confusing:

The function dlsym() [...] return[s] the address where that symbol is loaded into memory.

If the symbol is loaded into memory, how can the address be NULL?

Since the value of the symbol could actually be NULL (so that a NULL return from dlsym() need not indicate an error)

With if (!start_tool) we check the address of the symbol to be !=NULL, not the symbol itself.

If the symbol is NULL, but the function returns the address to the symbol, I would need to dereference the address to see the NULL, i.e. *start_tool == NULL? What do I miss here?

openmp/runtime/src/ompt-general.cpp

349

I would argue, that dlerror is cleared at this point, but calling the function anyways should not harm.

[AndreyChurbanov]

In D112174#3077754, @protze.joachim wrote:

start_tool = (ompt_start_tool_t)dlsym(h, "ompt_start_tool");

I find the dlsym description confusing:

The function dlsym() [...] return[s] the address where that symbol is loaded into memory.

If the symbol is loaded into memory, how can the address be NULL?

Since the value of the symbol could actually be NULL (so that a NULL return from dlsym() need not indicate an error)

With if (!start_tool) we check the address of the symbol to be !=NULL, not the symbol itself.

If the symbol is NULL, but the function returns the address to the symbol, I would need to dereference the address to see the NULL, i.e. *start_tool == NULL? What do I miss here?

The description of dlsym has a note (https://man7.org/linux/man-pages/man3/dlsym.3.html):

There are several scenarios when the address of a global symbol
is NULL.  For example, a symbol can be placed at zero address by
the linker, via a linker script or with --defsym command-line
option.  Undefined weak symbols also have NULL value.  Finally,
the symbol value may be the result of a GNU indirect function
(IFUNC) resolver function that returns NULL as the resolved
value.  In the latter case, dlsym() also returns NULL without
error.

So, theoretically both dlsym and following dlerror can return NULL.
I don't see a problem in using verbose recommended way of checking functions return values,
just to be on the safe side.

[protze.joachim]

Thanks for the clarification.
lgtm

openmp/runtime/src/ompt-general.cpp

#if KMP_OS_UNIX
       OMPT_VERBOSE_INIT_PRINT("Opening %s... ", fname);
       void *h = dlopen(fname, RTLD_LAZY);
       if (!h) {
         OMPT_VERBOSE_INIT_CONTINUED_PRINT("Failed: %s\n", dlerror());
       } else {
         OMPT_VERBOSE_INIT_CONTINUED_PRINT("Success. \n");
         OMPT_VERBOSE_INIT_PRINT("Searching for ompt_start_tool in %s... ",
                                 fname);
+        dlerror(); // Clear any existing error
         start_tool = (ompt_start_tool_t)dlsym(h, "ompt_start_tool");
         if (!start_tool) {
-          OMPT_VERBOSE_INIT_CONTINUED_PRINT("Failed: %s\n", dlerror());
+          char *error = dlerror();
+          if (error != NULL) {
+            OMPT_VERBOSE_INIT_CONTINUED_PRINT("Failed: %s\n", error);
+          } else {
+            OMPT_VERBOSE_INIT_CONTINUED_PRINT("Failed: %s\n",
+                                              "ompt_start_tool = NULL");
+          }
         } else
 #elif KMP_OS_WINDOWS
       OMPT_VERBOSE_INIT_PRINT("Opening %s... ", fname);
       HMODULE h = LoadLibrary(fname);
       if (!h) {
         OMPT_VERBOSE_INIT_CONTINUED_PRINT("Failed: Error %u\n", GetLastError());
       } else {
         OMPT_VERBOSE_INIT_CONTINUED_PRINT("Success. \n");