[1/2,libbacktrace] Handle realloc returning NULL if size == 0

Message ID 20181122123541.GA2333@delia
State New
Headers show
Series
  • [1/2,libbacktrace] Handle realloc returning NULL if size == 0
Related show

Commit Message

Tom de Vries Nov. 22, 2018, 12:35 p.m.
Hi,

If realloc is called with size 0, realloc can return NULL.

When this happens in the backtrace_vector_release in alloc.c, the error
callback is called, which should not be the case.

Fix this by testing for size == 0 before calling the error callback.

Build and tested on x86_64, with mmap.c replaced by alloc.c to ensure that
backtrace_vector_release in alloc.c is tested.

OK for trunk if bootstrap and reg-test on x86_64 succeeds?

Thanks,
- Tom

[libbacktrace] Handle realloc returning NULL if size == 0

2018-11-22  Tom de Vries  <tdevries@suse.de>

	* Makefile.am (check_PROGRAMS): Add unittest.
	* Makefile.in: Regenerate.
	* alloc.c (backtrace_vector_release): Handle realloc returning NULL if
	* size == 0.
	* unittest.c: New file.

---
 libbacktrace/Makefile.am |  5 +++
 libbacktrace/Makefile.in | 25 ++++++++++---
 libbacktrace/alloc.c     |  2 +-
 libbacktrace/unittest.c  | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 119 insertions(+), 5 deletions(-)

Comments

Joseph Myers Nov. 22, 2018, 6:16 p.m. | #1
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.

-- 
Joseph S. Myers
joseph@codesourcery.com

Patch

diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am
index 3c1bd49dd7b..a2111ee7f67 100644
--- a/libbacktrace/Makefile.am
+++ b/libbacktrace/Makefile.am
@@ -90,6 +90,11 @@  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
 btest_LDADD = libbacktrace.la
diff --git a/libbacktrace/Makefile.in b/libbacktrace/Makefile.in
index 60a9d887dba..2d62ce20b9a 100644
--- a/libbacktrace/Makefile.in
+++ b/libbacktrace/Makefile.in
@@ -121,7 +121,7 @@  build_triplet = @build@
 host_triplet = @host@
 target_triplet = @target@
 check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3)
-@NATIVE_TRUE@am__append_1 = btest stest ztest edtest
+@NATIVE_TRUE@am__append_1 = unittest btest stest ztest edtest
 @HAVE_ZLIB_TRUE@@NATIVE_TRUE@am__append_2 = -lz
 @HAVE_PTHREAD_TRUE@@NATIVE_TRUE@am__append_3 = ttest
 @HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_4 = dtest
@@ -158,8 +158,8 @@  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) stest$(EXEEXT) \
-@NATIVE_TRUE@	ztest$(EXEEXT) edtest$(EXEEXT)
+@NATIVE_TRUE@am__EXEEXT_1 = unittest$(EXEEXT) btest$(EXEEXT) \
+@NATIVE_TRUE@	stest$(EXEEXT) ztest$(EXEEXT) edtest$(EXEEXT)
 @HAVE_PTHREAD_TRUE@@NATIVE_TRUE@am__EXEEXT_2 = ttest$(EXEEXT)
 @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@am__EXEEXT_3 =  \
 @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@	ctestg$(EXEEXT) \
@@ -202,6 +202,10 @@  ttest_OBJECTS = $(am_ttest_OBJECTS)
 ttest_LINK = $(LIBTOOL) $(AM_V_lt) --tag=CC $(AM_LIBTOOLFLAGS) \
 	$(LIBTOOLFLAGS) --mode=link $(CCLD) $(ttest_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)
@@ -246,7 +250,7 @@  am__v_CCLD_1 =
 SOURCES = $(libbacktrace_la_SOURCES) $(EXTRA_libbacktrace_la_SOURCES) \
 	$(btest_SOURCES) $(ctesta_SOURCES) $(ctestg_SOURCES) \
 	$(edtest_SOURCES) $(stest_SOURCES) $(ttest_SOURCES) \
-	$(ztest_SOURCES)
+	$(unittest_SOURCES) $(ztest_SOURCES)
 am__can_run_installinfo = \
   case $$AM_UPDATE_INFO_DIR in \
     n|no|NO) false;; \
@@ -655,6 +659,8 @@  libbacktrace_la_LIBADD = \
 
 libbacktrace_la_DEPENDENCIES = $(libbacktrace_la_LIBADD)
 TESTS = $(check_PROGRAMS) $(am__append_4)
+@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
@@ -800,6 +806,10 @@  ttest$(EXEEXT): $(ttest_OBJECTS) $(ttest_DEPENDENCIES) $(EXTRA_ttest_DEPENDENCIE
 	@rm -f ttest$(EXEEXT)
 	$(AM_V_CCLD)$(ttest_LINK) $(ttest_OBJECTS) $(ttest_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)
@@ -1088,6 +1098,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..2f7ad956088 100644
--- a/libbacktrace/alloc.c
+++ b/libbacktrace/alloc.c
@@ -146,7 +146,7 @@  backtrace_vector_release (struct backtrace_state *state ATTRIBUTE_UNUSED,
 			  void *data)
 {
   vec->base = realloc (vec->base, vec->size);
-  if (vec->base == NULL)
+  if (vec->base == NULL && vec->size != 0)
     {
       error_callback (data, "realloc", errno);
       return 0;
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);
+}