Re: Allocator mismatches

2009-04-05 Thread Nelson Bolyard
Neil wrote, On 2009-04-02 01:19:
 Nelson Bolyard wrote:
 
 If you have public redistributable header files that declare the MSVCRT
 memory guards, please send a pointer to them.  Thanks.
 
 
 I can't actually find the documentation I originally read about it; this
  is the nearest that I get from a search: 
 http://msdn.microsoft.com/en-us/library/aa298452(VS.60).aspx

Neil, if your code to do this is in a patch or checked in somewhere,
a pointer to that code would be appreciated.
I think I'd like to play with NSPR to have it do what you said you were
doing.  If I can see how you did it, I won't have to reinvent the wheel.

Thanks, and regards,
/Nelson
-- 
dev-tech-crypto mailing list
dev-tech-crypto@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-tech-crypto


Re: Allocator mismatches

2009-04-05 Thread Neil

Nelson Bolyard wrote:


Neil, if your code to do this is in a patch or checked in somewhere, a pointer 
to that code would be appreciated.
 

I was going to email you a diff, but it turns out that there's an error 
in my methodology and I wasn't catching people misusing PL_strfree :-(


--
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


Re: Allocator mismatches

2009-04-02 Thread Neil

Nelson Bolyard wrote:


If you have public redistributable header files that declare the MSVCRT memory 
guards, please send a pointer to them.  Thanks.
 

I can't actually find the documentation I originally read about it; this 
is the nearest that I get from a search:

http://msdn.microsoft.com/en-us/library/aa298452(VS.60).aspx

--
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


Re: Allocator mismatches

2009-04-02 Thread Neil

Nelson Bolyard wrote:


Benjamin Smedberg wrote, On 2009-03-31 08:35:
 


In the other cases, we should just fix the bug, which is (I think) why Neil 
posted originally.
   


I wish him Godspeed in fixing (or even filing) those bugs.
 

I filed bugs 486404/5 yesterday, and have a possible patch for the 
former, but I'm not sure of the way forward for the latter.


--
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


Re: Allocator mismatches

2009-04-01 Thread Nelson Bolyard
Benjamin Smedberg wrote, On 2009-03-31 08:35:
 On 3/30/09 5:34 PM, Nelson B Bolyard wrote:
 
 The problem is mixing DLLs that use standard VCRT with those that use 
 Mozilla's modified VCRT.
 
 As long as there are bugs in the browser of the sort that Neil has
 found, developers of libraries upon which the browser depends, who
 develop with the free version of MSVC, CANNOT test their own library
 builds against Firefox builds from Mozilla.
 
 On the contrary, extension developers certainly *do*, because the bugs
 that Neil has found rarely affect extension developers.

Wonderful.  But this has no relevance to NSS or NSPR.

 In the other cases, we should just fix the bug, which is (I think) why
 Neil posted originally.

I wish him Godspeed in fixing (or even filing) those bugs.
-- 
dev-tech-crypto mailing list
dev-tech-crypto@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-tech-crypto


Re: Allocator mismatches

2009-03-31 Thread Jean-Marc Desperrier

Nelson B Bolyard wrote:

The problem is in the way that Mozilla builds JEMalloc for FF on Windows.
They build a replacement for the Microsoft C RunTime Library.  This
replacement is a hybrid, built in part from JEMalloc source code, and in
part from Microsoft's source code for MSVCRT, which source code is ONLY
available with the professional edition of the MSVC compiler.


You know Nelson, I spent a little time investigating this, and whilst it 
would be some work to create a fully open source alternative, it would 
not be *that* much work. A good starting point is the reimplementation 
of MSVCRT in Wine (but there might be also some low level .obj to 
rewrite in addition *and* Wine does not have the latest version of MSVCRT).


So the best solution would be for one of those people that this 
situation really annoys to just do it instead of complaining.

--
dev-tech-crypto mailing list
dev-tech-crypto@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-tech-crypto


Re: Allocator mismatches

2009-03-31 Thread Neil

Nelson B Bolyard wrote:


The problem is in the way that Mozilla builds JEMalloc for FF on Windows. They 
build a replacement for the Microsoft C RunTime Library.

Ah, but I'm hoping that any solution that they manage to implement for 
the Mac will be portable to VC Express, since it will only replace 
NS_Alloc/Free, not malloc/free.



But please tell us, which libjar has the horrifying number of mismatches?  If 
it's NSS's, we'd like to know about them.
 


I didn't even know NSS had a libjar! And it's not that horrifying, really.


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) ?
 


I meant PR_Free.  There is no PL_Free, AFAIK.
 


There is PL_strfree though.


Under which component should I file those bugs in pk12util?
   


If the bug is in a source file under security/nss/cmd then component tools.


Yes, it's pk12util.c itself.

I don't have a tool that makes it easy. All I did was piggy-back on a feature of the MSVC debug heap, 
   


Another feature of the professional package only, sadly.
 


I use VC2005Express...

--
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


Re: Allocator mismatches

2009-03-31 Thread Wan-Teh Chang
On Sun, Mar 29, 2009 at 5:01 PM, Neil n...@parkwaycc.co.uk wrote:

 Using the above criteria, I found the following allocator mismatches under
 security/*

 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.

For these four files under mozilla/security/manager, please file a bug
report for product=Core, component=Security: PSM, and cc Kai
Engert (:kaie) and Honza Bambas.

 pk12util.c: uses PR_Free for a string allocated with malloc, and PORT_ZFree
 for a string allocated with PL_strdup.

For this file, please file a bug for product=NSS, component=Tools.

Thanks,
Wan-Teh
-- 
dev-tech-crypto mailing list
dev-tech-crypto@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-tech-crypto


Re: Allocator mismatches

2009-03-31 Thread Benjamin Smedberg
On 3/30/09 5:34 PM, Nelson B Bolyard wrote:

 The problem is mixing DLLs that use standard VCRT with those that use
 Mozilla's modified VCRT.
 
 As long as there are bugs in the browser of the sort that Neil has found,
 developers of libraries upon which the browser depends, who develop with
 the free version of MSVC, CANNOT test their own library builds against
 Firefox builds from Mozilla.

On the contrary, extension developers certainly *do*, because the bugs that
Neil has found rarely affect extension developers. In the other cases, we
should just fix the bug, which is (I think) why Neil posted originally.

--BDS
-- 
dev-tech-crypto mailing list
dev-tech-crypto@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-tech-crypto


Re: Allocator mismatches

2009-03-31 Thread Nelson Bolyard
Neil wrote:
 Nelson B Bolyard wrote:

 I don't have a tool that makes it easy. All I did was piggy-back on a
 feature of the MSVC debug heap,   
 Another feature of the professional package only, sadly.

 I use VC2005Express...

Hmm.  Perhaps it is the source code to the debug RTL that is only
available in the professional edition.

If you have public redistributable header files that declare the
MSVCRT memory guards, please send a pointer to them.  Thanks.


-- 
dev-tech-crypto mailing list
dev-tech-crypto@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-tech-crypto


Re: Allocator mismatches

2009-03-31 Thread Nelson Bolyard
Jean-Marc Desperrier wrote:
 Nelson B Bolyard wrote:
 The problem is in the way that Mozilla builds JEMalloc for FF on Windows.
 They build a replacement for the Microsoft C RunTime Library.  This
 replacement is a hybrid, built in part from JEMalloc source code, and in
 part from Microsoft's source code for MSVCRT, which source code is ONLY
 available with the professional edition of the MSVC compiler.
 
 You know Nelson, I spent a little time investigating this, and whilst it
 would be some work to create a fully open source alternative, it would
 not be *that* much work. A good starting point is the reimplementation
 of MSVCRT in Wine (but there might be also some low level .obj to
 rewrite in addition *and* Wine does not have the latest version of MSVCRT).
 
 So the best solution would be for one of those people that this
 situation really annoys to just do it instead of complaining.

Perhaps if NSS was developed exclusively for use in Firefox, that would
make sense.  But of all the many products that use NSS, only Firefox
uses this unusual CRT.  I can't justify taking time away from developing
NSS features (or fixing NSS bugs) that affect all products that use NSS
just to go and develop a special library that will enable me to test my
NSS builds with FF nightly builds.  So my choices are:

a) Don't test my NSS builds with FF at all, or
b) test my NSS builds with old FF builds from before the time when FF
used its own custom CRT,
c) build my own FF from scratch, just to avoid any dependence on that
custom CRT.
d) acquire VC2005pro, and try to build mozilla's CRT with that.

Since the advent of the new custom CRT, I think most NSS developers do a,
frankly.  Presently I do b.  I'm working on d in my copious spare time.

-- 
dev-tech-crypto mailing list
dev-tech-crypto@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-tech-crypto


Re: Allocator mismatches

2009-03-30 Thread Nelson B Bolyard
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.

 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.  :(

 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.

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.

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.

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  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%2Ftree=mozilla

 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

Re: Allocator mismatches

2009-03-30 Thread Neil

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%2Ftree=mozilla
 


Thanks for the clarification.


Using the above criteria, I found the following allocator mismatches under

Re: Allocator mismatches

2009-03-30 Thread Benjamin Smedberg
On 3/30/09 4:00 AM, Nelson B Bolyard wrote:

 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.

What do you mean by everything? It is certainly possible to mix an extension
DLL compiled against the static CRT with Firefox compiled with the jemalloc
CRT. This is in fact the recommended configuration.

 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.  :(

Indeed. We also recommend that people don't pass allocated objects across
DLL boundaries without using a common allocator function such as
PR_Malloc/PR_Free or NS_Alloc/NS_Free. This is precisely why the Mozilla
platform has common allocator functions.

--BDS

-- 
dev-tech-crypto mailing list
dev-tech-crypto@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-tech-crypto


Re: Allocator mismatches

2009-03-30 Thread Nelson B Bolyard
Neil wrote, On 2009-03-30 02:10:
 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.

The problem is in the way that Mozilla builds JEMalloc for FF on Windows.
They build a replacement for the Microsoft C RunTime Library.  This
replacement is a hybrid, built in part from JEMalloc source code, and in
part from Microsoft's source code for MSVCRT, which source code is ONLY
available with the professional edition of the MSVC compiler.  There is
an ed script that modifies the MSVC source, so that not even a single
line of the MSVC source code is found in Mozilla's respository that builds
the replacement VCRT.  So, you cannot build the replacement VCRT without
the professional edition compiler, which isn't cheap.

Now, you might think that this would not be a problem if the replacements
DLL and its companion .lib file (with which one links to use the CRT)
were distributed to developers, but, as I understand it, based on the
explanation I received from someone at Mozilla, Microsoft's license
prohibits any redistribution of that .lib file.

 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%2Ftree=mozilla
 
 Thanks for the clarification.

You're welcome.  But please tell us, which libjar has the horrifying
number of mismatches?  If it's NSS's, we'd like to know about them.

 pk12util.c: uses PR_Free for a string allocated with malloc,
 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) ?

I meant PR_Free.  There is no PL_Free, AFAIK.

 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.

Yes, That's precisely the issue to which I was referring.  I'd like to
have PL_ use PR's allocator, but I don't want to create circular
dependencies.  I think we'd have to change PR to use an allocator in PL,
to keep the dependencies acyclic, and that would be a BIG undertaking. :(

 I didn't find any bugs in NSPR. Under which component should I file those
 bugs in pk12util?

If the bug is in a source file under security/nss/cmd then component
tools.  If it's under security/nss/lib then component libraries.


 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, 

Another feature of the professional package only, sadly.

 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. 

We should do something similar for PR_'s allocator functions.

 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...

:-)
-- 
dev-tech-crypto mailing list
dev-tech-crypto@lists.mozilla.org

Allocator mismatches

2009-03-29 Thread Neil
Firefox may want to switch to using the jemalloc allocator on the Mac. 
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. In particular, I am counting NS_Free, PR_Free, PL_strfree 
and free as separate deallocators, which might strike you as a bit 
harsh, as you probably only need to care about NS_Free. Note that 
nsXPIDLCString and .Adopt() counts as an NS_Free, while nsCRT::strdup 
counts as a PR_Malloc, and I'm currently lumping PR_smprintf_free in 
with free, and PORT_Free with whatever it calls.


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.)


Using the above criteria, I found the following allocator mismatches 
under security/*


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.


pk12util.c: uses PR_Free for a string allocated with malloc, and 
PORT_ZFree for a string allocated with PL_strdup.


Would you be interested in patches for some or all of these? Who would 
want to review them?


--
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