Re: [PATCH 1/5][libbacktrace] Factor out backtrace_vector_free

["Ian Lance Taylor via gcc-patches" <gcc-patches at gcc dot gnu dot org>]

On Wed, Nov 28, 2018 at 3:15 PM, Tom de Vries <tdevries@suse.de> wrote:
>
> this patch factors out new function backtrace_vector_free.
>
> Bootstrapped and reg-tested on x86_64.
>
> OK for trunk?

We should only add new files if we really absolutely must, as this
package is copied around to a lot of places (e.g.,
libsanitizer/libbacktrace) and adding files here requires
modifications in all those places.

If we do have to add a new file, which I don't think we do, it should
not have a leading "backtrace-" in the name, as none of the existing
files have such a prefix.

Ian



> Thanks,
> - Tom
>
> [libbacktrace] Factor out backtrace_vector_free
>
> 2018-11-28  Tom de Vries  <tdevries@suse.de>
>
>         * Makefile.am (libbacktrace_la_SOURCES): Add backtrace-vector.
>         * Makefile.in: Regenerate.
>         * backtrace-vector.c: New file.
>         (backtrace_vector_free): New fuction, factored out of ...
>         * dwarf.c (read_line_info): ... here.
>         * internal.h (backtrace_vector_free): Declare.
>
>         * libbacktrace/Makefile.am (libsanitizer_libbacktrace_la_SOURCES): Add
>         backtrace-vector.c.
>         * libbacktrace/Makefile.in: Regenerate.
>         * libbacktrace/backtrace-rename.h (backtrace_vector_free): Define to
>         __asan_backtrace_vector_free.
>
> ---
>  libbacktrace/Makefile.am                     |  3 +-
>  libbacktrace/Makefile.in                     |  5 +--
>  libbacktrace/backtrace-vector.c              | 48 ++++++++++++++++++++++++++++
>  libbacktrace/dwarf.c                         |  4 +--
>  libbacktrace/internal.h                      |  7 ++++
>  libsanitizer/libbacktrace/Makefile.am        |  1 +
>  libsanitizer/libbacktrace/Makefile.in        | 12 ++++++-
>  libsanitizer/libbacktrace/backtrace-rename.h |  1 +
>  8 files changed, 74 insertions(+), 7 deletions(-)
>
> diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am
> index 13e94f27aef..a267984e800 100644
> --- a/libbacktrace/Makefile.am
> +++ b/libbacktrace/Makefile.am
> @@ -47,7 +47,8 @@ libbacktrace_la_SOURCES = \
>         posix.c \
>         print.c \
>         sort.c \
> -       state.c
> +       state.c \
> +       backtrace-vector.c
>
>  BACKTRACE_FILES = \
>         backtrace.c \
> diff --git a/libbacktrace/Makefile.in b/libbacktrace/Makefile.in
> index 2d62ce20b9a..cc75c88c348 100644
> --- a/libbacktrace/Makefile.in
> +++ b/libbacktrace/Makefile.in
> @@ -152,7 +152,7 @@ CONFIG_CLEAN_VPATH_FILES =
>  LTLIBRARIES = $(noinst_LTLIBRARIES)
>  am__DEPENDENCIES_1 =
>  am_libbacktrace_la_OBJECTS = atomic.lo dwarf.lo fileline.lo posix.lo \
> -       print.lo sort.lo state.lo
> +       print.lo sort.lo state.lo backtrace-vector.lo
>  libbacktrace_la_OBJECTS = $(am_libbacktrace_la_OBJECTS)
>  AM_V_lt = $(am__v_lt_@AM_V@)
>  am__v_lt_ = $(am__v_lt_@AM_DEFAULT_V@)
> @@ -624,7 +624,8 @@ libbacktrace_la_SOURCES = \
>         posix.c \
>         print.c \
>         sort.c \
> -       state.c
> +       state.c \
> +       backtrace-vector.c
>
>  BACKTRACE_FILES = \
>         backtrace.c \
> diff --git a/libbacktrace/backtrace-vector.c b/libbacktrace/backtrace-vector.c
> new file mode 100644
> index 00000000000..11dffd48184
> --- /dev/null
> +++ b/libbacktrace/backtrace-vector.c
> @@ -0,0 +1,48 @@
> +/* backtrace-vector.c -- Utility functions for backtrace_vector.
> +   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 "config.h"
> +
> +#include <stdlib.h>
> +#include <sys/types.h>
> +
> +#include "backtrace.h"
> +#include "internal.h"
> +
> +void
> + backtrace_vector_free (struct backtrace_state *state,
> +                       struct backtrace_vector *vec,
> +                       backtrace_error_callback error_callback, void *data)
> +{
> +  vec->alc += vec->size;
> +  vec->size = 0;
> +  backtrace_vector_release (state, vec, error_callback, data);
> +}
> diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
> index 4e93f120820..13d0aa4bcd8 100644
> --- a/libbacktrace/dwarf.c
> +++ b/libbacktrace/dwarf.c
> @@ -2057,9 +2057,7 @@ read_line_info (struct backtrace_state *state, struct dwarf_data *ddata,
>    return 1;
>
>   fail:
> -  vec.vec.alc += vec.vec.size;
> -  vec.vec.size = 0;
> -  backtrace_vector_release (state, &vec.vec, error_callback, data);
> +  backtrace_vector_free (state, &vec.vec, error_callback, data);
>    free_line_header (state, hdr, error_callback, data);
>    *lines = (struct line *) (uintptr_t) -1;
>    *lines_count = 0;
> diff --git a/libbacktrace/internal.h b/libbacktrace/internal.h
> index bff8ed470e4..710693bcf66 100644
> --- a/libbacktrace/internal.h
> +++ b/libbacktrace/internal.h
> @@ -257,6 +257,13 @@ extern int backtrace_vector_release (struct backtrace_state *state,
>                                      backtrace_error_callback error_callback,
>                                      void *data);
>
> +/* Free the space managed by VEC.  This will reset VEC.  */
> +
> +extern void backtrace_vector_free (struct backtrace_state *state,
> +                                  struct backtrace_vector *vec,
> +                                  backtrace_error_callback error_callback,
> +                                  void *data);
> +
>  /* Read initial debug data from a descriptor, and set the
>     fileline_data, syminfo_fn, and syminfo_data fields of STATE.
>     Return the fileln_fn field in *FILELN_FN--this is done this way so
> diff --git a/libsanitizer/libbacktrace/Makefile.am b/libsanitizer/libbacktrace/Makefile.am
> index 9c752272be1..b3756c39305 100644
> --- a/libsanitizer/libbacktrace/Makefile.am
> +++ b/libsanitizer/libbacktrace/Makefile.am
> @@ -55,6 +55,7 @@ libsanitizer_libbacktrace_la_SOURCES = \
>         ../../libbacktrace/posix.c \
>         ../../libbacktrace/sort.c \
>         ../../libbacktrace/state.c \
> +       ../../libbacktrace/backtrace-vector.c \
>         ../../libiberty/cp-demangle.c \
>         bridge.cc
>
> diff --git a/libsanitizer/libbacktrace/Makefile.in b/libsanitizer/libbacktrace/Makefile.in
> index f0e190a81d8..6c06d5d0dba 100644
> --- a/libsanitizer/libbacktrace/Makefile.in
> +++ b/libsanitizer/libbacktrace/Makefile.in
> @@ -144,7 +144,8 @@ CONFIG_CLEAN_VPATH_FILES =
>  LTLIBRARIES = $(noinst_LTLIBRARIES)
>  am__DEPENDENCIES_1 =
>  am_libsanitizer_libbacktrace_la_OBJECTS = atomic.lo dwarf.lo \
> -       fileline.lo posix.lo sort.lo state.lo cp-demangle.lo bridge.lo
> +       fileline.lo posix.lo sort.lo state.lo backtrace-vector.lo \
> +       cp-demangle.lo bridge.lo
>  libsanitizer_libbacktrace_la_OBJECTS =  \
>         $(am_libsanitizer_libbacktrace_la_OBJECTS)
>  AM_V_lt = $(am__v_lt_@AM_V@)
> @@ -400,6 +401,7 @@ libsanitizer_libbacktrace_la_SOURCES = \
>         ../../libbacktrace/posix.c \
>         ../../libbacktrace/sort.c \
>         ../../libbacktrace/state.c \
> +       ../../libbacktrace/backtrace-vector.c \
>         ../../libiberty/cp-demangle.c \
>         bridge.cc
>
> @@ -484,6 +486,7 @@ distclean-compile:
>
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/alloc.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/atomic.Plo@am__quote@
> +@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/backtrace-vector.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/bridge.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/cp-demangle.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/dwarf.Plo@am__quote@
> @@ -560,6 +563,13 @@ state.lo: ../../libbacktrace/state.c
>  @AMDEP_TRUE@@am__fastdepCC_FALSE@      DEPDIR=$(DEPDIR) $(CCDEPMODE) $(depcomp) @AMDEPBACKSLASH@
>  @am__fastdepCC_FALSE@  $(AM_V_CC@am__nodep@)$(LIBTOOL) $(AM_V_lt) --tag=CC $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS) -c -o state.lo `test -f '../../libbacktrace/state.c' || echo '$(srcdir)/'`../../libbacktrace/state.c
>
> +backtrace-vector.lo: ../../libbacktrace/backtrace-vector.c
> +@am__fastdepCC_TRUE@   $(AM_V_CC)$(LIBTOOL) $(AM_V_lt) --tag=CC $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS) -MT backtrace-vector.lo -MD -MP -MF $(DEPDIR)/backtrace-vector.Tpo -c -o backtrace-vector.lo `test -f '../../libbacktrace/backtrace-vector.c' || echo '$(srcdir)/'`../../libbacktrace/backtrace-vector.c
> +@am__fastdepCC_TRUE@   $(AM_V_at)$(am__mv) $(DEPDIR)/backtrace-vector.Tpo $(DEPDIR)/backtrace-vector.Plo
> +@AMDEP_TRUE@@am__fastdepCC_FALSE@      $(AM_V_CC)source='../../libbacktrace/backtrace-vector.c' object='backtrace-vector.lo' libtool=yes @AMDEPBACKSLASH@
> +@AMDEP_TRUE@@am__fastdepCC_FALSE@      DEPDIR=$(DEPDIR) $(CCDEPMODE) $(depcomp) @AMDEPBACKSLASH@
> +@am__fastdepCC_FALSE@  $(AM_V_CC@am__nodep@)$(LIBTOOL) $(AM_V_lt) --tag=CC $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS) -c -o backtrace-vector.lo `test -f '../../libbacktrace/backtrace-vector.c' || echo '$(srcdir)/'`../../libbacktrace/backtrace-vector.c
> +
>  cp-demangle.lo: ../../libiberty/cp-demangle.c
>  @am__fastdepCC_TRUE@   $(AM_V_CC)$(LIBTOOL) $(AM_V_lt) --tag=CC $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CFLAGS) $(CFLAGS) -MT cp-demangle.lo -MD -MP -MF $(DEPDIR)/cp-demangle.Tpo -c -o cp-demangle.lo `test -f '../../libiberty/cp-demangle.c' || echo '$(srcdir)/'`../../libiberty/cp-demangle.c
>  @am__fastdepCC_TRUE@   $(AM_V_at)$(am__mv) $(DEPDIR)/cp-demangle.Tpo $(DEPDIR)/cp-demangle.Plo
> diff --git a/libsanitizer/libbacktrace/backtrace-rename.h b/libsanitizer/libbacktrace/backtrace-rename.h
> index 2555fe508c2..e2494e3686d 100644
> --- a/libsanitizer/libbacktrace/backtrace-rename.h
> +++ b/libsanitizer/libbacktrace/backtrace-rename.h
> @@ -15,6 +15,7 @@
>  #define backtrace_vector_finish __asan_backtrace_vector_finish
>  #define backtrace_vector_grow __asan_backtrace_vector_grow
>  #define backtrace_vector_release __asan_backtrace_vector_release
> +#define backtrace_vector_free __asan_backtrace_vector_free
>
>  #define cplus_demangle_builtin_types __asan_cplus_demangle_builtin_types
>  #define cplus_demangle_fill_ctor __asan_cplus_demangle_fill_ctor