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

Clang inspired fixes, add clang CI permutation #172

Closed
wants to merge 8 commits into from
Closed
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
1 change: 1 addition & 0 deletions .github/actions/setup-alpine/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ runs:
automake \
bash \
build-base \
clang \
evelikov marked this conversation as resolved.
Show resolved Hide resolved
git \
gtk-doc \
libtool \
Expand Down
1 change: 1 addition & 0 deletions .github/actions/setup-archlinux/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ runs:
pacman --noconfirm -Sy archlinux-keyring

pacman --noconfirm -Su \
clang \
git \
gtk-doc \
linux-headers \
Expand Down
1 change: 1 addition & 0 deletions .github/actions/setup-debian/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ runs:
autoconf \
automake \
build-essential \
clang \
gcc-multilib \
git \
gtk-doc-tools \
Expand Down
2 changes: 2 additions & 0 deletions .github/actions/setup-fedora/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ runs:
dnf install -y \
autoconf \
automake \
clang \
compiler-rt \
gcc \
git \
gtk-doc \
Expand Down
1 change: 1 addition & 0 deletions .github/actions/setup-ubuntu/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ runs:
autoconf \
automake \
build-essential \
clang \
gcc-multilib \
git \
gtk-doc-tools \
Expand Down
36 changes: 18 additions & 18 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,26 @@ permissions:

jobs:
build:
env:
CC: ${{ matrix.compiler }}
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
build: ['meson', 'autotools']
compiler: ['clang','gcc']
container:
- name: 'ubuntu:22.04'
multilib: 'true'
- name: 'ubuntu:24.04'
meson_setup: '-D b_sanitize=address,undefined'
multilib: 'true'
- name: 'alpine:latest'
meson_setup: '-D docs=false -D b_sanitize=none'
- name: 'archlinux:multilib-devel'
meson_setup: '-D b_sanitize=address,undefined'
multilib: 'true'
- name: 'fedora:latest'
meson_setup: '-D b_sanitize=address,undefined'
- name: 'alpine:latest'
meson_setup: '-D docs=false'
- name: 'debian:unstable'
meson_setup: '-D b_sanitize=address,undefined'
multilib: 'true'
- name: 'fedora:latest'
meson_setup: '-D b_sanitize=none'
- name: 'ubuntu:22.04'
multilib: 'true'
- name: 'ubuntu:24.04'
multilib: 'true'

container:
Expand All @@ -49,16 +49,16 @@ jobs:
with:
sparse-checkout: .github

- uses: ./.github/actions/setup-ubuntu
if: ${{ startsWith(matrix.container.name, 'ubuntu') }}
- uses: ./.github/actions/setup-archlinux
if: ${{ startsWith(matrix.container.name, 'archlinux') }}
- uses: ./.github/actions/setup-fedora
if: ${{ startsWith(matrix.container.name, 'fedora') }}
- uses: ./.github/actions/setup-alpine
if: ${{ startsWith(matrix.container.name, 'alpine') }}
- uses: ./.github/actions/setup-archlinux
if: ${{ startsWith(matrix.container.name, 'archlinux') }}
- uses: ./.github/actions/setup-debian
if: ${{ startsWith(matrix.container.name, 'debian') }}
- uses: ./.github/actions/setup-fedora
if: ${{ startsWith(matrix.container.name, 'fedora') }}
- uses: ./.github/actions/setup-ubuntu
if: ${{ startsWith(matrix.container.name, 'ubuntu') }}

- name: Checkout the whole project
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
Expand Down Expand Up @@ -113,7 +113,7 @@ jobs:

- name: configure (32bit) (meson)
if: ${{ matrix.build == 'meson' && matrix.container.multilib == 'true' }}
run: mkdir build32 && cd build32 && CC='gcc -m32' meson setup . ..
run: mkdir build32 && cd build32 && CC="$CC -m32" meson setup . ..

- name: build (32bit) (meson)
if: ${{ matrix.build == 'meson' && matrix.container.multilib == 'true' }}
Expand Down
2 changes: 1 addition & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ EXTRA_DIST += \
meson_options.txt \
testsuite/meson.build \
scripts/build-scdoc.sh \
scripts/sanitizer-env.sh \
scripts/test-gtkdoc.sh \
scripts/test-wrapper.sh \
scripts/kmod-symlink.sh

AM_CPPFLAGS = \
Expand Down
1 change: 1 addition & 0 deletions build-dev.ini
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ xz = 'enabled'
zlib = 'enabled'
openssl = 'enabled'
werror = true
b_sanitize = 'address,undefined'

[built-in options]
buildtype = 'debugoptimized'
17 changes: 8 additions & 9 deletions libkmod/libkmod-elf.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ struct kmod_elf {
_elf_dbg(elf, __FILE__, __LINE__, __func__, __VA_ARGS__); \
} while (0);

static inline void _elf_dbg(const struct kmod_elf *elf, const char *fname, unsigned line,
const char *func, const char *fmt, ...)
_printf_format_(5, 6) static inline void _elf_dbg(const struct kmod_elf *elf,
const char *fname, unsigned line,
const char *func, const char *fmt, ...)
{
va_list args;

Expand Down Expand Up @@ -150,14 +151,14 @@ static inline int elf_set_uint(const struct kmod_elf *elf, uint64_t offset, uint
size_t i;

ELFDBG(elf,
"size=%" PRIu16 " offset=%" PRIu64 " value=%" PRIu64 " write memory=%p\n",
"size=%" PRIu64 " offset=%" PRIu64 " value=%" PRIu64 " write memory=%p\n",
size, offset, value, changed);

assert(size <= sizeof(uint64_t));
assert(offset + size <= elf->size);
if (offset + size > elf->size) {
ELFDBG(elf,
"out of bounds: %" PRIu64 " + %" PRIu16 " = %" PRIu64 "> %" PRIu64
"out of bounds: %" PRIu64 " + %" PRIu64 " = %" PRIu64 "> %" PRIu64
" (ELF size)\n",
offset, size, offset + size, elf->size);
return -1;
Expand Down Expand Up @@ -313,9 +314,7 @@ struct kmod_elf *kmod_elf_new(const void *memory, off_t size)
elf->header.section.entry_size, elf->header.strings.section);

if (elf->header.section.entry_size != shdr_size) {
ELFDBG(elf,
"unexpected section entry size: %" PRIu16 ", expected %" PRIu16
"\n",
ELFDBG(elf, "unexpected section entry size: %" PRIu16 ", expected %zu\n",
elf->header.section.entry_size, shdr_size);
goto invalid;
}
Expand Down Expand Up @@ -1105,7 +1104,7 @@ int kmod_elf_get_dependency_symbols(const struct kmod_elf *elf,

name = elf_get_mem(elf, str_off + name_off);
if (name[0] == '\0') {
ELFDBG(elf, "empty symbol name at index %" PRIu64 "\n", i);
ELFDBG(elf, "empty symbol name at index %d\n", i);
continue;
}

Expand Down Expand Up @@ -1192,7 +1191,7 @@ int kmod_elf_get_dependency_symbols(const struct kmod_elf *elf,

name = elf_get_mem(elf, str_off + name_off);
if (name[0] == '\0') {
ELFDBG(elf, "empty symbol name at index %" PRIu64 "\n", i);
ELFDBG(elf, "empty symbol name at index %d\n", i);
continue;
}

Expand Down
14 changes: 13 additions & 1 deletion meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,17 @@ add_project_link_arguments(
language : 'c'
)

# Clang as of v18, relies on statically linking the sanitizers. This causes two
# distinct issues:
# - the shared library is underlinked, so the build fails
# - the modules (that we dlopen/ld_preload) are underlinked so the tests fail
#
# Force shared libasan (GCC defaults to shared and this toggle doesn't exist),
# which combined with the LD_PRELOAD in our wrapper makes everything happy.
if get_option('b_sanitize') != 'none' and cc.get_id() == 'clang'
add_project_arguments('-shared-libasan', language : 'c')
add_project_link_arguments('-shared-libasan', language : 'c')
endif
################################################################################
# Options
################################################################################
Expand Down Expand Up @@ -453,10 +464,11 @@ endforeach
# ------------------------------------------------------------------------------

if get_option('build-tests')
bash = find_program('bash')
sanitizer_env = find_program('scripts/sanitizer-env.sh')
setup_modules = find_program('scripts/setup-modules.sh')
setup_rootfs = find_program('scripts/setup-rootfs.sh')
top_include = include_directories('.')
test_wrapper = find_program('scripts/test-wrapper.sh')
subdir('testsuite')
endif

Expand Down
29 changes: 29 additions & 0 deletions scripts/sanitizer-env.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/bin/bash

# set -euo pipefail # don't set these, since this script is sourced

if [[ ${CC-} == *gcc* ]]; then
OUR_PRELOAD=$("$CC" -print-file-name=libasan.so)
elif [[ ${CC-} == *clang* ]]; then
OUR_PRELOAD=$("$CC" -print-file-name=libclang_rt.asan-x86_64.so)
else
echo "Unknown compiler CC=\"${CC-}\" - gcc and clang are supported."
echo "Assuming \"gcc\", manually set the variable and retry if needed."
echo
OUR_PRELOAD=$(gcc -print-file-name=libasan.so)
fi

if test -n "$OUR_PRELOAD"; then
# In some cases, like in Fedora, the file is a script which cannot be
# preloaded. Attempt to extract the details from within.
if grep -q INPUT "$OUR_PRELOAD"; then
input=$(sed -r -n "s/INPUT \( (.*) \)/\1/p" "$OUR_PRELOAD")
test -f "$input" && OUR_PRELOAD="$input"
fi

LD_PRELOAD=${LD_PRELOAD+${LD_PRELOAD}:}$OUR_PRELOAD
export LD_PRELOAD
echo "LD_PRELOAD has been set to \"$LD_PRELOAD\"."
echo "The sanitizer might report issues with ANY process you execute."
fi
unset OUR_PRELOAD
16 changes: 0 additions & 16 deletions scripts/test-wrapper.sh

This file was deleted.

9 changes: 6 additions & 3 deletions testsuite/README
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,11 @@ pay attention when writing a test:
modules from module-playground. Update the latter script to include any
modules your test needs.

8 - Tests can be run individually, outside of 'make check'. strace and gdb work
too, as long as you tell them to operate on child process.
8 - Tests can be run individually, outside of 'meson test'. When running with
sanitizers, make sure to 'source scripts/sanitizer-env.sh'. Sanitizers are
not guararnteed to work well with others like strace and gdb. strace and gdb
work too, as long as you tell them to operate on child process.

9 - Make sure test passes when using "default" build flags, i.e. by running
'autogen.sh c'
'meson setup --native-file build-dev.ini ...', which by default enables the
sanitizers.
30 changes: 20 additions & 10 deletions testsuite/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -94,18 +94,28 @@ _testsuite = [
'test-weakdep',
]

if get_option('b_sanitize') != 'none'
source_env = 'source @0@;'.format(sanitizer_env.full_path())
else
source_env = ''
endif

foreach input : _testsuite
exec = executable(
input,
files('@[email protected]'.format(input)),
include_directories : top_include,
c_args : testsuite_c_args,
link_with : [libshared, libkmod_internal, libtestsuite],
build_by_default : false,
)
test(
input,
test_wrapper,
args : executable(
input,
files('@[email protected]'.format(input)),
include_directories : top_include,
c_args : testsuite_c_args,
link_with : [libshared, libkmod_internal, libtestsuite],
build_by_default : false,
),
depends : [internal_kmod_symlinks, create_rootfs, test_override_mods],
bash,
args : [
'-c',
'@0@@1@'.format(source_env, exec.full_path()),
],
depends : [exec, internal_kmod_symlinks, create_rootfs, test_override_mods],
)
endforeach
2 changes: 1 addition & 1 deletion testsuite/testsuite.c
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ static inline int test_run_child(const struct test *t, int fdout[2], int fderr[2
}

if (stat_mstamp(&rootfsst) > stat_mstamp(&stampst)) {
ERR("rootfs %s is dirty, please run 'make rootfs' before running this test\n",
lucasdemarchi marked this conversation as resolved.
Show resolved Hide resolved
ERR("rootfs %s is dirty, please run 'meson compile testsuite/stamp-rootfs' before running this test\n",
rootfs);
exit(EXIT_FAILURE);
}
Expand Down