* [PATCH] sim/m32c: fix memory leaks in opc2c @ 2021-04-05 14:58 Simon Marchi 2021-04-05 16:23 ` Mike Frysinger 2021-04-08 4:50 ` Mike Frysinger 0 siblings, 2 replies; 16+ messages in thread From: Simon Marchi @ 2021-04-05 14:58 UTC (permalink / raw) To: gdb-patches When building with AddressSanitizer, sim/m32c fails with: ./opc2c -l r8c.out /home/simark/src/binutils-gdb/sim/m32c/r8c.opc > r8c.c sim_log: r8c.out ================================================================= ==3919390==ERROR: LeakSanitizer: detected memory leaks Direct leak of 485 byte(s) in 131 object(s) allocated from: #0 0x7ffff761fa69 in __interceptor_strdup /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cpp:452 #1 0x555555557abe in dump_lines /home/simark/src/binutils-gdb/sim/m32c/opc2c.c:243 #2 0x555555558bc9 in emit_indirect /home/simark/src/binutils-gdb/sim/m32c/opc2c.c:382 #3 0x555555558c66 in emit_indirect /home/simark/src/binutils-gdb/sim/m32c/opc2c.c:387 #4 0x55555555c463 in main /home/simark/src/binutils-gdb/sim/m32c/opc2c.c:718 #5 0x7ffff741fb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24) Direct leak of 360 byte(s) in 100 object(s) allocated from: #0 0x7ffff761fa69 in __interceptor_strdup /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cpp:452 #1 0x555555557abe in dump_lines /home/simark/src/binutils-gdb/sim/m32c/opc2c.c:243 #2 0x555555558bc9 in emit_indirect /home/simark/src/binutils-gdb/sim/m32c/opc2c.c:382 #3 0x55555555c463 in main /home/simark/src/binutils-gdb/sim/m32c/opc2c.c:718 #4 0x7ffff741fb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24) Direct leak of 4 byte(s) in 1 object(s) allocated from: #0 0x7ffff7677459 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:145 #1 0x55555555b3df in main /home/simark/src/binutils-gdb/sim/m32c/opc2c.c:658 #2 0x7ffff741fb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24) Fix the leak in dump_lines by free-ing the elements of varnames. Fix the leak in main by removing the vlist variable, which seems unused. sim/m32c/ChangeLog: * opc2c.c (dump_lines): Free elements of varnames. (main): Remove vlist variable. Change-Id: I5a92fd229da623bd32841b083c54c5c868455e91 --- sim/m32c/opc2c.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sim/m32c/opc2c.c b/sim/m32c/opc2c.c index 082d4e9b8a42..7d1604f4fb3a 100644 --- a/sim/m32c/opc2c.c +++ b/sim/m32c/opc2c.c @@ -286,6 +286,9 @@ dump_lines (opcode * op, int level, Indirect * ind) printf ("%*s%s", level, "", op->lines[i]); if (op->comment) printf ("%*s}\n", level, ""); + + for (i = 0; i < vn; i++) + free (varnames[i]); } void @@ -507,7 +510,6 @@ main (int argc, char **argv) FILE *in; int lineno = 0; int i; - VaryRef *vlist; if (argc > 2 && strcmp (argv[1], "-l") == 0) { @@ -655,8 +657,6 @@ main (int argc, char **argv) qsort (opcodes, n_opcodes, sizeof (opcodes[0]), op_cmp); - vlist = (VaryRef *) malloc (n_varies * sizeof (VaryRef)); - for (i = 0; i < n_opcodes; i++) { int j, b, v; -- 2.30.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sim/m32c: fix memory leaks in opc2c 2021-04-05 14:58 [PATCH] sim/m32c: fix memory leaks in opc2c Simon Marchi @ 2021-04-05 16:23 ` Mike Frysinger 2021-04-05 18:46 ` Simon Marchi 2021-04-08 4:50 ` Mike Frysinger 1 sibling, 1 reply; 16+ messages in thread From: Mike Frysinger @ 2021-04-05 16:23 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches On 05 Apr 2021 10:58, Simon Marchi via Gdb-patches wrote: > Fix the leak in dump_lines by free-ing the elements of varnames. i dislike stuffing a bunch of free's at the end of a program's lifetime just to satisfy a tool that is not normally used. which isn't to say LSAN isn't useful, just that i think we should do better. 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. -mike ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sim/m32c: fix memory leaks in opc2c 2021-04-05 16:23 ` Mike Frysinger @ 2021-04-05 18:46 ` Simon Marchi 2021-04-05 21:51 ` Mike Frysinger 0 siblings, 1 reply; 16+ messages in thread From: Simon Marchi @ 2021-04-05 18:46 UTC (permalink / raw) To: gdb-patches On 2021-04-05 12:23 p.m., Mike Frysinger wrote:> On 05 Apr 2021 10:58, Simon Marchi via Gdb-patches wrote: >> Fix the leak in dump_lines by free-ing the elements of varnames. > > i dislike stuffing a bunch of free's at the end of a program's lifetime just > to satisfy a tool that is not normally used. which isn't to say LSAN isn't > useful, just that i think we should do better. Why though? Because it adds execution time where not necessary? > 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. Simon ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sim/m32c: fix memory leaks in opc2c 2021-04-05 18:46 ` Simon Marchi @ 2021-04-05 21:51 ` Mike Frysinger 2021-04-06 1:36 ` Simon Marchi 0 siblings, 1 reply; 16+ messages in thread From: Mike Frysinger @ 2021-04-05 21:51 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches On 05 Apr 2021 14:46, Simon Marchi via Gdb-patches wrote: > On 2021-04-05 12:23 p.m., Mike Frysinger wrote:> On 05 Apr 2021 10:58, Simon Marchi via Gdb-patches wrote: > >> Fix the leak in dump_lines by free-ing the elements of varnames. > > > > i dislike stuffing a bunch of free's at the end of a program's lifetime just > > to satisfy a tool that is not normally used. which isn't to say LSAN isn't > > useful, just that i think we should do better. > > Why though? Because it adds execution time where not necessary? when the process exits, the kernel releases all the memory at once. there's no point to calling free() on all your allocated buffers before exiting. it only wastes time with the C library heap accounting & syscalls. i get that we're talking about opc2c here which is used only twice to generate two other files, so in the larger scheme of things, it's barely noise. i'm trying to define standard patterns we can apply in general for "the next one". > > 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. -mike ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sim/m32c: fix memory leaks in opc2c 2021-04-05 21:51 ` Mike Frysinger @ 2021-04-06 1:36 ` Simon Marchi 2021-04-06 10:41 ` Mike Frysinger 0 siblings, 1 reply; 16+ messages in thread From: Simon Marchi @ 2021-04-06 1:36 UTC (permalink / raw) To: gdb-patches 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:> On 05 Apr 2021 10:58, Simon Marchi via Gdb-patches wrote: >>>> Fix the leak in dump_lines by free-ing the elements of varnames. >>> >>> i dislike stuffing a bunch of free's at the end of a program's lifetime just >>> to satisfy a tool that is not normally used. which isn't to say LSAN isn't >>> useful, just that i think we should do better. >> >> Why though? Because it adds execution time where not necessary? > > when the process exits, the kernel releases all the memory at once. there's > no point to calling free() on all your allocated buffers before exiting. it > only wastes time with the C library heap accounting & syscalls. > > i get that we're talking about opc2c here which is used only twice to generate > two other files, so in the larger scheme of things, it's barely noise. i'm > trying to define standard patterns we can apply in general for "the next one". I understand. I'm not that worried about that kind of performance hit though. >>> 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? Although I'm always in favor of using C++ for just not having to think about free-ing stuff. Simon ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sim/m32c: fix memory leaks in opc2c 2021-04-06 1:36 ` Simon Marchi @ 2021-04-06 10:41 ` Mike Frysinger 2021-04-06 13:28 ` Simon Marchi 0 siblings, 1 reply; 16+ messages in thread From: Mike Frysinger @ 2021-04-06 10:41 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches 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) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sim/m32c: fix memory leaks in opc2c 2021-04-06 10:41 ` Mike Frysinger @ 2021-04-06 13:28 ` Simon Marchi 2021-04-06 13:45 ` Tom Tromey 2021-04-06 22:45 ` Mike Frysinger 0 siblings, 2 replies; 16+ messages in thread From: Simon Marchi @ 2021-04-06 13:28 UTC (permalink / raw) To: gdb-patches On 2021-04-06 6:41 a.m., Mike Frysinger wrote:> On 05 Apr 2021 21:36, Simon Marchi via Gdb-patches wrote: >> 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. Oh ok, didn't know about that. >> 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 I might try to take it on as a side project, but this is clearly too big as an immediate solution. What do you think of the patch below that sets ASAN_OPTIONS while running the tools? From e61e8132666f1ff87590075e2be88f379bc17aed Mon Sep 17 00:00:00 2001 From: Simon Marchi <simon.marchi@polymtl.ca> Date: Tue, 6 Apr 2021 08:54:00 -0400 Subject: [PATCH] sim: set ASAN_OPTIONS=detect_leaks=0 when running igen and opc2c The igen/dgen and opc2c tools leak their heap-allocated memory (on purpose) at program exit, which makes AddressSanitizer fail the tool execution. This breaks the build, as it makes the tool return a non-zero exit code. Fix that by disabling leak detection through the setting of that environment variable. I also changed the opc2c rules for m32c to go through a temporary file. What happened is that the failing opc2c would produce an incomplete file (probably because ASan exits the process before stdout is flushed). This meant that further make attempts didn't try to re-create the file, as it already existed. A "clean" was therefore necessary. This can also happen in regular builds if the user interrupts the build (^C) in the middle of the opc2c execution and tries to resume it. Going to a temporary file avoids this issue. sim/m32c/ChangeLog: * Makefile.in: Set ASAN_OPTIONS when running opc2c. sim/mips/ChangeLog: * Makefile.in: Set ASAN_OPTIONS when running igen. sim/mn10300/ChangeLog: * Makefile.in: Set ASAN_OPTIONS when running igen. sim/ppc/ChangeLog: * Makefile.in: Set ASAN_OPTIONS when running igen. sim/v850/ChangeLog: * Makefile.in: Set ASAN_OPTIONS when running igen. Change-Id: I00f21d4dc1aff0ef73471925d41ce7c23e83e082 --- sim/m32c/Makefile.in | 8 ++++++-- sim/mips/Makefile.in | 25 +++++++++++++++---------- sim/mn10300/Makefile.in | 7 ++++++- sim/ppc/Makefile.in | 10 ++++++++-- sim/v850/Makefile.in | 7 ++++++- 5 files changed, 41 insertions(+), 16 deletions(-) diff --git a/sim/m32c/Makefile.in b/sim/m32c/Makefile.in index 6bc5c5b743d9..0b604685eca9 100644 --- a/sim/m32c/Makefile.in +++ b/sim/m32c/Makefile.in @@ -46,11 +46,15 @@ LIBS = $B/bfd/libbfd.a $B/libiberty/libiberty.a arch = m32c +OPC2C_NO_LSAN = ASAN_OPTIONS=detect_leaks=0 ./opc2c + r8c.c : r8c.opc opc2c - ./opc2c -l r8c.out $(srcdir)/r8c.opc > r8c.c + $(OPC2C_NO_LSAN) -l r8c.out $(srcdir)/r8c.opc > r8c.c.tmp + mv r8c.c.tmp r8c.c m32c.c : m32c.opc opc2c - ./opc2c -l m32c.out $(srcdir)/m32c.opc > m32c.c + $(OPC2C_NO_LSAN) -l m32c.out $(srcdir)/m32c.opc > m32c.c.tmp + mv m32c.c.tmp m32c.c opc2c : opc2c.o safe-fgets.o $(LINK_FOR_BUILD) $^ diff --git a/sim/mips/Makefile.in b/sim/mips/Makefile.in index c66c6e839782..5f514128412e 100644 --- a/sim/mips/Makefile.in +++ b/sim/mips/Makefile.in @@ -8,6 +8,11 @@ SHELL = @SHELL@ srcdir=@srcdir@ srcroot=$(srcdir)/../../ +# igen leaks memory, and therefore makes AddressSanitizer unhappy. Disable +# leak detection while running it. + +IGEN_NO_LSAN = ASAN_OPTIONS=detect_leaks=0 ../igen/igen + # Object files created by various simulator generators. @@ -149,7 +154,7 @@ BUILT_SRC_FROM_IGEN = \ $(BUILT_SRC_FROM_IGEN): tmp-igen tmp-igen: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) - ../igen/igen \ + $(IGEN_NO_LSAN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -220,7 +225,7 @@ BUILT_SRC_FROM_M16 = \ $(BUILT_SRC_FROM_M16): tmp-m16 tmp-m16: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) - ../igen/igen \ + $(IGEN_NO_LSAN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -255,7 +260,7 @@ tmp-m16: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) $(SHELL) $(srcdir)/../../move-if-change tmp-model.c m16_model.c $(SHELL) $(srcdir)/../../move-if-change tmp-support.h m16_support.h $(SHELL) $(srcdir)/../../move-if-change tmp-support.c m16_support.c - ../igen/igen \ + $(IGEN_NO_LSAN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -292,7 +297,7 @@ tmp-m16: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) $(SHELL) $(srcdir)/../../move-if-change tmp-model.c m32_model.c $(SHELL) $(srcdir)/../../move-if-change tmp-support.h m32_support.h $(SHELL) $(srcdir)/../../move-if-change tmp-support.c m32_support.c - ../igen/igen \ + $(IGEN_NO_LSAN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -346,7 +351,7 @@ BUILT_SRC_FROM_MICROMIPS = \ $(BUILT_SRC_FROM_MICROMIPS): tmp-micromips tmp-micromips: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) - ../igen/igen \ + $(IGEN_NO_LSAN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -391,7 +396,7 @@ tmp-micromips: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) micromips16_support.h $(SHELL) $(srcdir)/../../move-if-change tmp-support.c \ micromips16_support.c - ../igen/igen \ + $(IGEN_NO_LSAN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -436,7 +441,7 @@ tmp-micromips: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) micromips32_support.h $(SHELL) $(srcdir)/../../move-if-change tmp-support.c \ micromips32_support.c - ../igen/igen \ + $(IGEN_NO_LSAN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -481,7 +486,7 @@ tmp-micromips: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) micromips_m32_support.h $(SHELL) $(srcdir)/../../move-if-change tmp-support.c \ micromips_m32_support.c - ../igen/igen \ + $(IGEN_NO_LSAN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -521,7 +526,7 @@ tmp-mach-multi: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) m16*) e="-B 16 -H 15 -o $(M16_DC) -F 16" ;; \ *) e="-B 32 -H 31 -o $(IGEN_DC) -F $${f}" ;; \ esac; \ - ../igen/igen \ + $(IGEN_NO_LSAN) \ $(IGEN_TRACE) \ $${e} \ -I $(srcdir) \ @@ -574,7 +579,7 @@ tmp-mach-multi: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) done touch tmp-mach-multi tmp-itable-multi: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) - ../igen/igen \ + $(IGEN_NO_LSAN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ diff --git a/sim/mn10300/Makefile.in b/sim/mn10300/Makefile.in index 6cdf9712882b..d0cc48309789 100644 --- a/sim/mn10300/Makefile.in +++ b/sim/mn10300/Makefile.in @@ -37,6 +37,11 @@ INCLUDE = mn10300_sim.h $(srcdir)/../../include/gdb/callback.h # List of extra flags to always pass to $(CC). SIM_EXTRA_CFLAGS = -DPOLL_QUIT_INTERVAL=0x20 +# igen leaks memory, and therefore makes AddressSanitizer unhappy. Disable +# leak detection while running it. + +IGEN_NO_LSAN = ASAN_OPTIONS=detect_leaks=0 ../igen/igen + ## COMMON_POST_CONFIG_FRAG idecode.o op_utils.o semantics.o: targ-vals.h @@ -69,7 +74,7 @@ IGEN_TRACE= # -G omit-line-numbers # -G trace-rule-selection -G trace-rule-rejec IGEN_INSN=$(srcdir)/mn10300.igen $(srcdir)/am33.igen $(srcdir)/am33-2.igen IGEN_DC=$(srcdir)/mn10300.dc tmp-igen: $(IGEN_INSN) $(IGEN_DC) ../igen/igen - ../igen/igen \ + $(IGEN_NO_LSAN) \ $(IGEN_TRACE) \ -G gen-direct-access \ -M mn10300,am33 -G gen-multi-sim=am33 \ diff --git a/sim/ppc/Makefile.in b/sim/ppc/Makefile.in index d2bd1d317754..4bcb0806ee1b 100644 --- a/sim/ppc/Makefile.in +++ b/sim/ppc/Makefile.in @@ -133,6 +133,12 @@ IGEN_FLAGS = \ $(IGEN_SMP) \ $(IGEN_LINE_NR) +# igen/dgen leak memory, and therefore makes AddressSanitizer unhappy. Disable +# leak detection while running them. + +IGEN_NO_LSAN = ASAN_OPTIONS=detect_leaks=0 ./igen +DGEN_NO_LSAN = ASAN_OPTIONS=detect_leaks=0 ./dgen + .NOEXPORT: MAKEOVERRIDES= @@ -666,7 +672,7 @@ ppc-config.h: $(CONFIG_FILE) tmp-dgen: dgen ppc-spr-table $(srcdir)/../../move-if-change - ./dgen $(DGEN_FLAGS) \ + $(DGEN_NO_LSAN) $(DGEN_FLAGS) \ -r $(srcdir)/ppc-spr-table \ -n spreg.h -hp tmp-spreg.h \ -n spreg.c -p tmp-spreg.c @@ -675,7 +681,7 @@ tmp-dgen: dgen ppc-spr-table $(srcdir)/../../move-if-change touch tmp-dgen tmp-igen: igen $(srcdir)/ppc-instructions $(srcdir)/altivec.igen $(srcdir)/e500.igen $(IGEN_OPCODE_RULES) $(srcdir)/../../move-if-change tmp-ld-decode tmp-ld-cache tmp-ld-insn tmp-filter - ./igen $(IGEN_FLAGS) \ + $(IGEN_NO_LSAN) $(IGEN_FLAGS) \ -o $(srcdir)/$(IGEN_OPCODE_RULES) \ -I $(srcdir) -i $(srcdir)/ppc-instructions \ -n icache.h -hc tmp-icache.h \ diff --git a/sim/v850/Makefile.in b/sim/v850/Makefile.in index 983fc79f93a7..d0e54530f450 100644 --- a/sim/v850/Makefile.in +++ b/sim/v850/Makefile.in @@ -41,6 +41,11 @@ NL_TARGET = -DNL_TARGET_v850 ## COMMON_POST_CONFIG_FRAG +# igen leaks memory, and therefore makes AddressSanitizer unhappy. Disable +# leak detection while running it. + +IGEN_NO_LSAN = ASAN_OPTIONS=detect_leaks=0 ../igen/igen + BUILT_SRC_FROM_IGEN = \ icache.h \ icache.c \ @@ -69,7 +74,7 @@ IGEN_TRACE= # -G omit-line-numbers # -G trace-rule-selection -G trace-rule-rejec IGEN_INSN=$(srcdir)/v850.igen IGEN_DC=$(srcdir)/v850-dc tmp-igen: $(IGEN_INSN) $(IGEN_DC) ../igen/igen - ../igen/igen \ + $(IGEN_NO_LSAN) \ $(IGEN_TRACE) \ -G gen-direct-access \ -G gen-zero-r0 \ -- 2.30.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sim/m32c: fix memory leaks in opc2c 2021-04-06 13:28 ` Simon Marchi @ 2021-04-06 13:45 ` Tom Tromey 2021-04-06 18:01 ` Philippe Waroquiers 2021-04-06 22:45 ` Mike Frysinger 1 sibling, 1 reply; 16+ messages in thread From: Tom Tromey @ 2021-04-06 13:45 UTC (permalink / raw) To: Simon Marchi via Gdb-patches Simon> I might try to take it on as a side project, but this is clearly too big Simon> as an immediate solution. What do you think of the patch below that Simon> sets ASAN_OPTIONS while running the tools? FWIW, I was surprised to hear that leak sanitizer reported this kind of leak. IIRC, valgrind has an option to ignore these, for just the reason that Mike pointed out -- there's no point to handling a leak just before exit. Anyway, I researched a little and came across this answer: https://stackoverflow.com/questions/51553115/how-do-i-make-leaksanitizer-ignore-end-of-program-leaks I don't know if that's something we'd want to enshrine in the tree, but at least now we know there's a way available if we want it. Tom ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sim/m32c: fix memory leaks in opc2c 2021-04-06 13:45 ` Tom Tromey @ 2021-04-06 18:01 ` Philippe Waroquiers 0 siblings, 0 replies; 16+ messages in thread From: Philippe Waroquiers @ 2021-04-06 18:01 UTC (permalink / raw) To: Tom Tromey, Simon Marchi via Gdb-patches On Tue, 2021-04-06 at 07:45 -0600, Tom Tromey wrote: > Simon> I might try to take it on as a side project, but this is clearly too big > Simon> as an immediate solution. What do you think of the patch below that > Simon> sets ASAN_OPTIONS while running the tools? > > FWIW, I was surprised to hear that leak sanitizer reported this kind of > leak. IIRC, valgrind has an option to ignore these, for just the reason > that Mike pointed out -- there's no point to handling a leak just before > exit. Effectively, with valgrind, you can control which leak kinds to show at exit, and you can control separately which leak kinds have to be considered as error: --show-leak-kinds=kind1,kind2,.. which leak kinds to show? [definite,possible] --errors-for-leak-kinds=kind1,kind2,.. which leak kinds are errors? [definite,possible] where kind is one of: definite indirect possible reachable all none You can similarly to the answer below do leak search from within the program itself. Philippe > > Anyway, I researched a little and came across this answer: > > https://stackoverflow.com/questions/51553115/how-do-i-make-leaksanitizer-ignore-end-of-program-leaks > > I don't know if that's something we'd want to enshrine in the tree, but > at least now we know there's a way available if we want it. > > Tom ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sim/m32c: fix memory leaks in opc2c 2021-04-06 13:28 ` Simon Marchi 2021-04-06 13:45 ` Tom Tromey @ 2021-04-06 22:45 ` Mike Frysinger 2021-04-07 1:45 ` Simon Marchi 1 sibling, 1 reply; 16+ messages in thread From: Mike Frysinger @ 2021-04-06 22:45 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches On 06 Apr 2021 09:28, Simon Marchi via Gdb-patches wrote: > On 2021-04-06 6:41 a.m., Mike Frysinger wrote: > > On 05 Apr 2021 21:36, Simon Marchi via Gdb-patches wrote: > >> 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. > > I might try to take it on as a side project, but this is clearly too big > as an immediate solution. that's totally understandable > What do you think of the patch below that > sets ASAN_OPTIONS while running the tools? i'm not keen on pushing it in this direction exactly. it would mean every caller would have to update & keep track. i think you could define an IGEN variable in common/Make-common.in and change all callers over to that. then that would be the only place you'd have to add any sanitizer related variables to. -mike ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sim/m32c: fix memory leaks in opc2c 2021-04-06 22:45 ` Mike Frysinger @ 2021-04-07 1:45 ` Simon Marchi 2021-04-07 11:38 ` Mike Frysinger 0 siblings, 1 reply; 16+ messages in thread From: Simon Marchi @ 2021-04-07 1:45 UTC (permalink / raw) To: gdb-patches > i'm not keen on pushing it in this direction exactly. it would mean every > caller would have to update & keep track. > > i think you could define an IGEN variable in common/Make-common.in and change > all callers over to that. then that would be the only place you'd have to add > any sanitizer related variables to. > -mike > I can do that, but I'm not sure where in common/Make-common.in I should add the variable. There seems to be a logic to the organization in that file, but I don't get it. Also, not that ppc has its own igen, so I guess it will still use its own definition. Another option is to just factor out the env var: DISABLE_LSAN = ASAN_OPTIONS=detect_leaks=0 ... and still use it in rules: target: source $(DISABLE_LSAN) ../igen/igen --blah Worst case, if somebody forgets to use DISABLE_LSAN somewhere, someone building with ASan will notice and it's a trivial fix to add it. Anyway, feel free to push the version you prefer, it will be faster than telling me how to do it :). Side-note, I saw some `@GMAKE_TRUE@` in Make-common.in. In GDB, we decided to require GNU make and remove that complexity, in case you want to do the same. Simon ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sim/m32c: fix memory leaks in opc2c 2021-04-07 1:45 ` Simon Marchi @ 2021-04-07 11:38 ` Mike Frysinger 2021-04-07 14:19 ` Simon Marchi 0 siblings, 1 reply; 16+ messages in thread From: Mike Frysinger @ 2021-04-07 11:38 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches On 06 Apr 2021 21:45, Simon Marchi via Gdb-patches wrote: > > i'm not keen on pushing it in this direction exactly. it would mean every > > caller would have to update & keep track. > > > > i think you could define an IGEN variable in common/Make-common.in and change > > all callers over to that. then that would be the only place you'd have to add > > any sanitizer related variables to. > > I can do that, but I'm not sure where in common/Make-common.in I should > add the variable. There seems to be a logic to the organization in that > file, but I don't get it. *shrug* there isn't much to it. you can put it after the POSTCOMPILE= line. > Also, not that ppc has its own igen, so I > guess it will still use its own definition. correct, for now, it's duplicated and we eat that cost > Another option is to just factor out the env var: > > DISABLE_LSAN = ASAN_OPTIONS=detect_leaks=0 > > ... and still use it in rules: > > target: source > $(DISABLE_LSAN) ../igen/igen --blah i'm not keen on doing sanitizer-specific vars. there's ASAN, LSAN, TSAN, UBSAN, KSAN, and prob more in the future. that's why having a common IGEN var and then having it use a generic name (SANITIZE_ENV?) would work best imo. > Side-note, I saw some `@GMAKE_TRUE@` in Make-common.in. In GDB, we > decided to require GNU make and remove that complexity, in case you want > to do the same. sim/ is changing to automake, so a lot of that stuff will go away entirely. i'm focusing on that rather than chipping away at smaller bits. you can see this with the igen/ dir: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=b6b1c790843087e67e85e7cfd3327a872c03c6bc although i need to do more groundwork in the C side first. -mike ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sim/m32c: fix memory leaks in opc2c 2021-04-07 11:38 ` Mike Frysinger @ 2021-04-07 14:19 ` Simon Marchi 2021-04-08 4:51 ` Mike Frysinger 0 siblings, 1 reply; 16+ messages in thread From: Simon Marchi @ 2021-04-07 14:19 UTC (permalink / raw) To: gdb-patches On 2021-04-07 7:38 a.m., Mike Frysinger wrote: > On 06 Apr 2021 21:45, Simon Marchi via Gdb-patches wrote: >>> i'm not keen on pushing it in this direction exactly. it would mean every >>> caller would have to update & keep track. >>> >>> i think you could define an IGEN variable in common/Make-common.in and change >>> all callers over to that. then that would be the only place you'd have to add >>> any sanitizer related variables to. >> >> I can do that, but I'm not sure where in common/Make-common.in I should >> add the variable. There seems to be a logic to the organization in that >> file, but I don't get it. > > *shrug* there isn't much to it. you can put it after the POSTCOMPILE= line. > >> Also, not that ppc has its own igen, so I >> guess it will still use its own definition. > > correct, for now, it's duplicated and we eat that cost > >> Another option is to just factor out the env var: >> >> DISABLE_LSAN = ASAN_OPTIONS=detect_leaks=0 >> >> ... and still use it in rules: >> >> target: source >> $(DISABLE_LSAN) ../igen/igen --blah > > i'm not keen on doing sanitizer-specific vars. there's ASAN, LSAN, TSAN, > UBSAN, KSAN, and prob more in the future. that's why having a common IGEN > var and then having it use a generic name (SANITIZE_ENV?) would work best > imo. > >> Side-note, I saw some `@GMAKE_TRUE@` in Make-common.in. In GDB, we >> decided to require GNU make and remove that complexity, in case you want >> to do the same. > > sim/ is changing to automake, so a lot of that stuff will go away entirely. > i'm focusing on that rather than chipping away at smaller bits. > > you can see this with the igen/ dir: > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=b6b1c790843087e67e85e7cfd3327a872c03c6bc > > although i need to do more groundwork in the C side first. > -mike > See updated patch below. From 1d03bd9b0173a319eddab82d41bfb1671bfcc82d Mon Sep 17 00:00:00 2001 From: Simon Marchi <simon.marchi@polymtl.ca> Date: Tue, 6 Apr 2021 08:54:00 -0400 Subject: [PATCH] sim: set ASAN_OPTIONS=detect_leaks=0 when running igen and opc2c The igen/dgen and opc2c tools leak their heap-allocated memory (on purpose) at program exit, which makes AddressSanitizer fail the tool execution. This breaks the build, as it makes the tool return a non-zero exit code. Fix that by disabling leak detection through the setting of that environment variable. I also changed the opc2c rules for m32c to go through a temporary file. What happened is that the failing opc2c would produce an incomplete file (probably because ASan exits the process before stdout is flushed). This meant that further make attempts didn't try to re-create the file, as it already existed. A "clean" was therefore necessary. This can also happen in regular builds if the user interrupts the build (^C) in the middle of the opc2c execution and tries to resume it. Going to a temporary file avoids this issue. sim/m32c/ChangeLog: * Makefile.in: Set ASAN_OPTIONS when running opc2c. sim/mips/ChangeLog: * Makefile.in: Set ASAN_OPTIONS when running igen. sim/mn10300/ChangeLog: * Makefile.in: Set ASAN_OPTIONS when running igen. sim/ppc/ChangeLog: * Makefile.in: Set ASAN_OPTIONS when running igen. sim/v850/ChangeLog: * Makefile.in: Set ASAN_OPTIONS when running igen. Change-Id: I00f21d4dc1aff0ef73471925d41ce7c23e83e082 --- sim/common/Make-common.in | 4 ++++ sim/m32c/Makefile.in | 10 ++++++++-- sim/mips/Makefile.in | 20 ++++++++++---------- sim/mn10300/Makefile.in | 2 +- sim/ppc/Makefile.in | 10 ++++++++-- sim/v850/Makefile.in | 2 +- 6 files changed, 32 insertions(+), 16 deletions(-) diff --git a/sim/common/Make-common.in b/sim/common/Make-common.in index c8445bce59ea..28f50abb2205 100644 --- a/sim/common/Make-common.in +++ b/sim/common/Make-common.in @@ -111,6 +111,10 @@ COMPILE.post = -c -o $@ COMPILE = $(COMPILE.pre) $(ALL_CFLAGS) $(COMPILE.post) POSTCOMPILE = @true +# igen leaks memory, and therefore makes AddressSanitizer unhappy. Disable +# leak detection while running it. +IGEN = ASAN_OPTIONS=detect_leaks=0 ../igen/igen + # Each simulator's Makefile.in defines one or more of these variables # to override our settings as necessary. There is no need to define these # in the simulator's Makefile.in if one is using the default value. In fact diff --git a/sim/m32c/Makefile.in b/sim/m32c/Makefile.in index 6bc5c5b743d9..186c9c063d32 100644 --- a/sim/m32c/Makefile.in +++ b/sim/m32c/Makefile.in @@ -46,11 +46,17 @@ LIBS = $B/bfd/libbfd.a $B/libiberty/libiberty.a arch = m32c +# opc2c leaks memory, and therefore makes AddressSanitizer unhappy. Disable +# leak detection while running it. +OPC2C = ASAN_OPTIONS=detect_leaks=0 ./opc2c + r8c.c : r8c.opc opc2c - ./opc2c -l r8c.out $(srcdir)/r8c.opc > r8c.c + $(OPC2C) -l r8c.out $(srcdir)/r8c.opc > r8c.c.tmp + mv r8c.c.tmp r8c.c m32c.c : m32c.opc opc2c - ./opc2c -l m32c.out $(srcdir)/m32c.opc > m32c.c + $(OPC2C) -l m32c.out $(srcdir)/m32c.opc > m32c.c.tmp + mv m32c.c.tmp m32c.c opc2c : opc2c.o safe-fgets.o $(LINK_FOR_BUILD) $^ diff --git a/sim/mips/Makefile.in b/sim/mips/Makefile.in index c66c6e839782..00906451c4cc 100644 --- a/sim/mips/Makefile.in +++ b/sim/mips/Makefile.in @@ -149,7 +149,7 @@ BUILT_SRC_FROM_IGEN = \ $(BUILT_SRC_FROM_IGEN): tmp-igen tmp-igen: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) - ../igen/igen \ + $(IGEN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -220,7 +220,7 @@ BUILT_SRC_FROM_M16 = \ $(BUILT_SRC_FROM_M16): tmp-m16 tmp-m16: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) - ../igen/igen \ + $(IGEN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -255,7 +255,7 @@ tmp-m16: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) $(SHELL) $(srcdir)/../../move-if-change tmp-model.c m16_model.c $(SHELL) $(srcdir)/../../move-if-change tmp-support.h m16_support.h $(SHELL) $(srcdir)/../../move-if-change tmp-support.c m16_support.c - ../igen/igen \ + $(IGEN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -292,7 +292,7 @@ tmp-m16: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) $(SHELL) $(srcdir)/../../move-if-change tmp-model.c m32_model.c $(SHELL) $(srcdir)/../../move-if-change tmp-support.h m32_support.h $(SHELL) $(srcdir)/../../move-if-change tmp-support.c m32_support.c - ../igen/igen \ + $(IGEN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -346,7 +346,7 @@ BUILT_SRC_FROM_MICROMIPS = \ $(BUILT_SRC_FROM_MICROMIPS): tmp-micromips tmp-micromips: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) - ../igen/igen \ + $(IGEN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -391,7 +391,7 @@ tmp-micromips: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) micromips16_support.h $(SHELL) $(srcdir)/../../move-if-change tmp-support.c \ micromips16_support.c - ../igen/igen \ + $(IGEN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -436,7 +436,7 @@ tmp-micromips: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) micromips32_support.h $(SHELL) $(srcdir)/../../move-if-change tmp-support.c \ micromips32_support.c - ../igen/igen \ + $(IGEN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -481,7 +481,7 @@ tmp-micromips: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) micromips_m32_support.h $(SHELL) $(srcdir)/../../move-if-change tmp-support.c \ micromips_m32_support.c - ../igen/igen \ + $(IGEN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ @@ -521,7 +521,7 @@ tmp-mach-multi: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) m16*) e="-B 16 -H 15 -o $(M16_DC) -F 16" ;; \ *) e="-B 32 -H 31 -o $(IGEN_DC) -F $${f}" ;; \ esac; \ - ../igen/igen \ + $(IGEN) \ $(IGEN_TRACE) \ $${e} \ -I $(srcdir) \ @@ -574,7 +574,7 @@ tmp-mach-multi: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) done touch tmp-mach-multi tmp-itable-multi: $(IGEN_INSN) $(IGEN_DC) ../igen/igen $(IGEN_INCLUDE) - ../igen/igen \ + $(IGEN) \ $(IGEN_TRACE) \ -I $(srcdir) \ -Werror \ diff --git a/sim/mn10300/Makefile.in b/sim/mn10300/Makefile.in index 6cdf9712882b..773b7f9a0c59 100644 --- a/sim/mn10300/Makefile.in +++ b/sim/mn10300/Makefile.in @@ -69,7 +69,7 @@ IGEN_TRACE= # -G omit-line-numbers # -G trace-rule-selection -G trace-rule-rejec IGEN_INSN=$(srcdir)/mn10300.igen $(srcdir)/am33.igen $(srcdir)/am33-2.igen IGEN_DC=$(srcdir)/mn10300.dc tmp-igen: $(IGEN_INSN) $(IGEN_DC) ../igen/igen - ../igen/igen \ + $(IGEN) \ $(IGEN_TRACE) \ -G gen-direct-access \ -M mn10300,am33 -G gen-multi-sim=am33 \ diff --git a/sim/ppc/Makefile.in b/sim/ppc/Makefile.in index d2bd1d317754..d9d01985404b 100644 --- a/sim/ppc/Makefile.in +++ b/sim/ppc/Makefile.in @@ -133,6 +133,12 @@ IGEN_FLAGS = \ $(IGEN_SMP) \ $(IGEN_LINE_NR) +# igen/dgen leak memory, and therefore makes AddressSanitizer unhappy. Disable +# leak detection while running them. + +IGEN = ASAN_OPTIONS=detect_leaks=0 ./igen +DGEN = ASAN_OPTIONS=detect_leaks=0 ./dgen + .NOEXPORT: MAKEOVERRIDES= @@ -666,7 +672,7 @@ ppc-config.h: $(CONFIG_FILE) tmp-dgen: dgen ppc-spr-table $(srcdir)/../../move-if-change - ./dgen $(DGEN_FLAGS) \ + $(DGEN) $(DGEN_FLAGS) \ -r $(srcdir)/ppc-spr-table \ -n spreg.h -hp tmp-spreg.h \ -n spreg.c -p tmp-spreg.c @@ -675,7 +681,7 @@ tmp-dgen: dgen ppc-spr-table $(srcdir)/../../move-if-change touch tmp-dgen tmp-igen: igen $(srcdir)/ppc-instructions $(srcdir)/altivec.igen $(srcdir)/e500.igen $(IGEN_OPCODE_RULES) $(srcdir)/../../move-if-change tmp-ld-decode tmp-ld-cache tmp-ld-insn tmp-filter - ./igen $(IGEN_FLAGS) \ + $(IGEN) $(IGEN_FLAGS) \ -o $(srcdir)/$(IGEN_OPCODE_RULES) \ -I $(srcdir) -i $(srcdir)/ppc-instructions \ -n icache.h -hc tmp-icache.h \ diff --git a/sim/v850/Makefile.in b/sim/v850/Makefile.in index 983fc79f93a7..250129c549f7 100644 --- a/sim/v850/Makefile.in +++ b/sim/v850/Makefile.in @@ -69,7 +69,7 @@ IGEN_TRACE= # -G omit-line-numbers # -G trace-rule-selection -G trace-rule-rejec IGEN_INSN=$(srcdir)/v850.igen IGEN_DC=$(srcdir)/v850-dc tmp-igen: $(IGEN_INSN) $(IGEN_DC) ../igen/igen - ../igen/igen \ + $(IGEN) \ $(IGEN_TRACE) \ -G gen-direct-access \ -G gen-zero-r0 \ -- 2.30.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sim/m32c: fix memory leaks in opc2c 2021-04-07 14:19 ` Simon Marchi @ 2021-04-08 4:51 ` Mike Frysinger 2021-04-08 13:52 ` Simon Marchi 0 siblings, 1 reply; 16+ messages in thread From: Mike Frysinger @ 2021-04-08 4:51 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches On 07 Apr 2021 10:19, Simon Marchi via Gdb-patches wrote: > See updated patch below. lgtm, thanks -mike ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sim/m32c: fix memory leaks in opc2c 2021-04-08 4:51 ` Mike Frysinger @ 2021-04-08 13:52 ` Simon Marchi 0 siblings, 0 replies; 16+ messages in thread From: Simon Marchi @ 2021-04-08 13:52 UTC (permalink / raw) To: gdb-patches On 2021-04-08 12:51 a.m., Mike Frysinger wrote: > On 07 Apr 2021 10:19, Simon Marchi via Gdb-patches wrote: >> See updated patch below. > > lgtm, thanks > -mike > Thanks, pushed. Simon ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sim/m32c: fix memory leaks in opc2c 2021-04-05 14:58 [PATCH] sim/m32c: fix memory leaks in opc2c Simon Marchi 2021-04-05 16:23 ` Mike Frysinger @ 2021-04-08 4:50 ` Mike Frysinger 1 sibling, 0 replies; 16+ messages in thread From: Mike Frysinger @ 2021-04-08 4:50 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches On 05 Apr 2021 10:58, Simon Marchi via Gdb-patches wrote: > Fix the leak in main by removing the vlist variable, which seems unused. fwiw, i extracted & merged this hunk alone since it seems obviously good -mike ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-04-08 13:52 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-05 14:58 [PATCH] sim/m32c: fix memory leaks in opc2c Simon Marchi 2021-04-05 16:23 ` Mike Frysinger 2021-04-05 18:46 ` Simon Marchi 2021-04-05 21:51 ` Mike Frysinger 2021-04-06 1:36 ` Simon Marchi 2021-04-06 10:41 ` Mike Frysinger 2021-04-06 13:28 ` Simon Marchi 2021-04-06 13:45 ` Tom Tromey 2021-04-06 18:01 ` Philippe Waroquiers 2021-04-06 22:45 ` Mike Frysinger 2021-04-07 1:45 ` Simon Marchi 2021-04-07 11:38 ` Mike Frysinger 2021-04-07 14:19 ` Simon Marchi 2021-04-08 4:51 ` Mike Frysinger 2021-04-08 13:52 ` Simon Marchi 2021-04-08 4:50 ` Mike Frysinger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).