[2/4,libbacktrace] Avoid realloc with size == 0 in backtrace_vector_release

Message ID 20181123205117.GB1927@delia
State New
Headers show
Series
  • Untitled series #9282
Related show

Commit Message

Tom de Vries Nov. 23, 2018, 8:51 p.m.
[ 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  <tdevries@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(-)

Comments

Jeff Law Nov. 26, 2018, 10:23 p.m. | #1
On 11/23/18 1:51 PM, Tom de Vries wrote:
> [ 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  <tdevries@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.

OK.

Are any of the subsequent patches in this series still relevant?

jeff

Patch

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);
+}