tools/perf/Makefile.config | 6 ++++++ tools/perf/util/disasm.c | 11 +++++++---- tools/perf/util/dwarf-aux.h | 1 + tools/perf/util/probe-finder.c | 5 +++++ 4 files changed, 19 insertions(+), 4 deletions(-)
I was looking into some lsan regressions and a latent issue with libdw, creating these fixes. A thought, we should probably simplify the libdw logic but rather than do it here I'll do it as a separate series on top of these. The issues I see are: 1) dwfl_thread_getframes is used to test for the presence of libdw-dwarf-unwind. The blame date on this function is 2013-05-30. As the function is 10 years old I think having libdw implies having dwfl_thread_getframes and so we can just merge the two pieces of logic instead of having different feature tests and ifdefs. 2) similarly, dwarf_getlocations has a blame date of 2013-08-23 so let's just make libdw tests test for this and make having libdw imply dwarf_getlocations support. 3) similarly, dwarf_getcfi has a blame date of 2009-06-24 so let's just make libdw tests test for this and make having libdw imply dwarf_getcfi support. 4) in Makefie.config feature-dwarf is a synonym for libdw support. I think using the name libdw is more intention revealing as dwarf can mean multiple things. Let's change HAVE_DWARF_SUPPORT to HAVE_LIBDW_SUPPORT and all similar dwarf vs libdw names. 5) We have "#if _ELFUTILS_PREREQ(0, 142)" testing for elfutils version 0.142. Elfutils 0.142 was released around 2009-06-13 (via git blame on the NEWS file). Let's remove the #if and ensure elfutils feature tests for at least 0.142. If someone were using an incredibly old version then they'd lose some elfutils support, but given the 15 year old age of the library I find it unlikely anyone is doing this. They can also just move to a newer version. From the mailing list I notice also overlap with the last patch and this series: https://lore.kernel.org/lkml/20240919013513.118527-1-yangjihong@bytedance.com/ Simplifying the libdw support will address some of those issues too. Ian Rogers (3): perf disasm: Fix capstone memory leak perf probe: Fix libdw memory leak perf build: Fix !HAVE_DWARF_GETLOCATIONS_SUPPORT tools/perf/Makefile.config | 6 ++++++ tools/perf/util/disasm.c | 11 +++++++---- tools/perf/util/dwarf-aux.h | 1 + tools/perf/util/probe-finder.c | 5 +++++ 4 files changed, 19 insertions(+), 4 deletions(-) -- 2.46.0.792.g87dc391469-goog
On Mon, Sep 23, 2024 at 05:37:17PM -0700, Ian Rogers wrote: > I was looking into some lsan regressions and a latent issue with > libdw, creating these fixes. > > A thought, we should probably simplify the libdw logic but rather than > do it here I'll do it as a separate series on top of these. The issues > I see are: > > 1) dwfl_thread_getframes is used to test for the presence of > libdw-dwarf-unwind. The blame date on this function is > 2013-05-30. As the function is 10 years old I think having libdw > implies having dwfl_thread_getframes and so we can just merge the > two pieces of logic instead of having different feature tests and > ifdefs. > > 2) similarly, dwarf_getlocations has a blame date of 2013-08-23 so > let's just make libdw tests test for this and make having libdw > imply dwarf_getlocations support. > > 3) similarly, dwarf_getcfi has a blame date of 2009-06-24 so let's > just make libdw tests test for this and make having libdw imply > dwarf_getcfi support. > > 4) in Makefie.config feature-dwarf is a synonym for libdw support. I > think using the name libdw is more intention revealing as dwarf can > mean multiple things. Let's change HAVE_DWARF_SUPPORT to > HAVE_LIBDW_SUPPORT and all similar dwarf vs libdw names. > > 5) We have "#if _ELFUTILS_PREREQ(0, 142)" testing for elfutils version > 0.142. Elfutils 0.142 was released around 2009-06-13 (via git blame > on the NEWS file). Let's remove the #if and ensure elfutils feature > tests for at least 0.142. If someone were using an incredibly old > version then they'd lose some elfutils support, but given the 15 > year old age of the library I find it unlikely anyone is doing > this. They can also just move to a newer version. Looking at the map file in libdw, the latest addition was 0.158 for dwfl_thread_getframes(). Probably we can add the version check to the feature test to make sure if it has all the required APIs. https://sourceware.org/git/?p=elfutils.git;a=blob;f=libdw/libdw.map;h=552588a94c0c1a1f2fd5b973553c784026e6de14;hb=HEAD#l274 > > From the mailing list I notice also overlap with the last patch and > this series: > https://lore.kernel.org/lkml/20240919013513.118527-1-yangjihong@bytedance.com/ > Simplifying the libdw support will address some of those issues too. Yeah I noticed that too and feel like it should go to perf-tools tree. Probably it doesn't clash with this so I think it's ok to have this in perf-tools-next. Thanks, Namhyung > > Ian Rogers (3): > perf disasm: Fix capstone memory leak > perf probe: Fix libdw memory leak > perf build: Fix !HAVE_DWARF_GETLOCATIONS_SUPPORT > > tools/perf/Makefile.config | 6 ++++++ > tools/perf/util/disasm.c | 11 +++++++---- > tools/perf/util/dwarf-aux.h | 1 + > tools/perf/util/probe-finder.c | 5 +++++ > 4 files changed, 19 insertions(+), 4 deletions(-) > > -- > 2.46.0.792.g87dc391469-goog >
On Tue, Sep 24, 2024 at 11:25 AM Namhyung Kim <namhyung@kernel.org> wrote: > > On Mon, Sep 23, 2024 at 05:37:17PM -0700, Ian Rogers wrote: > > I was looking into some lsan regressions and a latent issue with > > libdw, creating these fixes. > > > > A thought, we should probably simplify the libdw logic but rather than > > do it here I'll do it as a separate series on top of these. The issues > > I see are: > > > > 1) dwfl_thread_getframes is used to test for the presence of > > libdw-dwarf-unwind. The blame date on this function is > > 2013-05-30. As the function is 10 years old I think having libdw > > implies having dwfl_thread_getframes and so we can just merge the > > two pieces of logic instead of having different feature tests and > > ifdefs. > > > > 2) similarly, dwarf_getlocations has a blame date of 2013-08-23 so > > let's just make libdw tests test for this and make having libdw > > imply dwarf_getlocations support. > > > > 3) similarly, dwarf_getcfi has a blame date of 2009-06-24 so let's > > just make libdw tests test for this and make having libdw imply > > dwarf_getcfi support. > > > > 4) in Makefie.config feature-dwarf is a synonym for libdw support. I > > think using the name libdw is more intention revealing as dwarf can > > mean multiple things. Let's change HAVE_DWARF_SUPPORT to > > HAVE_LIBDW_SUPPORT and all similar dwarf vs libdw names. > > > > 5) We have "#if _ELFUTILS_PREREQ(0, 142)" testing for elfutils version > > 0.142. Elfutils 0.142 was released around 2009-06-13 (via git blame > > on the NEWS file). Let's remove the #if and ensure elfutils feature > > tests for at least 0.142. If someone were using an incredibly old > > version then they'd lose some elfutils support, but given the 15 > > year old age of the library I find it unlikely anyone is doing > > this. They can also just move to a newer version. > > Looking at the map file in libdw, the latest addition was 0.158 for > dwfl_thread_getframes(). Probably we can add the version check to the > feature test to make sure if it has all the required APIs. > > https://sourceware.org/git/?p=elfutils.git;a=blob;f=libdw/libdw.map;h=552588a94c0c1a1f2fd5b973553c784026e6de14;hb=HEAD#l274 > > > > > From the mailing list I notice also overlap with the last patch and > > this series: > > https://lore.kernel.org/lkml/20240919013513.118527-1-yangjihong@bytedance.com/ > > Simplifying the libdw support will address some of those issues too. > > Yeah I noticed that too and feel like it should go to perf-tools tree. > Probably it doesn't clash with this so I think it's ok to have this in > perf-tools-next. I think the comments wrt libdw are covered in the series cleaning up libdw: https://lore.kernel.org/lkml/20240924160418.1391100-1-irogers@google.com/ so these fixes should be good to land? Thanks, Ian
On Mon, Sep 30, 2024 at 10:11:34PM -0700, Ian Rogers wrote: > On Tue, Sep 24, 2024 at 11:25 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > On Mon, Sep 23, 2024 at 05:37:17PM -0700, Ian Rogers wrote: > > > I was looking into some lsan regressions and a latent issue with > > > libdw, creating these fixes. > > > > > > A thought, we should probably simplify the libdw logic but rather than > > > do it here I'll do it as a separate series on top of these. The issues > > > I see are: > > > > > > 1) dwfl_thread_getframes is used to test for the presence of > > > libdw-dwarf-unwind. The blame date on this function is > > > 2013-05-30. As the function is 10 years old I think having libdw > > > implies having dwfl_thread_getframes and so we can just merge the > > > two pieces of logic instead of having different feature tests and > > > ifdefs. > > > > > > 2) similarly, dwarf_getlocations has a blame date of 2013-08-23 so > > > let's just make libdw tests test for this and make having libdw > > > imply dwarf_getlocations support. > > > > > > 3) similarly, dwarf_getcfi has a blame date of 2009-06-24 so let's > > > just make libdw tests test for this and make having libdw imply > > > dwarf_getcfi support. > > > > > > 4) in Makefie.config feature-dwarf is a synonym for libdw support. I > > > think using the name libdw is more intention revealing as dwarf can > > > mean multiple things. Let's change HAVE_DWARF_SUPPORT to > > > HAVE_LIBDW_SUPPORT and all similar dwarf vs libdw names. > > > > > > 5) We have "#if _ELFUTILS_PREREQ(0, 142)" testing for elfutils version > > > 0.142. Elfutils 0.142 was released around 2009-06-13 (via git blame > > > on the NEWS file). Let's remove the #if and ensure elfutils feature > > > tests for at least 0.142. If someone were using an incredibly old > > > version then they'd lose some elfutils support, but given the 15 > > > year old age of the library I find it unlikely anyone is doing > > > this. They can also just move to a newer version. > > > > Looking at the map file in libdw, the latest addition was 0.158 for > > dwfl_thread_getframes(). Probably we can add the version check to the > > feature test to make sure if it has all the required APIs. > > > > https://sourceware.org/git/?p=elfutils.git;a=blob;f=libdw/libdw.map;h=552588a94c0c1a1f2fd5b973553c784026e6de14;hb=HEAD#l274 > > > > > > > > From the mailing list I notice also overlap with the last patch and > > > this series: > > > https://lore.kernel.org/lkml/20240919013513.118527-1-yangjihong@bytedance.com/ > > > Simplifying the libdw support will address some of those issues too. > > > > Yeah I noticed that too and feel like it should go to perf-tools tree. > > Probably it doesn't clash with this so I think it's ok to have this in > > perf-tools-next. > > I think the comments wrt libdw are covered in the series cleaning up libdw: > https://lore.kernel.org/lkml/20240924160418.1391100-1-irogers@google.com/ > so these fixes should be good to land? Sure, I'll just adjust the errno.h part as it's picked up by Arnaldo to the perf-tools already. I'll merge the branch later. Thanks, Namhyung
On 24/09/2024 1:37 am, Ian Rogers wrote: > I was looking into some lsan regressions and a latent issue with > libdw, creating these fixes. > > A thought, we should probably simplify the libdw logic but rather than > do it here I'll do it as a separate series on top of these. The issues > I see are: > > 1) dwfl_thread_getframes is used to test for the presence of > libdw-dwarf-unwind. The blame date on this function is > 2013-05-30. As the function is 10 years old I think having libdw > implies having dwfl_thread_getframes and so we can just merge the > two pieces of logic instead of having different feature tests and > ifdefs. > > 2) similarly, dwarf_getlocations has a blame date of 2013-08-23 so > let's just make libdw tests test for this and make having libdw > imply dwarf_getlocations support. > > 3) similarly, dwarf_getcfi has a blame date of 2009-06-24 so let's > just make libdw tests test for this and make having libdw imply > dwarf_getcfi support. > > 4) in Makefie.config feature-dwarf is a synonym for libdw support. I > think using the name libdw is more intention revealing as dwarf can > mean multiple things. Let's change HAVE_DWARF_SUPPORT to > HAVE_LIBDW_SUPPORT and all similar dwarf vs libdw names. > > 5) We have "#if _ELFUTILS_PREREQ(0, 142)" testing for elfutils version > 0.142. Elfutils 0.142 was released around 2009-06-13 (via git blame > on the NEWS file). Let's remove the #if and ensure elfutils feature > tests for at least 0.142. If someone were using an incredibly old > version then they'd lose some elfutils support, but given the 15 > year old age of the library I find it unlikely anyone is doing > this. They can also just move to a newer version. > > From the mailing list I notice also overlap with the last patch and > this series: > https://lore.kernel.org/lkml/20240919013513.118527-1-yangjihong@bytedance.com/ > Simplifying the libdw support will address some of those issues too. > > Ian Rogers (3): > perf disasm: Fix capstone memory leak > perf probe: Fix libdw memory leak > perf build: Fix !HAVE_DWARF_GETLOCATIONS_SUPPORT > > tools/perf/Makefile.config | 6 ++++++ > tools/perf/util/disasm.c | 11 +++++++---- > tools/perf/util/dwarf-aux.h | 1 + > tools/perf/util/probe-finder.c | 5 +++++ > 4 files changed, 19 insertions(+), 4 deletions(-) > Reviewed-by: James Clark <james.clark@linaro.org>
© 2016 - 2024 Red Hat, Inc.