C23 fix for syscallsreal.c and pid_syscallsreal.c by gc00 · Pull Request #1240 · dmtcp/dmtcp

gc00 · 2026-03-11T23:15:31Z

src/syscallsreal.c and src/plugin/pid/pid_syscallsreal.c were going to break when we use a compiler enforcing C23. C23 requires signatures in fnc pointers (to be aligned with C++). Luckily, C23 also makes the built-in, typeof(), part of the C standard. GCC and CLANG always provided typeof(). But now, all compliant compilers must provide it. So, it's safe for us to use it.

And I also cleaned up some minor errors along the way:

sched_setparam: We were discarding the second argument, param, when we interposed
close_range was created only with Linux 5.9 and glibc-2.34. We were not testing this
We were missing several include files that had the declarations for the functions that we were interposing on. (Since C23 requires the signature for fnc. pointers, we now need to include the proper .h files.)

I also tested on Rocky Linux 8 (Linux 4.18 and glibc-2.28). This is an older O/S. So, hopefully, we're not breaking DMTCP on older Linuxes.

Summary by CodeRabbit

Refactor
- Consolidated wrapper macros across system call handlers for improved type safety and consistency.
- Refactored function pointer types to use more specific signatures rather than generic types.
- Reorganized system headers and function declarations for clearer code structure.
Bug Fixes
- Enhanced error handling to abort immediately when required library symbols are missing, improving diagnostics.

coderabbitai · 2026-03-11T23:15:51Z

Warning

Rate limit exceeded

@gc00 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 35 minutes and 19 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fdaf62bc-4aa2-4a99-83b4-808222f105a1

📥 Commits

Reviewing files that changed from the base of the PR and between a4992aa and 118596f.

📒 Files selected for processing (3)

src/dmtcp_coordinator.cpp
src/plugin/pid/pid_syscallsreal.c
src/syscallsreal.c

📝 Walkthrough

Walkthrough

This pull request refactors function pointer wrapper macros and type definitions across multiple files to improve type safety and consistency. Changes include optimizing the short_name implementation, updating function pointer signatures in restart headers, and consolidating scattered wrapper macro patterns into generic REAL_FUNC_PASSTHROUGH forms with an enhanced error handling path for missing libc symbols.

Changes

Cohort / File(s)	Summary
Type Safety and Signature Updates `src/mtcp/mtcp_restart.h`, `src/mtcp/mtcp_restart.c`	Added forward declaration for `struct RestoreInfo`, changed `fnptr_t` signature to accept `struct RestoreInfo *`, introduced new `fnptr_post_restart_t` typedef for post-restart callbacks, and updated function pointer instantiation with explicit type casting.
Macro Refactoring - Plugin PID `src/plugin/pid/pid_syscallsreal.c`	Consolidated wrapper macro patterns by replacing legacy REAL_FUNC_PASSTHROUGH_TYPED/VOID/NORETURN variants with unified REAL_FUNC_PASSTHROUGH macros; added system headers (sched.h, sys/ioctl.h, termios.h); added extern declaration for `__clone`; updated 15+ real function wrappers to use new generic macro form with corrected argument passing.
Macro Refactoring - Core Syscalls `src/syscallsreal.c`	Replaced per-type wrapper macro usage with generic REAL_FUNC_PASSTHROUGH across many wrappers; introduced aggressive error path on symbol lookup failure (abort with diagnostic); added deprecation warning suppression pragma; added extern declarations for `__clone` and `__sigpause`; reformatted `_real_clone` signature; added conditional compilation guards for `close_range` (Linux 5.9+ and GLIBC 2.34).
Code Optimization `src/dmtcp_coordinator.cpp`	Simplified `short_name` implementation to derive base name directly from input using const char* without intermediate buffer copy.
Comment Formatting `test/plugin/sleep2/sleep2.c`	Repositioned inline comment before declaration for improved readability; no functional changes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Fixed SYS_close_range in syscall wrapper #1218: Related refactoring of syscall wrapper handling with conditional compilation guards for the close_range syscall to ensure compatibility across different libc and kernel versions.

Suggested reviewers

karya0
xuyao0127

Poem

🐰 Macros unified, function pointers refined, type safety intertwined,
No more scattered patterns cluttering our design,
With eager error paths and forward-declared grace,
Our wrappers now dance at a harmonious pace! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 6.41% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'C23 fix for syscallsreal.c and pid_syscallsreal.c' accurately captures the primary change—addressing C23 compatibility issues in two critical syscall wrapper files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/syscallsreal.c (1)
486-490: Consider using REAL_FUNC_PASSTHROUGH_NORETURN for _real_exit.

exit() is a noreturn function. While the current code works (since exit() never returns), using REAL_FUNC_PASSTHROUGH_NORETURN would be more semantically correct and consistent with _real_pthread_exit at line 961.
♻️ Suggested improvement
 LIB_PRIVATE
 void
 _real_exit(int status)
 {
-  REAL_FUNC_PASSTHROUGH(exit) (status);
+  REAL_FUNC_PASSTHROUGH_NORETURN(exit) (status);
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/syscallsreal.c` around lines 486 - 490, Replace the passthrough macro
used in _real_exit to the noreturn variant: currently _real_exit uses
REAL_FUNC_PASSTHROUGH(exit) (status) but since exit() is marked noreturn you
should call REAL_FUNC_PASSTHROUGH_NORETURN(exit) instead to be semantically
correct and consistent with the treatment of _real_pthread_exit; update the
invocation in the _real_exit function accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/plugin/pid/pid_syscallsreal.c`:
- Around line 434-435: The wrapper for pthread_exit (the symbol
_real_pthread_exit) incorrectly uses REAL_FUNC_PASSTHROUGH which expands to a
return; change it to REAL_FUNC_PASSTHROUGH_NORETURN so the call invokes the real
pthread_exit without returning; locate the occurrence of
REAL_FUNC_PASSTHROUGH(pthread_exit) in the _real_pthread_exit implementation and
replace it with REAL_FUNC_PASSTHROUGH_NORETURN(pthread_exit) to match other
noreturn wrappers (e.g., the style used in the _real_* implementations in
syscallsreal.c).

---

Nitpick comments:
In `@src/syscallsreal.c`:
- Around line 486-490: Replace the passthrough macro used in _real_exit to the
noreturn variant: currently _real_exit uses REAL_FUNC_PASSTHROUGH(exit) (status)
but since exit() is marked noreturn you should call
REAL_FUNC_PASSTHROUGH_NORETURN(exit) instead to be semantically correct and
consistent with the treatment of _real_pthread_exit; update the invocation in
the _real_exit function accordingly.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f1d9d792-5f3c-4659-9659-0e3e012dc0ec

📥 Commits

Reviewing files that changed from the base of the PR and between e654d4b and a4992aa.

📒 Files selected for processing (6)

src/dmtcp_coordinator.cpp
src/mtcp/mtcp_restart.c
src/mtcp/mtcp_restart.h
src/plugin/pid/pid_syscallsreal.c
src/syscallsreal.c
test/plugin/sleep2/sleep2.c

xuyao00

LGTM,

xuyao00

LGTM

gc00 requested review from karya0 and xuyao00 March 11, 2026 23:15

gc00 added the bug label Mar 11, 2026

coderabbitai Bot reviewed Mar 11, 2026

View reviewed changes

Comment thread src/plugin/pid/pid_syscallsreal.c Outdated

dmtcp_coordinator.cpp:short_name:Fix comp. warning

ab08366

gc00 force-pushed the c23-fix-more branch 2 times, most recently from c527f16 to 703ccaf Compare May 11, 2026 01:47

C23 requires args for fnc pointer declaration

118596f

gc00 force-pushed the c23-fix-more branch from 703ccaf to 118596f Compare May 11, 2026 01:55

xuyao00 reviewed May 11, 2026

View reviewed changes

xuyao00 approved these changes May 11, 2026

View reviewed changes

xuyao00 merged commit 8def95b into dmtcp:main May 11, 2026
2 checks passed

coderabbitai Bot mentioned this pull request May 26, 2026

Introduce single-DSO internal plugin architecture #1251

Merged

coderabbitai Bot mentioned this pull request Jun 28, 2026

Removes about 200 compiler pedantic warnings, etc. #1268

Merged

Navigation Menu

Uh oh!

C23 fix for syscallsreal.c and pid_syscallsreal.c - #1240

C23 fix for syscallsreal.c and pid_syscallsreal.c#1240
xuyao00 merged 2 commits into
dmtcp:mainfrom
gc00:c23-fix-more

gc00 commented Mar 11, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 11, 2026 •

edited

Loading

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

xuyao00 left a comment

Uh oh!

xuyao00 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

gc00 commented Mar 11, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xuyao00 left a comment

Choose a reason for hiding this comment

Uh oh!

xuyao00 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gc00 commented Mar 11, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 11, 2026 •

edited

Loading