php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #79356 FFI:structure/union alignment [ext/ffi/tests/022.phpt] failure
Submitted: 2020-03-09 05:49 UTC Modified: 2020-03-12 08:52 UTC
From: vibhutisawant18 at gmail dot com Assigned: dmitry (profile)
Status: Closed Package: Testing related
PHP Version: master-Git-2020-03-09 (Git) OS: Ubuntu 16.04
Private report: No CVE-ID: None
View Add Comment Developer Edit
Anyone can comment on a bug. Have a simpler test case? Does it work for you on a different platform? Let us know!
Just going to say 'Me too!'? Don't clutter the database with that please !
Your email address:
MUST BE VALID
Solve the problem:
7 + 49 = ?
Subscribe to this entry?

 
 [2020-03-09 05:49 UTC] vibhutisawant18 at gmail dot com
Description:
------------
FFI:structure/union alignment [ext/ffi/tests/022.phpt] fails on Big endian systems.
 
The TC fails in the following code block:
 
if (substr(PHP_OS, 0, 3) != 'WIN') {
    test_size(32, "struct  {char a; uint32_t b __attribute__((aligned));}");
    test_align(16, "struct  {char a; uint32_t b __attribute__((aligned));}");
}
 
The __attribute__((aligned(__BIGGEST_ALIGNMENT__))) returns 8 bytes on s390x, whereas its 16 bytes on x86 architecture.
Hence the test functions output observed on s390x is :
test_size expects the size returned to be 16.
test_align expects the alignment returned to be  8.

Added a patch with TC changes specific to s390x. Kindly let me know if I shall raise a PR with this changes.
Also Could you please ensure if additional source code changes are needed?


Patches

FFI_structure_union_alignment_s390x (last revision 2020-03-09 05:53 UTC by vibhutisawant18 at gmail dot com)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2020-03-09 05:53 UTC] vibhutisawant18 at gmail dot com
The following patch has been added/updated:

Patch Name: FFI_structure_union_alignment_s390x
Revision:   1583733196
URL:        https://bugs.php.net/patch-display.php?bug=79356&patch=FFI_structure_union_alignment_s390x&revision=1583733196
 [2020-03-09 08:17 UTC] nikic@php.net
This test also fails for me on a x86-64, so not big endian specific.
 [2020-03-09 09:50 UTC] vibhutisawant18 at gmail dot com
ext/ffi/tests/022.phpt passed on x86-64.PFB the test log.

root@a11efdc33877:~/php/php-src# sapi/cli/php -f "ext/ffi/tests/022.phpt"
--TEST--
FFI 022: structure/union alignment
--SKIPIF--
--INI--
ffi.enable=1
--FILE--
ok
--EXPECT--
ok
root@a11efdc33877:~/php/php-src# arch
x86_64
root@a11efdc33877:~/php/php-src#
 [2020-03-09 10:09 UTC] nikic@php.net
Something odd is going on here. The test passes for me on 7.4, but fails on master. Adding a bit more debug output I get:

FAIL: sizeof(struct  {char a; uint32_t b __attribute__((aligned));})
Expected: 32
Actual: 64
FAIL: alignof(struct  {char a; uint32_t b __attribute__((aligned));})
Expected: 16
Actual: 32

So I'm seeing a higher size/alignment for some reason.
 [2020-03-09 10:21 UTC] nikic@php.net
Okay, I see now. __attribute__((aligned)) is the same as __attribute__((__BIGGEST_ALIGNMENT__)). I happened to use an -march=sandybridge build, where __BIGGEST_ALIGNMENT__ is 32.
 [2020-03-09 10:24 UTC] nikic@php.net
-Assigned To: +Assigned To: dmitry
 [2020-03-09 10:24 UTC] nikic@php.net
@dmitry: I would suggest that we remove support for __attribute__((aligned)) without specified alignment, because it is not interoperable. If PHP was combined with a different target architecture than the library we're linking against, the alignment will be incorrect. We should force an explicit specification of the alignment instead. What do you think?
 [2020-03-11 09:07 UTC] dmitry@php.net
I'm not sure, if we should remove support for __attribute__((aligned)) without specified alignment completely. Probably, we should export __BIGGEST_ALIGNMENT__ constant and may be add a warning about dependency.
 [2020-03-11 09:11 UTC] nikic@php.net
@dmitry: Do you know of some particular case where this is being used?
 [2020-03-11 12:34 UTC] dmitry@php.net
@nikic: I don't know, but in case GCC supports this, I would prefer to support this as well. 

The proposed fix https://github.com/php/php-src/pull/5256
 [2020-03-12 08:52 UTC] dmitry@php.net
-Status: Assigned +Status: Closed
 [2020-03-12 08:52 UTC] dmitry@php.net
ext/ffi/tests/022.phpt failure is fixed by exporting FFI::__BIGGEST_ALIGNMENT__ constant.

Anyway, __attribute__((aligned)) without argument may cause invalid alignment assumption, if library and PHP were compiled with different options. The same situation with C.
 
PHP Copyright © 2001-2020 The PHP Group
All rights reserved.
Last updated: Fri Dec 04 12:01:23 2020 UTC