Exception stack support and stacktrace improvements #24

rhoneyager-tomorrow · 2024-02-27T19:36:28Z

Description

This makes debugging ioda and ufo a lot easier.

Enable C++11-style nested exceptions

This PR reworks the oops applications just a bit to enable nested C++11-style exceptions in the JEDI codes. This is used extensively in ioda, where nested exceptions should provide additional information about variable and file names.

This should probably be backported to eckit in several locations, particularly in eckit/testing/Test.h to add this to unit testing, and eckit/exception for a general fix. Adding it at the oops level, however, is sufficient when diagnosing runtime issues with increasing numbers of observation types in experiments.

Improvements to stack trace code

This PR reworks the stack trace code just a bit to improve the robustness of the code and to enable traces outside of the oops sigfpe checks. Additional comments have been added regarding why Boost's stacktrace is used in favor of eckit's. eckit does not depend on Boost, otherwise I would also consider making this change over there.

Nested exceptions example:

Here's a somewhat synthetic example of this code's error output. It shows what should happen when IODA tries to access a nonexistent variable. This is a bit verbose, but nested exceptions convey explanations at different processing levels.

Exception: oops::TimeIodaIO<IODA> terminating...
Exception: level: 0
	Reason:	An exception occurred inside ioda while opening a variable.
	source_filename:	/backup/ryan/src/JCSDA/jedi-bundle/stacktraces/ioda/src/engines/ioda/src/ioda/Has_Variables.cpp
	source_function:	ioda::Variable ioda::detail::Has_Variables_Base::open(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &) const
	source_line:	108
	stacktrace:	 
 0# util::stacktrace_current[abi:cxx11]() at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/oops/src/oops/util/Stacktrace.cc:23
 1# ioda::Exception::add_call_stack() at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/ioda/src/engines/ioda/src/ioda/Exception.cpp:46
 2# ioda::Exception::Exception(char const*, ioda::detail::compat::source_location::source_location const&, ioda::Options const&) at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/ioda/src/engines/ioda/src/ioda/Exception.cpp:25
 3# ioda::detail::Has_Variables_Base::open(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/ioda/src/engines/ioda/src/ioda/Has_Variables.cpp:109
 4# ioda::TimeIodaIO<ioda::IodaTrait>::execute(eckit::Configuration const&, bool) const at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/build/ioda/include/ioda/mains/timeIodaIO.h:59
 5# oops::Run::execute(oops::Application const&, eckit::mpi::Comm const&) at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/oops/src/oops/runs/Run.cc:186
 6# main at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/ioda/src/mains/timeIodaIO.cc:20
 7# __libc_start_call_main in /lib64/libc.so.6
 8# __libc_start_main_alias_1 in /lib64/libc.so.6
 9# _start in ../../bin/time_IodaIO.x

Exception: level: 1
	Reason:	Variable not found.
	name:	test
	source_filename:	/backup/ryan/src/JCSDA/jedi-bundle/stacktraces/ioda/src/engines/ioda/src/ioda/Engines/ObsStore/Variables.cpp
	source_function:	std::shared_ptr<ioda::ObsStore::Variable> ioda::ObsStore::Has_Variables::open(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &) const
	source_line:	189
	stacktrace:	 
 0# util::stacktrace_current[abi:cxx11]() at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/oops/src/oops/util/Stacktrace.cc:23
 1# ioda::Exception::add_call_stack() at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/ioda/src/engines/ioda/src/ioda/Exception.cpp:46
 2# ioda::Exception::Exception(char const*, ioda::detail::compat::source_location::source_location const&, ioda::Options const&) at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/ioda/src/engines/ioda/src/ioda/Exception.cpp:25
 3# ioda::ObsStore::Has_Variables::open(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/ioda/src/engines/ioda/src/ioda/Engines/ObsStore/Variables.cpp:189
 4# ioda::Engines::ObsStore::ObsStore_HasVariables_Backend::open(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/ioda/src/engines/ioda/src/ioda/Engines/ObsStore/ObsStore-variables.cpp:261
 5# ioda::detail::Has_Variables_Base::open(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/ioda/src/engines/ioda/src/ioda/Has_Variables.cpp:105
 6# ioda::TimeIodaIO<ioda::IodaTrait>::execute(eckit::Configuration const&, bool) const at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/build/ioda/include/ioda/mains/timeIodaIO.h:59
 7# oops::Run::execute(oops::Application const&, eckit::mpi::Comm const&) at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/oops/src/oops/runs/Run.cc:186
 8# main at /backup/ryan/src/JCSDA/jedi-bundle/stacktraces/ioda/src/mains/timeIodaIO.cc:20
 9# __libc_start_call_main in /lib64/libc.so.6
10# __libc_start_main_alias_1 in /lib64/libc.so.6
11# _start in ../../bin/time_IodaIO.x

Dependencies

None, but I might make a ioda PR soon-ish that would depend on this.

Checklist

I have performed a self-review of my own code
I have made corresponding changes to the documentation
I have run the unit tests before creating the PR

climbfuji · 2024-02-29T16:43:33Z

Can we review this PR here before I bring over the commits to JCSDA-internal?

srherbener

Thanks for these improvements!

For my understanding with the exception handling improvements. Do I have this right?

As long as the exceptions at the various function levels are based on std::exception, this should work at getting all of the messages printed.
At the point a non std::exception is hit, the unwinding stops as well as the messaging.
The eckit and ioda exception classes are both based on std::exception so they should be good with this unwinding method

Also, have one question (which is probably outside the scope of this PR). An issue we have seen is where Fortran throws the exception and it doesn't appear to get caught (or processed correctly) on the C++ side. Perhaps this change will help, but the issue could also be that the Fortran exception is not based on std::exception, correct? I'm guessing in that case, one would need to wrap an interface around the C++ std::exception and have Fortran use that (which is perhaps what fckit provides?).

rhoneyager-tomorrow · 2024-02-29T21:09:21Z

Thanks for these improvements!

For my understanding with the exception handling improvements. Do I have this right?

As long as the exceptions at the various function levels are based on std::exception, this should work at getting all of the messages printed.

Yes

At the point a non std::exception is hit, the unwinding stops as well as the messaging.

Yes. While the C++11 standard allows exceptions that do not inherit from std::exception, all of the post-C++11 features like std::exception_ptr require this dependence.

The eckit and ioda exception classes are both based on std::exception so they should be good with this unwinding method

Yes

Also, have one question (which is probably outside the scope of this PR). An issue we have seen is where Fortran throws the exception and it doesn't appear to get caught (or processed correctly) on the C++ side. Perhaps this change will help, but the issue could also be that the Fortran exception is not based on std::exception, correct? I'm guessing in that case, one would need to wrap an interface around the C++ std::exception and have Fortran use that (which is perhaps what fckit provides?).

You cannot reliably throw exceptions across language boundaries. It is undefined behavior. This definitely will interfere with any type of object or resource handle initialization, finalization, or destruction. This was why ioda's C interface, for example, was written to catch all exceptions at the interface layer and terminate.

You can, however, catch and encapsulate or translate the exceptions right before you pass through different languages in the codebase. I think I had some prototype code for doing this in one or more branches of jckit. Somewhere in there you can also find a basic implementation of C-style exceptions using setjmp/longjmp. Similar examples in Fortran exist on the web, like this one.

srherbener · 2024-03-01T18:03:31Z

Thank you @rhoneyager-tomorrow for the information - very helpful!

ctgh

Many thanks for adding this feature - it should make understanding exceptions much easier.

ctgh · 2024-03-07T10:19:00Z

src/oops/util/Stacktrace.cc

+#ifndef BOOST_STACKTRACE_USE_NOOP
+  std::ostringstream s;
+  s << boost::stacktrace::stacktrace() << std::endl;
+  std::string sout = s.str();


This could be const.

ctgh · 2024-03-07T10:19:20Z

src/test/util/Stacktrace.cc

@@ -0,0 +1,19 @@
+/*
+ * (C) Copyright 2021-2023 UCAR


ctgh · 2024-03-07T10:20:11Z

src/test/util/Stacktrace.cc

+#include <iostream>
+
+int main(int argc, char **argv) {
+  std::cout << "Stacktrace:\n" << util::stacktrace_current() << std::endl;


Just a suggestion but you could use oops::Log::info() here instead of std::cout.

climbfuji · 2024-03-08T17:38:52Z

Since we have two approvals for this, I'll go ahead and

update the branch from develop
create a PR in jcsda-internal

climbfuji · 2024-03-08T18:18:48Z

The PR for jcsda-internal is https://github.com/JCSDA-internal/oops/pull/2578

I will keep this PR open until https://github.com/JCSDA-internal/oops/pull/2578 is merged

fmahebert · 2024-03-08T20:50:17Z

Thanks for this improvement, @rhoneyager-tomorrow !

climbfuji · 2024-03-08T21:05:55Z

PR was merged in jcsda-internal and should show up on JCSDA public develop shortly. Thanks for this contribution @rhoneyager-tomorrow

rhoneyager-tomorrow added 6 commits February 26, 2024 16:00

Adding stacktrace support

095e4df

Merge remote-tracking branch 'origin/develop' into feature/stacktrace

a21ff2e

.

21e3e43

Implemented a stacktrace exporter in oops.

10b238a

oops applications should walk the C++ exception stack on error.

a77f28c

Coding norms

19f1506

climbfuji requested review from srherbener and shlyaeva February 29, 2024 16:43

srherbener approved these changes Feb 29, 2024

View reviewed changes

rhoneyager-tomorrow mentioned this pull request Feb 29, 2024

Better stacktraces in ioda JCSDA/ioda#14

Closed

4 tasks

shlyaeva requested review from fmahebert and ctgh March 6, 2024 14:38

ctgh approved these changes Mar 7, 2024

View reviewed changes

climbfuji closed this Mar 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exception stack support and stacktrace improvements #24

Exception stack support and stacktrace improvements #24

rhoneyager-tomorrow commented Feb 27, 2024

climbfuji commented Feb 29, 2024

srherbener left a comment

rhoneyager-tomorrow commented Feb 29, 2024 •

edited

Loading

srherbener commented Mar 1, 2024

ctgh left a comment

ctgh Mar 7, 2024

ctgh Mar 7, 2024

ctgh Mar 7, 2024

climbfuji commented Mar 8, 2024

climbfuji commented Mar 8, 2024

fmahebert commented Mar 8, 2024

climbfuji commented Mar 8, 2024

Exception stack support and stacktrace improvements #24

Exception stack support and stacktrace improvements #24

Conversation

rhoneyager-tomorrow commented Feb 27, 2024

Description

Enable C++11-style nested exceptions

Improvements to stack trace code

Nested exceptions example:

Dependencies

Checklist

climbfuji commented Feb 29, 2024

srherbener left a comment

Choose a reason for hiding this comment

rhoneyager-tomorrow commented Feb 29, 2024 • edited Loading

srherbener commented Mar 1, 2024

ctgh left a comment

Choose a reason for hiding this comment

ctgh Mar 7, 2024

Choose a reason for hiding this comment

ctgh Mar 7, 2024

Choose a reason for hiding this comment

ctgh Mar 7, 2024

Choose a reason for hiding this comment

climbfuji commented Mar 8, 2024

climbfuji commented Mar 8, 2024

fmahebert commented Mar 8, 2024

climbfuji commented Mar 8, 2024

rhoneyager-tomorrow commented Feb 29, 2024 •

edited

Loading