[ was: Re: [PATCH 1/2][libbacktrace] Handle realloc returning NULL if size == 0 ] On Thu, Nov 22, 2018 at 06:16:20PM +0000, Joseph Myers wrote: > On Thu, 22 Nov 2018, Tom de Vries wrote: > > > Hi, > > > > If realloc is called with size 0, realloc can return NULL. > > Note that, as of C17, realloc with size 0 is marked as an obsolescent > feature (because of inconsistencies between implementations regarding > whether the old object is deallocated). So it would be advisable for code > intended to be portable to avoid calling realloc with size 0 at all. >
Updated patch to avoid realloc with size 0. OK for trunk? Thanks, - Tom [libbacktrace] Avoid realloc with size == 0 in backtrace_vector_release As of C17, realloc with size 0 is marked as an obsolescent feature. Fix this in backtrace_vector_release by using free instead. Bootstrapped and reg-tested on x86_64. 2018-11-22 Tom de Vries <tdevr...@suse.de> * alloc.c (backtrace_vector_release): Handle vec->size == 0 using free instead of realloc. * Makefile.am (check_PROGRAMS): Add unittest. * Makefile.in: Regenerate. * unittest.c: New file. --- libbacktrace/Makefile.am | 4 +++ libbacktrace/Makefile.in | 25 ++++++++++--- libbacktrace/alloc.c | 13 ++++++- libbacktrace/unittest.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 5 deletions(-) diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am index ee47daee415..4b28984c4e0 100644 --- a/libbacktrace/Makefile.am +++ b/libbacktrace/Makefile.am @@ -89,6 +89,10 @@ check_PROGRAMS = TESTS = $(check_PROGRAMS) if NATIVE +unittest_SOURCES = unittest.c testlib.c +unittest_LDADD = libbacktrace.la + +check_PROGRAMS += unittest btest_SOURCES = btest.c testlib.c btest_CFLAGS = $(AM_CFLAGS) -g -O diff --git a/libbacktrace/Makefile.in b/libbacktrace/Makefile.in index 2b984bc127e..c79b67636c9 100644 --- a/libbacktrace/Makefile.in +++ b/libbacktrace/Makefile.in @@ -124,7 +124,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \ $(am__EXEEXT_4) $(am__EXEEXT_5) $(am__EXEEXT_6) \ $(am__EXEEXT_7) $(am__EXEEXT_8) $(am__EXEEXT_9) \ $(am__EXEEXT_10) $(am__EXEEXT_11) $(am__EXEEXT_12) -@NATIVE_TRUE@am__append_1 = btest +@NATIVE_TRUE@am__append_1 = unittest btest @HAVE_MMAP_TRUE@@NATIVE_TRUE@am__append_2 = btest_with_alloc @NATIVE_TRUE@am__append_3 = stest @HAVE_MMAP_TRUE@@NATIVE_TRUE@am__append_4 = stest_with_alloc @@ -170,7 +170,7 @@ AM_V_lt = $(am__v_lt_@AM_V@) am__v_lt_ = $(am__v_lt_@AM_DEFAULT_V@) am__v_lt_0 = --silent am__v_lt_1 = -@NATIVE_TRUE@am__EXEEXT_1 = btest$(EXEEXT) +@NATIVE_TRUE@am__EXEEXT_1 = unittest$(EXEEXT) btest$(EXEEXT) @HAVE_MMAP_TRUE@@NATIVE_TRUE@am__EXEEXT_2 = btest_with_alloc$(EXEEXT) @NATIVE_TRUE@am__EXEEXT_3 = stest$(EXEEXT) @HAVE_MMAP_TRUE@@NATIVE_TRUE@am__EXEEXT_4 = stest_with_alloc$(EXEEXT) @@ -275,6 +275,10 @@ ttest_with_alloc_LINK = $(LIBTOOL) $(AM_V_lt) --tag=CC \ $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=link $(CCLD) \ $(ttest_with_alloc_CFLAGS) $(CFLAGS) $(AM_LDFLAGS) $(LDFLAGS) \ -o $@ +@NATIVE_TRUE@am_unittest_OBJECTS = unittest.$(OBJEXT) \ +@NATIVE_TRUE@ testlib.$(OBJEXT) +unittest_OBJECTS = $(am_unittest_OBJECTS) +@NATIVE_TRUE@unittest_DEPENDENCIES = libbacktrace.la @NATIVE_TRUE@am_ztest_OBJECTS = ztest-ztest.$(OBJEXT) \ @NATIVE_TRUE@ ztest-testlib.$(OBJEXT) ztest_OBJECTS = $(am_ztest_OBJECTS) @@ -335,8 +339,8 @@ SOURCES = $(libbacktrace_la_SOURCES) $(EXTRA_libbacktrace_la_SOURCES) \ $(ctestg_with_alloc_SOURCES) $(edtest_SOURCES) \ $(edtest_with_alloc_SOURCES) $(stest_SOURCES) \ $(stest_with_alloc_SOURCES) $(ttest_SOURCES) \ - $(ttest_with_alloc_SOURCES) $(ztest_SOURCES) \ - $(ztest_with_alloc_SOURCES) + $(ttest_with_alloc_SOURCES) $(unittest_SOURCES) \ + $(ztest_SOURCES) $(ztest_with_alloc_SOURCES) am__can_run_installinfo = \ case $$AM_UPDATE_INFO_DIR in \ n|no|NO) false;; \ @@ -745,6 +749,8 @@ libbacktrace_la_LIBADD = \ libbacktrace_la_DEPENDENCIES = $(libbacktrace_la_LIBADD) TESTS = $(check_PROGRAMS) $(am__append_12) +@NATIVE_TRUE@unittest_SOURCES = unittest.c testlib.c +@NATIVE_TRUE@unittest_LDADD = libbacktrace.la @NATIVE_TRUE@btest_SOURCES = btest.c testlib.c @NATIVE_TRUE@btest_CFLAGS = $(AM_CFLAGS) -g -O @NATIVE_TRUE@btest_LDADD = libbacktrace.la @@ -941,6 +947,10 @@ ttest_with_alloc$(EXEEXT): $(ttest_with_alloc_OBJECTS) $(ttest_with_alloc_DEPEND @rm -f ttest_with_alloc$(EXEEXT) $(AM_V_CCLD)$(ttest_with_alloc_LINK) $(ttest_with_alloc_OBJECTS) $(ttest_with_alloc_LDADD) $(LIBS) +unittest$(EXEEXT): $(unittest_OBJECTS) $(unittest_DEPENDENCIES) $(EXTRA_unittest_DEPENDENCIES) + @rm -f unittest$(EXEEXT) + $(AM_V_CCLD)$(LINK) $(unittest_OBJECTS) $(unittest_LDADD) $(LIBS) + ztest$(EXEEXT): $(ztest_OBJECTS) $(ztest_DEPENDENCIES) $(EXTRA_ztest_DEPENDENCIES) @rm -f ztest$(EXEEXT) $(AM_V_CCLD)$(ztest_LINK) $(ztest_OBJECTS) $(ztest_LDADD) $(LIBS) @@ -1293,6 +1303,13 @@ recheck: all $(check_PROGRAMS) am__force_recheck=am--force-recheck \ TEST_LOGS="$$log_list"; \ exit $$? +unittest.log: unittest$(EXEEXT) + @p='unittest$(EXEEXT)'; \ + b='unittest'; \ + $(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \ + --log-file $$b.log --trs-file $$b.trs \ + $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \ + "$$tst" $(AM_TESTS_FD_REDIRECT) btest.log: btest$(EXEEXT) @p='btest$(EXEEXT)'; \ b='btest'; \ diff --git a/libbacktrace/alloc.c b/libbacktrace/alloc.c index 7070afbf2aa..522b59dd59f 100644 --- a/libbacktrace/alloc.c +++ b/libbacktrace/alloc.c @@ -145,12 +145,23 @@ backtrace_vector_release (struct backtrace_state *state ATTRIBUTE_UNUSED, backtrace_error_callback error_callback, void *data) { + vec->alc = 0; + + if (vec->size == 0) + { + /* As of C17, realloc with size 0 is marked as an obsolescent feature, use + free instead. */ + free (vec->base); + vec->base = NULL; + return 1; + } + vec->base = realloc (vec->base, vec->size); if (vec->base == NULL) { error_callback (data, "realloc", errno); return 0; } - vec->alc = 0; + return 1; } diff --git a/libbacktrace/unittest.c b/libbacktrace/unittest.c new file mode 100644 index 00000000000..576aa080935 --- /dev/null +++ b/libbacktrace/unittest.c @@ -0,0 +1,92 @@ +/* unittest.c -- Test for libbacktrace library + Copyright (C) 2018 Free Software Foundation, Inc. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + (1) Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + + (2) Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in + the documentation and/or other materials provided with the + distribution. + + (3) The name of the author may not be used to + endorse or promote products derived from this software without + specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR +IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED +WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +DISCLAIMED. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, +INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES +(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) +HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, +STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING +IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +POSSIBILITY OF SUCH DAMAGE. */ + +#include <assert.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> + +#include "filenames.h" + +#include "backtrace.h" +#include "backtrace-supported.h" + +#include "testlib.h" + +#include "internal.h" + +static unsigned count; + +static void +error_callback (void *vdata ATTRIBUTE_UNUSED, const char *msg ATTRIBUTE_UNUSED, + int errnum ATTRIBUTE_UNUSED) +{ + ++count; +} + +static int +test1 (void) +{ + int res; + int failed; + + struct backtrace_vector vec; + + memset (&vec, 0, sizeof vec); + + backtrace_vector_grow (state, 100, error_callback, NULL, &vec); + vec.alc += vec.size; + vec.size = 0; + + count = 0; + res = backtrace_vector_release (state, &vec, error_callback, NULL); + failed = res != 1 || count != 0; + + printf ("%s: unittest backtrace_vector_release size == 0\n", + failed ? "FAIL": "PASS"); + + if (failed) + ++failures; + + return failures; +} + +int +main (int argc ATTRIBUTE_UNUSED, char **argv) +{ + state = backtrace_create_state (argv[0], BACKTRACE_SUPPORTS_THREADS, + error_callback_create, NULL); + + test1 (); + + exit (failures ? EXIT_FAILURE : EXIT_SUCCESS); +}