Skip to content

Failing build with -Dasan=ON and memory leak in minimal build #14055

Closed

Description

@AxelKrypton

Check duplicate issues.

  • Checked for duplicates

Description

As requested by @Axel-Naumann in #13130 this is a follow up of my note there.

Basically I recently built and linked to ROOT in an external project (in a Docker image) and I got all unit tests failing because of a memory leak detected by the address sanitiser. As ROOT was set up using CMake via

cmake -DCMAKE_INSTALL_PREFIX=../root_install ../root_src -Dxrootd=OFF

the backtrace reported in my previous note was pretty small. Hence, I tried to compile ROOT as

cmake -DCMAKE_INSTALL_PREFIX=../root_install ../root_src -Dxrootd=OFF -Dasan=ON

but this failed with this output.

Then I tried to make a minimal build via

cmake -DCMAKE_INSTALL_PREFIX=../root_install ../root_src -Dxrootd=OFF -Dasan=ON -Dminimal=ON

and this succeeded and I got my Docker image built. Inside it the tests of my application were still failing as expected, but this time the backtrace was way larger. In this file you find it all.

Reproducer

As the build fails because of a leak, I am pretty much expecting that the two issues are related and I did not invest more time in building up a MWE to reproduce the second issue. However, I would naively guess that linking to the build library in any executable with the address sanitiser on would make it.

ROOT version

In the Docker container in the minimal build ROOT binary folder, I get:

root@fc0d5912f35b:/SMASH/root_install/bin# ./root.exe -b -q
   ------------------------------------------------------------------
  | Welcome to ROOT 6.28/06                        https://root.cern |
  | (c) 1995-2023, The ROOT Team; conception: R. Brun, F. Rademakers |
  | Built for linuxx8664gcc on Aug 28 2023, 11:29:15                 |
  | From tags/v6-28-06@v6-28-06                                      |
  | With c++ (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0                   |
  | Try '.help'/'.?', '.demo', '.license', '.credits', '.quit'/'.q'  |
   ------------------------------------------------------------------

Installation method

Build from source with CMake version 3.22.1

Operating system

Ubuntu 22.04.3 LTS

Additional context

The same problem occurred with previous ROOT v6.28 versions, whereas switching back to v6.26.10 made the problem disappear.

Activity

  1. hahnjo commented on Nov 15, 2023

    @hahnjo
    Member

    see also #7968; can you try compiling with Clang?

  2. AxelKrypton commented on Nov 16, 2023

    @AxelKrypton
    Author

    Indeed, compiling with Clang after having setting up via (slightly different from above as I used a different docker image)

    cmake -DCMAKE_CXX_STANDARD=17 -DCMAKE_INSTALL_PREFIX=../root_install ../root_src -Dxrootd=OFF -Dasan=ON -Dbuiltin_afterimage=ON
    

    did succeed. Here the ROOT version from its executable:

    root@4bdbf4041f81:/SMASH/root_install/bin# ./root -b -q
       ------------------------------------------------------------------
      | Welcome to ROOT 6.28/06                        https://root.cern |
      | (c) 1995-2023, The ROOT Team; conception: R. Brun, F. Rademakers |
      | Built for linuxx8664gcc on Aug 28 2023, 11:29:15                 |
      | From tags/v6-28-06@v6-28-06                                      |
      | With Ubuntu clang version 13.0.1-2ubuntu2.2                      |
      | Try '.help'/'.?', '.demo', '.license', '.credits', '.quit'/'.q'  |
       ------------------------------------------------------------------
    

    However, the tests of my application still fail with a very similar backtrace. So in my opinion the issue still exists. Feel free to rename the issue title, if you think it is appropriate.

  3. hahnjo commented on Nov 16, 2023

    @hahnjo
    Member

    @AxelKrypton I'm sorry, I didn't read your initial report closely enough: what you see is a leak report that we turn off due to how ROOT uses the LLVM and Clang libraries:

    #define DETECT_LEAKS ":detect_leaks=0"

    I don't remember the details, but it's possible that the method we're using to suppress the leak sanitizer doesn't work with GCC and that's why switching to Clang solved the issue during build. For your own application, you need to pull in that same code, which should happen automatically if you use ROOT_EXECUTABLE. Can you check how your application is created?

    If it's not possible to use ROOT_EXECUTABLE, you can also manually link in ROOT::ROOTStaticSanitizerConfig. Alternatively you can try setting the environment variable ASAN_OPTIONS="detect_leaks=0". Hope this helps, let me know if you have further problems!

  4. AxelKrypton commented on Nov 16, 2023

    @AxelKrypton
    Author

    🤔 Just to understand, is it a known real leak or a false positive?

    I initially thought it was something we in any case could not fix on our side and I also suppressed the sanitiser leak as described here (not sure this is an official documentation, but it's something google brought to my attention). In particular I added

    cd root_install/etc/
    printf '\n#Full libCling for SMASH\nleak:libCling\n' >> lsan-root.supp
    

    together with

    # This will let sanitizer suppress leaks according to suppressions file list of patterns
    ENV LSAN_OPTIONS="suppressions=${ROOTSYS}/etc/lsan-root.supp:print_suppressions=0"

    to my Dockerfile.

    Doing so, all the unit tests pass, but I then monitored memory usage over a long run of our application and a linear growth was evident (plot left below). Nothing extreme, sure, but I thought it was worth reporting, especially since in v6.26.10 the problem does not occur (plot right below). 🤷‍♂️

  5. self-assigned this
    on Nov 16, 2023
  6. hahnjo commented on Nov 16, 2023

    @hahnjo
    Member

    🤔 Just to understand, is it a known real leak or a false positive?

    It is a "real" leak that in my understanding we are not urgently planning to fix. Basically it comes from Clang's internal data structures that are "lost" during shutdown. With clang used as a compiler, the libraries make quite some effort to clean up all data structures and free memory allocations to make sanitizers clean. I think this mode was never implemented in Cling, also because it's not really relevant: after process termination, the OS will clean up all virtual memory anyway.

    I initially thought it was something we in any case could not fix on our side and I also suppressed the sanitiser leak as described here (not sure this is an official documentation, but it's something google brought to my attention).

    Yes, the upstream equivalent to that GitLab instance would be https://github.com/root-project/root/tree/master/core/sanitizer

    Doing so, all the unit tests pass, but I then monitored memory usage over a long run of our application and a linear growth was evident (plot left below). Nothing extreme, sure, but I thought it was worth reporting, especially since in v6.26.10 the problem does not occur (plot right below). 🤷‍♂️

    Ok, good that all unit tests pass, so you're not using uninitialized memory 👍 for the memory leak, do you have a semi-standalone program that shows the growing memory usage on 6.28, but not on 6.26? Does it involve TChains, as reported in #13130?

  7. AxelKrypton commented on Nov 17, 2023

    @AxelKrypton
    Author

    Thanks for the insight.

    for the memory leak, do you have a semi-standalone program that shows the growing memory usage on 6.28, but not on 6.26?

    Not really.

    Does it involve TChains, as reported in #13130?

    It might indirectly. I never dealt with the part of our application interacting with ROOT but after a quick look, we mostly directly use TTree. However, we link to and use HepMC, which makes heavier use of ROOT.

    Now, I just realised that many (if not most) of our unit tests do not use ROOT, but only link to its libraries. In particular, I tried to add a trivial unit test file

    #include "vir/test.h"  // This include has to be first
    
    TEST(){ std::cout << "Hello world\n"; }

    and compiling as the other ones, i.e. linking it to

    $ ldd trivial_tests | grep -i 'root'
    	libGui.so => /SMASH/root_install/lib/libGui.so (0x00007f21967ee000)
    	libCore.so => /SMASH/root_install/lib/libCore.so (0x00007f2195e98000)
    	libImt.so => /SMASH/root_install/lib/libImt.so (0x00007f2195e74000)
    	libRIO.so => /SMASH/root_install/lib/libRIO.so (0x00007f219541f000)
    	libNet.so => /SMASH/root_install/lib/libNet.so (0x00007f2195280000)
    	libHist.so => /SMASH/root_install/lib/libHist.so (0x00007f21949cb000)
    	libGraf.so => /SMASH/root_install/lib/libGraf.so (0x00007f21946ea000)
    	libGraf3d.so => /SMASH/root_install/lib/libGraf3d.so (0x00007f21945ab000)
    	libGpad.so => /SMASH/root_install/lib/libGpad.so (0x00007f2194408000)
    	libROOTVecOps.so => /SMASH/root_install/lib/libROOTVecOps.so (0x00007f21941b1000)
    	libTree.so => /SMASH/root_install/lib/libTree.so (0x00007f2193e65000)
    	libTreePlayer.so => /SMASH/root_install/lib/libTreePlayer.so (0x00007f2193b41000)
    	libRint.so => /SMASH/root_install/lib/libRint.so (0x00007f2193ae4000)
    	libPostscript.so => /SMASH/root_install/lib/libPostscript.so (0x00007f2193953000)
    	libMatrix.so => /SMASH/root_install/lib/libMatrix.so (0x00007f21935d5000)
    	libPhysics.so => /SMASH/root_install/lib/libPhysics.so (0x00007f2193539000)
    	libMathCore.so => /SMASH/root_install/lib/libMathCore.so (0x00007f2193113000)
    	libThread.so => /SMASH/root_install/lib/libThread.so (0x00007f2193072000)
    	libMultiProc.so => /SMASH/root_install/lib/libMultiProc.so (0x00007f219305b000)
    	libROOTDataFrame.so => /SMASH/root_install/lib/libROOTDataFrame.so (0x00007f2192dd3000)
    	libHepMC3rootIO.so.3 => /SMASH/hepmc3_install/lib/libHepMC3rootIO.so.3 (0x00007f2192bb0000)
    	libtbb.so.2 => /SMASH/root_install/lib/libtbb.so.2 (0x00007f2184ecd000)
    	libvdt.so => /SMASH/root_install/lib/libvdt.so (0x00007f2184910000)
    	libGenVector.so => /SMASH/root_install/lib/libGenVector.so (0x00007f21847b9000)
    	libROOTNTuple.so => /SMASH/root_install/lib/libROOTNTuple.so (0x00007f218455a000)

    makes the sanitiser running the test still complain as above. 🤔 I guess I could create a standalone hello-world C++ code and compile it with the sanitiser linking to ROOT as our unit test does and I should get the same report. Let me know if you need so. Now, this behaviour (leaking just linking) should make it not too difficult to locate the issue. Do you do something with globals or static variables? Or do you have a _init and/or _fini function?

    By the way, I think we might reduce which ROOT libraries we link against and we have an issue about this, although it is not a urgent aspect at the moment.

  8. hahnjo commented on Nov 20, 2023

    @hahnjo
    Member

    In particular, I tried to add a trivial unit test file [...] and compiling as the other ones, i.e. linking it to [...] makes the sanitiser running the test still complain as above.

    Depending on how you compile it, you might get -fsanitize=address from the ROOT C++ flags. But in general yes, the libraries are sanitized and will operate no matter if your application itself is built with sanitizers or not.

    I guess I could create a standalone hello-world C++ code and compile it with the sanitiser linking to ROOT as our unit test does and I should get the same report. Let me know if you need so.

    That would be as expected, so I don't think it's worth the effort.

    Now, this behaviour (leaking just linking) should make it not too difficult to locate the issue. Do you do something with globals or static variables? Or do you have a _init and/or _fini function?

    As explained before, what the AddressSanitizer is complaining about is well understood and it also doesn't explain growing memory usage when not interacting with ROOT. This latter is what needs a clear reproducer to dig out the problem.

  9. AxelKrypton commented on Nov 20, 2023

    @AxelKrypton
    Author

    This latter is what needs a clear reproducer to dig out the problem.

    I agree. This is actually what I was meaning by making an hello-world program that does not use ROOT, compile it linking to ROOT and with sanitiser on, and see if the same leak report is given. I would expect so and, if not, then there is something I definitely do not get in our application. 🙈

  10. ferdymercury commented on Jun 14, 2024

    @ferdymercury
    Collaborator

    Related PR #15803

  11. pcanal commented on Jun 14, 2024

    @pcanal
    Member

    Doing so, all the unit tests pass, but I then monitored memory usage over a long run of our application and a linear growth was evident (plot left below). Nothing extreme, sure, but I thought it was worth reporting, especially since in v6.26.10 the problem does not occur (plot right below).

    Do you see the same growth without ASAN?

  12. hahnjo commented on Sep 24, 2024

    @hahnjo
    Member

    @AxelKrypton any updates on this? Looking over the past discussion, we are aware of the (intentional) memory leaks in libCling (even though we fixed one case in #16150 that was a mistake; the fix will appear in 6.34). I still have trouble understanding how just linking to ROOT and not using it should introduce a memory leak in the application... Lacking more feedback, I would propose to close this issue for the moment...

  13. AxelKrypton commented on Sep 24, 2024

    @AxelKrypton
    Author

    I could not find time to provide you with more information. You will surely understand that it is not a trivial check, as I need to build a container compiling ROOT as you suggest and then build and run my application. However, IMO, you also have all the ingredients to give it a try. Following this note above, you should be able to reproduce the problem. There I wrote

    I guess I could create a standalone hello-world C++ code and compile it with the sanitiser linking to ROOT as our unit test does and I should get the same report.

    did you try? 🤔

    It is on my to-do list to come back to this at some point, but I have not really found time yet. I plan to try to use more recent ROOT versions and check if the problem disappear. Especially as you mentioned it, 6.34 is a good first candidate. I can report here the outcome. As side remark, closing the issue does not feel useful. 🫠

  14. hahnjo commented on Sep 24, 2024

    @hahnjo
    Member

    However, IMO, you also have all the ingredients to give it a try. Following this note above, you should be able to reproduce the problem. There I wrote

    I guess I could create a standalone hello-world C++ code and compile it with the sanitiser linking to ROOT as our unit test does and I should get the same report.

    did you try? 🤔

    No, because this was already discussed: The report is real, but it's a well understood problem in the way ROOT and Cling use the LLVM and Clang libraries and will not be fixed. That part of the issue has been addressed from my point of view, please let me know if there is something to clarify.

    Then in #14055 (comment) you made a second report about memory usage which you clarified in #14055 (comment) happens even if only linking the ROOT libraries. As mentioned, this seemed specific to your setup and I was thus waiting for a reproducer from your side. Without that, I'm not sure what I would be able to do here?

  15. AxelKrypton commented on Sep 24, 2024

    @AxelKrypton
    Author

    Do you mean that the sanitiser report when running the following unit test

    #include "vir/test.h"  // This include has to be first -> https://github.com/mattkretz/virtest
    
    TEST(){ std::cout << "Hello world\n"; }

    compiled linking to ROOT is about the leakage you usually suppress? 🤔 If so, I misunderstood you, sorry. In that case, if you need to close the issue for your internal workflow, go ahead. Still the reported leakage above is a real one which appeared in v6.28 and was not there in v6.26.

    I will at some point try to come up with a standalone reproducer. I am not sure how straightforward will it be. However, I would start checking the memory usage with newer ROOT versions of the same setup used above.

  16. hahnjo commented on Sep 24, 2024

    @hahnjo
    Member

    Do you mean that the sanitiser report when running the following unit test

    #include "vir/test.h"  // This include has to be first -> https://github.com/mattkretz/virtest
    
    TEST(){ std::cout << "Hello world\n"; }

    compiled linking to ROOT is about the leakage you usually suppress? 🤔

    I just tried and with this simple example I don't see anything, which is totally expected because it doesn't even initialize ROOT. The other reports that you provided before were the leakage we know about, or in the case of TemplateIdAnnotation are fixed by #16150

    Still the reported leakage above is a real one which appeared in v6.28 and was not there in v6.26.

    Yes, that's why I didn't close the issue back then and came across this when re-evaluating open issues. I totally understand that creating a reproducer takes time, but I fear without it there isn't much we can do without it...

  17. AxelKrypton commented on Sep 24, 2024

    @AxelKrypton
    Author

    I just tried and with this simple example I don't see anything, which is totally expected because it doesn't even initialize ROOT. The other reports that you provided before were the leakage we know about, or in the case of TemplateIdAnnotation are fixed by #16150

    Thanks. I guess what I get from the sanitiser is what you suppress. This clarifies further the situation.

    I totally understand that creating a reproducer takes time, but I fear without it there isn't much we can do without it...

    Fair enough. I will try to find some spare hours for this. In the best case scenario ROOT 6.34 does not show the problem and I can leave here a comment and close the issue... 🤞

  18. AxelKrypton commented on Sep 27, 2024

    @AxelKrypton
    Author

    @hahnjo I did not want to let this sit for too long and as days ahead are even busier for me I invested now some more time on this. The good news is that I can pretty confidently say that the memory leak reported above in the left plot has nothing to do with ROOT 🎉 and it was an unlucky coincidence which made me think it is due to ROOT. 😓 I still have to understand this memory behaviour in detail, but it is off-topic here. 🙈

    I was about simply closing the issue, when I remembered checking old issues in our project that our tests were just passing with ROOT v6.26 and started to fail reporting the above mentioned sanitiser complaint from v6.28. I now checked and these failure occur as well with v6.30 and v6.32. As far as I understand from our exchange above, we can safely suppress these adding leak:libCling' to lsan-root.supp file (I would not exclude more than the bare minimum in order to avoid suppress potential real problems/errors in the future). However, I would like to ask you a feedback about the fact this suppression was not necessary in v6.26. Is this expected for you? 🤔

  19. hahnjo commented on Sep 30, 2024

    @hahnjo
    Member

    @hahnjo I did not want to let this sit for too long and as days ahead are even busier for me I invested now some more time on this. The good news is that I can pretty confidently say that the memory leak reported above in the left plot has nothing to do with ROOT 🎉 and it was an unlucky coincidence which made me think it is due to ROOT. 😓 I still have to understand this memory behaviour in detail, but it is off-topic here. 🙈

    Ok, thanks for cross-checking!

    I was about simply closing the issue, when I remembered checking old issues in our project that our tests were just passing with ROOT v6.26 and started to fail reporting the above mentioned sanitiser complaint from v6.28. I now checked and these failure occur as well with v6.30 and v6.32. As far as I understand from our exchange above, we can safely suppress these adding leak:libCling' to lsan-root.supp file (I would not exclude more than the bare minimum in order to avoid suppress potential real problems/errors in the future).

    Yes, I believe from an application point of view, you can treat libCling as a black box.

    However, I would like to ask you a feedback about the fact this suppression was not necessary in v6.26. Is this expected for you? 🤔

    If the leaks are about TemplateId (as they were in the log file(s) you shared), then yes this is a problem we introduced in v6.28. It was eventually fixed in #16150 which will appear in v6.34 (later this year). Sorry for not recognizing this as a real problem earlier (the fix was also made difficult because it was "caused" by an upstream change in LLVM/Clang when we upgraded).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions