ext/bcmath: Fixed an issue where macros may become undefined

[SakiTakamachi]

Failed nightly test:
https://github.com/php/php-src/actions/runs/8994680502/job/24708536894

/usr/bin/ld: ext/bcmath/libbcmath/src/doaddsub.o: in function `_bc_do_sub':
/home/runner/work/php-src/php-src/ext/bcmath/libbcmath/src/doaddsub.c:186: undefined reference to `BSWAP64'
/usr/bin/ld: /home/runner/work/php-src/php-src/ext/bcmath/libbcmath/src/doaddsub.c:187: undefined reference to `BSWAP64'
/usr/bin/ld: /home/runner/work/php-src/php-src/ext/bcmath/libbcmath/src/doaddsub.c:206: undefined reference to `BSWAP64'
collect2: error: ld returned 1 exit status
make: *** [Makefile:3[13](https://github.com/php/php-src/actions/runs/8994680502/job/24708536894#step:8:14): sapi/cli/php] Error 1
make: *** Waiting for unfinished jobs....
/usr/bin/ld: ext/bcmath/libbcmath/src/doaddsub.o: in function `_bc_do_sub':
/home/runner/work/php-src/php-src/ext/bcmath/libbcmath/src/doaddsub.c:186: undefined reference to `BSWAP64'
/usr/bin/ld: /home/runner/work/php-src/php-src/ext/bcmath/libbcmath/src/doaddsub.c:[18](https://github.com/php/php-src/actions/runs/8994680502/job/24708536894#step:8:19)7: undefined reference to `BSWAP64'
/usr/bin/ld: /home/runner/work/php-src/php-src/ext/bcmath/libbcmath/src/doaddsub.c:206: undefined reference to `BSWAP64'
collect2: error: ld returned 1 exit status
make: *** [Makefile:325: sapi/fpm/php-fpm] Error 1
/usr/bin/ld: ext/bcmath/libbcmath/src/doaddsub.o: in function `_bc_do_sub':
/home/runner/work/php-src/php-src/ext/bcmath/libbcmath/src/doaddsub.c:186: undefined reference to `BSWAP64'
/usr/bin/ld: /home/runner/work/php-src/php-src/ext/bcmath/libbcmath/src/doaddsub.c:187: undefined reference to `BSWAP64'
/usr/bin/ld: /home/runner/work/php-src/php-src/ext/bcmath/libbcmath/src/doaddsub.c:[20](https://github.com/php/php-src/actions/runs/8994680502/job/24708536894#step:8:21)6: undefined reference to `BSWAP64'
collect2: error: ld returned 1 exit status
make: *** [Makefile:358: sapi/phpdbg/phpdbg] Error 1

My GitHub Actions (After this fix):
https://github.com/SakiTakamachi/php-src/actions/runs/9001412048/job/24727651914

[SakiTakamachi]

@nielsdos
Honestly, I don't understand why it becomes undefined in the original code... Do you have any idea what I'm missing?

[nielsdos]

The problem is that the nightly run puts >/dev/null at the end, so we can't see the warnings of the macro.
I'd recommend rerunning that one run but without the output redirection so we can understand the problem.
It's probably a naming conflict of some sorts, in which case we need to prefix the macro names. And actually, that would be good in general to prefix them in any case.

[SakiTakamachi]

Thanks, I'm testing a branch where I just modified the Workflow
https://github.com/SakiTakamachi/php-src/actions/runs/9004591232
https://github.com/SakiTakamachi/php-src/actions/runs/9004591232/job/24737978432

[SakiTakamachi]

There were no useful logs.

I'm not sure what the cause is, but I know that the changes in this PR will solve the problem, so would it be reasonable to merge it?

[nielsdos]

I'm not sure what the cause is, but I know that the changes in this PR will solve the problem, so would it be reasonable to merge it?

I don't think it's entirely right.
The error means that the function BSWAP64 was not found, which means that there is probably a define active somewhere that does this: #define BSWAP64 BSWAP64. This would cause the macro to not be defined and would cause the function to not be defined either.
Furthermore, the inlines don't actually work because the static keyword is missing. (You can test this by removing the macro defines).

I have a test commit here that I'm running the failing nightly workflow on: nielsdos@a7add80
To avoid the define conflict I prefixed the macros with BC_ and to avoid the inline issue I added the static keyword.
Workflow started here: https://github.com/nielsdos/php-src/actions/runs/9016370472

[SakiTakamachi]

Indeed, I should have added a prefix. This was code that imitated ext/hash.

php-src/ext/hash/murmur/endianness.h

Line 11 in 2956f55

#if defined(_MSC_VER)

However, while working on this problem, I noticed that when __has_builtin(__builtin_bswap32) is false, the inline function is used even if __GNUC__ is defined.

So it seems like a good idea to merge my patch with yours.

[SakiTakamachi]

Looking at your Nightly results, it looks like your fixing policy is correct!

[SakiTakamachi]

@nielsdos
I used your patch

[nielsdos]

Lgtm