Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix asan issue with tiny profiler #4066

Closed
wants to merge 1 commit into from

Conversation

marchdf
Copy link
Contributor

@marchdf marchdf commented Aug 6, 2024

Summary

Fix a container-overflow AddressSanitizer error.

Additional background

A RelWithDebInfo, clang 18 build gave me the following:

TinyProfiler total time across processes [min...avg...max]: 0.2733 ... 0.2733 ... 0.2733
=================================================================
==53679==ERROR: AddressSanitizer: container-overflow on address 0x60800000d768 at pc 0x000102d32e54 bp 0x00016d107dc0 sp 0x00016d107db8
WRITE of size 8 at 0x60800000d768 thread T0
    #0 0x102d32e50 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>::__init_copy_ctor_external(char const*, unsigned long) string:2240
    #1 0x102f00028 in amrex::TinyProfiler::PrintStats(std::__1::map<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, amrex::TinyProfiler::Stats, std::__1::less<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>, std::__1::allocator<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const, amrex::TinyProfiler::Stats>>>&, double) AMReX_TinyProfiler.cpp:422
    #2 0x102effdb0 in amrex::TinyProfiler::Finalize(bool) AMReX_TinyProfiler.cpp:353
    #3 0x102e4eadc in amrex::Finalize(amrex::AMReX*) AMReX.cpp:743
    #4 0x102cf7da4 in main main.cpp:64
    #5 0x189f63fd4 in start+0x968 (dyld:arm64+0xfffffffffff63fd4)
    #6 0xb000fffffffffffc  (<unknown module>)

This change makes that error go away. Interestingly, I didn't see this error with a Debug build.

Checklist

The proposed changes:

  • fix a bug or incorrect behavior in AMReX
  • add new capabilities to AMReX
  • changes answers in the test suite to more than roundoff level
  • are likely to significantly affect the results of downstream AMReX users
  • include documentation in the code and/or rst files, if appropriate

@marchdf
Copy link
Contributor Author

marchdf commented Aug 6, 2024

hmmm maybe this is a false positive. From https://github.com/google/sanitizers/wiki/AddressSanitizerContainerOverflow

False positives
We know about at least one kind of false positive container overflow reports. If a part of the application is built with asan and another part is not instrumented, and both parts use e.g. instrumented std::vector, asan may report non-existent container overflow. This happens because instrumented and non-instrumented bits of std::vector, inlined and not, are mixed during linking, so you end up with incompletely instrumented std::vector.

@marchdf
Copy link
Contributor Author

marchdf commented Aug 6, 2024

Confirmed this was a false positive. I needed to propogate asan options through to amrex.

@marchdf marchdf closed this Aug 6, 2024
@marchdf marchdf deleted the fix-tinyprof branch August 6, 2024 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant