This is an archive of the discontinued LLVM Phabricator instance.

[clang-repl] Re-implement clang-interpreter as a test case.
ClosedPublic

Authored by v.g.vassilev on Jul 29 2021, 2:41 AM.

Details

Summary

The current infrastructure in lib/Interpreter has a tool, clang-repl, very similar to clang-interpreter which also allows incremental compilation.

This patch moves clang-interpreter as a test case and drops it as conditionally built example as we already have clang-repl in place.

Diff Detail

Event Timeline

v.g.vassilev created this revision.Jul 29 2021, 2:41 AM
v.g.vassilev requested review of this revision.Jul 29 2021, 2:41 AM

LGTM, but a clang dev should probably check this out too.

v.g.vassilev added a subscriber: rnk.
v.g.vassilev added inline comments.
clang/unittests/Interpreter/InterpreterTest.cpp
128

@rnk, would that test here be fine for both win32 and win64. Sorry for asking but I do not have an easy access to a Windows machine.

rsmith accepted this revision.Jul 30 2021, 12:20 PM
rsmith added inline comments.Jul 30 2021, 12:20 PM
clang/unittests/Interpreter/InterpreterTest.cpp
58

I don't follow why this can't be static. Is there some problem with using the address of this function if it's static?

This revision is now accepted and ready to land.Jul 30 2021, 12:20 PM
v.g.vassilev added inline comments.Jul 30 2021, 12:28 PM
clang/unittests/Interpreter/InterpreterTest.cpp
58

That's a standard comment for each GetExecutablePath and I copy pasted it as done elsewhere. That requirement might be lifted in recent systems but I do not know the platform coverage to check.

Try to fix the issues reported by the pre-merge-checks. clang-format.

Try to appease the windows pre-merge bot.

Split the exception handling test into a separate unittest and compile it with exceptions on.

Try fix the win pre-merge bot. Clang-format.

v.g.vassilev added a subscriber: sgraenitz.

Disable the case exception test of windows.

cc: @sgraenitz

rsmith added inline comments.Aug 30 2021, 4:42 PM
clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
74–77

Presumably this is redundant now we don't run the test on Windows?

102–106

Isn't this because you're passing no arguments to main when you call it later in the test? You're not passing any arguments on line 123/124.

124–125

I think this should probably be cast to int(*)(int, const char**) instead. But the name main is special, and might not have its declared type, so selecting a different function name would have less risk of weird behavior. I'd also suggest you remove the parameters if you're not going to use them.

v.g.vassilev marked 3 inline comments as done.

Address comments.

rsmith accepted this revision.Aug 31 2021, 10:16 AM
v.g.vassilev added inline comments.Aug 31 2021, 11:11 AM
clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
102–106

Ah, indeed. Thanks!

124–125

Fixed. The past intent was to keep it as close as possible to the original example.

This revision was landed with ongoing or failed builds.Aug 31 2021, 10:24 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2021, 10:24 PM
thakis added a subscriber: thakis.Sep 1 2021, 5:28 AM

The test fails for me on macOS (normal cmake build):

% tools/clang/unittests/Interpreter/ExceptionTests/ClangReplInterpreterExceptionTests
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from InterpreterTest
[ RUN      ] InterpreterTest.CatchException
In file included from <<< inputs >>>:1:
input_line_0:2:10: fatal error: 'stdexcept' file not found
#include <stdexcept>
         ^~~~~~~~~~~

The reason is probably that C++ stdlib headers are part of the toolchain, not the sysroot, on macOS. So self-built clang can't include libc++(abi) headers without some hoops.

Tests should generally be standalone though anyways. Can we remove these includes and just throw some basic type?

Finally, hardcoding the path between unit test binary and the rest of the build dir is fairly unusual. Can't this be a lit-style test that uses the normal substitution logic we use in lit?

thakis added a comment.Sep 1 2021, 5:30 AM

Also failing on the bots: https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/23343/execution/node/39/log/ (that's for the original land, but from what I can tell nothing has changed in this regard for the reland, and I'm still seeing the same failure at head).

I'll revert this for now to unbreak mac bots.

The test fails for me on macOS (normal cmake build):

% tools/clang/unittests/Interpreter/ExceptionTests/ClangReplInterpreterExceptionTests
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from InterpreterTest
[ RUN      ] InterpreterTest.CatchException
In file included from <<< inputs >>>:1:
input_line_0:2:10: fatal error: 'stdexcept' file not found
#include <stdexcept>
         ^~~~~~~~~~~

The reason is probably that C++ stdlib headers are part of the toolchain, not the sysroot, on macOS. So self-built clang can't include libc++(abi) headers without some hoops.

Tests should generally be standalone though anyways. Can we remove these includes and just throw some basic type?

Good point. I think I can avoid including stdexcept.

Finally, hardcoding the path between unit test binary and the rest of the build dir is fairly unusual. Can't this be a lit-style test that uses the normal substitution logic we use in lit?

I did not find a good way to 'find back' the clang resource-dir and it seems to be put in a constant place. The test needs to have a compiled and interpreted parts -- it throws an exception from the interpreter land and expects a compiled binary to be able to catch it. I am not sure if I know how to do that in lit.

thakis added a comment.Sep 1 2021, 5:51 AM

I did not find a good way to 'find back' the clang resource-dir and it seems to be put in a constant place. The test needs to have a compiled and interpreted parts -- it throws an exception from the interpreter land and expects a compiled binary to be able to catch it. I am not sure if I know how to do that in lit.

Maybe this could be a binary with a main() function instead of a unit test, and you pass the path to the resource dir and to clang-interpreter to it, and then that sets up an exception handler and execs clang-interpreter? Or it could stay mostly as-is except make it a binary instead of a test, and call it from lit from a .test file (and pass in resource dir). That'd be more in line what other clang tests do.

(See e.g. the %resource_dir substitution, set in llvm/utils/lit/lit/llvm/config.py, used in a few places.)

thakis added a comment.Sep 3 2021, 6:18 AM

Thanks for fixing! Intel macs are happy now.

But the test is still failing on arm macs: http://45.33.8.238/macm1/17292/step_7.txt

FAIL: Clang-Unit :: Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException (28440 of 28440)
******************** TEST 'Clang-Unit :: Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException' FAILED ********************
Script:
--
/Users/thakis/src/llvm-project/out/gn/obj/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests --gtest_filter=InterpreterTest.CatchException
--
Note: Google Test filter = InterpreterTest.CatchException
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from InterpreterTest
[ RUN      ] InterpreterTest.CatchException
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
....

Please take a look.

Thanks for fixing! Intel macs are happy now.

But the test is still failing on arm macs: http://45.33.8.238/macm1/17292/step_7.txt

FAIL: Clang-Unit :: Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException (28440 of 28440)
******************** TEST 'Clang-Unit :: Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException' FAILED ********************
Script:
--
/Users/thakis/src/llvm-project/out/gn/obj/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests --gtest_filter=InterpreterTest.CatchException
--
Note: Google Test filter = InterpreterTest.CatchException
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from InterpreterTest
[ RUN      ] InterpreterTest.CatchException
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
....

Please take a look.

Yes, thanks for this information -- it fails also on hexagon so reverted it again :(

Thanks for fixing! Intel macs are happy now.

But the test is still failing on arm macs: http://45.33.8.238/macm1/17292/step_7.txt

FAIL: Clang-Unit :: Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException (28440 of 28440)
******************** TEST 'Clang-Unit :: Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException' FAILED ********************
Script:
--
/Users/thakis/src/llvm-project/out/gn/obj/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests --gtest_filter=InterpreterTest.CatchException
--
Note: Google Test filter = InterpreterTest.CatchException
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from InterpreterTest
[ RUN      ] InterpreterTest.CatchException
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
libunwind: malformed __unwind_info at 0x192029E3C bad second level page
....

Please take a look.

It looks like we hit https://bugs.llvm.org/show_bug.cgi?id=49692

cc: @karies, @Hahnfeld

I agree. Unfortunately, as described in the report, this is something that needs to be fixed on Apples side first.

karies added inline comments.Sep 6 2021, 2:52 AM
clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt
2

I don't understand the term "to expect" the thrown exception. Please ignore if that's a known term of art.

clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
93

Why not just throw Name; to avoid #include <stdexcept>?

100

Consider fwd declaring printf to avoid inclusion of stdio.h.

102

How is that provoking a test failure? What about exit(1) or whatever works for gtest?

104

To me, the wording difference between "In JIT" and "From JIT" doesn't signal anything. Maybe "the first could be "To be caught in JIT" and the second "To be caught in binary"?

Upload the newest version of this patch. It has several improvements that came from various bot failures. We just need to outline the llvm::consumeError to be able to call it from exceptions land and fix the last known failure on hexagon.

v.g.vassilev marked 5 inline comments as done.

address comments

clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
102

we use EXPECT_ANY_THROW and expect output on the stdout. I guess that line is not reachable.

104

The intent was to show that we can move the clang-interpreter example with relatively small amount of changes to the libInterpreter interfaces. we have changed that goal significantly and maybe we should also change the strings, too.

thakis added inline comments.Oct 8 2021, 4:22 AM
clang/unittests/Interpreter/CMakeLists.txt
4

Why are these additions needed here again? This change doesn't add any code to ClangReplInterpreterTests as far as I can tell. Are these remnants from before the exception tests were in a different binary?

v.g.vassilev added inline comments.Oct 8 2021, 11:13 AM
clang/unittests/Interpreter/CMakeLists.txt
4

Thanks for noticing that. The story was that clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt required ${LLVM_TARGETS_TO_BUILD} Core OrcJIT Support as a dependency -- I was not sure why but I added the same here. I am testing now if I can get rid of both. I do not understand why we will need different set of dependencies for these tests as they should be using more or less the same set of functionality...

Hi. I think our clang builders are failing from this after the reland (https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8833962834812688769/overview):

Script:
--
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests --gtest_filter=InterpreterTest.CatchException
--
Note: Google Test filter = InterpreterTest.CatchException
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from InterpreterTest
[ RUN      ] InterpreterTest.CatchException
JIT session error: Symbols not found: [ _Unwind_Resume, _ZSt9terminatev, _ZTVN10__cxxabiv117__class_type_infoE, __cxa_allocate_exception, __cxa_begin_catch, __cxa_end_catch, __cxa_free_exception, __cxa_throw, __gxx_personality_v0 ]
Failure value returned from cantFail wrapped call
Failed to materialize symbols: { (main, { __clang_call_terminate, _ZTI16custom_exception, _ZN16custom_exceptionC2EPKc, _ZTS16custom_exception, throw_exception }) }
UNREACHABLE executed at llvm/include/llvm/Support/Error.h:778!

Would you be able to send out a fix or revert?

Hi. I think our clang builders are failing from this after the reland (https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8833962834812688769/overview):

Script:
--
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests --gtest_filter=InterpreterTest.CatchException
--
Note: Google Test filter = InterpreterTest.CatchException
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from InterpreterTest
[ RUN      ] InterpreterTest.CatchException
JIT session error: Symbols not found: [ _Unwind_Resume, _ZSt9terminatev, _ZTVN10__cxxabiv117__class_type_infoE, __cxa_allocate_exception, __cxa_begin_catch, __cxa_end_catch, __cxa_free_exception, __cxa_throw, __gxx_personality_v0 ]
Failure value returned from cantFail wrapped call
Failed to materialize symbols: { (main, { __clang_call_terminate, _ZTI16custom_exception, _ZN16custom_exceptionC2EPKc, _ZTS16custom_exception, throw_exception }) }
UNREACHABLE executed at llvm/include/llvm/Support/Error.h:778!

Would you be able to send out a fix or revert?

Thanks for reverting. I will look into this failure.

v.g.vassilev added inline comments.Oct 26 2021, 1:20 PM
clang/unittests/Interpreter/CMakeLists.txt
4

I dropped these dependencies as you mentioned -- thanks again.

thakis added inline comments.Oct 26 2021, 3:44 PM
clang/unittests/Interpreter/CMakeLists.txt
4

Looks like they're back in the reland. Mind dropping them again?

v.g.vassilev marked an inline comment as done.Oct 27 2021, 3:14 AM
v.g.vassilev added inline comments.
clang/unittests/Interpreter/CMakeLists.txt
4

Yeah, thanks for keeping an eye on this. Not sure what I did wrong there.. Too many reverts I guess...

Hi,

We're seeing a problem with this patch in our downstream (not public) buildbots. With an asan-built compiler we see the following:

10:08:55 FAIL: Clang-Unit :: Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException (25832 of 79960)
10:08:55 ******************** TEST 'Clang-Unit :: Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException' FAILED ********************
10:08:55 Script:
10:08:55 --
10:08:55 /repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests --gtest_filter=InterpreterTest.CatchException
10:08:55 --
10:08:55 Note: Google Test filter = InterpreterTest.CatchException
10:08:55 [==========] Running 1 test from 1 test suite.
10:08:55 [----------] Global test environment set-up.
10:08:55 [----------] 1 test from InterpreterTest
10:08:55 [ RUN      ] InterpreterTest.CatchException
10:08:55 libunwind: __unw_add_dynamic_fde: bad fde: FDE is really a CIE
10:08:55 libc++abi: terminating with uncaught exception of type custom_exception
10:08:55  #0 0x000000000052072b backtrace /repo/uabelho/flacc_6.144/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:4205:13
10:08:55  #1 0x0000000001873774 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/../lib/Support/Unix/Signals.inc:565:13
10:08:55  #2 0x000000000186c328 llvm::sys::RunSignalHandlers() /repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/../lib/Support/Signals.cpp:0:5
10:08:55  #3 0x00000000018745a8 SignalHandler(int) /repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/../lib/Support/Unix/Signals.inc:0:3
10:08:55  #4 0x00007f61246da630 __restore_rt sigaction.c:0:0
10:08:55  #5 0x00007f6122025387 raise (/lib64/libc.so.6+0x36387)
10:08:55  #6 0x00007f6122026a78 abort (/lib64/libc.so.6+0x37a78)
10:08:55  #7 0x000000000cbdedd6 (/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests+0xcbdedd6)
10:08:55  #8 0x000000000cbe4407 (/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests+0xcbe4407)
10:08:55  #9 0x000000000cbdecb3 std::__terminate(void (*)()) crtstuff.c:0:0
10:08:55 #10 0x000000000cbdca26 (/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests+0xcbdca26)
10:08:55 #11 0x000000000cbdc9c0 __cxxabiv1::exception_cleanup_func(_Unwind_Reason_Code, _Unwind_Exception*) cxa_exception.cpp:0:0
10:08:55 #12 0x00007f611ea00171 
10:08:55 
10:08:55 ********************
10:08:55 Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
10:25:10 
10:25:10 1 warning(s) in tests
10:25:10 ********************
10:25:10 Failed Tests (1):
10:25:10   Clang-Unit :: Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException
10:25:10

Does an asan-build run clean for everyone else?

v.g.vassilev marked an inline comment as done.Oct 29 2021, 8:32 AM

Hi,

We're seeing a problem with this patch in our downstream (not public) buildbots. With an asan-built compiler we see the following:

10:08:55 FAIL: Clang-Unit :: Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException (25832 of 79960)
10:08:55 ******************** TEST 'Clang-Unit :: Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException' FAILED ********************
10:08:55 Script:
10:08:55 --
10:08:55 /repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests --gtest_filter=InterpreterTest.CatchException
10:08:55 --
10:08:55 Note: Google Test filter = InterpreterTest.CatchException
10:08:55 [==========] Running 1 test from 1 test suite.
10:08:55 [----------] Global test environment set-up.
10:08:55 [----------] 1 test from InterpreterTest
10:08:55 [ RUN      ] InterpreterTest.CatchException
10:08:55 libunwind: __unw_add_dynamic_fde: bad fde: FDE is really a CIE
10:08:55 libc++abi: terminating with uncaught exception of type custom_exception
10:08:55  #0 0x000000000052072b backtrace /repo/uabelho/flacc_6.144/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:4205:13
10:08:55  #1 0x0000000001873774 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/../lib/Support/Unix/Signals.inc:565:13
10:08:55  #2 0x000000000186c328 llvm::sys::RunSignalHandlers() /repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/../lib/Support/Signals.cpp:0:5
10:08:55  #3 0x00000000018745a8 SignalHandler(int) /repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/../lib/Support/Unix/Signals.inc:0:3
10:08:55  #4 0x00007f61246da630 __restore_rt sigaction.c:0:0
10:08:55  #5 0x00007f6122025387 raise (/lib64/libc.so.6+0x36387)
10:08:55  #6 0x00007f6122026a78 abort (/lib64/libc.so.6+0x37a78)
10:08:55  #7 0x000000000cbdedd6 (/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests+0xcbdedd6)
10:08:55  #8 0x000000000cbe4407 (/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests+0xcbe4407)
10:08:55  #9 0x000000000cbdecb3 std::__terminate(void (*)()) crtstuff.c:0:0
10:08:55 #10 0x000000000cbdca26 (/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests+0xcbdca26)
10:08:55 #11 0x000000000cbdc9c0 __cxxabiv1::exception_cleanup_func(_Unwind_Reason_Code, _Unwind_Exception*) cxa_exception.cpp:0:0
10:08:55 #12 0x00007f611ea00171 
10:08:55 
10:08:55 ********************
10:08:55 Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
10:25:10 
10:25:10 1 warning(s) in tests
10:25:10 ********************
10:25:10 Failed Tests (1):
10:25:10   Clang-Unit :: Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException
10:25:10

Does an asan-build run clean for everyone else?

Can you share your cmake config line, the target triple and architecture?

Can you share your cmake config line, the target triple and architecture?

CC='/mycompiler/compiler-clang/bin/clang -march=corei7' CXX='/mycompiler/compiler-clang/bin/clang++ -march=corei7' LDFLAGS='-L/mycompiler/compiler-clang/lib64 -Wl,--disable-new-dtags,-R/mycompiler/compiler-clang/lib/x86_64-unknown-linux-gnu/c++:/mycompiler/compiler-clang/lib64:/proj/bbi_twh/wh_bbi/x86_64-Linux3/z3/4.8.8-1/lib64 -ldl' cmake /repo/llvm --debug-trycompile -G Ninja -DCMAKE_MAKE_PROGRAM=ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_FLAGS='-Wno-error=unused-command-line-argument' -DCMAKE_CXX_FLAGS='-stdlib=libc++' -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_INSTALL_PREFIX=/compiler-clang -DLLVM_APPEND_VC_REV=OFF -DLLVM_CCACHE_DIR=/repo/.ccache -DLLVM_CCACHE_BUILD=ON -DLLVM_CCACHE_PROGRAM=/app/vbuild/RHEL7-x86_64/ccache/3.2.2/bin/ccache -DLLVM_CCACHE_NOHASHDIR=ON -DLLVM_CCACHE_BASEDIR=/repo -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_ENABLE_WERROR=ON -DLLVM_ENABLE_PROJECTS='clang;clang-tools-extra;lld' -DLLVM_ENABLE_Z3_SOLVER=ON -DLLVM_Z3_INSTALL_DIR=/proj/bbi_twh/wh_bbi/x86_64-Linux3/z3/4.8.8-1 -DLLVM_LIT_ARGS='-sv --threads 14' -DLLVM_ENABLE_PLUGINS=OFF -DLLVM_USE_SANITIZER='Address' -DLLVM_ENABLE_LIBPFM=OFF -DLLVM_ENABLE_LIBXML2=OFF -DLLVM_ENABLE_TERMINFO=OFF -DLLVM_STATIC_LINK_CXX_STDLIB=ON -DCLANG_ENABLE_ARCMT=OFF -DCLANG_TOOLING_BUILD_AST_INTROSPECTION=OFF -DCLANG_ROUND_TRIP_CC1_ARGS=OFF

If we run without z3 it still fails, but in another way:

[ RUN      ] InterpreterTest.CatchException
JIT session error: Symbols not found: [ __gxx_personality_v0, _ZSt9terminatev, _ZTVN10__cxxabiv117__class_type_infoE, __cxa_allocate_exception, __cxa_begin_catch, __cxa_end_catch, __cxa_free_exception ]
Failure value returned from cantFail wrapped call
Failed to materialize symbols: { (main, { __clang_call_terminate, _ZN16custom_exceptionC2EPKc, throw_exception, _ZTS16custom_exception, _ZTI16custom_exception }) }
UNREACHABLE executed at /repo/uabelho/master-github/llvm/include/llvm/Support/Error.h:788!
 #0 0x000000000051c0fb backtrace /repo/uabelho/flacc_6.144/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:4205:13
 #1 0x000000000186f144 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /repo/uabelho/master-github/llvm/build-all-bbisdk-asan/../lib/Support/Unix/Signals.inc:565:13
 #2 0x0000000001867cf8 llvm::sys::RunSignalHandlers() /repo/uabelho/master-github/llvm/build-all-bbisdk-asan/../lib/Support/Signals.cpp:0:5
 #3 0x000000000186ff78 SignalHandler(int) /repo/uabelho/master-github/llvm/build-all-bbisdk-asan/../lib/Support/Unix/Signals.inc:0:3
 #4 0x00007fdcac5bd630 __restore_rt sigaction.c:0:0
 #5 0x00007fdcab8e0387 raise (/lib64/libc.so.6+0x36387)
 #6 0x00007fdcab8e1a78 abort (/lib64/libc.so.6+0x37a78)
 #7 0x0000000001708875 operator<< /repo/uabelho/master-github/llvm/build-all-bbisdk-asan/../include/llvm/Support/raw_ostream.h:221:7
 #8 0x0000000001708875 operator<< /repo/uabelho/master-github/llvm/build-all-bbisdk-asan/../include/llvm/Support/raw_ostream.h:231:18
 #9 0x0000000001708875 llvm::llvm_unreachable_internal(char const*, char const*, unsigned int) /repo/uabelho/master-github/llvm/build-all-bbisdk-asan/../lib/Support/ErrorHandling.cpp:204:19
#10 0x0000000000590b65 unsigned long llvm::cantFail<unsigned long>(llvm::Expected<unsigned long>, char const*) /repo/uabelho/master-github/llvm/include/llvm/Support/Error.h:777:12
#11 0x000000000058e5b8 (anonymous namespace)::InterpreterTest_CatchException_Test::TestBody() /repo/uabelho/master-github/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:0:0
#12 0x00000000018abce8 os_stack_trace_getter /repo/uabelho/master-github/llvm/build-all-bbisdk-asan/../utils/unittest/googletest/src/gtest.cc:5635:7
#13 0x00000000018abce8 testing::Test::Run() /repo/uabelho/master-github/llvm/build-all-bbisdk-asan/../utils/unittest/googletest/src/gtest.cc:2515:9
#14 0x00000000018aeb03 testing::TestInfo::Run() /repo/uabelho/master-github/llvm/build-all-bbisdk-asan/../utils/unittest/googletest/src/gtest.cc:0:11
#15 0x00000000018aff71 testing::TestSuite::Run() /repo/uabelho/master-github/llvm/build-all-bbisdk-asan/../utils/unittest/googletest/src/gtest.cc:0:28
#16 0x00000000018dfaa9 testing::internal::UnitTestImpl::RunAllTests() /repo/uabelho/master-github/llvm/build-all-bbisdk-asan/../utils/unittest/googletest/src/gtest.cc:0:44
#17 0x00000000018ddc8e testing::UnitTest::Run() /repo/uabelho/master-github/llvm/build-all-bbisdk-asan/../utils/unittest/googletest/src/gtest.cc:4925:10
#18 0x000000000188fbce main /repo/uabelho/master-github/llvm/build-all-bbisdk-asan/../utils/unittest/UnitTestMain/TestMain.cpp:50:3
#19 0x00007fdcab8cc555 __libc_start_main (/lib64/libc.so.6+0x22555)
#20 0x00000000004e59c7 _start (/repo/uabelho/master-github/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests+0x4e59c7)

Can you share your cmake config line, the target triple and architecture?

CC='/mycompiler/compiler-clang/bin/clang -march=corei7' CXX='/mycompiler/compiler-clang/bin/clang++ -march=corei7' LDFLAGS='-L/mycompiler/compiler-clang/lib64 -Wl,--disable-new-dtags,-R/mycompiler/compiler-clang/lib/x86_64-unknown-linux-gnu/c++:/mycompiler/compiler-clang/lib64:/proj/bbi_twh/wh_bbi/x86_64-Linux3/z3/4.8.8-1/lib64 -ldl' cmake /repo/llvm --debug-trycompile -G Ninja -DCMAKE_MAKE_PROGRAM=ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_FLAGS='-Wno-error=unused-command-line-argument' -DCMAKE_CXX_FLAGS='-stdlib=libc++' -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_INSTALL_PREFIX=/compiler-clang -DLLVM_APPEND_VC_REV=OFF -DLLVM_CCACHE_DIR=/repo/.ccache -DLLVM_CCACHE_BUILD=ON -DLLVM_CCACHE_PROGRAM=/app/vbuild/RHEL7-x86_64/ccache/3.2.2/bin/ccache -DLLVM_CCACHE_NOHASHDIR=ON -DLLVM_CCACHE_BASEDIR=/repo -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_ENABLE_WERROR=ON -DLLVM_ENABLE_PROJECTS='clang;clang-tools-extra;lld' -DLLVM_ENABLE_Z3_SOLVER=ON -DLLVM_Z3_INSTALL_DIR=/proj/bbi_twh/wh_bbi/x86_64-Linux3/z3/4.8.8-1 -DLLVM_LIT_ARGS='-sv --threads 14' -DLLVM_ENABLE_PLUGINS=OFF -DLLVM_USE_SANITIZER='Address' -DLLVM_ENABLE_LIBPFM=OFF -DLLVM_ENABLE_LIBXML2=OFF -DLLVM_ENABLE_TERMINFO=OFF -DLLVM_STATIC_LINK_CXX_STDLIB=ON -DCLANG_ENABLE_ARCMT=OFF -DCLANG_TOOLING_BUILD_AST_INTROSPECTION=OFF -DCLANG_ROUND_TRIP_CC1_ARGS=OFF

Can you also paste the configure output of cmake?

Can you also paste the configure output of cmake?

Attached in file.txt

rnk added a subscriber: dblaikie.Nov 1 2021, 10:37 AM

So, to back up a bit, do I understand correctly that this change adds tests to the check-clang test suite that JIT compiles C++ code for the host and throws C++ exceptions? Can we reconsider that?

We have a policy of not running execution tests in the clang test suite because we know they always turn out to be unreliable, flaky, and highly dependent on the environment rather than the code under test. Integration tests are great, everyone needs them, but they should definitely not be part of the default set of tests that developers run anywhere and everywhere and expect to work out of the box, regardless of the environment. +@dblaikie to confirm if I am off base here about our testing strategy, and maybe Lang can advise us about past JIT testing strategies.

Hi,

We're seeing a problem with this patch in our downstream (not public) buildbots. With an asan-built compiler we see the following:

...
10:08:55 [ RUN      ] InterpreterTest.CatchException
10:08:55 libunwind: __unw_add_dynamic_fde: bad fde: FDE is really a CIE
10:08:55 libc++abi: terminating with uncaught exception of type custom_exception
...

I suspect that this is a Linux distro that's using libunwind as the unwinder, at least for this test. Linux distros typically use libgcc_s for unwinding. The two libraries have different behavior for their __register_frame functions: libunwind's function expects to be passed a single FDE, libgcc_s's expects to be passed an entire .eh_frame section. We try to guess the unwinder in the JIT and use the appropriate argument (see [1][2]), but when we get it wrong this is often the result: we try to pass an .eh-frame section to libunwind's __register_frame and it errors out on a CIE at the start of the section.

@uabelho -- In your setup I'm seeing:

-- Looking for __unw_add_dynamic_fde
-- Looking for __unw_add_dynamic_fde - not found

So the question is, how are we failing to find __unw_add_dynamic_fde during config, only to crash in it during the test? Is use of libunwind on your platform expected?

side note: Peter Housel recently posted https://reviews.llvm.org/D111863 to add a new registration API with consistent behavior. Hopefully in the future we can rely on dynamic detection of this to eliminate the issue for users of future libunwinds.

So, to back up a bit, do I understand correctly that this change adds tests to the check-clang test suite that JIT compiles C++ code for the host and throws C++ exceptions? Can we reconsider that?

We have a policy of not running execution tests in the clang test suite because we know they always turn out to be unreliable, flaky, and highly dependent on the environment rather than the code under test. Integration tests are great, everyone needs them, but they should definitely not be part of the default set of tests that developers run anywhere and everywhere and expect to work out of the box, regardless of the environment. +@dblaikie to confirm if I am off base here about our testing strategy, and maybe Lang can advise us about past JIT testing strategies.

The JIT has always used execution tests. It's difficult to test the JIT properly without them, since it doesn't have persistent output.

In practice the trick has always been to tighten the criteria for where these can run until things stabilize. We're seeing more failures on the test cases that Vassil is writing because he's pushing the boundaries of ORC's feature set, but I think the solution should still be the same: fix the JIT when we find bugs there, and restrict the tests to environments where they're expected to pass.

[1] https://github.com/llvm/llvm-project/commit/957334382cd12ec07b46c0ddfdcc220731f6d80f
[2] https://llvm.org/PR44074

So, to back up a bit, do I understand correctly that this change adds tests to the check-clang test suite that JIT compiles C++ code for the host and throws C++ exceptions? Can we reconsider that?

We have a policy of not running execution tests in the clang test suite because we know they always turn out to be unreliable, flaky, and highly dependent on the environment rather than the code under test. Integration tests are great, everyone needs them, but they should definitely not be part of the default set of tests that developers run anywhere and everywhere and expect to work out of the box, regardless of the environment. +@dblaikie to confirm if I am off base here about our testing strategy, and maybe Lang can advise us about past JIT testing strategies.

Yeah, seems we've had some precedent for this for a while, as @lhames says (doing some archaeology I see one of the clang-interpreter tests added here, for instance: https://github.com/llvm/llvm-project/commit/1d487617f20ab9df65ab60d6cf9431ef288312ab

I push back pretty hard on end-to-end tests in clang in most cases - one place that slips through either untested or with end to end tests is anything in MCOptions, etc, since they aren't in IR and are only observable through their effect on the resulting assembly. And JITs - I didn't actually know we had live JIT tests up in clang - and yeah, would rather we didn't - perhaps via a mode in such frontends that emits the IR at various points, but it'll be a pretty significant investment to make something fairly robust, I would imagine (sort of like lldb reproducers replays (which are being removed due to unmaintainability, by the sounds of it) but being able to stub out the actual execution) - hmm, perhaps a remote execution mock would be feasible? That'd at least remove the execution part, make it portable to run on non-native environments - but wouldn't remove the end-to-end-ness, for that maybe you'd need a mock target that worked as though it ran IR natively, perhaps? Not sure.

Then maybe the more end-to-end-y tests could move out to cross-project-tests.

Hi,

We're seeing a problem with this patch in our downstream (not public) buildbots. With an asan-built compiler we see the following:

...
10:08:55 [ RUN      ] InterpreterTest.CatchException
10:08:55 libunwind: __unw_add_dynamic_fde: bad fde: FDE is really a CIE
10:08:55 libc++abi: terminating with uncaught exception of type custom_exception
...

I suspect that this is a Linux distro that's using libunwind as the unwinder, at least for this test. Linux distros typically use libgcc_s for unwinding. The two libraries have different behavior for their __register_frame functions: libunwind's function expects to be passed a single FDE, libgcc_s's expects to be passed an entire .eh_frame section. We try to guess the unwinder in the JIT and use the appropriate argument (see [1][2]), but when we get it wrong this is often the result: we try to pass an .eh-frame section to libunwind's __register_frame and it errors out on a CIE at the start of the section.

@uabelho -- In your setup I'm seeing:

-- Looking for __unw_add_dynamic_fde
-- Looking for __unw_add_dynamic_fde - not found

So the question is, how are we failing to find __unw_add_dynamic_fde during config, only to crash in it during the test? Is use of libunwind on your platform expected?

Thanks for your reply. Unfortunately I'm not really sure how it's configured or what is expected. I'll try to involve someone who knows.

lhames added a comment.Nov 2 2021, 1:16 PM

So, to back up a bit, do I understand correctly that this change adds tests to the check-clang test suite that JIT compiles C++ code for the host and throws C++ exceptions? Can we reconsider that?

We have a policy of not running execution tests in the clang test suite because we know they always turn out to be unreliable, flaky, and highly dependent on the environment rather than the code under test. Integration tests are great, everyone needs them, but they should definitely not be part of the default set of tests that developers run anywhere and everywhere and expect to work out of the box, regardless of the environment. +@dblaikie to confirm if I am off base here about our testing strategy, and maybe Lang can advise us about past JIT testing strategies.

Yeah, seems we've had some precedent for this for a while, as @lhames says (doing some archaeology I see one of the clang-interpreter tests added here, for instance: https://github.com/llvm/llvm-project/commit/1d487617f20ab9df65ab60d6cf9431ef288312ab

Also all of the MJCIT and Orc tests in LLVM.

I push back pretty hard on end-to-end tests in clang in most cases - one place that slips through either untested or with end to end tests is anything in MCOptions, etc, since they aren't in IR and are only observable through their effect on the resulting assembly. And JITs - I didn't actually know we had live JIT tests up in clang - and yeah, would rather we didn't - perhaps via a mode in such frontends that emits the IR at various points, but it'll be a pretty significant investment to make something fairly robust, I would imagine (sort of like lldb reproducers replays (which are being removed due to unmaintainability, by the sounds of it) but being able to stub out the actual execution) - hmm, perhaps a remote execution mock would be feasible? That'd at least remove the execution part, make it portable to run on non-native environments - but wouldn't remove the end-to-end-ness, for that maybe you'd need a mock target that worked as though it ran IR natively, perhaps? Not sure.

The LLVM MCJIT and Orc tests have been stable enough once we constrained the environments that they run in. I don't see why this would be any less stable. The features being exercised (essentially the object runtimes) are likely to be more stable in practice than the environments being tested in the LLDB tests.

Then maybe the more end-to-end-y tests could move out to cross-project-tests.

Are you suggesting execution tests for cross-project-tests?

So, to back up a bit, do I understand correctly that this change adds tests to the check-clang test suite that JIT compiles C++ code for the host and throws C++ exceptions? Can we reconsider that?

We have a policy of not running execution tests in the clang test suite because we know they always turn out to be unreliable, flaky, and highly dependent on the environment rather than the code under test. Integration tests are great, everyone needs them, but they should definitely not be part of the default set of tests that developers run anywhere and everywhere and expect to work out of the box, regardless of the environment. +@dblaikie to confirm if I am off base here about our testing strategy, and maybe Lang can advise us about past JIT testing strategies.

Yeah, seems we've had some precedent for this for a while, as @lhames says (doing some archaeology I see one of the clang-interpreter tests added here, for instance: https://github.com/llvm/llvm-project/commit/1d487617f20ab9df65ab60d6cf9431ef288312ab

Also all of the MJCIT and Orc tests in LLVM.

I think @rnk was asking/drawing a distinction between execution tests in LLVM (which is at least only testing LLVM code in LLVM) and execution tests in Clang (which is both execution, and cross-project) - so I was just talking about the Clang->LLVM->execution tests. Though I think in LLVM there'd also be some opportunity to find some ways to isolate/test JIT infrastructure with less execution (had sort of hoped in the early days of Orc that the layered architecture would allow more isolated/unit testing to robustly test pieces without execution for that testing).

I push back pretty hard on end-to-end tests in clang in most cases - one place that slips through either untested or with end to end tests is anything in MCOptions, etc, since they aren't in IR and are only observable through their effect on the resulting assembly. And JITs - I didn't actually know we had live JIT tests up in clang - and yeah, would rather we didn't - perhaps via a mode in such frontends that emits the IR at various points, but it'll be a pretty significant investment to make something fairly robust, I would imagine (sort of like lldb reproducers replays (which are being removed due to unmaintainability, by the sounds of it) but being able to stub out the actual execution) - hmm, perhaps a remote execution mock would be feasible? That'd at least remove the execution part, make it portable to run on non-native environments - but wouldn't remove the end-to-end-ness, for that maybe you'd need a mock target that worked as though it ran IR natively, perhaps? Not sure.

The LLVM MCJIT and Orc tests have been stable enough once we constrained the environments that they run in. I don't see why this would be any less stable. The features being exercised (essentially the object runtimes) are likely to be more stable in practice than the environments being tested in the LLDB tests.

Then maybe the more end-to-end-y tests could move out to cross-project-tests.

Are you suggesting execution tests for cross-project-tests?

I think cross-project-tests (originally debuginfo-tests) has always had/been a place for execution tests. Not all of them, and still value in having lit-style non-execution cross-project tests where possible for more robust/variation testing where possible, with fewer full end-to-end-including-execution tests where possible (always going to be fuzzy lines of what's worth the cost/benefit, etc - and we're all going to have different points on that spectrum).

lhames added a comment.Nov 2 2021, 5:09 PM

Yeah, seems we've had some precedent for this for a while, as @lhames says (doing some archaeology I see one of the clang-interpreter tests added here, for instance: https://github.com/llvm/llvm-project/commit/1d487617f20ab9df65ab60d6cf9431ef288312ab

Also all of the MJCIT and Orc tests in LLVM.

I think @rnk was asking/drawing a distinction between execution tests in LLVM (which is at least only testing LLVM code in LLVM) and execution tests in Clang (which is both execution, and cross-project) - so I was just talking about the Clang->LLVM->execution tests. Though I think in LLVM there'd also be some opportunity to find some ways to isolate/test JIT infrastructure with less execution (had sort of hoped in the early days of Orc that the layered architecture would allow more isolated/unit testing to robustly test pieces without execution for that testing).

Orc has plenty of isolated unit testing and regression testing too, it's just that the nature of the project (dynamic compilation, and with relatively few resources dedicated to it) makes end to end testing attractive for now too. Perhaps in the long term we could build out enough mocking infrastructure to be confident that the JIT works without running any JIT'd code, but we're a fair way off that.

I think testing of the clang-repl is probably in a similar place: In theory you could build infrastructure to test out all the interesting states that a REPL could put your AST into. In practice we don't have the resources to do that yet, and without execution tests we're left with essentially no testing of the feature.

Execution testing seems like the right compromise for now, as long as we can make sure that there's not too much noise from the testers. The lesson from the LLVM execution tests is that this is possible without an excessive amount of work.

I think cross-project-tests (originally debuginfo-tests) has always had/been a place for execution tests. Not all of them, and still value in having lit-style non-execution cross-project tests where possible for more robust/variation testing where possible, with fewer full end-to-end-including-execution tests where possible (always going to be fuzzy lines of what's worth the cost/benefit, etc - and we're all going to have different points on that spectrum).

Yeah. Cross-project tests sounds like a great place to add more JIT tests, and maybe it's the correct home for clang-repl tests, but I think for now the right trade off is still to have execution tests and just restrict them to running on known-good configs.

clang/test/CMakeLists.txt