Skip to content

Better fix for when CFLAGS includes -D_FORTIFY_SOURCE - #226

Merged
dimpase merged 1 commit into
sagemath:mainfrom
tornaria:fortify
Apr 5, 2026
Merged

Better fix for when CFLAGS includes -D_FORTIFY_SOURCE#226
dimpase merged 1 commit into
sagemath:mainfrom
tornaria:fortify

Conversation

@tornaria

@tornaria tornaria commented Feb 9, 2025

Copy link
Copy Markdown
Member

The module signals.pyx has to be built with fortify disabled, because of
a false positive in one of the longjmp(), more precisely in the call

cylongjmp(trampoline_setup, 1)

which appears in setup_trampoline().

Often distributions will add -D_FORTIFY_SOURCE=2 to CFLAGS, and so
this has to be overridden when compiling signals.pyx by adding
-U_FORTIFY_SOURCE to the compiler arguments. The former solution,
using add_project_arguments() doesn't work since it will add flags
before the ones provided in CFLAGS.

Instead, add -U_FORTIFY_SOURCE to the arguments in the definition of
the extension module signal, which will add it to the command line
after the ones provided in CFLAGS. In addition, fortify is disabled
only for building signal.pyx.

@tornaria
tornaria requested a review from tobiasdiez February 16, 2025 13:30

@tobiasdiez tobiasdiez 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.

LGTM

The module signals.pyx has to be built with fortify disabled, because of
a false positive in one of the `longjmp()`, more precisely in the call
```
cylongjmp(trampoline_setup, 1)
```
which appears in `setup_trampoline()`.

Often distributions will add `-D_FORTIFY_SOURCE=2` to `CFLAGS`, and so
this has to be overridden when compiling `signals.pyx` by adding
`-U_FORTIFY_SOURCE` to the compiler arguments. The former solution,
using `add_project_arguments()` doesn't work since it will add flags
*before* the ones provided in `CFLAGS`.

Instead, add `-U_FORTIFY_SOURCE` to the arguments in the definition of
the extension module `signal`, which will add it to the command line
*after* the ones provided in `CFLAGS`. In addition, fortify is disabled
only for building `signal.pyx`.
@tornaria

tornaria commented Apr 4, 2026

Copy link
Copy Markdown
Member Author

@cxzhong this is better than removing the check in implementation.c.

Try

CFLAGS="-D_FORTIFY_SOURCE -O2 -Wall" meson setup build && ninja -C build

to see why this is useful. With this PR, it works just fine.

@tornaria

tornaria commented Apr 4, 2026

Copy link
Copy Markdown
Member Author

@tobiasdiez this is still useful, I only rebased on current main.

@tobiasdiez

Copy link
Copy Markdown
Contributor

@dimpase could you please merge this?

@dimpase
dimpase merged commit 61cff30 into sagemath:main Apr 5, 2026
19 checks passed
@tornaria
tornaria deleted the fortify branch April 5, 2026 21:20
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