From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id g6qkKClibGCCTgAAWB0awg (envelope-from ) for ; Tue, 06 Apr 2021 09:29:13 -0400 Received: by simark.ca (Postfix, from userid 112) id 987D41E939; Tue, 6 Apr 2021 09:29:13 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 79E831E54D for ; Tue, 6 Apr 2021 09:29:11 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 0B46B384601F; Tue, 6 Apr 2021 13:29:11 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0B46B384601F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1617715751; bh=OXHqy+UKdBAnukfirLHcOQhcqt29gFQiA9nFlHF2WNs=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=vGAreL1XMbTUsLEIqs128y2FICoGPcTk2h6Gr63p3eQ3QrzCOGtqcR6GMnaK91+dn svMhF4Z2UHTJpl1Ac7hUJsIyGdQWQzSDUJwyORaEI1pojXOonK7OaPef0YnZGXOvCD 62gVXcfUm/KGprvgOBi9XMU/dcD5LYfOQq5AMkYU= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 456DE384601F for ; Tue, 6 Apr 2021 13:29:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 456DE384601F Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 136DSxq2018855 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 6 Apr 2021 09:29:04 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 136DSxq2018855 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id C489E1E54D for ; Tue, 6 Apr 2021 09:28:59 -0400 (EDT) Subject: Re: [PATCH] sim/m32c: fix memory leaks in opc2c To: gdb-patches@sourceware.org References: <20210405145856.3925296-1-simon.marchi@polymtl.ca> Message-ID: Date: Tue, 6 Apr 2021 09:28:59 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Tue, 6 Apr 2021 13:28:59 +0000 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces@sourceware.org Sender: "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 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