Uh oh!
There was an error while loading. Please reload this page.
fix cross compilation failure due to size_t typecast in define - #5128
fix cross compilation failure due to size_t typecast in define#5128pmjdebruijn wants to merge 1 commit into
Conversation
nikic
commented
Jan 29, 2020
This fix is not right. Probably just a matter of including whatever header defines |
pmjdebruijn
commented
Jan 29, 2020
Thanks for your fast response. Adding an include for stddef.h or malloc.h to Zend/zend_alloc.h doesn't seem to help... However, I just realized br-arm-full target for buildroot actually uses uclibc as opposed to glibc. As a frame of reference, these builds are happening as part of the buildroot framework, which has a test script called "test-pkg", which builds a specific package for a predefined set of architecture and library combinations. |
nikic
commented
Jan 29, 2020
Ah sorry, I didn't properly understand what the issue is: It's not the use of size_t itself, but the fact that we end up expanding to |
There was a problem hiding this comment.
As C unfortunately doesn't have a suffix for size_t, I'd suggest replacing this with Z_UL(8). It may differ from size_t, but only on x32 ABI.
There was a problem hiding this comment.
How about just getting the size of size_t during the configure stage and making it a define?
pmjdebruijn
commented
Jan 29, 2020
As a sidenote, the build failure only occurs in specific scenarios, "crosscompiling" from x86-64 to x86-64 for a glibc target is fine for example. |
pmjdebruijn
commented
Jan 30, 2020
I just did a little more digging, maybe this is actually a GCC version issue? I just noticed that the reference compiler for br-arm-full is gcc 5.5.0, which is getting on the older side of things... |
pmjdebruijn
commented
Jan 30, 2020
So I just finished x86-64 "crosscompile" with uclibc and gcc 5.5.0, and it built just fine. So presumably that means this really it is an architecture specific issue. |
nikic
commented
Jan 30, 2020
@pmjdebruijn Usually this constant is determined by configure, and only computed in zend_alloc.h if cross compilation is used. |
pmjdebruijn
commented
Jan 30, 2020
Out of curiousity, what is the consequence of getting this wrong? Subpar performance? |
The following commit introduces a cross-compilation failure: 93c728b "Try to control ZEND_MM_ALIGNED_SIZE type" br-arm-full/build/php-7.4.2/Zend/zend_alloc.h:30:38: error: missing binary operator before token "8" ^ br-arm-full/build/php-7.4.2/ext/opcache/ZendAccelerator.c:1380:7: note: in expansion of macro ‘ZEND_MM_ALIGNMENT’
nikic
commented
Jan 30, 2020
To be honest, I'm not totally sure ... I think theoretically assuming a too large alignment can be a correctness issue, as we assume all allocations to have this alignment. As we use our own allocator (with controlled alignment) unless you explicitly disable it, and as the minimum alignment of system malloc is usually either 8 or 16, I don't think it matters in practice. In any case, I've merged a variant of this patch (with Z_L -> Z_UL) into 7.4. |
The following commit introduces a cross-compilation failure:
93c728b
"Try to control ZEND_MM_ALIGNED_SIZE type"