Skip to content

ext/pcntl: Bump num_signals to uint16_t#21347

Open
NattyNarwhal wants to merge 1 commit intophp:PHP-8.5from
NattyNarwhal:pcntl-overflow-8.5
Open

ext/pcntl: Bump num_signals to uint16_t#21347
NattyNarwhal wants to merge 1 commit intophp:PHP-8.5from
NattyNarwhal:pcntl-overflow-8.5

Conversation

@NattyNarwhal
Copy link
Member

On AIX, NSIG is def'd as SIGMAX64+1, and SIGMAX64 itself is def'd as 255:

$ grep -Rw SIGMAX64 /QOpenSys/usr/include/
/QOpenSys/usr/include/sys/signal.h:#define SIGMAX64 255
/QOpenSys/usr/include/sys/signal.h:#define SIGMAX SIGMAX64
/QOpenSys/usr/include/sys/signal.h:#define NSIG64               (SIGMAX64+1)

...this causes an overflow when we set num_signals from the value of NSIG, per GCC:

/rpmbuild/BUILD/php-8.5.3/ext/pcntl/pcntl.c:216:25: warning: large integer implicitly truncated to unsigned type [-Woverflow]
  PCNTL_G(num_signals) = NSIG;
                         ^~~~

...when we try to use pcntl to i.e. install a signal handler, we get an error from pcntl:

Fatal error: Uncaught ValueError: pcntl_signal(): Argument #1 ($signal) must be less than 0 in phar:///QOpenSys/pkgs/bin/composer/vendor/seld/signal-handler/src/SignalHandler.php:491

The easiest way to deal with this silly AIX behaviour is to just promote the storage size.

On AIX, NSIG is def'd as SIGMAX64+1, and SIGMAX64 itself is def'd as
255:

```
$ grep -Rw SIGMAX64 /QOpenSys/usr/include/
/QOpenSys/usr/include/sys/signal.h:#define SIGMAX64 255
/QOpenSys/usr/include/sys/signal.h:#define SIGMAX SIGMAX64
/QOpenSys/usr/include/sys/signal.h:#define NSIG64               (SIGMAX64+1)
```

...this causes an overflow when we set num_signals from the value of
NSIG, per GCC:

```
/rpmbuild/BUILD/php-8.5.3/ext/pcntl/pcntl.c:216:25: warning: large integer implicitly truncated to unsigned type [-Woverflow]
  PCNTL_G(num_signals) = NSIG;
                         ^~~~
```

...when we try to use pcntl to i.e. install a signal handler, we get an
error from pcntl:

```
Fatal error: Uncaught ValueError: pcntl_signal(): Argument #1 ($signal) must be less than 0 in phar:///QOpenSys/pkgs/bin/composer/vendor/seld/signal-handler/src/SignalHandler.php:491
```

The easiest way to deal with this silly AIX behaviour is to just promote
the storage size.
@devnexen
Copy link
Member

devnexen commented Mar 5, 2026

looks safe change, did you have a quick look with pahole or similar ?

@NattyNarwhal
Copy link
Member Author

I haven't used pahole, no. I'm guessing this does slightly blow up the size of the structure, as the bools and previous uint8 would have been packed into 32 bits.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants