Nelson B Bolyard wrote:
Neil wrote, On 2009-03-29 17:01:
Firefox may want to switch to using the jemalloc allocator on the Mac.
Well, I hope it won't mean for the Mac what it has meant on Windows.
On Windows, the JEMalloc code can ONLY be built with the non-free
"professional" version of Microsoft Visual Studio, and CANNOT be built with the
free version. Consequently, developers who use the free version must build EVERYTHING
themselves, and cannot mix and match libraries they build with libraries they download
from Mozilla's site.
This has greatly increased the perception, among many developers who develop
for Windows, that Firefox has effectively reverted to being MoCo's corporate
product, and not quite the truly open product that it once was, and still
claims to be. It's open, but its components cannot be mixed and matched with
builds from other builders, even for the same platform.
I'm sure a similar system could be used to enable people to build using
VC++ Express, using the jemalloc allocator only for NS_Alloc/Free etc.
As I understand it, this is tricky because you can't globally replace malloc/free. While in theory it would be possible to replace NS_Free and/or PR_Free, this runs into problems when the code doesn't match its allocators.
Right.
In particular, I am counting NS_Free, PR_Free, PL_strfree and free as separate
deallocators,
Oh, it's worse than that. It you link a shared library or executable with the "static" version of the c run
time on Windows, then that shared library/executable has its own heap. It's "free" is a different
deallocator than the "free" in other shared libs. So, unless you're careful to completely avoid linking with
static C run time libraries, every library's "free" is a separate deallocator. This wouldn't be so bad if
everyone avoided using static c RTL, but Mozilla actually recommends that people DO use the static RTL, to work around
the afore-mentioned problem with JEMalloc on Windows. :(
Ah yes, I hadn't considered that someone might try to use free on memory
allocated with malloc in a separate module.
which might strike you as a bit harsh, as you probably only need to care about NS_Free.
That's an interesting statement. It suggests some mismatch between the people
you intended to read this message, and the people who actually read the
dev-tech-crypto mailing list.
Developers of NSS and NSPR _NEVER_ use NS_Free and never care about it. They
all hang out in dev-tech-crypto.
Thanks for clearing that up.
Developers of PSM (which is part of Necko or Gecko, I think) care about all of
those allocators you mentioned. They occasionally (but somewhat rarely, I
think) hang out in dev-tech-crypto. I don't know where else they hang out.
That's a shame, because I'd like to know!
Developers of mail/news are yet a third breed. :) They don't hang out in
dev-tech-crypto at all, AFAIK. (sniffle)
Note that nsXPIDLCString and .Adopt() counts as an NS_Free, while nsCRT::strdup
counts as a PR_Malloc,
That sounds confusing.
and I'm currently lumping PR_smprintf_free in with free, and PORT_Free with
whatever it calls.
Hmm. All the PR_ functions use free, which is the free for NSPR's heap. I
think of all the PR_ functions as having their own heap, for which the free
function (as seen by code outside of NSPR) is PR_Free. So, effectively,
PR_smprintf_free is the same as PR_Free, both using NSPR's heap.
You're right, I had misremembered. I put it down to the earlier confusion!
PORT_Free is a wrapper for PR_Free. NSS tries pretty hard to only use NSPR's
allocator for allocation of any objects whose addresses NSS may give out to its
callers. NSS may occasionally use malloc/calloc/free for data items that are
always purely private to NSS, never being passed to other libraries to
(potentially) be freed by those other libraries. If you find a place where NSS
allocates memory using a non-NSPR allocator and passes that memory to code
outside of NSS to be freed, that's a bug.
I instrumented the code and I ran all the mochitests (which include some SSL) and I could
only find a few (<20) allocator mismatches in Gecko/Toolkit. By comparison, there are a
horrifying number of allocator mismatches in mailnews (nsDirPrefs.cpp on its own manages to
use one deallocator for a variable allocated by one of three allocators) and several in
libjar, narrowly relegating you to third in a hypothetical "hall of shame". (I
haven't filed bugs on mailnews or libjar yet either.)
I wonder who is the "you" relegated to third place. NSS? PSM? mailnews?
Which libjar? There are two. NSS has one used in NSS utility programs, and
Firefox has one. Firefox doesn't use NSS's and NSS utilities don't use Firefox's.
See http://mxr.mozilla.org/mozilla/find?string=jar%2F&tree=mozilla
Thanks for the clarification.
Using the above criteria, I found the following allocator mismatches under
security/*
OK, so, under security/* you have PSM and NSS. Different groups of developers
with different constraints.
nsNSSCallbacks.cpp: HandshakeCallback calls Adopt(cipherName) instead of Assign
and PORT_Free. In fact, it can leak cipherName.
nsNSSCertificate.cpp: several times it Adopt()s the result of CERT_Hexify, when
it should use PORT_Free.
nsNSSIOLayer.cpp: both adopts the result of and allocates an outparam using
nsCRT::strdup, when both times it should use NS_strdup, while SSL_RevealURL
allocates with PL_strdup but charCleaner uses PR_FREEIF.
nsSSLStatus.cpp: allocates an outparam using PL_strdup, when it should use
ToNewCString.
OK, the above issues are all PSM issues. Bugs for those would be filed against
product: Core, component: Security/PSM.
Thanks, I will file those bugs later this week.
pk12util.c: uses PR_Free for a string allocated with malloc,
That's a bug in an NSS utility program. The NSS team will definitely be
interested in fixing that. Please file an NSS bug. But know that that won't
have any effect on Firefox.
It will, because the mochitests use pk12util.
and PORT_ZFree for a string allocated with PL_strdup.
PORT_ZFree is a wrapper for PR_Free. The question is: is it correct to
allocate with PL_strdup (part of NSPR) and free with PL_Free (also part of
NSPR, but part of a separate NSPR shared library) ?
Today, the answer is almost certainly "No, it's not correct". I wonder if we
could change that, so that NSPR effectively has just one allocator (that is, on heap)
used by both PL_ and PR_ functions, or if that would break the world as we know it.
Well, as I recall, PL_ functions go straight to malloc/free, but the PR_
functions could be compiled to have their own allocator. There is the
additional issue that those functions live in different libraries, so
that if they were compiled statically then they would still be mismatcched.
Whether right or wrong, good or bad, NSS intentionally confuses PR_ and PL_
having #defines that give PR_ names to the PL_ functions. That's probably
hurting this.
Would you be interested in patches for some or all of these? Who would want to
review them?
Definitely yes for any bugs in NSS or NSPR. You can ask me to review all the
NSS and NSPR bug patches. I may reassign that task to others for some bugs.
I didn't find any bugs in NSPR. Under which component should I file
those bugs in pk12util?
For PSM, the developers are Kai Engert and Honza Bombas. Again, I'd say file
bugs, and ask one of them to review. You might write directly to them for
their suggestions on reviewer selection for PSM bugs.
I can't advise you about mailnews.
Thanks, you've given me quite enough advice already :-)
By the way, what tool do you use to find these mismatches of allocators and
deallocators? I think those issues haven't gotten as much attention as they
deserve simply because we're not aware of tools that make it easy.
I don't have a tool that makes it easy. All I did was piggy-back on a
feature of the MSVC debug heap, and I deliberately overwrite the MSVC
memory guard areas (which are normally set to 0xFDFDFDFD); for example,
NS_Alloc might write 0x0303FDFD, and NS_Free would check for that value
and overwrite the correct value (thus faking out free), but when there's
a mismatch either NS_Free or free will complain as appropriate. I fixed
the few mismatched calls to free first, as they're harder to breakpoint,
then NS_Free, PR_Free and finally PL_strfree. With all my patches I can
now run all three mochitest suites without any allocator mismatches.
Thanks for researching this, Neil.
I actually wanted to audit mailnews, but I had to get the rest of Gecko
working first! Actually, I guess I should be doing more important things
first, such as adding a key for Paste as Quotation...
--
Warning: May contain traces of nuts.
--
dev-tech-crypto mailing list
dev-tech-crypto@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-tech-crypto
