27175 - Add PR_FindFunctionSymbol()

Status: RESOLVED FIXED

(NSPR :: NSPR, enhancement, P3)

Description

[John G. Myers]

prlink.h should add the functions:

NSPR_API(void (*)()) PR_FindFunctionSymbol(PRLibrary *lib, const char *name);

NSPR_API(void (*)()) PR_FindFunctionSymbolAndLibrary(const char *name, PRLibrary 
**lib);

According to the ANSI C standard, it is not portable to cast a function pointer 
to (void *) or back.  Function pointers are allowed to be larger than data 
pointers.

The casting appears to work on all of NSPR's current platforms, but on some of 
them the compiler generates a warning when the value returned by 
PR_Find'Symbol() is cast to a function pointer.

Deferring this to NSPR 4.1 would be OK.

Comment 1

[srinivas]

Attachment: Patch containing the fix.

Comment 2

[srinivas]

The patch defines a new type for function pointers

typedef void (*PRFuncPtr)(void);

and two new functions

NSPR_API(PRFuncPtr) PR_FindFunctionSymbol(PRLibrary *lib, const char *name);
NSPR_API(PRFuncPtr) PR_FindFunctionSymbolAndLibrary(const char *name, PRLibrary* 
*lib);

Comment 3

[srinivas]

Checked in the patch and also added the new functions to pr/tests/dlltest.c

Files modified:

/cvsroot/mozilla/nsprpub/pr/include/prlink.h,v  <--  prlink.h
new revision: 3.6; previous revision: 3.5

/cvsroot/mozilla/nsprpub/pr/src/linking/prlink.c,v  <--  prlink.c
new revision: 3.36; previous revision: 3.35

/cvsroot/mozilla/nsprpub/pr/tests/dlltest.c,v  <--  dlltest.c
new revision: 3.4; previous revision: 3.3

Comment 4

[srinivas]

Marking resolved.

Comment 5

[Wan-Teh Chang]

I reviewed the patch and made the following changes:

Changed the PRFuncPtr type to void (*)(), to be consistent
with the type of the 'fp' field of the PRStaticLinkTable
structure.
/cvsroot/mozilla/nsprpub/pr/include/prlink.h, revision 3.7

Reverted PR_FindSymbol and PR_FindSymbolAndLibrary to their
original definition and define the new functions
PR_FindFunctionSymbol and PR_FindFunctionSymbolAndLibrary
in terms of them.  This is to minimize the risk of the new code
breaking the build on platforms that we haven't tested it on.
The only problem with this is that the cast from void* to
void (*)() may lose precision if function pointers are larger
than data pointers. 
/cvsroot/mozilla/nsprpub/pr/src/linking/prlink.c, revision 3.37

Comment 6

[srinivas]

This modification breaks the solution in the first patch. The loss of 
information in the, potentially, narrowing conversion of the cast from a 
function pointer to a (void *) is the very problem that needs to be fixed here.

On some platforms such as Unix, the native function-address-lookup 
interface returns a void *, and in these cases the implementation of 
PR_FindSymbol and PR_FindFunctionSymbol can be the same.

On other platforms such as Win32, the native function-address-lookup interface 
is properly defined to return a pointer to a funtion. When the function and data 
pointers are of different size, the patch in rev 3.37 may cause the 
PR_FindFunctionSymbol to return invalid values.

If it is intended that the original implementation of PR_FindSymbol not 
be modified, then PR_FindFunctionSymbol should be implemented in parallel to 
not make incorrect conversions of the values returned by the native 
function-address-lookup interfaces.

Defining PR_FindFunctionSymbol in terms of PR_FindSymbol simply moves the 
compiler warning from the build of client code to that of NSPR code.

Comment 7

[Wan-Teh Chang]

It is not portable to cast a function pointer
to (void *) either, so defining PR_FindSymbol
in terms of PR_FindFunctionSymbol would also
cause a compiler warning.

Cleaning up the load library API is one of
the tasks of NSPR 4.1, so there will be more
changes to come.  I'm planning to add
PR_FindDataSymbol, which returns (void *),
and have PR_FindDataSymbol and
PR_FindFunctionSymbol share the same
internal symbol lookup function (pr_FindSymbolInLib
in prlink.c) that takes a symbol type parameter,
a la the shl_findsym function of HP-UX.

I reverted your patch because it still does the
nonportable PRFuncPtr=>(void *) cast, and because
I'm planning to do more internal reorg of the
prlink.c code. Sorry if I wasn't clear.