Skip to content

Backtrace support for flang #118179

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

Merged
merged 20 commits into from
Dec 10, 2024
Merged

Backtrace support for flang #118179

merged 20 commits into from
Dec 10, 2024

Conversation

dty2
Copy link
Contributor

@dty2 dty2 commented Nov 30, 2024

Fixed build failures in old PRs due to missing files

Copy link

github-actions bot commented Nov 30, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Comment on lines 93 to 97
# function checks
find_package(Backtrace)
set(HAVE_BACKTRACE ${Backtrace_FOUND})
set(BACKTRACE_HEADER ${Backtrace_HEADER})

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please could you try putting this in flang/runtime/CMakeLists.txt

@tblah tblah requested a review from jeanPerier December 2, 2024 10:21
@@ -29,6 +29,7 @@ NORETURN void RTNAME(ProgramEndStatement)(NO_ARGUMENTS);
// Extensions
NORETURN void RTNAME(Exit)(int status DEFAULT_VALUE(EXIT_SUCCESS));
NORETURN void RTNAME(Abort)(NO_ARGUMENTS);
void RTNAME(Backtrace)(NO_ARGUMENTS);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeanPerier pointed out we don't need any of the changes in flang/lib in this case: pointing you at fdate

Suggested change
void RTNAME(Backtrace)(NO_ARGUMENTS);
void FORTRAN_PROCEDURE_NAME(Backtrace)(NO_ARGUMENTS);

With this you shouldn't need any of the frontend changes (flang/lib, and associated header changes).

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates. Do you still need help to merge?

Copy link
Contributor

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit on the ABORT change, LGTM otherwise, thanks for the update!

[[noreturn]] void RTNAME(Abort)() {
// TODO: Add backtrace call, unless with `-fno-backtrace`.
PrintBacktrace();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make that one also conditional on #ifdef HAVE_BACKTRACE I do not think ABORT should crash with "Handle the case when a backtrace is not available\n" on platforms that have no backtrace.

It could lead people to believe that the abort is related to BACKTRACE while it is not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, thanks for pointing this out.

@dty2
Copy link
Contributor Author

dty2 commented Dec 9, 2024

Thanks for the updates. Do you still need help to merge?

Thank you for your corrections to my questions. I am very grateful. If you think there is no problem, you can merge it.

@tblah tblah merged commit e8baa79 into llvm:main Dec 10, 2024
9 checks passed
@brad0
Copy link
Contributor

brad0 commented Jan 31, 2025

It looks like the runtime should link against libexecinfo on *BSD's. Backtrace_LIBRARY is filed in. I am not sure of how to go about linking with the runtime in runtime/CMakeLists.txt

FAILED: tools/flang/unittests/Runtime/FlangRuntimeTests
: && /usr/bin/c++ -pipe  -fno-ret-protector -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -Wno-deprecated-copy -Wno-string-conversion -Wno-ctad-maybe-unsupported -Wno-unused-command-line-argument -Wstring-conversion           -Wcovered-switch-default -Wno-nested-anon-types -O2 -Wl,--color-diagnostics    -Wl,--gc-sections  -Xlinker --dependency-file=tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/link.d tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/AccessTest.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Allocatable.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/ArrayConstructor.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/BufferTest.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/CharacterTest.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/CommandTest.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Complex.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/CrashHandlerFixture.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Derived.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/ExternalIOTest.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Format.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Inquiry.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/ListInputTest.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/LogicalFormatTest.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Matmul.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/MatmulTranspose.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/MiscIntrinsic.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Namelist.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Numeric.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/NumericalFormatTest.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Pointer.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Ragged.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Random.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Reduction.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/RuntimeCrashTest.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Stop.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Support.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Time.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/TemporaryStack.cpp.o tools/flang/unittests/Runtime/CMakeFiles/FlangRuntimeTests.dir/Transformational.cpp.o -o tools/flang/unittests/Runtime/FlangRuntimeTests  -Wl,-z,origin,-rpath,/home/brad/tmp/llvm-build/lib  lib/libllvm_gtest_main.a  lib/libllvm_gtest.a  -lpthread  lib/libFortranRuntime.a  -lpthread  lib/libFortranDecimal.a  lib/libclang-cpp.so.21.0git  lib/libLLVM.so.21.0git  -Wl,-rpath-link,/usr/X11R6/lib:/usr/local/lib && :
ld: error: undefined symbol: backtrace
>>> referenced by stop.cpp
>>>               stop.cpp.o:(PrintBacktrace()) in archive lib/libFortranRuntime.a
>>> did you mean: backtrace_
>>> defined in: lib/libFortranRuntime.a(stop.cpp.o)

ld: error: undefined symbol: backtrace_symbols
>>> referenced by stop.cpp
>>>               stop.cpp.o:(PrintBacktrace()) in archive lib/libFortranRuntime.a
c++: error: linker command failed with exit code 1 (use -v to see invocation)

@tblah
Copy link
Contributor

tblah commented Jan 31, 2025

Hi @brad0, thanks for looking into this.

I don't have a BSD machine to test but I would guess something like the following would work in flang/runtime/CMakeLists.txt

set(linked_libraries
  FortranDecimal
)
if(CMAKE_SYSTEM_NAME MATCHES ".+BSD")
  list(APPEND linked_libraries "libexecinfo")
endif

# For each usage of add_flang_library use the new list instead of just FortranDecimal:
add_flang_library(FortranRuntime
  ${sources}
  LINK_LIBS
  ${link_libraries}
)

You can find CMake's documentation here (e.g. for list(APPEND )) https://cmake.org/cmake/help/latest/command/list.html#append

@brad0
Copy link
Contributor

brad0 commented Feb 1, 2025

@tblah Ok, let me play around with that and see what I can come up with. Thanks.

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.

4 participants