[tz] tzcode: uninitialized sp->charcnt gives clang analyzer warning

Paul Eggert eggert at cs.ucla.edu
Tue Aug 10 19:51:03 UTC 2021


On 8/9/21 3:55 PM, Jan Engelhardt wrote:

> Inside tzload, if malloc fails, then, by POSIX standardese, it ought to set
> errno. However, clang - rightfully, I think - does not make any particular
> assumptions about malloc and has found and reported the case whereby this
> malloc returns with NULL _and_ errno is 0.

localtime.c assumes POSIX behavior for other functions (e.g., 'open', 
'read'), which may help explain why assuming POSIX for 'malloc' hasn't 
been a problem in practice.

Still, I take your point that there could be platforms that support 
POSIX 'open', 'read', etc. but not POSIX 'malloc', or static analyzers 
like 'clang' that assume that the platform's malloc does not conform to 
POSIX. So I took the usual way out in the spirit of HAVE_POSIX_DECLS 
etc. by adding a compile-time option HAVE_MALLOC_ERRNO which you can set 
to 0 if your platform's malloc departs from standard practice. See the 
attached proposed patches.

With these patches you should be able to run clang this way:

   clang --analyze -Xanalyzer -analyzer-output=text localtime.c \
     -DALL_STATE -DHAVE_MALLOC_ERRNO=0

and get a clean report.


> The malloc(3) page on Linux systems
> mentions the corner-cases in which errno=0 can happen, namely "private malloc
> implementations".)

That man page is outdated, as it was written when the GNU C Library was 
looser and goosier about memory allocation. Your email prompted me to 
send in a patch here:

https://lore.kernel.org/linux-man/20210810193708.10277-1-eggert@cs.ucla.edu/

and I expect the man page to be updated in due course.

The glibc manual says that any memory allocator replacement functions 
must be compatible with glibc, right down to errno. Among other things, 
it says "for example, the replacement 'free' should also preserve 
'errno'"; this is a property of GNU 'free' that I believe will appear in 
the next POSIX revision.

> Using calloc instead of malloc, or just setting the field to zero, should
> have little ill effect, even cosmetically.

I can see an ill effect, in that clearing storage merely to pacify 
static analysis false alarms can mask other errors. Valid reports of 
uninitialized variables can be useful to developers, and invariably 
clearing storage to zero can suppress these useful reports and can lead 
to less-reliable software.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Port-to-platforms-where-malloc-does-not-set-errno.patch
Type: text/x-patch
Size: 3558 bytes
Desc: not available
URL: <https://mm.icann.org/pipermail/tz/attachments/20210810/420e2159/0001-Port-to-platforms-where-malloc-does-not-set-errno.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Makefile-Document-HAVE_MALLOC_ERRNO.patch
Type: text/x-patch
Size: 884 bytes
Desc: not available
URL: <https://mm.icann.org/pipermail/tz/attachments/20210810/420e2159/0002-Makefile-Document-HAVE_MALLOC_ERRNO.patch>


More information about the tz mailing list