0000680: Defective determination of SBRK_OK - MantisBT

ID	Project	Category	View Status	Date Submitted	Last Update

0000680	LDMud 3.5	Compilation, Installation	public	2009-09-20 10:06	2011-02-13 21:38

Reporter	zesstra	Assigned To	zesstra
Priority	normal	Severity	minor	Reproducibility	always
Status	resolved	Resolution	fixed
Platform	i686	OS	MacOS X	OS Version	10.5.x
Target Version	3.5.0	Fixed in Version	3.5.0

Summary	0000680: Defective determination of SBRK_OK
Description	sbrk/brk() are mostly deprecated in the newer POSIX standards and on MacOS, brk() ist not available and sbrk() only simulates a 4MB (AFAIR) large heap. Unfortunately, our check for sbrk() succeeds (because it is possible to get some memory) and the resulting driver would be non-functional, because it can only allocate about 4MB of memory. This does not show up, because on MacOS we are not able to replace the system malloc. In that case, SBRK_OK is automatically disabled again. But there may be systems, where our SBRK_OK malfunctions and it is possible to replace the system malloc. Therefore we should repair our autoconf check somehow. I can only think of allocating more memory during the test. But how much? For MacOS it would be > 4MB. But is that good enough? Can anyone think of a better check? Note: On most systems, the fallback from SBRK_OK is to use mmap(). In that case, we can still replace malloc(). If mmap() is also not available, malloc ist used and can obviously not be replaced any longer.
Additional Information	An older mail, I write about this (sorry for the german): ich habe gerade nochmal ueber meine Crashes und moegliche Fixes gegruebelt und dabei ist mir folgendes aufgefallen: Irgendwie laufen unsere Tests fuer SBRK_OK & Co nicht so richtig gut. Erstmal Begriffsbestimmung: SBRK_OK: (s)brk() nutzbar. Momentan wird SBRK_OK auch zusaetzlich im Sinne von "malloc() ist durch Definition eines eigenen malloc() ersetzbar" benutzt. HAVE_MMAP: mmap() verfuegbar MALLOC_SBRK: sbrk oder mmap nutzen, falls verfuegbar, REPLACE_MALLOC aktivieren und den Systemallokator ersetzen. Standardmaessig definiert. Unser configure-Test testet nun in erster Linie die letztgenannte Teilbedingung von SBRK_OK. Auf MacOS ist der Allokator nicht so simpel zu ersetzen und der Test schaltet SBRK_OK auf MacOS folglich auch aus. Der Test auf funktionsfaehiges sbrk ist jedoch in diesem Falle Mist, weil MacOS ein Minimal-sbrk hat, mit welchem man die Breakadresse um einige Kilobyte verschieben kann. Unser Testprogramm braucht nicht viel und wuerde daher SBRK_OK setzen, wenn das Ersetzen funktionieren wuerde. Wuerde jedoch in keinem lauffaehigen Driver enden. HAVE_MMAP ist auf fast allen Systemen verfuegbar, MALLOC_SBRK ist standardmaessig gesetzt, d.h. es wird bei !SBRK_OK fast immer versucht, den System-Allokator zu ersetzen, auch wenn das eigentlich nicht mit unserer Methode geht. Folge sind moegliche Crashes (strdup alloziert per Systemallokator, Speicher wird ueber Aufruf unseres free() freigegeben). Das ganze Problem resultiert eigentlich aus a) unzureichenden Tests und b) der semantischen Vermischung von "brk ist verfuegbar" und "malloc ersetzbar" in SBRK_OK. Ich waer dafuer 1) SBRK_OK nur bei wirklich nutzbarem brk() zu setzen und dafuer nur noch diese Bedeutung haben zu lassen und 2) MALLOC_SBRK nur dann zu aktivieren, wenn malloc ersetzbar ist. D.h. fuer MALLOC_SBRK koennte man im wesentlichen den Test nehmen, den wir gerade in configure fuer SBRK_OK haben. Fragt sich aber, wie man feststellt, ob brk() wirklich funktionsfaehig ist. Einfach blind zu versuchen, mehr Speicher als bisher via brk() zu allozieren scheint mir irgendwie so toll auch nicht zu sein.
Tags	No tags attached.

zesstra 2009-09-20 15:14 administrator ~0001287	OK, I started work on cleaning this up a little bit (nothing is applied yet): 1) configure contains a (preliminary) check for a working sbrk/brk, which tries to allocate 50MB with sbrk/brk. If successful, SBRK_OK is defined. 2) configure contains a check for a replaceable malloc. This is very similar to the old check for SBRK_OK. If malloc seems to be replaceable, MALLOC_REPLACEABLE is defined. So, now we have: SBRK_OK: allocating with sbrk/brk() works (machine.h) MALLOC_REPLACEABLE: malloc is replaceable (machine.h) HAVE_MMAP: mmap() is available (machine.h) These three are completely independent of each other. MALLOC_SBRK: the user requested to use sbrk if available (default, config.h) REPLACE_MALLOC: the system malloc is to be replaced by our own functions (slaballoc.c, smalloc.c) I changed some precompiler logic in the allocators to make use of this. So the logic is now as follows: 1) the allocators checks first if MALLOC_SBRK is requested. If yes, SBRK_OK and MALLOC_REPLACEABLE must be defined as well. If yes, REPLACE_MALLOC is set. Otherwise, MALLOC_SBRK is undefined and sbrk ist not used. 2) If HAVE_MMAP and MALLOC_REPLACEABLE are defined, mmap() is used instead of sbrk() and REPLACE_MALLOC is defined. 3) If HAVE_MMAP is defined, but not MALLOC_REPLACEABLE, mmap() is used, but we don't attempt to replace malloc. With mmap(), this is possible, because we don't interfere with any system allocator using sbrk, brk or mmap. 4) the allocators then don't check only for SBRK_OK anymore, but for MALLOC_SBRK (which is only defined, if requested and the prerequisites are met).

zesstra 2009-09-21 16:16 administrator ~0001290	I applied some experimental patches as r2748-2750, which implement the described behaviour in 0000680:0001287 (at least I hope so). I will keep this issue open for a while, because this stuff should be tested on several platforms. And there are probably improvements possible.

zesstra 2010-07-13 14:10 administrator ~0001872	Ok, I consider this resolved in 3.5.x for now.

Date Modified	Username	Field	Change
2009-09-20 10:06	zesstra	New Issue
2009-09-20 14:45	zesstra	Additional Information Updated
2009-09-20 15:14	zesstra	Note Added: 0001287
2009-09-20 15:48	zesstra	Status	new => assigned
2009-09-20 15:48	zesstra	Assigned To	=> zesstra
2009-09-21 16:16	zesstra	Note Added: 0001290
2009-10-04 16:02	zesstra	Relationship added	child of 0000687
2010-07-13 14:09	zesstra	Project	LDMud => LDMud 3.5
2010-07-13 14:10	zesstra	Note Added: 0001872
2010-07-13 14:10	zesstra	Status	assigned => resolved
2010-07-13 14:10	zesstra	Fixed in Version	=> 3.5.0
2010-07-13 14:10	zesstra	Resolution	open => fixed
2011-02-13 21:38	zesstra	Target Version	=> 3.5.0