classification
Title: AIX compile error with Modules/posixmodule.c: Function argument assignment between types "unsigned long" and "struct fsid_t" is not allowed
Type: compile error Stage: resolved
Components: Build Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: David.Edelsohn, Michael.Felt, vstinner, xdegaye
Priority: normal Keywords: patch

Created on 2017-12-20 19:02 by Michael.Felt, last changed 2018-01-05 18:30 by David.Edelsohn. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 4972 merged Michael.Felt, 2017-12-22 09:46
Messages (21)
msg308775 - (view) Author: Michael Felt (Michael.Felt) * Date: 2017-12-20 19:02
current level: commit 4b965930e8625f77cb0e821daf5cc40e85b45f84 (HEAD -> master, upstream/master, origin/master, origin/HEAD)

Build message:
michael@x071:[/data/prj/python/git/python3-3.7.X]make
xlc_r  -DNDEBUG -O -I/opt/include -O2 -qmaxmem=-1 -qarch=pwr5      -I. -I./Include -I/opt/include   -DPy_BUILD_CORE  -c ./Modules/posixmodule.c -o Modules/posixmodule.o
"./Modules/posixmodule.c", line 5514.11: 1506-131 (W) Explicit dimension specification or initializer required for an auto or static array.
"./Modules/posixmodule.c", line 9328.64: 1506-280 (S) Function argument assignment between types "unsigned long" and "struct fsid_t" is not allowed.
Makefile:1765: recipe for target 'Modules/posixmodule.o' failed
make: *** [Modules/posixmodule.o] Error 1

Details (note rewrite from listing for line 5514)

     5514 | PyDoc_VAR(os_sched_param__doc__);
     5514 + static char os_sched_param__doc__[];
"./Modules/posixmodule.c", line 5514.11: 1506-131 (W) Explicit dimension specification or initializer required for an auto or static array.
     5515 |
     5516 | static PyStructSequence_Field sched_param_fields[] = {
     5517 |     {"sched_priority", "the scheduling priority"},
     5518 |     {0}
     5519 | };
msg308790 - (view) Author: Michael Felt (Michael.Felt) * Date: 2017-12-20 19:36
OOps - wrong error! 

It is the new fsid variable!

michael@x071:[/data/prj/python/git/python3-3.7.0.a3]xlc_r  -DNDEBUG -O -I/opt/include -O2 -qmaxmem=-1 -qarch=pwr5      -I. -I./Include -I/opt/in>
"./Modules/posixmodule.c", line 5514.11: 1506-131 (W) Explicit dimension specification or initializer required for an auto or static array.
"./Modules/posixmodule.c", line 9328.64: 1506-280 (S) Function argument assignment between types "unsigned long" and "struct fsid_t" is not allowed.

     9326 |     PyStructSequence_SET_ITEM(v, 9, PyLong_FromLong((long) st.f_namemax));
     9326 +     (((PyTupleObject *)(v))->ob_item[9] = PyLong_FromLong((long) st.f_namemax));
     9327 | #endif
"./Modules/posixmodule.c", line 9328.64: 1506-280 (S) Function argument assignment between types "unsigned long" and "struct fsid_t" is not allowed
.
     9328 |     PyStructSequence_SET_ITEM(v, 10, PyLong_FromUnsignedLong(st.f_fsid));
     9328 +     (((PyTupleObject *)(v))->ob_item[10] = PyLong_FromUnsignedLong(st.f_fsid));
     9329 |     if (PyErr_Occurred()) {
     9330 |         Py_DECREF(v);
     9330 +         do { PyObject *_py_decref_tmp = (PyObject *)(v); if (   --(_py_decref_tmp)->ob_refcnt != 0) ; else (      (*(((PyObject*)(_py_d
ecref_tmp))->ob_type)->tp_dealloc)((PyObject *)(_py_decref_tmp))); } while (0);
msg308794 - (view) Author: Michael Felt (Michael.Felt) * Date: 2017-12-20 19:49
FYI: from /usr/include/types.h

/* typedef for the File System Identifier (fsid).  This must correspond
 * to the "struct fsid" structure in _ALL_SOURCE below.
 */
typedef struct fsid_t {
#ifdef __64BIT_KERNEL
        unsigned long val[2];
#else  /* __64BIT_KERNEL */
#ifdef _ALL_SOURCE
        unsigned int val[2];
#else  /* _ALL_SOURCE */
        unsigned int __val[2];
#endif /* _ALL_SOURCE */
#endif /* __64BIT_KERNEL */
} fsid_t;

And, currently I am building in 32-bit mode, with a 64BIT kernel (AIX 6.1) - so I expect it to be 2 unsigned_longs.

However, I am not smart enough to find a solution: I tried:
    PyStructSequence_SET_ITEM(v, 10, PyLong_FromUnsignedLong((unsigned long) st.f_fsid));

and     PyStructSequence_SET_ITEM(v, 10, PyLong_FromUnsignedLong((unsigned long *) st.f_fsid));

Both return:

"./Modules/posixmodule.c", line 9328.80: 1506-117 (S) Operand must be a scalar type.
msg308818 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2017-12-20 21:15
The PPC64 AIX 3.x Python buildbot (http://buildbot.python.org/all/#/builders/10) has been failing upon this same error for over a month.

Can you please try with:

PyStructSequence_SET_ITEM(v, 10, PyLong_FromUnsignedLong(st.f_fsid.val[0]));

Or if statvfs.f_fsid is a pointer to fsid_t, try this instead:

PyStructSequence_SET_ITEM(v, 10, PyLong_FromUnsignedLong(st.f_fsid->val[0]));
msg308930 - (view) Author: Michael Felt (Michael.Felt) * Date: 2017-12-22 09:08
Well, replied via email response - but it did not show up here.

You suggestion worked. I'll start a PR and we can see if your suggestion also works for all the other platforms.
msg308993 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2017-12-24 10:09
> The PPC64 AIX 3.x Python buildbot (http://buildbot.python.org/all/#/builders/10) has been failing upon this same error for over a month.

Michael Felt answered by email:

> Not quite a month: 8 days ago (test 357 was the first with this error). Great suggestion!

Right, f_fsid has been added only few days ago by issue 32143.
msg309332 - (view) Author: Michael Felt (Michael.Felt) * Date: 2018-01-01 16:17
Did some additional research. The code can work asis when _ALL_SOURCE is undefined. Or, in other words - there does not have to be a variable syntax issue or separate code for AIX.

As you read this - please remember that since issue #32143 was 'resolved' AIX cannot compile.

* https://www.gnu.org/software/autoconf/manual/autoconf-2.64/html_node/Posix-Variants.html

describes _ALL_SOURCE as a macro for AIX3.

Looking at AIX: /usr/include/sys/types.h - by undefining _ALL_SOURCE the typedefs in the header files are equivalent enough that the new code added for os.statvfs(path) is compiled by both gcc and xlc.

As examples (from Centos)
/usr/include/bits/typesizes.h:72:#define __FSID_T_TYPE          struct { int __val[2]; }

And from AIX:
 * /usr/include/sys/types.h
  +360  / * typedef for the File System Identifier (fsid).  This must correspond
  +361    * to the "struct fsid" structure in _ALL_SOURCE below.
  +362    * /
  +363  typedef struct fsid_t {
  +364  #ifdef __64BIT_KERNEL
  +365          unsigned long val[2];
  +366  #else  / * __64BIT_KERNEL * /
  +367  #ifdef _ALL_SOURCE
  +368          unsigned int val[2];
  +369  #else  / * _ALL_SOURCE * /
  +370          unsigned int __val[2];
  +371  #endif / * _ALL_SOURCE * /
  +372  #endif / * __64BIT_KERNEL * /
  +373  } fsid_t;
 */

(Note - a variation of the text above was in the pull request https://github.com/python/cpython/pull/4972. Per reviewer request - this is removed and a discussion is now open here.)

Thank you for your comments.
msg309369 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-01-02 11:48
Can you please post the definition of the statvfs structure on AIX.
msg309376 - (view) Author: David Edelsohn (David.Edelsohn) * Date: 2018-01-02 15:27
struct statvfs {
        ulong_t    f_bsize;     /* preferred file system block size          */
        ulong_t    f_frsize;    /* fundamental file system block size        */
        fsblkcnt_t f_blocks;    /* total # of blocks of f_frsize in fs       */ 
        fsblkcnt_t f_bfree;     /* total # of free blocks                    */
        fsblkcnt_t f_bavail;    /* # of blocks available to non super user   */
        fsfilcnt_t f_files;     /* total # of file nodes (inode in JFS)      */
        fsfilcnt_t f_ffree;     /* total # of free file nodes                */
        fsfilcnt_t f_favail;    /* # of nodes available to non super user    */
#ifdef _ALL_SOURCE
        fsid_t     f_fsid;      /* file system id                            */
#else
        ulong_t    f_fsid;      /* file system id                            */
#ifndef __64BIT__
        ulong_t    f_fstype;    /* file system type                          */
#endif
#endif  /* _ALL_SOURCE */
        char       f_basetype[_FSTYPSIZ]; /* Filesystem type name (eg. jfs)  */
        ulong_t    f_flag;      /* bit mask of flags                         */
        ulong_t    f_namemax;   /* maximum filename length                   */
        char       f_fstr[32];  /* filesystem-specific string */
        ulong_t    f_filler[16];/* reserved for future use                   */
};
msg309378 - (view) Author: David Edelsohn (David.Edelsohn) * Date: 2018-01-02 15:30
/* typedef for the File System Identifier (fsid).  This must correspond 
 * to the "struct fsid" structure in _ALL_SOURCE below.
 */
typedef struct fsid_t {
#ifdef __64BIT_KERNEL
        unsigned long val[2];
#else  /* __64BIT_KERNEL */
#ifdef _ALL_SOURCE
        unsigned int val[2];
#else  /* _ALL_SOURCE */
        unsigned int __val[2];
#endif /* _ALL_SOURCE */
#endif /* __64BIT_KERNEL */
} fsid_t;

#ifdef _KERNEL
typedef struct fsid32_t {
        unsigned int val[2];
} fsid32_t;
#endif /* __64BIT_KERNEL */

typedef struct fsid64_t {
#if defined(_ALL_SOURCE) && (defined(__64BIT__) || defined(_LONG_LONG))
        uint64_t val[2];
#else /* _ALL_SOURCE */
        uint32_t __val[4];
#endif /* _ALL_SOURCE */
} fsid64_t;

/* typedef for the File System Identifier (fsid) */
struct fsid {
#ifndef __64BIT_KERNEL
        unsigned int    val[2];
#else
        unsigned long   val[2];
#endif
};
msg309412 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-01-03 11:53
The _ALL_SOURCE feature test macro is set by the AC_USE_SYSTEM_EXTENSIONS macro in configure.ac.
It is not clear why the _ALL_SOURCE macro is currently needed by Python.

The _ALL_SOURCE feature test macro is defined here: https://www.ibm.com/support/knowledgecenter/en/SSLTBW_2.1.0/com.ibm.zos.v2r1.bpxbd00/ftms.htm
msg309413 - (view) Author: David Edelsohn (David.Edelsohn) * Date: 2018-01-03 13:41
_ALL_SOURCE is overkill. It probably is too big a club for this regression. However, the AIX header definition of fsid compatible with the current Python posixmodule.c code is bracketed by _ALL_SOURCE.

AFAICT, the change to posixmodule.c made assumptions about fsid based on Linux and not required by POSIX. This didn't simply introduce a testsuite regression, but broke the build on AIX. The posixmodule.c code should be corrected or should be reverted.
msg309418 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-01-03 17:03
The following patch may be less invasive and more explicit:

diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index 38b6c80e6b..e0bb4ba869 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -9325,7 +9325,11 @@ _pystatvfs_fromstructstatvfs(struct statvfs st) {
     PyStructSequence_SET_ITEM(v, 8, PyLong_FromLong((long) st.f_flag));
     PyStructSequence_SET_ITEM(v, 9, PyLong_FromLong((long) st.f_namemax));
 #endif
+#if defined(_AIX) && defined(_ALL_SOURCE)
+    PyStructSequence_SET_ITEM(v, 10, PyLong_FromUnsignedLong(st.f_fsid.val[0]));
+#else
     PyStructSequence_SET_ITEM(v, 10, PyLong_FromUnsignedLong(st.f_fsid));
+#endif
     if (PyErr_Occurred()) {
         Py_DECREF(v);
         return NULL;
msg309419 - (view) Author: Michael Felt (Michael.Felt) * Date: 2018-01-03 17:21
On 03/01/2018 14:41, David Edelsohn wrote:
> David Edelsohn <dje.gcc@gmail.com> added the comment:
>
> _ALL_SOURCE is overkill. It probably is too big a club for this regression.

Maybe. Clearly it is a big club. The documentation - if you can find any 
- is ancient and/or confusing. For example the xlc compiler reference 
quide (v12 is what I referenced, not the latest, but what I found) only 
has a single reference to any "_SOURCE" macro (-D_POSIX_SOURCE).

I do not know 'why' AIX and Linux differ in the way they name things. I 
do not want to care either - which is why I, personally, am less 
concerned about the size of the club. People much more clever than I 
decided that many variable names should be without two underscores 
(_ALL_SOURCE is defined) while others felt they must have two 
underscores (_ALL_SOURCE is undefined).

IMHO: 15+ years ago IBM (AIX) worked to find a method to simplify 
porting. And, I hope somewhere someone knows what these all meant. The 
practice seems to be to always define _ALL_SOURCE (see configure.ac:
   +882  # checks for UNIX variants that set C preprocessor variables
   +883  AC_USE_SYSTEM_EXTENSIONS
   +884
https://www.gnu.org/software/autoconf/manual/autoconf-2.64/html_node/Posix-Variants.html

Here is where I read that _ALL_SOURCE is for AIX3. I can neither deny 
nor affirm that that is (still) accurate. But that is what working with 
autotools does. Throws a sauce over everything that may, or maynot be 
what is needed.

I considered it 'interesting' that <sys/types.h> at least talks a bit 
about _POSIX_SOURCE and _ALL_SOURCE.

>   However, the AIX header definition of fsid compatible with the current Python posixmodule.c code is bracketed by _ALL_SOURCE.
>
> AFAICT, the change to posixmodule.c made assumptions about fsid based on Linux and not required by POSIX. This didn't simply introduce a testsuite regression, but broke the build on AIX. The posixmodule.c code should be corrected or should be reverted.

Maybe reverting the change is better than using the "big club". But, 
asis, AIX is dead to development. Was it possible to have AIX included 
in the PR test process I would hope that the PR would never have been 
merged.

IMHO - this is a spelling issue, going back years. But if you use a 
UK-only spelling checker and try and use only US spelling rules - and 
v.v. - there will be 'issues'. What is the solution with the most 
clarity? Above my pay grade to answer.

In any case the previous issue that saw adding fsid as a solution was 
not fully tested across all platforms. Currently AIX is broken. Someone 
needs to decide how to restore a supported platform - and where the 
discussion on fsid either restarts or continues.

In short - I am just a messenger - you are the experts. :)

>
> ----------
> nosy: +vstinner
> title: AIX (xlc_r) compile error with Modules/posixmodule.c: Function argument assignment between types "unsigned long" and "struct fsid_t" is not allowed -> AIX compile error with Modules/posixmodule.c: Function argument assignment between types "unsigned long" and "struct fsid_t" is not allowed
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue32390>
> _______________________________________
>
msg309423 - (view) Author: Michael Felt (Michael.Felt) * Date: 2018-01-03 17:53
Ignore my last comment - I have a headache. If I could edit/delete it I would.

aka "reset 2018 - here I come!"
msg309425 - (view) Author: Michael Felt (Michael.Felt) * Date: 2018-01-03 18:06
On 03/01/2018 18:03, Xavier de Gaye wrote:
> Xavier de Gaye <xdegaye@gmail.com> added the comment:
>
> The following patch may be less invasive and more explicit:
>
> diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
> index 38b6c80e6b..e0bb4ba869 100644
> --- a/Modules/posixmodule.c
> +++ b/Modules/posixmodule.c
> @@ -9325,7 +9325,11 @@ _pystatvfs_fromstructstatvfs(struct statvfs st) {
>       PyStructSequence_SET_ITEM(v, 8, PyLong_FromLong((long) st.f_flag));
>       PyStructSequence_SET_ITEM(v, 9, PyLong_FromLong((long) st.f_namemax));
>   #endif
> +#if defined(_AIX) && defined(_ALL_SOURCE)
> +    PyStructSequence_SET_ITEM(v, 10, PyLong_FromUnsignedLong(st.f_fsid.val[0]));
> +#else
>       PyStructSequence_SET_ITEM(v, 10, PyLong_FromUnsignedLong(st.f_fsid));
> +#endif
>       if (PyErr_Occurred()) {
>           Py_DECREF(v);
>           return NULL;
>
> ----------
Applied to the PR. Thx.
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue32390>
> _______________________________________
>
msg309500 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-01-05 12:02
New changeset 502d551c6d782963d26957a9e5ff1588946f233f by xdegaye (Michael Felt) in branch 'master':
bpo-32390: Fix compilation failure on AIX after f_fsid was added to os.statvfs() (#4972)
https://github.com/python/cpython/commit/502d551c6d782963d26957a9e5ff1588946f233f
msg309501 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-01-05 12:03
Thanks Michael for your contribution in fixing this issue.
msg309508 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-01-05 14:42
A compilation error is a blocking bug. It is short and short, it can be
backported to 2.7 and 3.6 no? Is ALL_SOURCE defined vy default?
msg309510 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-01-05 17:07
The compilation error occurs only on the master branch, issue 32143 is the enhancement that is the  initial cause of this problem. This went unnoticed when issue 32143 was resolved because the AIX buildbots were already failing at that time for another reason.
msg309514 - (view) Author: David Edelsohn (David.Edelsohn) * Date: 2018-01-05 18:30
The AIX buildbots has been exhibiting testsuite failures, but not build (compile) failures. The buildbots do not visibly distinguish between the two cases in a strong manner.

We can disable / expect failure for the few, additional testcases on AIX so that the buildbots will report "success" to make new failures more obvious.
History
Date User Action Args
2018-01-05 18:30:02David.Edelsohnsetmessages: + msg309514
2018-01-05 17:07:55xdegayesetmessages: + msg309510
2018-01-05 14:42:31vstinnersetmessages: + msg309508
2018-01-05 12:03:32xdegayesetstatus: open -> closed
resolution: fixed
messages: + msg309501

stage: patch review -> resolved
2018-01-05 12:02:04xdegayesetmessages: + msg309500
2018-01-03 18:06:06Michael.Feltsetmessages: + msg309425
2018-01-03 17:53:52Michael.Feltsetmessages: + msg309423
2018-01-03 17:21:14Michael.Feltsetmessages: + msg309419
2018-01-03 17:03:53xdegayesetmessages: + msg309418
2018-01-03 13:41:10David.Edelsohnsetnosy: + vstinner

messages: + msg309413
title: AIX (xlc_r) compile error with Modules/posixmodule.c: Function argument assignment between types "unsigned long" and "struct fsid_t" is not allowed -> AIX compile error with Modules/posixmodule.c: Function argument assignment between types "unsigned long" and "struct fsid_t" is not allowed
2018-01-03 11:53:58xdegayesetmessages: + msg309412
2018-01-02 15:30:42David.Edelsohnsetmessages: + msg309378
2018-01-02 15:27:36David.Edelsohnsetnosy: + David.Edelsohn
messages: + msg309376
2018-01-02 11:48:01xdegayesetmessages: + msg309369
2018-01-01 16:17:04Michael.Feltsetmessages: + msg309332
2017-12-24 10:09:04xdegayesetmessages: + msg308993
2017-12-22 09:46:42Michael.Feltsetkeywords: + patch
stage: patch review
pull_requests: + pull_request4863
2017-12-22 09:08:02Michael.Feltsetmessages: + msg308930
2017-12-20 21:15:08xdegayesetnosy: + xdegaye
messages: + msg308818
2017-12-20 19:49:51Michael.Feltsetmessages: + msg308794
2017-12-20 19:36:36Michael.Feltsetmessages: + msg308790
title: AIX (xlc_r) compile error with Modules/posixmodule.c: Explicit dimension specification or initializer required -> AIX (xlc_r) compile error with Modules/posixmodule.c: Function argument assignment between types "unsigned long" and "struct fsid_t" is not allowed
2017-12-20 19:02:31Michael.Feltcreate