google / google/leveldb

Would you consider adding static/dynamic analysis (clang-tidy, ASan, UBSan) to CI?

Open
#1,276 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
39.4k
Forks
8.2k
PR merge metrics
No merged PRs in 30d

Description

Hi LevelDB maintainers,

First, thank you for your work on this project! I noticed that the current CI pipeline (`.github/workflows/build.yml`) includes comprehensive builds and tests across platforms/compilers, but it doesn’t seem to include:

1. **Static analysis** (e.g., `clang-tidy`, `cppcheck`)
- Could help catch potential code quality issues early (e.g., style violations, misuse of APIs).

2. **Advanced dynamic checks** (e.g., ASan, UBSan, TSan)
- ASan (AddressSanitizer) for memory errors.
- UBSan (UndefinedBehaviorSanitizer) for undefined behavior.
- TSan (ThreadSanitizer) for data races (if multithreading is used).

### Why this might be useful:
- These tools are widely used in C++ projects (e.g., Chromium, LLVM) to catch subtle bugs.
- They complement existing tests by focusing on **security and stability** issues that might not trigger test failures.
- Sanitizers are especially valuable for a storage engine like LevelDB, where memory safety and data integrity are critical.

### Example of potential additions:
```yaml
# Static analysis with clang-tidy
- name: Run clang-tidy
if: ${{ matrix.compiler == 'clang' }}
run: |
cmake -B "${{ env.CMAKE_BUILD_DIR }}" -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
run-clang-tidy -p "${{ env.CMAKE_BUILD_DIR }}"

# Dynamic analysis with sanitizers
env:
SANITIZER_FLAGS: "-fsanitize=address,undefined -fno-omit-frame-pointer"
CFLAGS: ${{ env.SANITIZER_FLAGS }}
CXXFLAGS: ${{ env.SANITIZER_FLAGS }}

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.