* [PATCH 0/2] libsframe: fix some memory leaks @ 2022-12-22 22:54 Indu Bhagat 2022-12-22 22:54 ` [PATCH 1/2] libsframe: fix a memory leak in sframe_decode Indu Bhagat ` (3 more replies) 0 siblings, 4 replies; 11+ messages in thread From: Indu Bhagat @ 2022-12-22 22:54 UTC (permalink / raw) To: binutils; +Cc: Indu Bhagat Hello, This patch set fixes some memory leaks in the libsframe and it's testsuite. Testing notes: - Reg tested natively on aarch64 and x86_64 Thanks, Indu Bhagat (2): libsframe: fix a memory leak in sframe_decode libsframe: testsuite: fix memory leaks in testcases libsframe/sframe-impl.h | 15 +++++++++++---- libsframe/sframe.c | 9 +++++++++ .../testsuite/libsframe.decode/be-flipping.c | 3 +++ libsframe/testsuite/libsframe.decode/frecnt-1.c | 3 +++ libsframe/testsuite/libsframe.decode/frecnt-2.c | 3 +++ 5 files changed, 29 insertions(+), 4 deletions(-) -- 2.37.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] libsframe: fix a memory leak in sframe_decode 2022-12-22 22:54 [PATCH 0/2] libsframe: fix some memory leaks Indu Bhagat @ 2022-12-22 22:54 ` Indu Bhagat 2022-12-22 22:54 ` [PATCH 2/2] libsframe: testsuite: fix memory leaks in testcases Indu Bhagat ` (2 subsequent siblings) 3 siblings, 0 replies; 11+ messages in thread From: Indu Bhagat @ 2022-12-22 22:54 UTC (permalink / raw) To: binutils; +Cc: Indu Bhagat sframe_decode () needs to malloc a temporary buffer of the same size as the input buffer (containing the SFrame section bytes) when endian flipping is needed. The decoder keeps the endian flipped contents in this buffer for its usage. This code is necessary when the target endianneess is not the same as host endianness. The malloc'd buffer needs to be kept track of, so that it can freed up in sframe_decoder_free () later. ChangeLog: * libsframe/sframe-impl.h (struct sframe_decoder_ctx): Add new member to keep track of the internally malloc'd buffer. * libsframe/sframe.c (sframe_decoder_free): Free it up. (sframe_decode): Update the reference to the buffer. --- libsframe/sframe-impl.h | 15 +++++++++++---- libsframe/sframe.c | 9 +++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/libsframe/sframe-impl.h b/libsframe/sframe-impl.h index 0e61c977886..340d3b35042 100644 --- a/libsframe/sframe-impl.h +++ b/libsframe/sframe-impl.h @@ -32,10 +32,17 @@ extern "C" struct sframe_decoder_ctx { - sframe_header sfd_header; /* SFrame header. */ - uint32_t *sfd_funcdesc; /* SFrame function desc entries table. */ - void *sfd_fres; /* SFrame FRE table. */ - int sfd_fre_nbytes; /* Number of bytes needed for SFrame FREs. */ + /* SFrame header. */ + sframe_header sfd_header; + /* SFrame function desc entries table. */ + uint32_t *sfd_funcdesc; + /* SFrame FRE table. */ + void *sfd_fres; + /* Number of bytes needed for SFrame FREs. */ + int sfd_fre_nbytes; + /* Reference to the internally malloc'd buffer, if any, for endian flipping + the original input buffer before decoding. */ + void *sfd_buf; }; struct sframe_encoder_ctx diff --git a/libsframe/sframe.c b/libsframe/sframe.c index b8fde2f04f8..e41c95b9944 100644 --- a/libsframe/sframe.c +++ b/libsframe/sframe.c @@ -548,6 +548,11 @@ sframe_decoder_free (sframe_decoder_ctx **decoder) free (dctx->sfd_fres); dctx->sfd_fres = NULL; } + if (dctx->sfd_buf != NULL) + { + free (dctx->sfd_buf); + dctx->sfd_buf = NULL; + } free (*decoder); *decoder = NULL; @@ -824,6 +829,10 @@ sframe_decode (const char *sf_buf, size_t sf_size, int *errp) return sframe_ret_set_errno (errp, SFRAME_ERR_BUF_INVAL); } frame_buf = tempbuf; + /* This buffer is malloc'd when endian flipping the contents of the input + buffer are needed. Keep a reference to it so it can be free'd up + later in sframe_decoder_free (). */ + dctx->sfd_buf = tempbuf; } else frame_buf = (char *)sf_buf; -- 2.37.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] libsframe: testsuite: fix memory leaks in testcases 2022-12-22 22:54 [PATCH 0/2] libsframe: fix some memory leaks Indu Bhagat 2022-12-22 22:54 ` [PATCH 1/2] libsframe: fix a memory leak in sframe_decode Indu Bhagat @ 2022-12-22 22:54 ` Indu Bhagat 2022-12-23 9:55 ` [PATCH 0/2] libsframe: fix some memory leaks Nick Clifton 2022-12-23 14:22 ` libsframe builder (Was: [PATCH 0/2] libsframe: fix some memory leaks) Mark Wielaard 3 siblings, 0 replies; 11+ messages in thread From: Indu Bhagat @ 2022-12-22 22:54 UTC (permalink / raw) To: binutils; +Cc: Indu Bhagat ChangeLog: * libsframe/testsuite/libsframe.decode/be-flipping.c: Free SFrame buffer. * libsframe/testsuite/libsframe.decode/frecnt-1.c: Likewise. * libsframe/testsuite/libsframe.decode/frecnt-2.c: Likewise. --- libsframe/testsuite/libsframe.decode/be-flipping.c | 3 +++ libsframe/testsuite/libsframe.decode/frecnt-1.c | 3 +++ libsframe/testsuite/libsframe.decode/frecnt-2.c | 3 +++ 3 files changed, 9 insertions(+) diff --git a/libsframe/testsuite/libsframe.decode/be-flipping.c b/libsframe/testsuite/libsframe.decode/be-flipping.c index d915f9bb278..378537e4e10 100644 --- a/libsframe/testsuite/libsframe.decode/be-flipping.c +++ b/libsframe/testsuite/libsframe.decode/be-flipping.c @@ -104,6 +104,9 @@ main (void) err = sframe_decoder_get_funcdesc (dctx, 0, &nfres, &fsize, &fstart, &finfo); TEST ("be-flipping: Decoder get FDE", err == 0); TEST ("be-flipping: Decoder FRE count", nfres == 5); + + free (sf_buf); + sf_buf = NULL; sframe_decoder_free (&dctx); return 0; diff --git a/libsframe/testsuite/libsframe.decode/frecnt-1.c b/libsframe/testsuite/libsframe.decode/frecnt-1.c index 49861ed4f5c..bffa2ef37fb 100644 --- a/libsframe/testsuite/libsframe.decode/frecnt-1.c +++ b/libsframe/testsuite/libsframe.decode/frecnt-1.c @@ -89,6 +89,9 @@ main (void) TEST ("frecnt-1: Decoder get FDE", err == 0); TEST ("frecnt-1: Decoder FRE count", nfres == 4); + free (sf_buf); + sf_buf = NULL; + sframe_decoder_free (&dctx); return 0; diff --git a/libsframe/testsuite/libsframe.decode/frecnt-2.c b/libsframe/testsuite/libsframe.decode/frecnt-2.c index 7c140d88c3a..cbb72fe5f08 100644 --- a/libsframe/testsuite/libsframe.decode/frecnt-2.c +++ b/libsframe/testsuite/libsframe.decode/frecnt-2.c @@ -94,6 +94,9 @@ main (void) TEST ("frecnt-2: Decode get FRE", nfres == 4); } + free (sf_buf); + sf_buf = NULL; + sframe_decoder_free (&dctx); return 0; -- 2.37.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] libsframe: fix some memory leaks 2022-12-22 22:54 [PATCH 0/2] libsframe: fix some memory leaks Indu Bhagat 2022-12-22 22:54 ` [PATCH 1/2] libsframe: fix a memory leak in sframe_decode Indu Bhagat 2022-12-22 22:54 ` [PATCH 2/2] libsframe: testsuite: fix memory leaks in testcases Indu Bhagat @ 2022-12-23 9:55 ` Nick Clifton 2022-12-23 15:35 ` Indu Bhagat 2022-12-23 14:22 ` libsframe builder (Was: [PATCH 0/2] libsframe: fix some memory leaks) Mark Wielaard 3 siblings, 1 reply; 11+ messages in thread From: Nick Clifton @ 2022-12-23 9:55 UTC (permalink / raw) To: Indu Bhagat, binutils Hi Indu, > This patch set fixes some memory leaks in the libsframe and it's testsuite. Patch series approved. Please apply. Would you be interested in becoming an official maintainer for the libsframe code ? That way you can approve your own patches - as long as they are specific to libsframe of course. Cheers Nick ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] libsframe: fix some memory leaks 2022-12-23 9:55 ` [PATCH 0/2] libsframe: fix some memory leaks Nick Clifton @ 2022-12-23 15:35 ` Indu Bhagat 2022-12-23 15:39 ` Nick Clifton 0 siblings, 1 reply; 11+ messages in thread From: Indu Bhagat @ 2022-12-23 15:35 UTC (permalink / raw) To: Nick Clifton, binutils On 12/23/22 01:55, Nick Clifton wrote: > Hi Indu, > >> This patch set fixes some memory leaks in the libsframe and it's >> testsuite. > > Patch series approved. Please apply. > > Would you be interested in becoming an official maintainer for the > libsframe > code ? That way you can approve your own patches - as long as they are > specific > to libsframe of course. > Yes, I am happy to do that. Thanks for reviewing. > Cheers > Nick > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] libsframe: fix some memory leaks 2022-12-23 15:35 ` Indu Bhagat @ 2022-12-23 15:39 ` Nick Clifton 0 siblings, 0 replies; 11+ messages in thread From: Nick Clifton @ 2022-12-23 15:39 UTC (permalink / raw) To: Indu Bhagat, binutils Hi Indu, >> Would you be interested in becoming an official maintainer for the libsframe >> code ? That way you can approve your own patches - as long as they are specific >> to libsframe of course. > > Yes, I am happy to do that. Brilliant - please create a patch to add yourself to the binutils/MAINTAINERS file as the maintainer for libsframe. And welcome to the wonderful world of maintainership! Cheers Nick ^ permalink raw reply [flat|nested] 11+ messages in thread
* libsframe builder (Was: [PATCH 0/2] libsframe: fix some memory leaks) 2022-12-22 22:54 [PATCH 0/2] libsframe: fix some memory leaks Indu Bhagat ` (2 preceding siblings ...) 2022-12-23 9:55 ` [PATCH 0/2] libsframe: fix some memory leaks Nick Clifton @ 2022-12-23 14:22 ` Mark Wielaard 2022-12-23 14:41 ` Frank Ch. Eigler 2022-12-23 15:24 ` Indu Bhagat 3 siblings, 2 replies; 11+ messages in thread From: Mark Wielaard @ 2022-12-23 14:22 UTC (permalink / raw) To: Indu Bhagat, binutils, buildbot [-- Attachment #1: Type: text/plain, Size: 517 bytes --] Hi Indu, On Thu, 2022-12-22 at 14:54 -0800, Indu Bhagat via Binutils wrote: > This patch set fixes some memory leaks in the libsframe and it's > testsuite. I see you are an enthusiastic user of the binutils-try buildbot. But changes under libsframe/ don't trigger new builds and the libsframe testsuite isn't actually ran. The following tweak to the buildbot master.cfg should fix that. Please let buildbot@sourceware.org know if you need more tweaks to better test the new code. Cheers, Mark [-- Attachment #2: 0001-Add-libsframe-to-binutils_files.patch --] [-- Type: text/x-patch, Size: 3031 bytes --] From b58f0c5e82062f9c1160b72bba62a425c993fc79 Mon Sep 17 00:00:00 2001 From: Mark Wielaard <mark@klomp.org> Date: Fri, 23 Dec 2022 14:44:53 +0100 Subject: [PATCH] Add libsframe to binutils_files Because gdb and binutils share a git repo we only trigger builds if they are part of gdb or binutils. libsframe is a new subdir part of binutils. Also add an explicit check-libsframe to binutils_step_checks. --- builder/master.cfg | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/builder/master.cfg b/builder/master.cfg index 1e17aa3..8a66ae8 100644 --- a/builder/master.cfg +++ b/builder/master.cfg @@ -671,6 +671,7 @@ c['schedulers'].append(gccrust_bootstrap_scheduler) binutils_files = ["bfd/", "binutils/", "gas/", "ld/", "libctf/", + "libsframe/", "gold/", "elfcpp/", "include/", "libiberty/", "opcodes/", "configure", "Makefile.in"] @@ -2106,14 +2107,17 @@ binutils_step_check = steps.Test( workdir='binutils-build', command=['make', util.Interpolate('-j%(prop:ncpus)s'), - 'check-ld', 'check-gas', 'check-binutils'], + 'check-ld', 'check-gas', 'check-binutils', + 'check-libsframe'], name='make check', logfiles={ "ld.sum": "ld/ld.sum", "ld.log": "ld/ld.log", "gas.sum": "gas/testsuite/gas.sum", "gas.log": "gas/testsuite/gas.log", "binutils.sum": "binutils/binutils.sum", - "binutils.log": "binutils/binutils.log" }, + "binutils.log": "binutils/binutils.log", + "libsframe.sum": "libsframe/libsframe.sum", + "libsframe.log": "libsframe/libsframe.log" }, haltOnFailure=False, flunkOnFailure=True) binutils_steps_bunsen = bunsen_logfile_upload_cpio_steps( ["*.sum", "*.log"], @@ -2123,7 +2127,8 @@ binutils_step_check_libctf = steps.Test( workdir='binutils-build', command=['make', util.Interpolate('-j%(prop:ncpus)s'), - 'check-ld', 'check-gas', 'check-binutils', 'check-libctf'], + 'check-ld', 'check-gas', 'check-binutils', 'check-libctf', + 'check-libsframe'], name='make check', logfiles={ "ld.sum": "ld/ld.sum", "ld.log": "ld/ld.log", @@ -2131,6 +2136,8 @@ binutils_step_check_libctf = steps.Test( "gas.log": "gas/testsuite/gas.log", "binutils.sum": "binutils/binutils.sum", "binutils.log": "binutils/binutils.log", + "libsframe.sum": "libsframe/libsframe.sum", + "libsframe.log": "libsframe/libsframe.log", "libctf.sum": "libctf/libctf.sum", "libctf.log": "libctf/libctf.log" }, haltOnFailure=False, flunkOnFailure=True) -- 2.18.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: libsframe builder (Was: [PATCH 0/2] libsframe: fix some memory leaks) 2022-12-23 14:22 ` libsframe builder (Was: [PATCH 0/2] libsframe: fix some memory leaks) Mark Wielaard @ 2022-12-23 14:41 ` Frank Ch. Eigler 2022-12-23 16:06 ` Mark Wielaard 2022-12-23 15:24 ` Indu Bhagat 1 sibling, 1 reply; 11+ messages in thread From: Frank Ch. Eigler @ 2022-12-23 14:41 UTC (permalink / raw) To: Mark Wielaard; +Cc: Indu Bhagat, binutils, buildbot Hi - > command=['make', > util.Interpolate('-j%(prop:ncpus)s'), > - 'check-ld', 'check-gas', 'check-binutils'], > + 'check-ld', 'check-gas', 'check-binutils', > + 'check-libsframe'], > name='make check', > logfiles={ "ld.sum": "ld/ld.sum", > "ld.log": "ld/ld.log", > "gas.sum": "gas/testsuite/gas.sum", > "gas.log": "gas/testsuite/gas.log", > "binutils.sum": "binutils/binutils.sum", > - "binutils.log": "binutils/binutils.log" }, > + "binutils.log": "binutils/binutils.log", > + "libsframe.sum": "libsframe/libsframe.sum", > + "libsframe.log": "libsframe/libsframe.log" }, > [...] (For the record, all of those .log/.sum files, and more, are pulled into the bunsen database automagically, so listing them all here for storage and presentation in the buildbot system is not really necessary.) - FChE ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: libsframe builder (Was: [PATCH 0/2] libsframe: fix some memory leaks) 2022-12-23 14:41 ` Frank Ch. Eigler @ 2022-12-23 16:06 ` Mark Wielaard 0 siblings, 0 replies; 11+ messages in thread From: Mark Wielaard @ 2022-12-23 16:06 UTC (permalink / raw) To: Frank Ch. Eigler; +Cc: Indu Bhagat, binutils, buildbot Hi Frank, On Fri, 2022-12-23 at 09:41 -0500, Frank Ch. Eigler wrote: > (For the record, all of those .log/.sum files, and more, are pulled > into the bunsen database automagically, so listing them all here for > storage and presentation in the buildbot system is not really > necessary.) You are of course right. But the bunsen link is not easily found. We really need to tweak the web-interface and emails sent to prominently feature the bunsen results link. They can be found in the stdio log output of the "upload to bunsen" step as https://builder.sourceware.org/testrun/<hashid> Cheers, Mark ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: libsframe builder (Was: [PATCH 0/2] libsframe: fix some memory leaks) 2022-12-23 14:22 ` libsframe builder (Was: [PATCH 0/2] libsframe: fix some memory leaks) Mark Wielaard 2022-12-23 14:41 ` Frank Ch. Eigler @ 2022-12-23 15:24 ` Indu Bhagat 2022-12-23 16:13 ` Mark Wielaard 1 sibling, 1 reply; 11+ messages in thread From: Indu Bhagat @ 2022-12-23 15:24 UTC (permalink / raw) To: Mark Wielaard, binutils, buildbot On 12/23/22 06:22, Mark Wielaard wrote: > Hi Indu, > > On Thu, 2022-12-22 at 14:54 -0800, Indu Bhagat via Binutils wrote: >> This patch set fixes some memory leaks in the libsframe and it's >> testsuite. > > I see you are an enthusiastic user of the binutils-try buildbot. But > changes under libsframe/ don't trigger new builds and the libsframe > testsuite isn't actually ran. > heh, binutils-try buildbot's enthusiastic user I am, yes. I did wonder if there are enough resources for users to be doing this so frequently (with almost every patch set before commit), I hope its not a problem ? Right, I noticed that libsframe/ testsuite is not being run, but binutils-try buildbot has still been useful to catch regressions to gas/ld by my code ... Asking about adding libsframe/ testsuite was on my list, thanks for doing that. Best Regards, Indu > The following tweak to the buildbot master.cfg should fix that. > > Please let buildbot@sourceware.org know if you need more tweaks to > better test the new code. > > Cheers, > > Mark ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: libsframe builder (Was: [PATCH 0/2] libsframe: fix some memory leaks) 2022-12-23 15:24 ` Indu Bhagat @ 2022-12-23 16:13 ` Mark Wielaard 0 siblings, 0 replies; 11+ messages in thread From: Mark Wielaard @ 2022-12-23 16:13 UTC (permalink / raw) To: Indu Bhagat, binutils, buildbot Hi Indu, On Fri, 2022-12-23 at 07:24 -0800, Indu Bhagat wrote: > heh, binutils-try buildbot's enthusiastic user I am, yes. I did > wonder > if there are enough resources for users to be doing this so frequently > (with almost every patch set before commit), I hope its not a problem ? It isn't a problem, a binutils build/check takes just a couple of minutes and at least for the x86_64 and i386 builders there are more than one worker. The most resource constrained one is the s390x worker, but it is still able to catch up. Worst case you have to wait an hour till your job is finally scheduled. You can always check whether there are a lot of pending jobs at https://builder.sourceware.org/buildbot/#/pendingbuildrequests (There are often a couple which take multiple hours but are only ran a few times a day) Cheers, Mark ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-12-23 16:13 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-22 22:54 [PATCH 0/2] libsframe: fix some memory leaks Indu Bhagat 2022-12-22 22:54 ` [PATCH 1/2] libsframe: fix a memory leak in sframe_decode Indu Bhagat 2022-12-22 22:54 ` [PATCH 2/2] libsframe: testsuite: fix memory leaks in testcases Indu Bhagat 2022-12-23 9:55 ` [PATCH 0/2] libsframe: fix some memory leaks Nick Clifton 2022-12-23 15:35 ` Indu Bhagat 2022-12-23 15:39 ` Nick Clifton 2022-12-23 14:22 ` libsframe builder (Was: [PATCH 0/2] libsframe: fix some memory leaks) Mark Wielaard 2022-12-23 14:41 ` Frank Ch. Eigler 2022-12-23 16:06 ` Mark Wielaard 2022-12-23 15:24 ` Indu Bhagat 2022-12-23 16:13 ` Mark Wielaard
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).