Skip to content

build: link libfastjson against libm for modf() #167

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.

Sign up for GitHub

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

Jump to bottom
Merged
rgerhards merged 1 commit into from
Sep 10, 2025
Merged

build: link libfastjson against libm for modf() #167

rgerhards merged 1 commit into from
Sep 10, 2025

Conversation

@Whissi
Copy link
Contributor

@Whissi Whissi commented Aug 9, 2025 •
edited
Loading

After upgrading to glibc-2.42, I noticed a runtime error in rsyslogd:

rsyslogd: Relink `/usr/lib64/libfastjson.so.4' with `/lib64/libm.so.6' for IFUNC symbol `modf'
Segmentation fault

This is due to changed modf implementation in glibc-2.42 causing IFUNC modf() resolution to fail at runtime in rsyslog due to missing direct link against libm.

Adding -lm to Libs ensures the shared library has a DT_NEEDED entry for libm.

@Whissi
build: link libfastjson against libm for modf()
919f7d2 
glibc-2.42 changed modf causing IFUNC modf() resolution to fail at runtime
in rsyslog due to missing direct link against libm.

Adding -lm to Libs ensures the shared library has a DT_NEEDED entry for libm.
@xgqt
Copy link

xgqt commented Aug 11, 2025

See also: https://bugs.gentoo.org/961289

Thanks for the patch!

@rgerhards rgerhards merged commit f1983a8 into rsyslog:master Sep 10, 2025
1 check passed
@xgqt
Copy link

xgqt commented Sep 10, 2025

Thanks!

@mbiebl
Copy link
Contributor

mbiebl commented Dec 5, 2025

@Whissi : adding -lm to Libs in libfastjson.pc does not ensure that the "shared library has a DT_NEEDED entry for libm".

It makes sure software linking against libfastjson and using pkg-config will get a -lm added to their build flags.
So, software like rsyslog would get a DT_NEEDED entry for libm. But since rsyslog already has -lm in its build flags, I wonder what exactly this PR would change.

@Whissi
Copy link
Contributor Author

Whissi commented Dec 5, 2025 •
edited
Loading

On systems with glibc-2.42 or newer, any application using modf will segfault at runtime when the function is resolved. This is due to an intentional change in glibc-2.42, which now requires linking against libm.

We observed exactly this behavior with rsyslogd, which calls modf indirectly through the linked libfastjson library.

Is there agreement on this point?

This PR ensures that any build system using pkg-config, like the one rsyslog uses, links against libm when linking against libfastjson, thereby preventing the crash.

Now, reading your message, I wonder whether the “proper” fix might actually be to have libfastjson link directly against libm. That way, users of the library, like rsyslogd, would only need to link against libfastjson itself. lddtree would then show (please notice the indention in front of libm.so which now shows it as a dependency of libfastjson):

 $ lddtree /usr/sbin/rsyslogd
/usr/sbin/rsyslogd (interpreter => /lib64/ld-linux-x86-64.so.2)
    libz.so.1 => /usr/lib64/libz.so.1
    libestr.so.0 => /usr/lib64/libestr.so.0
    libfastjson.so.4 => /usr/lib64/libfastjson.so.4
        libm.so.6 => /lib64/libm.so.6
    libuuid.so.1 => /usr/lib64/libuuid.so.1
    libc.so.6 => /lib64/libc.so.6

instead of current output

 $ lddtree /usr/sbin/rsyslogd
/usr/sbin/rsyslogd (interpreter => /lib64/ld-linux-x86-64.so.2)
    libz.so.1 => /usr/lib64/libz.so.1
    libestr.so.0 => /usr/lib64/libestr.so.0
    libfastjson.so.4 => /usr/lib64/libfastjson.so.4
    libm.so.6 => /lib64/libm.so.6
    libuuid.so.1 => /usr/lib64/libuuid.so.1
    libc.so.6 => /lib64/libc.so.6

Is that what you meant?

@mbiebl
Copy link
Contributor

mbiebl commented Dec 5, 2025

This PR ensures that rsyslogd links against libm when linking against libfastjson, which prevents the crash.

Your commit message says:
"Adding -lm to Libs ensures the shared library has a DT_NEEDED entry for libm."

What you repeated above is basically what I said, which is my point.

The commit message is incorrect/misleading and I wanted to clear up the confusion.

Also, rsyslog already has a NEEDED entry on libm:

$ objdump -x /usr/sbin/rsyslogd | grep NEEDED | grep libm
  NEEDED               libm.so.6

This comes in via https://github.com/rsyslog/rsyslog/blob/main/runtime/Makefile.am#L123 afaics (-lm was added 4 years ago).
Which makes we wonder why this was an issue on Gentoo in the first place?

And yes, I think the PR should be changed as follows:

  • libfastjson should link against libm directly (given it uses functions from libm)
  • in libfastjson.pc, libm should be moved to Libs.private for static builds to work

@mbiebl
Copy link
Contributor

mbiebl commented Dec 5, 2025

  • in libfastjson.pc, libm should be moved to Libs.private for static builds to work

Unless statically linking is not something that libfastjson wants to support.

@mbiebl
Copy link
Contributor

mbiebl commented Dec 5, 2025

And yes, I think the PR should be changed as follows:

  • libfastjson should link against libm directly (given it uses functions from libm)
  • in libfastjson.pc, libm should be moved to Libs.private for static builds to work

And a new upstream release with these changes would make sense.
@rgerhards ^

Whissi added a commit to Whissi/libfastjson that referenced this pull request Dec 5, 2025
@Whissi
Revert "build: link libfastjson against libm for modf()"
24e7150 
Pull Request rsyslog#167 modified libfastjson.pc to add an explicit dependency
on -lm through pkg-config. While this worked around missing modf() resolution
under glibc ≥ 2.42, it pushed the responsibility onto applications.

The correct solution is for libfastjson itself to link against libm so
that dependent applications do not need to adjust their linker flags.

This commit removes the pkg-config modifications introduced by commit 919f7d2.
Whissi added a commit to Whissi/libfastjson that referenced this pull request Dec 5, 2025
@Whissi
build: Use AC_CHECK_LIB to ensure libfastjson links against libm
2e7330e 
glibc ≥ 2.42 no longer provides modf() through implicit linkage, so any
library using modf() must explicitly link libm. The previous approach
conditionally added -lm only if isnan() failed to link, which is no
longer sufficient and did not guarantee that libfastjson itself linked
against libm.

Replace the isnan-based conditional check with an explicit
AC_CHECK_LIB([m], [modf]) so that -lm is always added when required.
This ensures libfastjson links directly against libm, allowing dependent
applications to link only against libfastjson without needing to add -lm
themselves.

Fixes: rsyslog#167
@Whissi Whissi mentioned this pull request Dec 5, 2025
Open
@Whissi
Copy link
Contributor Author

Whissi commented Dec 5, 2025

Many thanks for your feedback! I created #171 to correct the fix. I would appreciate your review on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Whissi @xgqt @mbiebl @rgerhards