Explicit header definitions for Open XL warnings #7321

Deigue · 2024-04-25T15:34:28Z

Open XL 2.1 on z/OS reports some of the implicit header declarations as errors and requires these to be explicitly defined. Most of the underlying changes are to address this concern and some other fixes pertaining to compilation errors.

(This is one part of the multiple changes added for supporting Open XL compilation on OMR)

port/zos390/omrsignal_context.h

port/zos390/omrintrospect.h

port/zos390/omrgetuserid.c

include_core/unix/thrdsup.h

port/zos390/omrintrospect.h

thread/unix/thrdsup.c

babsingh · 2024-05-07T18:34:26Z

port/zos390/omrosdump.c

 #if defined(J9ZOS390) && !defined(OMR_EBCDIC)
 #include "atoe.h"
 #endif
 #include "portnls.h"

+


new line is not needed; it should be removed.

port/zos390/omrintrospect.h

port/zos390/omrosdump.c

port/zos390/omrintrospect.h

babsingh · 2024-05-23T03:48:10Z

jenkins build all

babsingh · 2024-05-23T12:42:08Z

jenkins build all

babsingh · 2024-05-23T21:23:16Z

@Deigue Please investigate the failures in the PR build jobs.

I looked at the zOS PR build: https://ci.eclipse.org/omr/job/PullRequest-zos_390-64/4399/console

Here are the errors:

08:46:14  ERROR CCN3172 /u/user1/workspace/Build/thread/unix/thrdsup.c:71    Parameter type list for function setenv contains parameters without identifiers.
08:46:14  ERROR CCN3172 /u/user1/workspace/Build/thread/unix/thrdsup.c:71    Parameter type list for function setenv contains parameters without identifiers.
08:46:14  ERROR CCN3172 /u/user1/workspace/Build/thread/unix/thrdsup.c:71    Parameter type list for function setenv contains parameters without identifiers.
08:46:14  ERROR CCN3276 /u/user1/workspace/Build/thread/unix/thrdsup.c:71    Syntax error: possible missing '{'?
08:46:14  WARNING CCN3449 /u/user1/workspace/Build/thread/unix/thrdsup.c:106   Missing return expression.
08:46:14  ERROR CCN3045 /u/user1/workspace/Build/thread/unix/thrdsup.c:116   Undeclared identifier init_once.
08:46:14  CCN0793(I) Compilation failed for file /u/user1/workspace/Build/thread/unix/thrdsup.c.  Object file not created.
08:46:14  FSUM3065 The COMPILE step ended with return code 12.
08:46:14  FSUM3017 Could not compile /u/user1/workspace/Build/thread/unix/thrdsup.c. Correct the errors and try again.

Deigue · 2024-05-24T01:12:05Z

@Deigue Please investigate the failures in the PR build jobs.

I looked at the zOS PR build: https://ci.eclipse.org/omr/job/PullRequest-zos_390-64/4399/console

Here are the errors:

08:46:14  ERROR CCN3172 /u/user1/workspace/Build/thread/unix/thrdsup.c:71    Parameter type list for function setenv contains parameters without identifiers.
08:46:14  ERROR CCN3172 /u/user1/workspace/Build/thread/unix/thrdsup.c:71    Parameter type list for function setenv contains parameters without identifiers.
08:46:14  ERROR CCN3172 /u/user1/workspace/Build/thread/unix/thrdsup.c:71    Parameter type list for function setenv contains parameters without identifiers.
08:46:14  ERROR CCN3276 /u/user1/workspace/Build/thread/unix/thrdsup.c:71    Syntax error: possible missing '{'?
08:46:14  WARNING CCN3449 /u/user1/workspace/Build/thread/unix/thrdsup.c:106   Missing return expression.
08:46:14  ERROR CCN3045 /u/user1/workspace/Build/thread/unix/thrdsup.c:116   Undeclared identifier init_once.
08:46:14  CCN0793(I) Compilation failed for file /u/user1/workspace/Build/thread/unix/thrdsup.c.  Object file not created.
08:46:14  FSUM3065 The COMPILE step ended with return code 12.
08:46:14  FSUM3017 Could not compile /u/user1/workspace/Build/thread/unix/thrdsup.c. Correct the errors and try again.

Sounds good, I can fix some of those. Can I also type the "build all" command to trigger jenkins to start a build to verify changes made after I push a commit against the branch?
Also regarding 08:46:14 ERROR CCN3045 /u/user1/workspace/Build/thread/unix/thrdsup.c:116 Undeclared identifier init_once. : Isn't this defined on Line 80 already, wondering how come this error shows even though that part of the code or its related declaration is unchanged by this PR? Or whether its just something that is showing up because there are other errors above it?

babsingh · 2024-05-24T19:46:46Z

Can I also type the "build all" command to trigger jenkins to start a build to verify changes made after I push a commit against the branch?

No. For access, you can try requesting through @0xdaryl (project lead) and @AdamBrousseau (devops).

You can also message me on Slack; I can launch the builds for you.

Also regarding 08:46:14 ERROR CCN3045 /u/user1/workspace/Build/thread/unix/thrdsup.c:116 Undeclared identifier init_once. : Isn't this defined on Line 80 already, wondering how come this error shows even though that part of the code or its related declaration is unchanged by this PR?

One of the changes might have implicitly triggered the above error. Fixing other errors might automatically fix it.

If it persists, the following thread might help resolve the above error:
https://community.ibm.com/community/user/ibmz-and-linuxone/discussion/xlc-complains-ccn3277-ccn3485-ccn3045-but-gcc-on-zlinux-does-not-complain

Open XL 2.1 on z/OS reports some of the implicit declarations as errors, and requires these to be explicitly defined. Most of the underlying changes are to address this concern and some other fixes pertaining to compilation errors. _EXT is defined in port/zos390/omrosdump.c as the cnap() function declaration within the ctype.h header is guarded behind the _EXT macro and required to be present for Open XL. Signed-off-by: Gaurav Chaudhari <gaurav.chaudhari@ibm.com>

babsingh · 2024-06-03T16:36:45Z

jenkins build all

Deigue · 2024-06-04T20:28:24Z

I see this in the latest jenkin build log
[2024-06-03T16:40:30.430Z] ERROR CCN3276 /u/user1/workspace/Build/thread/unix/thrdsup.c:71 Syntax error: possible missing '{'?
But as per the latest tweaks/changes to thrdsup.c L71 looks ok, and had compiled fine on my side.

Does something still look syntactically wrong? Because I used the method signatures from official documentation and verified against headers for the params so not sure what it is going forward...

babsingh · 2024-06-04T20:44:15Z

The errors are slightly different in the OSX builds:

12:39:37  /Users/omr/workspace/Build/thread/unix/thrdsup.c:71:64: error: expected function body after function declarator
12:39:37  int setenv(const char *name, const char *value, int overwrite) __THROW;
12:39:37                                                                 ^
12:39:37  /Users/omr/workspace/Build/thread/unix/thrdsup.c:72:32: error: expected function body after function declarator
12:39:37  char *getenv(const char *name) __THROW;
12:39:37                                 ^
12:39:37  2 errors generated.

babsingh · 2024-06-04T20:45:06Z

Only the AIX and zOS builds have the following error:

12:39:38  "/home/omr/workspace/Build/thread/unix/thrdsup.c", line 71.64: 1506-276 (S) Syntax error: possible missing '{'?
12:39:38  "/home/omr/workspace/Build/thread/unix/thrdsup.c", line 80.28: 1506-277 (S) Syntax error: possible missing ';' or ','?
12:39:38  "/home/omr/workspace/Build/thread/unix/thrdsup.c", line 116.23: 1506-045 (S) Undeclared identifier init_once.

This looks like a side-effect of the thrdsup.c changes, and related to #7321 (comment).

babsingh · 2024-06-04T20:46:41Z

On Windows, there is a test failure:

13:18:48  20: [----------] 12 tests from PriorityInterrupt
14:39:08  Cancelling nested steps due to timeout
14:39:08  Sending interrupt signal to process
14:39:19  20/24 Test #20: threadtest ........................***Failed  4844.66 sec

Deigue · 2024-06-05T17:45:46Z

Just a quick update re some internal discussions on the right way to fix this problem.
We ideally want to not redeclare these things again, and stdlib should be responsible for correctly declaring them.

On further digging, seems like setenv() is causing a declaration problem in Context with Open XL as such:

cd /jit/team/gauravc/repos/omr/build/thread && /xlc210/usr/lpp/IBM/cnw/v2r1/openxl/bin/ibm-clang64  -D__STDC_LIMIT_MACROS -D_ALL_SOURCE -D_ISOC99_SOURCE -D_OPEN_THREADS=3 -D_POSIX_SOURCE -D_XOPEN_SOURCE_EXTENDED -DJ9ZOS390 -DJ9ZOS39064 -DLONGLONG -DOMR_EBCDIC -DSUPPORTS_THREAD_LOCAL -DZOS -I/jit/team/gauravc/repos/omr/build/thread -I/jit/team/gauravc/repos/omr/thread/. -I/jit/team/gauravc/repos/omr/thread/zos390 -I/jit/team/gauravc/repos/omr/thread/unix -I/jit/team/gauravc/repos/omr/thread/common -I/jit/team/gauravc/repos/omr/include_core -I/jit/team/gauravc/repos/omr/build  -fstrict-aliasing -mzos-target=ZOSV2R4 -m64 -march=arch10   -fvisibility=default -o CMakeFiles/j9thr_obj.dir/unix/thrdsup.c.o   -c /jit/team/gauravc/repos/omr/thread/unix/thrdsup.c
/jit/team/gauravc/repos/omr/thread/unix/thrdsup.c:340:7: error: call to undeclared function 'setenv'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  340 |                 if (setenv("_EDC_PTHREAD_YIELD", "-2", 1) != 0) {
      |                     ^
/jit/team/gauravc/repos/omr/thread/unix/thrdsup.c:340:7: note: did you mean 'getenv'?
/usr/include/stdlib.h:479:14: note: 'getenv' declared here
  479 |     char *   getenv (const char *) __THROW;
      |              ^
1 error generated.

After looking at the stdlib zos headers, I see that the declare was guarded by _EXT and perhaps this needed to be defined.
Adding the following at the start of the thrdsup.c file fixes the compilation errors.

#if defined(J9ZOS390)
#define _EXT
#endif /* defined(J9ZOS390) */

Currently discussing a few things with compiler team before finalizing changes..

do we need to explicitly define _EXT? Why can we not just specific #include <stdlib.h> and have things work?
do we want to be more specific if it is needed? Rather use a #if defined(__open_xl_) to guard the _EXT definition. This doesnt cause a problem with XLC, maybe just brings a warning .. so not sure if we want to keep it broader guard or not.
@babsingh question/suggestion from you that I am looking for, but I see that port/zos390/omrosdump.c I also added a _EXT definition, would you rather this be gaurded with one of the above as well?

I think I can make the tweaks once I get a better understanding on the above.

Deigue requested review from babsingh, mstoodle and 0xdaryl as code owners April 25, 2024 15:34

github-actions bot added arch:z comp:core comp:port comp:thread comp:utilities labels Apr 25, 2024

Deigue force-pushed the openxl-declarations branch from dd43d97 to 4f257e0 Compare April 26, 2024 16:02

babsingh reviewed Apr 30, 2024

View reviewed changes

Deigue force-pushed the openxl-declarations branch from 4f257e0 to 2e98c96 Compare May 6, 2024 20:12

babsingh reviewed May 9, 2024

View reviewed changes

Deigue force-pushed the openxl-declarations branch 2 times, most recently from 6b4576b to 660db3c Compare May 13, 2024 16:41

babsingh reviewed May 13, 2024

View reviewed changes

port/zos390/omrintrospect.h Outdated Show resolved Hide resolved

Deigue force-pushed the openxl-declarations branch 2 times, most recently from bb2756c to 47b5f0f Compare May 21, 2024 20:07

Deigue force-pushed the openxl-declarations branch from 47b5f0f to c1c2105 Compare June 3, 2024 16:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Explicit header definitions for Open XL warnings #7321

Explicit header definitions for Open XL warnings #7321

Deigue commented Apr 25, 2024

babsingh May 7, 2024

Deigue May 13, 2024

babsingh commented May 23, 2024

babsingh commented May 23, 2024

babsingh commented May 23, 2024

Deigue commented May 24, 2024

babsingh commented May 24, 2024 •

edited

babsingh commented Jun 3, 2024

Deigue commented Jun 4, 2024

babsingh commented Jun 4, 2024

babsingh commented Jun 4, 2024 •

edited

babsingh commented Jun 4, 2024

Deigue commented Jun 5, 2024

Explicit header definitions for Open XL warnings #7321

Are you sure you want to change the base?

Explicit header definitions for Open XL warnings #7321

Conversation

Deigue commented Apr 25, 2024

babsingh May 7, 2024

Choose a reason for hiding this comment

Deigue May 13, 2024

Choose a reason for hiding this comment

babsingh commented May 23, 2024

babsingh commented May 23, 2024

babsingh commented May 23, 2024

Deigue commented May 24, 2024

babsingh commented May 24, 2024 • edited

babsingh commented Jun 3, 2024

Deigue commented Jun 4, 2024

babsingh commented Jun 4, 2024

babsingh commented Jun 4, 2024 • edited

babsingh commented Jun 4, 2024

Deigue commented Jun 5, 2024

babsingh commented May 24, 2024 •

edited

babsingh commented Jun 4, 2024 •

edited