[PATCH] sim/m32c: fix memory leaks in opc2c
Mike Frysinger
vapier@gentoo.org
Tue Apr 6 10:41:49 GMT 2021
On 05 Apr 2021 21:36, Simon Marchi via Gdb-patches wrote:
> On 2021-04-05 5:51 p.m., Mike Frysinger wrote:
> > On 05 Apr 2021 14:46, Simon Marchi via Gdb-patches wrote:
> >> On 2021-04-05 12:23 p.m., Mike Frysinger wrote:
> >>> in other codebases, i've done things like:
> >>> #ifdef __SANITIZE_ADDRESS__
> >>> # define ENABLE_CLEANUP_ONEXIT 1
> >>> #else
> >>> # define ENABLE_CLEANUP_ONEXIT 0
> >>> #endif
> >>>
> >>> then this could be written:
> >>>
> >>> if (ENABLE_CLEANUP_ONEXIT) {
> >>> for (i = 0; i < vn; i++)
> >>> free (varnames[i]);
> >>> }
> >>>
> >>> wdyt ? feel free to bikeshed the name. not sure if there's prior art in
> >>> the tree currently.
> >>
> >> I find this complexity a bit unnecessary, versus just free-ing the
> >> stuff. But I don't really mind, we can do as you like, I just want to
> >> my build to be fixed!
> >>
> >> Note that the igen tool doesn't free anything, so it's next on the list
> >> of things that break the -fsanizite=address build. I started to update
> >> it to free things, it's a bit tedious but it should be do-able.
> >>
> >> Another option would be to change the Makefile to call igen with the
> >> ASAN_OPTIONS=detect_leaks=0 environment variable.
> >
> > ah yes, this is exactly what i mean wrt "the tip of the iceberg" and it being
> > "a slippery slope" ;). first it's the small build tools, then the larger
> > build tools, then the programs we actually install, ...
> >
> > maybe an alternative would be to convert these to C++. then it would handle
> > stack/heap resources for us.
>
> If you convert to C++ and the memory is managed automatically, isn't it
> exactly the same (runtime-wise) as free-ing everything by hand in C?
while i'm sure there is some automatic heap freeing, C++ stdlib can do
smarter memory management, whether it be caches, or stack based.
> Although I'm always in favor of using C++ for just not having to think
> about free-ing stuff.
right, and we don't have to debate this :). i can let it go for the sake
of the gains with the better language.
NB: this would only apply to build-time tools like opc2c.
to be clear, if you want to take on changing these tools to C++, i'd be
more than happy to help review. but if you feel that's too much to take
on, we can do the aforementioned conditional frees.
-mike
--- a/sim/Makefile.am
+++ b/sim/Makefile.am
@@ -36,6 +36,7 @@ DISTCLEANFILES =
MOSTLYCLEANFILES = core
COMPILE_FOR_BUILD = $(CC_FOR_BUILD) $(AM_CPPFLAGS) $(CFLAGS_FOR_BUILD)
+CXXCOMPILE_FOR_BUILD = $(CXX_FOR_BUILD) $(AM_CPPFLAGS) $(CXXFLAGS_FOR_BUILD)
LINK_FOR_BUILD = $(CC_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) -o $@
# Generate nltvals.def for newlib/libgloss using devo and build tree.
--- a/sim/m4/sim_ac_toolchain.m4
+++ b/sim/m4/sim_ac_toolchain.m4
@@ -27,15 +27,21 @@ AC_PROG_INSTALL
dnl Setup toolchain settings for build-time tools..
if test "x$cross_compiling" = "xno"; then
: "${CC_FOR_BUILD:=\$(CC)}"
+ : "${CXX_FOR_BUILD:=\$(CXX)}"
: "${CFLAGS_FOR_BUILD:=\$(CFLAGS)}"
+ : "${CXXFLAGS_FOR_BUILD:=\$(CXXFLAGS)}"
: "${LDFLAGS_FOR_BUILD:=\$(LDFLAGS)}"
else
: "${CC_FOR_BUILD:=gcc}"
+ : "${CXX_FOR_BUILD:=g++}"
: "${CFLAGS_FOR_BUILD:=-g -O}"
+ : "${CXXFLAGS_FOR_BUILD:=-g -O}"
: "${LDLFAGS_FOR_BUILD:=}"
fi
AC_SUBST(CC_FOR_BUILD)
+AC_SUBST(CXX_FOR_BUILD)
AC_SUBST(CFLAGS_FOR_BUILD)
+AC_SUBST(CXXFLAGS_FOR_BUILD)
AC_SUBST(LDFLAGS_FOR_BUILD)
AC_SUBST(CFLAGS)
More information about the Gdb-patches
mailing list