Skip to content

build: preserve default CFLAGS for libseccomp - #5456

Closed
hakman wants to merge 1 commit into
opencontainers:mainfrom
hakman:fix-seccomp-cflags
Closed

hakman wants to merge 1 commit into
opencontainers:mainfrom
hakman:fix-seccomp-cflags

Conversation

@hakman

@hakman hakman commented Sep 12, 2026

Copy link
Copy Markdown

When CFLAGS is unset, the per-architecture libseccomp build loop resets it to an empty value and set_cross_vars exports it. This prevents Autoconf from applying its default -g -O2 flags, leaving libseccomp unoptimized. Release builds link against these per-architecture copies, including for the native architecture.

Restore an unset CFLAGS before each architecture build instead of exporting an empty value. This lets configure choose its defaults when set_cross_vars does not add architecture-specific CFLAGS, while preserving explicitly empty and custom caller values.

In an amd64 comparison of matched runc v1.5.1 builds with libseccomp 2.6.0 on an Azure D16ds_v5, building libseccomp with -g -O2 instead of empty CFLAGS reduced mean runc run time with containerd's RuntimeDefault seccomp profile from 43.16 ms to 36.24 ms, approximately 16%, over 200 samples per variant.

@kolyshkin kolyshkin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my.

I think the right approach here should use something like

if [ -v CFLAGS ]; then
   local original_cflags="$CFLAGS"
fi
...
# Reset CFLAGS.
if [ -v original_cflags ]; then
    CFLAGS="$original_cflags"
else
    unset CFLAGS
fi

@kolyshkin
kolyshkin requested a review from cyphar September 13, 2026 06:05
The per-architecture loop resets CFLAGS to an empty value when the caller
leaves it unset. set_cross_vars then exports it, causing configure to skip
its default -g -O2 flags. Release builds link against these per-architecture
copies of libseccomp, including for the native architecture.

Unset CFLAGS when it was originally unset, allowing configure to apply
its defaults on architectures that do not add their own CFLAGS. Preserve
explicitly empty and custom caller values.

Signed-off-by: Ciprian Hacman <ciprian@hakman.dev>
@hakman

hakman commented Sep 13, 2026

Copy link
Copy Markdown
Author

Thanks for the review @kolyshkin, updated.

@kolyshkin kolyshkin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this works!

Yet better idea is to not change CFLAGS in set_cross_vars, so we won't have to save/restore it

@kolyshkin

Copy link
Copy Markdown
Contributor

Yet better idea is to not change CFLAGS in set_cross_vars, so we won't have to save/restore it

We also have the same issue in script/release_build.sh.

Opened #5464 as an alternative

@hakman

hakman commented Sep 14, 2026

Copy link
Copy Markdown
Author

Yet better idea is to not change CFLAGS in set_cross_vars, so we won't have to save/restore it

We also have the same issue in script/release_build.sh.

Opened #5464 as an alternative

Thanks, looks like a more complete alternative. I don't think there's any reason to keep this one open anymore.

@hakman hakman closed this Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants