Re: [PATCHv2] strip: keep .ctf section in stripped file
Hi, On Tue, 2022-12-20 at 22:35 +0100, Mark Wielaard wrote: > On Wed, Jun 01, 2022 at 10:55:27AM -0500, Guillermo E. Martinez via > Elfutils-devel wrote: > > This is the second version patch to avoid remove the CTF section in > > stripped files. Changes from v1: > > > > - Add description in tests/run-strip-remove-keep-ctf.sh > > mentioning how to regenerate test input file (testfile-ctf) > > > > Please let me know your thoughts. > > > > [...] > > > > CTF debug format was designed to be present in stripped files, so > > this section should not be removed, so a new --remove-ctf option > > is added to indicate explicitly that .ctf section will be stripped > > out from binary file. > > Sorry, I see I never reviewed this v2 variant. I know we tried to > coordinate with binutils so eu-strip and binutils strip would do the > same thing. And that Jose had an idea for a new section flag to > automatically detect what section should/shouldn't be stripped (into a > separate .debug file). What was the conclusion of that? Any update on this? How should .ctf sections be dealt with by strip like tools? Thanks, Mark
Re: [PATCHv2] strip: keep .ctf section in stripped file
> Hi, > > On Tue, 2022-12-20 at 22:35 +0100, Mark Wielaard wrote: >> On Wed, Jun 01, 2022 at 10:55:27AM -0500, Guillermo E. Martinez via >> Elfutils-devel wrote: >> > This is the second version patch to avoid remove the CTF section in >> > stripped files. Changes from v1: >> > >> > - Add description in tests/run-strip-remove-keep-ctf.sh >> > mentioning how to regenerate test input file (testfile-ctf) >> > >> > Please let me know your thoughts. >> > >> > [...] >> > >> > CTF debug format was designed to be present in stripped files, so >> > this section should not be removed, so a new --remove-ctf option >> > is added to indicate explicitly that .ctf section will be stripped >> > out from binary file. >> >> Sorry, I see I never reviewed this v2 variant. I know we tried to >> coordinate with binutils so eu-strip and binutils strip would do the >> same thing. And that Jose had an idea for a new section flag to >> automatically detect what section should/shouldn't be stripped (into a >> separate .debug file). What was the conclusion of that? > > Any update on this? > How should .ctf sections be dealt with by strip like tools? I think the conclusion was that having a NOSTRIP flag in the object file is far from trivial, because what "strip" means for several people and tools is not clear. So I guess we are stuck with the old keep-a-list method. Or we may add NOSTRIP meaning "_never ever_ strip this section, period, no matter what."
Re: [PATCHv2] strip: keep .ctf section in stripped file
On Wed, Feb 22, 2023 at 05:42:45PM +0100, Mark Wielaard wrote: > Hi, > Hi Mark, > On Tue, 2022-12-20 at 22:35 +0100, Mark Wielaard wrote: > > On Wed, Jun 01, 2022 at 10:55:27AM -0500, Guillermo E. Martinez via > > Elfutils-devel wrote: > > > This is the second version patch to avoid remove the CTF section in > > > stripped files. Changes from v1: > > > > > > - Add description in tests/run-strip-remove-keep-ctf.sh > > > mentioning how to regenerate test input file (testfile-ctf) > > > > > > Please let me know your thoughts. > > > > > > [...] > > > > > > CTF debug format was designed to be present in stripped files, so > > > this section should not be removed, so a new --remove-ctf option > > > is added to indicate explicitly that .ctf section will be stripped > > > out from binary file. > > > > Sorry, I see I never reviewed this v2 variant. I know we tried to > > coordinate with binutils so eu-strip and binutils strip would do the > > same thing. And that Jose had an idea for a new section flag to > > automatically detect what section should/shouldn't be stripped (into a > > separate .debug file). What was the conclusion of that? > > Any update on this? > How should .ctf sections be dealt with by strip like tools? > Sorry for late reply, The conclusion was basically not use section flags to identify which section should be stripped out or not, so, it requires other mechanisms for explicitly specifying which sections should be removed, as eu-strip does using arguments. https://sourceware.org/bugzilla/show_bug.cgi?id=29737 Worth it mention here, that my last test shows that .ctf section is not stripped by ue-strip because it doesn't have a "debug" section name. Thanks, guillermo
[Bug libdw/29434] Memory leak in `dwarf_getscopes`
https://sourceware.org/bugzilla/show_bug.cgi?id=29434 --- Comment #17 from Mark Wielaard --- So the code here changed a little with this patch: commit b7c7d8776ed46e2237d18fb15c6b72e83cfa259b Author: Mark Wielaard Date: Sun Jan 22 00:31:57 2023 +0100 libdw: Search for abstract origin in the correct CU With gcc -flto the abstract origin of an inlined subroutine could be in a different CU. dwarf_getscopes might return an empty scope if it cannot find the abstract origin scope. So make sure to search in the We also tried to add the origin match in pc_record directly in the current inlined scope. This always failed, causing to do a needless traversal, followed by the full CU scan in dwarf_getscopes. Just always stop the pc_record search and then do the CU origin_match in dwarf_getscopes. Signed-off-by: Mark Wielaard Which makes the condition of the first check slightly different: - if (result == 0 && a.scopes != NULL) -result = __libdw_visit_scopes (0, &cu, NULL, &origin_match, NULL, &a); + if (result >= 0 && a.scopes != NULL && a.inlined > 0) +{ + /* We like the find the inline function's abstract definition + scope, but that might be in a different CU. */ + cu.die = CUDIE (a.inlined_origin.cu); + result = __libdw_visit_scopes (0, &cu, NULL, &origin_match, NULL, &a); +} So with that I think my proposed patch in comment #3 might work. But I have not been able to replicate the issue. So cannot easily check. -- You are receiving this mail because: You are on the CC list for the bug.
[PATCH] libdw: Fix dwarf_getscopes memory leak on error
When there is an error in dwarf_getscopes after the initial scopes have been allocated, e.g. when looking for the inlined scopes, then the scopes would leak. Fix this by explicitly free the scopes on error. https://sourceware.org/bugzilla/show_bug.cgi?id=29434 Signed-off-by: Mark Wielaard --- libdw/ChangeLog | 5 + libdw/dwarf_getscopes.c | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/libdw/ChangeLog b/libdw/ChangeLog index e0cd8f21..fefc53af 100644 --- a/libdw/ChangeLog +++ b/libdw/ChangeLog @@ -1,3 +1,8 @@ +2023-02-22 Mark Wielaard + + * dwarf_getscopes.c (origin_match): Don't free a->scopes. + (dwarf_getscopes): Free a->scopes on error. + 2023-02-14 Mark Wielaard * dwarf_getlocation.c (__libdw_intern_expression): Correct check diff --git a/libdw/dwarf_getscopes.c b/libdw/dwarf_getscopes.c index 833f1ac5..ce073b13 100644 --- a/libdw/dwarf_getscopes.c +++ b/libdw/dwarf_getscopes.c @@ -101,7 +101,7 @@ origin_match (unsigned int depth, struct Dwarf_Die_Chain *die, void *arg) Dwarf_Die *scopes = realloc (a->scopes, nscopes * sizeof scopes[0]); if (scopes == NULL) { - free (a->scopes); + /* a->scopes will be freed by dwarf_getscopes on error. */ __libdw_seterrno (DWARF_E_NOMEM); return -1; } @@ -200,6 +200,8 @@ dwarf_getscopes (Dwarf_Die *cudie, Dwarf_Addr pc, Dwarf_Die **scopes) if (result > 0) *scopes = a.scopes; + else if (result < 0) +free (a.scopes); return result; } -- 2.31.1
[Bug libdw/29434] Memory leak in `dwarf_getscopes`
https://sourceware.org/bugzilla/show_bug.cgi?id=29434 --- Comment #18 from Mark Wielaard --- I could replicate a leak when there was an error looking up the inlined scopes, so I submitted the patch to fix that: https://inbox.sourceware.org/elfutils-devel/2023023901.1089881-1-m...@klomp.org/T/#u Hopefully that also fixes the original issue (which I have been unable to replicate). -- You are receiving this mail because: You are on the CC list for the bug.
Re: [PATCH RFC] backends: Add RISC-V object attribute printing
Hi, On Fri, Nov 25, 2022 at 05:29:19PM +0100, Martin Liška wrote: > On 10/13/22 16:53, Mark Wielaard wrote: > > On Wed, 2022-08-10 at 11:27 +0200, Andreas Schwab via Elfutils-devel > > wrote: > >> This does not work yet. The RISC-V attribute tags use the same > >> convention as the GNU attributes: odd numbered tags take a string > >> value, > >> even numbered ones an integer value, but print_attributes assumes the > >> ARM numbering scheme by default for non-GNU attributes. > > > > Yeah, I see this comment in print_attributes: > > > > /* GNU style tags have either a uleb128 value, > > when lowest bit is not set, or a string > > when the lowest bit is set. > > "compatibility" (32) is special. It has > > both a string and a uleb128 value. For > > non-gnu we assume 6 till 31 only take ints. > > XXX see arm backend, do we need a separate > > hook? */ > > > > Maybe we need a flag in the backend to tell whether attributes follow > > the "gnu_vendor" convention? So that could be checked at: > > > > bool gnu_vendor = (q - name == sizeof "gnu" > > && !memcmp (name, "gnu", sizeof "gnu")); > > gnu_vendor |= ebl->has_gnu_attributes; > > > > Or something similar? > > Andreas: Can you please take a look at this? Has anybody had time to look at this? Thanks, Mark
Re: [PATCHv2] strip: keep .ctf section in stripped file
Hi, On Wed, Feb 22, 2023 at 11:12:07AM -0600, Guillermo E. Martinez wrote: > The conclusion was basically not use section flags to identify which > section should be stripped out or not, so, it requires other mechanisms > for explicitly specifying which sections should be removed, as eu-strip > does using arguments. > > https://sourceware.org/bugzilla/show_bug.cgi?id=29737 OK, thanks. > Worth it mention here, that my last test shows that .ctf section is not > stripped by ue-strip because it doesn't have a "debug" section name. Are you sure that is what happens? It might depend on whether or not you give eu-strip -g or not. With -g only debug symbols and .debug sections are removed, but it keeps any other unused/unallocated symbol/section. Without -g I would expect eu-strip to remove the .ctf section (unless it is an allocated section or referenced from an allocated section/symbol table). Cheers, Mark