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

feat(bb): stack traces for check_circuit #6851

Merged
merged 16 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions barretenberg/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -287,3 +287,68 @@ python3 -m http.server <port>
```

and tunnel the port through ssh.

### Debugging Verifification Failures

The CicuitChecker::check_circuit function is used to get the gate index and block information about a failing circuit constraint.
If you are in a scenario where you have a failing call to check_circuit and wish to get more information out of it than just the gate index, you can use this feature to get a stack trace, see example below.

Usage instructions:
- On ubuntu (or our mainframe accounts) use `sudo apt-get install libdw-dev` to support trace printing
- Use `cmake --preset clang16-dbg-fast-circuit-check-traces` and `cmake --build --preset clang16-dbg-fast-circuit-check-traces` to enable the backward-cpp dependency through the CHECK_CIRCUIT_STACKTRACES CMake variable.
- Run any case where you have a failing check_circuit call, you will now have a stack trace illuminating where this constraint was added in code.

Caveats:
- This works best for code that is not overly generic, i.e. where just the sequence of function calls carries a lot of information. It is possible to tag extra data along with the stack trace, this can be done as a followup, please leave feedback if desired.
- There are certain functions like `assert_equals` that can cause gates that occur _before_ them to fail. If this would be useful to automatically report, please leave feedback.

Example:
```
[ RUN ] standard_circuit_constructor.test_check_circuit_broken
Stack trace (most recent call last):
#4 Source "_deps/gtest-src/googletest/src/gtest.cc", line 2845, in Run
2842: if (!Test::HasFatalFailure() && !Test::IsSkipped()) {
2843: // This doesn't throw as all user code that can throw are wrapped into
2844: // exception handling code.
>2845: test->Run();
2846: }
2847:
2848: if (test != nullptr) {
#3 Source "_deps/gtest-src/googletest/src/gtest.cc", line 2696, in Run
2693: // GTEST_SKIP().
2694: if (!HasFatalFailure() && !IsSkipped()) {
2695: impl->os_stack_trace_getter()->UponLeavingGTest();
>2696: internal::HandleExceptionsInMethodIfSupported(this, &Test::TestBody,
2697: "the test body");
2698: }
#2 | Source "_deps/gtest-src/googletest/src/gtest.cc", line 2657, in HandleSehExceptionsInMethodIfSupported<testing::Test, void>
| 2655: #if GTEST_HAS_EXCEPTIONS
| 2656: try {
| >2657: return HandleSehExceptionsInMethodIfSupported(object, method, location);
| 2658: } catch (const AssertionException&) { // NOLINT
| 2659: // This failure was reported already.
Source "_deps/gtest-src/googletest/src/gtest.cc", line 2621, in HandleExceptionsInMethodIfSupported<testing::Test, void>
2618: }
2619: #else
2620: (void)location;
>2621: return (object->*method)();
2622: #endif // GTEST_HAS_SEH
2623: }
#1 Source "/mnt/user-data/adam/aztec-packages/barretenberg/cpp/src/barretenberg/circuit_checker/standard_circuit_builder.test.cpp", line 464, in TestBody
461: uint32_t d_idx = circuit_constructor.add_variable(d);
462: circuit_constructor.create_add_gate({ a_idx, b_idx, c_idx, fr::one(), fr::one(), fr::neg_one(), fr::zero() });
463:
> 464: circuit_constructor.create_add_gate({ d_idx, c_idx, a_idx, fr::one(), fr::neg_one(), fr::neg_one(), fr::zero() });
465:
466: bool result = CircuitChecker::check(circuit_constructor);
467: EXPECT_EQ(result, false);
#0 Source "/mnt/user-data/adam/aztec-packages/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/standard_circuit_builder.cpp", line 22, in create_add_gate
19: {
20: this->assert_valid_variables({ in.a, in.b, in.c });
21:
> 22: blocks.arithmetic.populate_wires(in.a, in.b, in.c);
23: blocks.arithmetic.q_m().emplace_back(FF::zero());
24: blocks.arithmetic.q_1().emplace_back(in.a_scaling);
25: blocks.arithmetic.q_2().emplace_back(in.b_scaling);
gate number4
```
10 changes: 9 additions & 1 deletion barretenberg/cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ configure_file(
# Add doxygen build command
find_package(Doxygen)
if (DOXYGEN_FOUND)
add_custom_target(build_docs
add_custom_target(build_docs
COMMAND ${DOXYGEN_EXECUTABLE} ${PROJECT_SOURCE_DIR}/docs/Doxyfile
WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
COMMENT "Generate documentation with Doxygen")
Expand All @@ -34,6 +34,8 @@ option(DISABLE_TBB "Intel Thread Building Blocks" ON)
option(COVERAGE "Enable collecting coverage from tests" OFF)
option(ENABLE_ASAN "Address sanitizer for debugging tricky memory corruption" OFF)
option(ENABLE_HEAVY_TESTS "Enable heavy tests when collecting coverage" OFF)
# Note: Must do 'sudo apt-get install libdw-dev' or equivalent
option(CHECK_CIRCUIT_STACKTRACES "Enable (slow) stack traces for check circuit" OFF)

if(CMAKE_SYSTEM_PROCESSOR MATCHES "aarch64" OR CMAKE_SYSTEM_PROCESSOR MATCHES "arm64")
message(STATUS "Compiling for ARM.")
Expand All @@ -45,6 +47,10 @@ if(CMAKE_SYSTEM_PROCESSOR MATCHES "aarch64" OR CMAKE_SYSTEM_PROCESSOR MATCHES "a
set(DISABLE_TBB 0)
endif()

if(CHECK_CIRCUIT_STACKTRACES)
add_compile_options(-DCHECK_CIRCUIT_STACKTRACES)
endif()

if(ENABLE_ASAN)
add_compile_options(-fsanitize=address)
add_link_options(-fsanitize=address)
Expand Down Expand Up @@ -136,6 +142,8 @@ include(cmake/gtest.cmake)
include(cmake/benchmark.cmake)
include(cmake/module.cmake)
include(cmake/msgpack.cmake)
include(cmake/backward-cpp.cmake)

add_subdirectory(src)
if (ENABLE_ASAN AND NOT(FUZZING))
find_program(LLVM_SYMBOLIZER_PATH NAMES llvm-symbolizer-16)
Expand Down
22 changes: 16 additions & 6 deletions barretenberg/cpp/CMakePresets.json
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,16 @@
"LDFLAGS": "-O2 -gdwarf-4"
}
},
{
"name": "clang16-dbg-fast-circuit-check-traces",
"displayName": "Optimized debug build with Clang-16 with stack traces for failing circuit checks",
"description": "Build with globally installed Clang-16 in optimized debug mode with stack traces for failing circuit checks",
"inherits": "clang16-dbg-fast",
"binaryDir": "build-debug-fast-circuit-check-traces",
"cacheVariables": {
"CHECK_CIRCUIT_STACKTRACES": "ON"
}
},
{
"name": "clang16-assert",
"binaryDir": "build-assert",
Expand Down Expand Up @@ -405,6 +415,11 @@
"inherits": "default",
"configurePreset": "clang16-dbg-fast"
},
{
"name": "clang16-dbg-fast-circuit-check-traces",
"inherits": "clang16-dbg-fast",
"configurePreset": "clang16-dbg-fast-circuit-check-traces"
},
{
"name": "clang16-assert",
"inherits": "default",
Expand Down Expand Up @@ -480,12 +495,7 @@
"configurePreset": "wasm",
"inheritConfigureEnvironment": true,
"jobs": 0,
"targets": [
"barretenberg.wasm",
"barretenberg",
"wasi",
"env"
]
"targets": ["barretenberg.wasm", "barretenberg", "wasi", "env"]
},
{
"name": "wasm-dbg",
Expand Down
11 changes: 11 additions & 0 deletions barretenberg/cpp/cmake/backward-cpp.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
if(CHECK_CIRCUIT_STACKTRACES)
include(FetchContent)

# Also requires one of: libbfd (gnu binutils), libdwarf, libdw (elfutils)
FetchContent_Declare(backward
GIT_REPOSITORY https://github.com/bombela/backward-cpp
GIT_TAG 51f0700452cf71c57d43c2d028277b24cde32502
SYSTEM # optional, the Backward include directory will be treated as system directory
)
FetchContent_MakeAvailable(backward)
endif()
38 changes: 38 additions & 0 deletions barretenberg/cpp/cmake/module.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,19 @@ function(barretenberg_module MODULE_NAME)
${TBB_IMPORTED_TARGETS}
)

if(CHECK_CIRCUIT_STACKTRACES)
target_link_libraries(
${MODULE_NAME}_objects
PUBLIC
Backward::Interface
)
target_link_options(
${MODULE_NAME}
PRIVATE
-ldw -lelf
)
endif()

# enable msgpack downloading via dependency (solves race condition)
add_dependencies(${MODULE_NAME} msgpack-c)
add_dependencies(${MODULE_NAME}_objects msgpack-c)
Expand Down Expand Up @@ -88,6 +101,19 @@ function(barretenberg_module MODULE_NAME)
)
list(APPEND exe_targets ${MODULE_NAME}_tests)

if(CHECK_CIRCUIT_STACKTRACES)
target_link_libraries(
${MODULE_NAME}_test_objects
PUBLIC
Backward::Interface
)
target_link_options(
${MODULE_NAME}_tests
PRIVATE
-ldw -lelf
)
endif()

if(WASM)
target_link_options(
${MODULE_NAME}_tests
Expand Down Expand Up @@ -229,6 +255,18 @@ function(barretenberg_module MODULE_NAME)
benchmark::benchmark
${TBB_IMPORTED_TARGETS}
)
if(CHECK_CIRCUIT_STACKTRACES)
target_link_libraries(
${BENCHMARK_NAME}_bench_objects
PUBLIC
Backward::Interface
)
target_link_options(
${BENCHMARK_NAME}_bench
PRIVATE
-ldw -lelf
)
endif()

# enable msgpack downloading via dependency (solves race condition)
add_dependencies(${BENCHMARK_NAME}_bench_objects msgpack-c)
Expand Down
12 changes: 12 additions & 0 deletions barretenberg/cpp/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -188,4 +188,16 @@ if(WASM)
-nostartfiles -Wl,--no-entry,--export-dynamic
)

if(CHECK_CIRCUIT_STACKTRACES)
target_link_libraries(
barretenberg.wasm
PUBLIC
Backward::Interface
)
target_link_options(
barretenberg.wasm
PRIVATE
-ldw -lelf
)
endif()
endif()
12 changes: 12 additions & 0 deletions barretenberg/cpp/src/barretenberg/bb/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,16 @@ if (NOT(FUZZING))
barretenberg
env
)
if(CHECK_CIRCUIT_STACKTRACES)
target_link_libraries(
bb
PUBLIC
Backward::Interface
)
target_link_options(
bb
PRIVATE
-ldw -lelf
)
endif()
endif()
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ class StandardCircuitChecker {
FF gate_sum = block.q_m()[i] * left * right + block.q_1()[i] * left + block.q_2()[i] * right +
block.q_3()[i] * output + block.q_c()[i];
if (!gate_sum.is_zero()) {
#ifdef CHECK_CIRCUIT_STACKTRACES
block.stack_traces.print(i);
#endif
info("gate number", i);
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ TEST(ultra_circuit_constructor, sort_with_edges_gate)
fr h = fr(8);

{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();
UltraCircuitBuilder circuit_constructor;
auto a_idx = circuit_constructor.add_variable(a);
auto b_idx = circuit_constructor.add_variable(b);
auto c_idx = circuit_constructor.add_variable(c);
Expand All @@ -339,7 +339,7 @@ TEST(ultra_circuit_constructor, sort_with_edges_gate)
}

{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();
UltraCircuitBuilder circuit_constructor;
auto a_idx = circuit_constructor.add_variable(a);
auto b_idx = circuit_constructor.add_variable(b);
auto c_idx = circuit_constructor.add_variable(c);
Expand All @@ -355,7 +355,7 @@ TEST(ultra_circuit_constructor, sort_with_edges_gate)
EXPECT_EQ(result, false);
}
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();
UltraCircuitBuilder circuit_constructor;
auto a_idx = circuit_constructor.add_variable(a);
auto b_idx = circuit_constructor.add_variable(b);
auto c_idx = circuit_constructor.add_variable(c);
Expand All @@ -371,7 +371,7 @@ TEST(ultra_circuit_constructor, sort_with_edges_gate)
EXPECT_EQ(result, false);
}
{
UltraCircuitBuilder circuit_constructor = UltraCircuitBuilder();
UltraCircuitBuilder circuit_constructor;
auto a_idx = circuit_constructor.add_variable(a);
auto c_idx = circuit_constructor.add_variable(c);
auto d_idx = circuit_constructor.add_variable(d);
Expand Down
Loading
Loading