[1/5,libbacktrace] Factor out backtrace_vector_free

Message ID 20181128231545.GA3883@delia
State New
Headers show
Series
  • [1/5,libbacktrace] Factor out backtrace_vector_free
Related show

Commit Message

Tom de Vries Nov. 28, 2018, 11:15 p.m.
Hi,

this patch factors out new function backtrace_vector_free.

Bootstrapped and reg-tested on x86_64.

OK for trunk?

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(-)

Comments

Richard Biener via Gcc-patches Nov. 28, 2018, 11:26 p.m. | #1
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
Tom de Vries Nov. 29, 2018, 12:33 p.m. | #2
On 29-11-18 00:26, Ian Lance Taylor wrote:
> 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.

> 


I see, thanks for the explanation.

How about his patch? It does not add a file, though it does add an
external function which requires a rename in libsanitizer/libbacktrace
(I don't know whether that requires changes in any other places).

[ Also, it inlines backtrace-vector.c into alloc.c and mmap.c, so it
duplicates code. If that is not acceptable, I could move it to
internal.h as static inline or static ATTRIBUTE_UNUSED. ]

Thanks,
- Tom
[libbacktrace] Factor out backtrace_vector_free

Factor out new function backtrace_vector_free.

Bootstrapped and reg-tested on x86_64.

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

	* alloc.c (backtrace_vector_free): New fuction, factored out of ...
	* dwarf.c (read_line_info): ... here.
	* mmap.c (backtrace_vector_free): Copy from alloc.c.
	* internal.h (backtrace_vector_free): Declare.

	* libbacktrace/backtrace-rename.h (backtrace_vector_free): Define to
	__asan_backtrace_vector_free.

---
 libbacktrace/alloc.c                         | 12 ++++++++++++
 libbacktrace/dwarf.c                         |  4 +---
 libbacktrace/internal.h                      |  7 +++++++
 libbacktrace/mmap.c                          | 12 ++++++++++++
 libsanitizer/libbacktrace/backtrace-rename.h |  1 +
 5 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/libbacktrace/alloc.c b/libbacktrace/alloc.c
index 522b59dd59f..d442b5d1762 100644
--- a/libbacktrace/alloc.c
+++ b/libbacktrace/alloc.c
@@ -165,3 +165,15 @@ backtrace_vector_release (struct backtrace_state *state ATTRIBUTE_UNUSED,
 
   return 1;
 }
+
+/* Free the space managed by VEC.  */
+
+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/libbacktrace/mmap.c b/libbacktrace/mmap.c
index 9f896a1bb99..080868c8a91 100644
--- a/libbacktrace/mmap.c
+++ b/libbacktrace/mmap.c
@@ -325,3 +325,15 @@ backtrace_vector_release (struct backtrace_state *state,
     vec->base = NULL;
   return 1;
 }
+
+/* Free the space managed by VEC.  */
+
+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/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
Richard Biener via Gcc-patches Nov. 29, 2018, 6:17 p.m. | #3
On Thu, Nov 29, 2018 at 4:33 AM, Tom de Vries <tdevries@suse.de> wrote:
> On 29-11-18 00:26, Ian Lance Taylor wrote:

>> 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.

>>

>

> I see, thanks for the explanation.

>

> How about his patch? It does not add a file, though it does add an

> external function which requires a rename in libsanitizer/libbacktrace

> (I don't know whether that requires changes in any other places).

>

> [ Also, it inlines backtrace-vector.c into alloc.c and mmap.c, so it

> duplicates code. If that is not acceptable, I could move it to

> internal.h as static inline or static ATTRIBUTE_UNUSED. ]


Yes, let's just use a static inline function or a macro.  Thanks.

Ian
Tom de Vries Nov. 30, 2018, 9:02 a.m. | #4
On 29-11-18 19:17, Ian Lance Taylor wrote:
> On Thu, Nov 29, 2018 at 4:33 AM, Tom de Vries <tdevries@suse.de> wrote:

>> On 29-11-18 00:26, Ian Lance Taylor wrote:

>>> 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.

>>>

>>

>> I see, thanks for the explanation.

>>

>> How about his patch? It does not add a file, though it does add an

>> external function which requires a rename in libsanitizer/libbacktrace

>> (I don't know whether that requires changes in any other places).

>>

>> [ Also, it inlines backtrace-vector.c into alloc.c and mmap.c, so it

>> duplicates code. If that is not acceptable, I could move it to

>> internal.h as static inline or static ATTRIBUTE_UNUSED. ]

> 

> Yes, let's just use a static inline function or a macro.  Thanks.


Committed as static inline.

Thanks,
- Tom
[libbacktrace] Factor out backtrace_vector_free

Factor out new function backtrace_vector_free.

Bootstrapped and reg-tested on x86_64.

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

	* internal.h (backtrace_vector_free): New static inline fuction,
	factored out of ...
	* dwarf.c (read_line_info): ... here.

---
 libbacktrace/dwarf.c    |  4 +---
 libbacktrace/internal.h | 12 ++++++++++++
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
index 34543747c8f..48ef3638a5f 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..548f9d70905 100644
--- a/libbacktrace/internal.h
+++ b/libbacktrace/internal.h
@@ -257,6 +257,18 @@ 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.  */
+
+static inline 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);
+}
+
 /* 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

Patch

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