UCM: gate brk/sbrk hooks on configure-time availability by GenericRikka · Pull Request #11198 · openucx/ucx

GenericRikka · 2026-02-19T23:06:24Z

What?

This PR makes UCM’s brk()/sbrk() hooking and related tests conditional on the functions being present on the target platform. Specifically:

Detect brk() and sbrk() at configure time.
Only build/enable the brk()/sbrk() replace layer when both are available.
Skip brk()/sbrk() mmap event hook registration and tests when they are not available.
Provide ENOSYS stubs for internal brk()/sbrk() wrappers when disabled.

Why?

Some supported/non-Linux platforms do not provide brk()/sbrk() (or do not expose them in a way compatible with UCM’s hooking approach). UCM currently builds and registers these hooks unconditionally, which can:

break the build due to missing symbols/headers, or
lead to undefined behavior if hook registration or wrappers are compiled in despite the functions not being usable.

How?

Add AC_CHECK_FUNCS([brk sbrk]) in the UCM configure logic and define HAVE_BRK_SBRK only when both are present.
Guard hook registration, event firing/tests, and the replace-layer definitions with #ifdef HAVE_BRK_SBRK.
When disabled, provide ENOSYS stub implementations so code paths remain well-defined and compilation remains clean.

Linux behavior is unchanged because brk()/sbrk() are detected as available and the existing code remains enabled.

Testing

Tested: Linux (build)
Tested: FreeBSD (build assumed via downstream port; runtime pending)

Some platforms do not provide brk()/sbrk(). UCM currently builds and registers hooks for these symbols unconditionally, which can break the build or lead to undefined behavior when the functions are absent. Detect brk/sbrk during configure and: - build/enable the brk/sbrk replace layer only when available - skip brk/sbrk mmap event tests and hook registration otherwise - provide ENOSYS stubs for the internal brk/sbrk wrappers when disabled This keeps Linux behavior unchanged and avoids enabling unsupported hooks on non-Linux targets. Tested: Linux (build) Tested: FreeBSD (build assumed via downstream port; runtime pending)

yosefe · 2026-02-20T07:59:01Z

@GenericRikka thank you for your contribution!
Contributions to OpenUCX require a signed individual or organization contributor license agreement (CLA).
Can you pls review and sign a copy of our CLA (https://openucx.org/license) and email the signed CLA to admin at ucfconsortium.org?

config/m4/ucm.m4

configure.ac

src/ucm/mmap/install.c

src/ucm/malloc/malloc_hook.c

src/ucm/mmap/install.c

GenericRikka · 2026-02-20T14:48:24Z

@GenericRikka thank you for your contribution! Contributions to OpenUCX require a signed individual or organization contributor license agreement (CLA). Can you pls review and sign a copy of our CLA (https://openucx.org/license) and email the signed CLA to admin at ucfconsortium.org?

Thanks for pointing that out! I sent the email and am currently awaiting confirmation. Will report back once I got the backsigned version / confirmation!

Meanwhile I’ll continue iterating on the review feedback.

GenericRikka · 2026-02-20T22:59:34Z

I should have addressed all review feedback. Thanks again for the effort and if anything else needs adjustment, I am happy to iterate / adjust further!

yosefe · 2026-02-22T15:32:55Z

config/m4/sysdep.m4

+    AC_CHECK_FUNCS([brk sbrk])
+
+    AS_IF([test "x$ac_cv_func_brk" = xyes], [
+        AC_DEFINE([HAVE_BRK], [1], [Define if brk() is available])


IMO it's not needed since AC_CHECK_FUNCS already defines HAVE_xx macros

yosefe · 2026-02-22T15:38:38Z

config/m4/ucm.m4

+               [], [mmap_hooks_happy=no], [#include <sys/syscall.h>])
+
+AS_IF([test "x$ac_cv_func_brk" = xyes],
+      [AC_CHECK_DECLS([SYS_brk], [], [mmap_hooks_happy=no],


why needed? we can check SYS_brk regardless of brk function, as before

yosefe · 2026-02-22T15:38:54Z

config/m4/ucm.m4

+AS_IF([test "x$ac_cv_func_brk" != xyes -o "x$ac_cv_func_sbrk" != xyes],
+      [bistro_hooks_happy=no])


not sure we need it during compile time

yosefe · 2026-02-22T15:41:32Z

src/ucm/malloc/malloc_hook.c

+#if !defined(HAVE_BRK)
+    events &= ~UCM_EVENT_BRK;
+#endif
+#if !defined(HAVE_SBRK)
+    events &= ~UCM_EVENT_SBRK;
+#endif
+    return events;


not sure we need this
usually users would request VM_UNMAP. And if they reuest BRK/SBRK explicitly and they are not available, why not fail?

yosefe · 2026-02-22T15:41:47Z

src/ucm/malloc/malloc_hook.c


 ucs_status_t ucm_malloc_install(int events)
 {
+#ifdef HAVE_SBRK


do we really need ifdef here?

I think we still need the #ifdef HAVE_SBRK here: ucm_malloc_sbrk itself is only built when HAVE_SBRK is defined, so referencing it unconditionally would fail to compile/link on platforms without sbrk().
If it is preferred to reduce #ifdefs, I can instead provide a stub ucm_malloc_sbrk implementation under !HAVE_SBRK that returns UCS_ERR_UNSUPPORTED, and then keep the handler definition unconditional.

yosefe · 2026-02-22T15:43:02Z

src/ucm/util/replace.c


 #endif

+#ifdef HAVE_BRK


why need this ifdef? i don't see we access brk() in the disabled code here

The real sbrk implementation in this file depends on ucm_orig_brk() and ucm_get_current_brk(), so some HAVE_BRK guarding is still needed.
That said, I agree the current guard is broader than ideal: the sbrk path should really be conditioned on HAVE_SBRK && HAVE_BRK, while the brk path should stay under HAVE_BRK alone. I will tighten that so the preprocessor logic matches the actual dependency more precisely.

yosefe · 2026-02-22T15:44:02Z

src/ucm/util/replace.c


 void *ucm_get_current_brk()
 {
+#ifdef HAVE_BRK


why needed? maybe check SYS_brk instead inside ucm_brk_syscall?

yosefe · 2026-02-22T15:44:49Z

src/ucm/util/sys.c


 void *ucm_brk_syscall(void *addr)
 {
+#ifdef HAVE_BRK


should we heck HAVE_DECL_SYS_BRK?

yosefe · 2026-02-22T15:45:37Z

configure.ac

     AM_CONDITIONAL([DOCS_ONLY], [false])
     m4_include([config/m4/compiler.m4])
     m4_include([config/m4/sysdep.m4])
+     UCX_CHECK_BRK_SBRK


can't we do it directly in ucm.m4 ?

config/m4/ucm.m4

src/ucm/malloc/malloc_hook.c

GenericRikka · 2026-02-26T00:19:59Z

Hi,

Thanks for the detailed review and comments on the PR — I appreciate the time.

I’m currently working through the feedback items and will push an updated revision once I’ve addressed them and re-tested. It may take me a little time due to a tight schedule this week, but it’s on my active list and I’ll follow up with concrete updates.

GenericRikka · 2026-03-09T17:59:56Z

I have updated the differential according to your review feedback. Thank you again for taking the time to provide such thorough comments.

Where I reacted with 👍, I fully agreed with the suggestion and updated the code accordingly. In the few places where I thought additional context would be useful, I left inline comments to explain my reasoning and adjusted the patch accordingly.

config/m4/ucm.m4

src/ucm/malloc/malloc_hook.c

src/ucm/mmap/install.c

src/ucm/malloc/malloc_hook.c

src/ucm/util/replace.c

GenericRikka · 2026-03-10T21:11:53Z

Update on the CLA process: I received the backsigned copy of my CLA form from the UCF admins.

src/ucm/util/replace.c

tvegas1

last comment

src/ucm/malloc/malloc_hook.c

src/ucm/util/sys.c

yosefe · 2026-03-14T11:09:41Z

Update on the CLA process: I received the backsigned copy of my CLA form from the UCF admins.

The CLA was accepted, thanks!

GenericRikka · 2026-03-14T14:29:59Z

It looks like the Azure UCX snapshot check may be stuck rather than failing normally.
It started on 2026-03-13 21:51 and is still running more than 16 hours later, whereas these checks usually finish for me within ~2–3 hours.
Since the PR is already approved, could someone with the needed permissions cancel and re-run the pipeline?

GenericRikka · 2026-03-17T00:04:43Z

Thanks for merging and the good review feedback!

Some platforms do not provide brk()/sbrk(). UCM currently builds and registers hooks for these symbols unconditionally, which can break the build or lead to undefined behavior when the functions are absent.

GenericRikka force-pushed the brk-sbrk-allocator branch from 12cd556 to bf3ec01 Compare February 19, 2026 23:15

AUTHORS: add Generic Rikka

d2de836

GenericRikka force-pushed the brk-sbrk-allocator branch from efe70d9 to d2de836 Compare February 19, 2026 23:26

BUILD: tighten HAVE_BRK_SBRK detection

f116ab6

GenericRikka force-pushed the brk-sbrk-allocator branch from 064baa2 to f116ab6 Compare February 20, 2026 06:30

yosefe requested a review from tvegas1 February 20, 2026 07:52

tvegas1 reviewed Feb 20, 2026

View reviewed changes

GenericRikka force-pushed the brk-sbrk-allocator branch 2 times, most recently from 28a302e to f8f3451 Compare February 20, 2026 23:38

yosefe requested changes Feb 22, 2026

View reviewed changes

tvegas1 reviewed Feb 23, 2026

View reviewed changes

config/m4/ucm.m4 Outdated Show resolved Hide resolved

config/m4/ucm.m4 Show resolved Hide resolved

src/ucm/malloc/malloc_hook.c Outdated Show resolved Hide resolved

tvegas1 reviewed Feb 23, 2026

View reviewed changes

src/ucm/malloc/malloc_hook.c Outdated Show resolved Hide resolved

GenericRikka force-pushed the brk-sbrk-allocator branch from f8f3451 to e14005b Compare March 9, 2026 17:58

GenericRikka requested a review from yosefe March 9, 2026 21:50

tvegas1 reviewed Mar 10, 2026

View reviewed changes

config/m4/ucm.m4 Show resolved Hide resolved

config/m4/ucm.m4 Outdated Show resolved Hide resolved

src/ucm/malloc/malloc_hook.c Outdated Show resolved Hide resolved

src/ucm/mmap/install.c Outdated Show resolved Hide resolved

src/ucm/malloc/malloc_hook.c Show resolved Hide resolved

GenericRikka force-pushed the brk-sbrk-allocator branch from e14005b to c51ee03 Compare March 10, 2026 13:50

tvegas1 reviewed Mar 10, 2026

View reviewed changes

src/ucm/util/replace.c Outdated Show resolved Hide resolved

src/ucm/util/replace.c Outdated Show resolved Hide resolved

GenericRikka force-pushed the brk-sbrk-allocator branch from c51ee03 to f4fcc74 Compare March 11, 2026 03:06

tvegas1 reviewed Mar 11, 2026

View reviewed changes

src/ucm/util/replace.c Outdated Show resolved Hide resolved

GenericRikka force-pushed the brk-sbrk-allocator branch from f4fcc74 to 688baee Compare March 11, 2026 16:09

tvegas1 reviewed Mar 11, 2026

View reviewed changes

src/ucm/util/replace.c Outdated Show resolved Hide resolved

GenericRikka force-pushed the brk-sbrk-allocator branch from 688baee to ab4732e Compare March 11, 2026 19:04

tvegas1 reviewed Mar 12, 2026

View reviewed changes

src/ucm/malloc/malloc_hook.c Outdated Show resolved Hide resolved

GenericRikka force-pushed the brk-sbrk-allocator branch from ab4732e to 1da856b Compare March 13, 2026 01:03

tvegas1 reviewed Mar 13, 2026

View reviewed changes

src/ucm/util/sys.c Outdated Show resolved Hide resolved

BUILD: Adjusted BRK/SBRK conditionals to review feedback.

275b244

GenericRikka force-pushed the brk-sbrk-allocator branch from 1da856b to 275b244 Compare March 13, 2026 21:50

yosefe approved these changes Mar 14, 2026

View reviewed changes

tvegas1 approved these changes Mar 16, 2026

View reviewed changes

tvegas1 enabled auto-merge (squash) March 16, 2026 14:29

tvegas1 merged commit c698e43 into openucx:master Mar 16, 2026
152 checks passed

		AS_IF([test "x$ac_cv_func_brk" != xyes -o "x$ac_cv_func_sbrk" != xyes],
		[bistro_hooks_happy=no])

Conversation

GenericRikka commented Feb 19, 2026

What?

Why?

How?

Testing

Uh oh!

yosefe commented Feb 20, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GenericRikka commented Feb 20, 2026

Uh oh!

GenericRikka commented Feb 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GenericRikka commented Feb 26, 2026

Uh oh!

GenericRikka commented Mar 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GenericRikka commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tvegas1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

yosefe commented Mar 14, 2026

Uh oh!

GenericRikka commented Mar 14, 2026

Uh oh!

Uh oh!

GenericRikka commented Mar 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

GenericRikka commented Mar 10, 2026 •

edited

Loading