posix: Use RTLD_DEFAULT instead of NULL in dlsym

[skliper]

In {{{OS_SymbolLookup}}} defined in {{{osloader.c}}} in the POSIX OSAL, [http://pubs.opengroup.org/onlinepubs/9699919799/functions/dlsym.html dlsym] is used to obtain the address of the specified symbol:
{{{
Function = dlsym((void *)0, SymbolName);
}}}

The behavior of {{{dlsym}}} when called with a null pointer for the first argument is not defined by POSIX. On GNU/Linux, this value corresponds to {{{RTLD_DEFAULT}}}, as found in {{{/usr/include/dlfcn.h}}}.
{{{

define RTLD_DEFAULT ((void *) 0)

}}}

When called with {{{RTLD_DEFAULT}}} as a first argument, "The identifier lookup happens in the normal global scope; that is, a search for an identifier using handle would find the same definition as a direct use of this identifier in the program code." This appears to be the intended behavior in the OSAL.

However, the value of {{{RTLD_DEFAULT}}} is implementation-defined according to POSIX. The macro, rather than the value it happens to have on GNU/Linux, should be used.

This would address part of #203.

[skliper]

Imported from trac issue 191. Created by tflemin1 on 2017-10-30T14:12:36, last modified: 2019-08-14T14:11:46

[davearch]

Hello, I wanted to fix an issue but it seems this one is already fixed on line 102 of osloader.c:
Function = dlsym(RTLD_DEFAULT, SymbolName);

Is there anything else that needs to be done for this?

[skliper]

Referenced code was refactored, current issue is at:

osal/src/os/portable/os-impl-posix-dl.c

Line 126 in 2284a6f

Function = dlsym((void *)0, SymbolName);

[jphickey]

Note that this code is used by both RTEMS and POSIX, and (IIRC) RTEMS does not define the RTLD_DEFAULT symbol. So whatever change is made here needs to take this into account and confirm that the code builds and runs on both platforms.

[skliper]

Well... it's used in src/os/rtems/osloader.c...

osal/src/os/rtems/osloader.c

Line 102 in 2284a6f

Function = dlsym(RTLD_DEFAULT, SymbolName);

[skliper]

Doesn't look to me like rtems includes the portable version?

[jphickey]

Hmm, made the previous comment from memory without looking at the code ....

All I can say is that there was some issue with using RTLD_DEFAULT across systems which is why it was hardcoded as NULL.

[skliper]

Removing good first issue label (at least until a specific fix is identified.)

Only posix osloader.c includes the os-impl-posix-dl.c.

In the context of POSIX is it an issue due to RTLD_DEFAULT being "reserved", maybe there was a system on which it wasn't defined? Should we add an #ifndef RTLD_DEFAULT such that we use it when available and define if not?

[jphickey]

I want to say it is something like the symbol was only available when defining _GNU_SOURCE or something of that nature. Maybe it has to do with the version of glibc in use - which means any fix should be tested on at least one older system as well as a newer system.

[jphickey]

Took a closer look at this.

I can confirm the reason that this is not using RTLD_DEFAULT right now is because the XPG6 standard does not actually define this symbol. Glibc only provides a value here as a GNU extension. Officially the XOpen/6 standard just says:

The RTLD_DEFAULT and RTLD_NEXT flags are reserved for future use

When building with _XOPEN_SOURCE=600 on glibc as the pc-linux BSP does, the symbol is undefined.

In contrast, regarding the RTEMS implementation, on that platform the RTLD_DEFAULT value is actually not null, it has a nonzero value.

It might make the most sense to just add an #ifdef and roll it into the changes already being worked for e.g. #285, as this code is changing anyway for that ticket. Any change made here is just eventually going to be a merge conflict.

[skliper]

Sounds good, re-added the milestone as a target for my tracking, not critical though.

[jphickey]

This is rolled into the larger change in issue #285, pull request #427. This contains a bit of preprocessor logic such that if the RTLD_DEFAULT symbol is provided by the C library, it will be used, but otherwise will default to NULL instead.

See: https://github.com/jphickey/osal/blob/1cf85b2a1662d22bad70d4b9797e0c52e515eb0c/src/os/portable/os-impl-posix-dl-symtab.c#L46-L64

[stanislaw]

See also this. There I had to use RTLD_DEFAULT to make things work on macOS.

[jphickey]

I think the approach proposed in the draft pull request should work on MacOS too, but I don't currently have a facility to test on this platform.

RTEMS has a similar issue where NULL doesn't work - one must actually use RTLD_DEFAULT, but POSIX doesn't actually require the symbol, so hence the need for a conditional.

One could make the case that we are actually invoking platform-defined behavior here by using NULL, as glibc considers RTLD_DEFAULT as a GNU extension. Pedantically speaking, since this symbol is a GNU extension and POSIX does not specify a standard way of looking up symbols in the base executable, that implicitly means the API as defined by OSAL is not implementable in a strict POSIX environment.....

[stanislaw]

I think the approach proposed in the draft pull request should work on MacOS too, but I don't currently have a facility to test on this platform.

It definitely should. I can try it later today and report the result.

[stanislaw]

@jphickey the code from your draft works for me here: https://github.com/nasa/osal/pull/352/files#diff-ea89b73e0d390f934db08d14699de6a4R77.

I also have to add this in order for some symbols including RTLD_DEFAULT to be available:

target_compile_definitions(osal_posix_impl PRIVATE -D_DARWIN_C_SOURCE)

otherwise I get:

# 1) In file included from /sandbox/cFS/osal/src/os/posix/osloader.c:32:
#/sandbox/cFS/osal/src/os/posix/../portable/os-impl-posix-dl.c:130:30: error: use of undeclared identifier 'RTLD_DEFAULT'
#    Function = dlsym((void *)RTLD_DEFAULT, SymbolName);
#                             ^
# 2) In file included from /sandbox/cFS/osal/src/os/posix/osfilesys.c:35:
# In file included from /usr/include/sys/mount.h:76:
# In file included from /usr/include/sys/attr.h:42:
# /usr/include/sys/ucred.h:96:11: error: unknown type name 'u_long'; did you mean 'long'?
#         volatile u_long         cr_ref;  /* reference count */
#                  ^
# /usr/include/sys/ucred.h:139:2: error: unknown type name 'u_int'
#         u_int   cr_version;             /* structure layout version */
#         ^

[skliper]

Closed by #443