Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130115002442.GU20323@brightrain.aerifal.cx>
Date: Mon, 14 Jan 2013 19:24:42 -0500
From: Rich Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: malloc(0) behaviour

On Mon, Jan 14, 2013 at 05:37:29PM -0600, Rob Landley wrote:
> On 01/14/2013 11:17:47 AM, Igmar Palsenberg wrote:
> >Hi,
> >
> >Is there a (good) reason for Musl to follow glibc's malloc(0)
> >behaviour ?
> 
> Because not doing so breaks some programs? (Dunno if it still does,
> but last time I tried switching that config option off in uClibc the
> linux from scratch build didn't finish.)

I don't think it breaks any programs; the ones that depend on
malloc(0) returning nonzero seem to use either gnulib or their own
hacks to wrap malloc if the system malloc does not have this behavior.
However, the bug that originally prompted me to switch the behavior
was that the detection could be broken. gnulib was compiling a tiny
test program to check the behavior of malloc, and with musl, it got
linked to __simple_malloc, which was giving the malloc(0)!=NULL
behavior. The full program then got linked to real malloc (where
malloc(0) was returning a null pointer), and horribly broke, reporting
spurious allocation failures and aborting.

At that time, I researched and discussed the topic and the conclusion
reached was that malloc(0)!=NULL makes more sense.

> >Musl returns a valid pointer, which is fine according
> >to the standard, but returning NULL is also fine.
> >
> >IMHO, returning NULL is better : It usually kills the program if
> >actual storage is
> >attempted.
> 
> It also kills the program if they're checking the return code of
> malloc() and treating NULL as an allocation failure indicator. NULL
> has a defined meaning of "failed", when a zero length allocation is
> trivial to satisfy and not necessarily wrong.
> 
> Should a zero length file read return -1 instead of 0? Is it the
> program's job to make sure it never makes a NOP syscall? Does adding
> special case checks in your code to avoid such NOP calls actually
> make the code smaller and simpler?

Indeed, another argument for malloc(0)!=NULL is to avoid having to
write special-case code to handle it. Wrapping malloc with a function
that replaces an argument of 0 with an argument of 1 is not just a
hack to get "GNU behavior"; it's what you probably want to be doing
anyway (even if there were no GNU to copy) if there's a chance you
might pass 0 to malloc. It leads to the simplest code. Of course it
would make sense to give this wrapper a proper name rather than using
macro hacks to "replace" malloc like gnulib does...

Or, they could use my convenient formula...

#undef malloc
#define malloc(n) malloc((n)|1)

which seems a lot cheaper than a wrapper. It can be applied to realloc
too, but might get expensive with calloc.

> >You also can't do that if a valid pointer
> >is returned, so I really can't grasp the reason for returning a
> >pointer at all,
> 
> Not indicating that the allocation failed and triggering an assert()
> when there isn't actually a problem with a legitimately zero length
> array that had nothing in it? (Both times I debugged why LFS stuff
> was failing that's what it turned out to be, but I didn't spend too
> much time on it before just switching the uClibc option on to
> support it.)

While in some cases it would be nice to get a fault, you don't usually
get a fault when trying to access past the end of a length-1 array, so
why should you expect one when trying to access past the end of a
"length-0 array"?

> >except to support buggy and lazy programming.
> 
> You're defining "lazy" here a "not adding a special case in the
> caller for every use of malloc()". That's certainly a point of view,
> but I'm not sure that's the word you want to use for it. "Not
> sufficiently defensive programming" maybe?

Well, doing nothing to account for the fact that malloc(0) "failing"
might not indicate a problematic OOM condition is "lazy" in my book.

> And "posix doesn't require this, therefore let's intentional break
> it to force any programs with technical posix compatability issues
> to use a different libc rather than change anything to humor us"...
> not seeing it.

When POSIX allows implementation options but one option has clear
technical merits over the others, as is the case here, I don't see any
good reason to take the low-quality option. Especially if most
existing implementations already take the high-quality one, as this
suggests to me it's somewhat reasonable for otherwise-portable
programs to assume the high-quality behavior.

> (Re: the "posix test" thread, having a ./configure --pedantic that
> builds a musl without gnu/dammit extensions and safety tape over the
> sharp bits sounds like a possible fun future thing. Having that be
> the default, or adding a lot of runtime checks rather than
> commenting stuff out at compile time, not so much...)

Sounds like a lot of work for something that could be better
accomplished with static analysis tools and such...

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.