Skip to content

Commit

Permalink
Add --enable-sanitizers (not on by default yet)
Browse files Browse the repository at this point in the history
This way we at least get unit test coverage (which...
our unit test coverage doesn't do much because our
main code paths require privileges or virt).

One main blocker to this is that rustc doesn't expose
first-class support for this yet:
rust-lang/rust#39699

At a practical level this works when building in release
mode but fails with `cargo test` for some reason; linker
arguments being pruned?  Not sure.

So I was able to use this when composing to find a bug,
but then for some other reason the client
side apparently infinite loops inside libsolv.

So we're not enabling this yet for those reasons, but
let's land the build infrastructure now.

```
(lldb) thread backtrace
* thread #4, name = 'pool-/usr/bin/r'
  * frame #0: 0x00007fd61b97200f libc.so.6`__memcpy_sse2_unaligned_erms + 623
    frame #1: 0x00007fd61cbc88e6 libasan.so.6`__asan::asan_realloc(void*, unsigned long, __sanitizer::BufferedStackTrace*) + 214
    frame #2: 0x00007fd61cc4b725 libasan.so.6`__interceptor_realloc + 245
    frame #3: 0x00007fd61baec43e libsolv.so.1`solv_realloc + 30
    frame #4: 0x00007fd61baf0414 libsolv.so.1`repodata_add_dirstr + 276
    frame #5: 0x00007fd61bb6f755 libsolvext.so.1`end_element + 53
    frame #6: 0x00007fd61b05855d libxml2.so.2`xmlParseEndTag1.constprop.0 + 317
    frame #7: 0x00007fd61b063548 libxml2.so.2`xmlParseTryOrFinish.isra.0 + 888
    frame #8: 0x00007fd61af7ed20 libxml2.so.2`xmlParseChunk + 560
    frame #9: 0x00007fd61bb727e7 libsolvext.so.1`solv_xmlparser_parse + 183
    frame #10: 0x00007fd61bb5ea0e libsolvext.so.1`repo_add_rpmmd + 254
    frame #11: 0x000055a4fce7a5f5 rpm-ostree`::load_filelists_cb(repo=<unavailable>, fp=<unavailable>) at dnf-sack.cpp:444:23
    frame #12: 0x000055a4fce7cad6 rpm-ostree`load_ext(_DnfSack*, libdnf::Repo*, _hy_repo_repodata, char const*, char const*, int (*)(s_Repo*, _IO_FILE*), _GError**) at dnf-sack.cpp:430:13
    frame #13: 0x000055a4fce7df60 rpm-ostree`dnf_sack_load_repo at dnf-sack.cpp:1789:26
    frame #14: 0x000055a4fce7eee9 rpm-ostree`dnf_sack_add_repo at dnf-sack.cpp:2217:28
    frame #15: 0x000055a4fce7f0fb rpm-ostree`dnf_sack_add_repos at dnf-sack.cpp:2271:32
    frame #16: 0x000055a4fce870ee rpm-ostree`dnf_context_setup_sack_with_flags at dnf-context.cpp:1796:29
    frame #17: 0x000055a4fcdf757f rpm-ostree`rpmostree_context_download_metadata at rpmostree-core.cxx:1206:44
    frame #18: 0x000055a4fcdf95c3 rpm-ostree`rpmostree_context_prepare at rpmostree-core.cxx:2001:48
    frame #19: 0x000055a4fce54ab7 rpm-ostree`rpmostree_sysroot_upgrader_prep_layering at rpmostree-sysroot-upgrader.cxx:1018:38
    frame #20: 0x000055a4fcdcb143 rpm-ostree`deploy_transaction_execute(_RpmostreedTransaction*, _GCancellable*, _GError**) at rpmostreed-transaction-types.cxx:1445:49
    frame #21: 0x000055a4fcdba4cd rpm-ostree`transaction_execute_thread(_GTask*, void*, void*, _GCancellable*) at rpmostreed-transaction.cxx:340:34
    frame #22: 0x00007fd61c58f7e2 libgio-2.0.so.0`g_task_thread_pool_thread + 114
    frame #23: 0x00007fd61c3d7e54 libglib-2.0.so.0`g_thread_pool_thread_proxy.lto_priv.0 + 116
    frame #24: 0x00007fd61c3d52b2 libglib-2.0.so.0`g_thread_proxy + 82
    frame #25: 0x00007fd61b8af3f9 libpthread.so.0`start_thread + 233
    frame #26: 0x00007fd61b9c9903 libc.so.6`__clone + 67
(lldb)
```
  • Loading branch information
cgwalters committed Feb 10, 2021
1 parent 25fa1ba commit a3c09a7
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 12 deletions.
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,7 @@ lto = true
[features]
sqlite-rpmdb-default = []
fedora-integration = []
# ASAN+UBSAN
sanitizers = []

default = []
9 changes: 7 additions & 2 deletions Makefile-rpm-ostree.am
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ rpmostree_common_libs = libglnx.la librpmostree-1.la librpmostreecxxrs.la

rpmostree_bin_common_libs = librpmostreeinternals.la $(rpmostree_common_libs)
librpmostreeinternals_la_CFLAGS = $(AM_CFLAGS) $(rpmostree_common_cflags)
librpmostreeinternals_la_CXXFLAGS = $(AM_CXXFLAGS) $(rpmostree_common_cflags)
# Note for now we only inject the sanitizer flags into our static library,
# because doing ASAN for a shared library is trickier.
librpmostreeinternals_la_CXXFLAGS = $(AM_CXXFLAGS) $(sanitizer_flags) $(rpmostree_common_cflags)
librpmostreeinternals_la_LIBADD = $(rpmostree_common_libs)

privdatadir=$(pkglibdir)
Expand All @@ -101,6 +103,9 @@ endif
if BUILDOPT_ENABLE_SQLITE_RPMDB_DEFAULT
cargo_build += --features sqlite-rpmdb-default
endif
if BUILDOPT_ASAN
cargo_build += --features sanitizers
endif

if RUST_DEBUG
cargo_target_dir=debug
Expand Down Expand Up @@ -136,7 +141,7 @@ endif
noinst_LTLIBRARIES += librpmostreecxxrs.la
librpmostreecxxrs_la_SOURCES = rpmostree-cxxrs.h rpmostree-cxxrs.cxx
# Suppress missing-declarations because https://github.com/dtolnay/cxx/issues/590
librpmostreecxxrs_la_CXXFLAGS = $(AM_CXXFLAGS) $(rpmostree_common_cflags) -Wno-missing-declarations
librpmostreecxxrs_la_CXXFLAGS = $(AM_CXXFLAGS) $(SANITIZER_FLAGS) $(rpmostree_common_cflags) -Wno-missing-declarations
librpmostreecxxrs_la_LIBADD = -lstdc++
GITIGNOREFILES += $(binding_generated_sources)
BUILT_SOURCES += $(binding_generated_sources)
Expand Down
6 changes: 5 additions & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,14 @@ endif
warnings_error_only_c = strict-prototypes missing-prototypes \
implicit-function-declaration int-conversion incompatible-pointer-types \
$(NULL)
sanitizer_flags =
if BUILDOPT_ASAN
sanitizer_flags += -fsanitize=address -fsanitize=undefined -fsanitize-undefined-trap-on-error
endif
# See the AM_CFLAGS in libostree for more information about -fno-strict-aliasing
AM_CFLAGS += -std=gnu11 -fno-strict-aliasing $(warning_flags) $(patsubst %,-Werror=%,$(warnings_error_only_c))
# Our default CXX flags
AM_CXXFLAGS += -std=c++17 -fno-strict-aliasing $(warning_flags)
AM_CXXFLAGS += -std=c++17 -fno-strict-aliasing $(warning_flags) $(sanitizer_flags)

EXTRA_DIST += autogen.sh COPYING

Expand Down
5 changes: 5 additions & 0 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ fn detect_fedora_feature() -> Result<()> {
}

fn main() -> Result<()> {
if std::env::var("CARGO_FEATURE_SANITIZERS").is_ok() {
// Force these on
println!("cargo:rustc-link-lib=ubsan");
println!("cargo:rustc-link-lib=asan");
}
let cwd = std::env::current_dir()?;
let cwd = cwd.to_str().expect("utf8 pwd");
println!("cargo:rustc-link-search={}/.libs", cwd);
Expand Down
14 changes: 6 additions & 8 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,11 @@ dnl if not set, which we definitely want; cmake doesn't do that.
AC_PROG_CXX
AM_PROG_CC_C_O

AC_MSG_CHECKING([for -fsanitize=address in CFLAGS])
if echo $CFLAGS | grep -q -e -fsanitize=address; then
AC_MSG_RESULT([yes])
using_asan=yes
else
AC_MSG_RESULT([no])
fi
AM_CONDITIONAL(BUILDOPT_ASAN, [test x$using_asan = xyes])
AC_ARG_ENABLE(sanitizers,
AS_HELP_STRING([--enable-sanitizers],
[Enable ASAN and UBSAN (default: no)]),,
[enable_sanitizers=no])
AM_CONDITIONAL(BUILDOPT_ASAN, [test x$enable_sanitizers != xno])

# Initialize libtool
LT_PREREQ([2.2.4])
Expand Down Expand Up @@ -150,6 +147,7 @@ echo "

introspection: $found_introspection
rojig: ${enable_rojig:-no}
ASAN + UBSAN: ${enable_sanitizers:-no}
gtk-doc: $enable_gtk_doc
rust: $rust_debug_release
cbindgen: ${cbindgen:-external}
Expand Down
6 changes: 5 additions & 1 deletion packaging/rpm-ostree.spec.in
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ BuildRequires: cargo
BuildRequires: rust
%endif

# Enable ASAN + UBSAN
%bcond_with sanitizers

# RHEL8 doesn't ship zchunk today. See also the comments
# in configure.ac around this as libdnf/librepo need to be in
# sync, and today we bundle libdnf but not librepo.
Expand Down Expand Up @@ -131,7 +134,8 @@ env NOCONFIGURE=1 ./autogen.sh
# the %%configure macro today assumes (reasonably) that one is building
# C/C++ and sets C{,XX}FLAGS
export RUSTFLAGS="%{build_rustflags}"
%configure --disable-silent-rules --enable-gtk-doc %{?sqlite_rpmdb_default}
%configure --disable-silent-rules --enable-gtk-doc %{?sqlite_rpmdb_default} %{?with_sanitizers:--enable-sanitizers}

%make_build

%install
Expand Down

0 comments on commit a3c09a7

Please sign in to comment.