552569 - misc fixes for compilation with Android/Bionic

Status: RESOLVED INVALID

(NSS :: Libraries, enhancement)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: fixes for NSS on Android/Bionic

NSS compilation on Android (with Bionic libc) needs a few small fixes to work around quirks there; there's no way to do GetLibraryFilePathname, there's no sysinfo, and pthread_atfork isn't available.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 432699 [details] [diff] [review]
fixes for NSS on Android/Bionic

Adding add'l r? as well -- not sure who's the right person to request review from here.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 432699 [details] [diff] [review]
fixes for NSS on Android/Bionic

I don't doubt that these changes help NSS to _compile_ on android
but I suspect that NSS doesn't actually WORK well with them.
NSS won't be very happy without a working PR_GetLibraryFilePathname
and it needs good entropy sources, not weaker ones.  
I suspect that the port to Android is going to require more than 
minimal incision surgery.

I wouldn't recommend committing these changes.  I think they're at 
best place holders until the right solutions can be found.  
Maybe you need to make a branch on which you can commit such changes
while you work out the rest of the port.

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Hmm, I couldn't quite follow how PR_GetLibraryFilePathname is used -- what is it used for?  It looks like it tries to pick which freebl shlib to load; we can hardcode one and preload it ahead of time -- we actually already do anyway, so a solution here might be to just have the relevant function check if symbols from it are already loaded in the process.  What problems would we see if GetLibraryFilePathname isn't working?  The browser seems to be able to visit https sites fine, though I haven't done extensive checks with that.

For the entropy piece, is not including struct sysinfo really going to break the world?  We still have everything else, we'd just be giving up one entropy source.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Comments:
1) Because Android is not yet a supported platform for NSPR, any "bugs" 
filed about Android support at this time are enhancement requests.
2) Because this patch would modify code inside the FIPS boundary, it cannot 
be taken before the 3.13 release, since all code inside the FIPS boundary
is now frozen in the 3.12.x releases.  So, I am targeting this bug for 3.13.

In answer to the question in comment 3:
NSS dynamically loads several of its shared libraries.  It does NOT require 
them to be in the PATH (on Windows) or the LD_LIBRARY_PATH (on **IX).  
It expects to find them in the same directory where its base shared libraries 
are found.  So it needs to be able to ask "in what directory was the base NSS 
shared lib found?".  That is what PR_GetLibraryFilePathname accomplishes.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Attachment: Above patch converted to form usable by bugzilla

For NSS and NSPR bugs, please submit patches that are CVS diffs.  The command
  cvs diff -Npu security/nss 
(or whatever directory you need) works very well for this purpose.

I have attempted to edit your patch to make it more usable by bugzilla's diff 
reviewing tools.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Attachment: above patch converted to CVS diff

My hand edit didn't work.  Here's a real CVS diff output.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

OK, I'm not going to give your patch a full review yet, but I want you to 
try something.  Please remove your patch from stubs.c and try doing a full 
clean/clobber rebuild of all of NSS with this environment variable set as 
follows

export FREEBL_NO_DEPEND=1

I think that will solve your problem.  If not, let me know.  If it does, 
then I'll have you submit a different patch.

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I think we can do this a different way.