Fix problems in relation to Fedora's C99 instrumentation #665
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
src/lib/libast/comp/conf.tab:
- Add include directives to fix implicit function declarations that
cause the PID_MAX test to silently fail. This compile error occurs
on all platforms when compiling with strict C99, but wasn't
exposed even after setting IFFEFLAGS to -d1 or -d2 (that's likely
a separate bug in conf.sh that'll need fixing). As a consequence,
it could cause following bug in the getconf builtin (now fixed
as of this commit):
$ /opt/ast/bin/getconf PID_MAX
30000 # Wrong, should be the platform value (e.g., 4194304)
src/lib/libast/{features/lib,sfio/sfhdr.h}:
- Delete the lib_poll_fd_2 feature test and associated code, which
allowed ksh to used non-compliant implementations of poll(3). This
feature test fails on POSIX-compliant systems with int-conversion
errors, which is the expected result. However, the flawed C99
instrumentation used by Red Hat marks the entire build as a failure
because of the fact those errors exist at all, even though the
build proceeds as normal. Fortunately, the operating systems
supported by ksh (afaik) all have proper, POSIX-compliant versions
of poll() and thus don't need this test. To work around the flawed
instrumentation, the unnecessary test has been jettisoned.
Note that the changes in this commit *do not* fix all the errors.
The ones that remain are caused by the incomplete exception list in
redhat-rpm-config, which doesn't include the necessary functions
ksh tests for. That must be fixed in redhat-rpm-config, not ksh.
A patch which adds the required exceptions to the Lua script can
be found in the following comment:
ksh93#664 (comment)
After applying the patch, the Lua script (which can be run via
/usr/lib/rpm/redhat/report-gcc-errors.sh) will ignore the
implicit function errors that are expected to occur in the feature
tests, which (after fixing conf.tab and _lib_poll) should allow
the build to succeed with Red Hat's instrumentation.
|
After applying the patch from #664 (comment) to redhat-rpm-config, I can confirm that the instrumented gcc did not report any errors when building Thank you very much for all your hard work and effort! |
|
I second that! Apologies for taking months to get around to this – real life kept having to take priority. |
McDutchie
pushed a commit
that referenced
this pull request
Sep 9, 2023
This commit applies the necessary changes to the ksh codebase to fix C99 compilation when using Red Hat's instrumented GCC[*], though note that a patch to redhat-rpm-config is also required and must be applied separately[*2]. [*1] https://fedoraproject.org/wiki/Toolchain/PortingToModernC [*2] #664 (comment) src/lib/libast/comp/conf.tab: - Add include directives to fix implicit function declarations that cause the PID_MAX test to silently fail. This compile error occurs on all platforms when compiling with strict C99, but wasn't exposed even after setting IFFEFLAGS to -d1 or -d2 (that's likely a separate bug in conf.sh that'll need fixing). As a consequence, it could cause following bug in the getconf builtin (now fixed as of this commit): $ /opt/ast/bin/getconf PID_MAX 30000 # Wrong, should be the platform value (e.g., 4194304) src/lib/libast/{features/lib,sfio/sfhdr.h}: - Delete the lib_poll_fd_2 feature test and associated code, which allowed ksh to used non-compliant implementations of poll(3). This feature test fails on POSIX-compliant systems with int-conversion errors, which is the expected result. However, the C99 instrumentation used by Red Hat marks the entire build as a failure because of the fact those errors exist at all, even though the build proceeds as normal. Fortunately, the operating systems supported by ksh (AFAIK) all have proper, POSIX-compliant versions of poll() and thus don't need this test. To work around the flawed instrumentation, the unnecessary test has been jettisoned.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request applies the necessary changes to the ksh codebase to (hopefully) fix C99 compilation when using Red Hat't instrumented GCC (cf. https://fedoraproject.org/wiki/Toolchain/PortingToModernC, though note that a patch to redhat-rpm-config is also required and must be applied separately, see #664 (comment)). Seeing as my previous attempt did not fully fix this problem, I would like @vmihalko to confirm that this pull request + the patch to redhat-rpm-config successfully fix the build errors that occur when building ksh with C99.
List of changes in the pull request:
src/lib/libast/comp/conf.tab:
- Add include directives to fix implicit function declarations that cause the PID_MAX test to silently fail. This compile error occurs on all platforms when compiling with strict C99, but wasn't exposed even after setting IFFEFLAGS to -d1 or -d2 (that's likely a separate bug in conf.sh that'll need fixing). As a consequence, it could cause following bug in the getconf builtin (now fixed as of this commit):
$ /opt/ast/bin/getconf PID_MAX 30000 # Wrong, should be the platform value (e.g., 4194304)src/lib/libast/{features/lib,sfio/sfhdr.h}:
- Delete the lib_poll_fd_2 feature test and associated code, which allowed ksh to used non-compliant implementations of poll(3). This feature test fails on POSIX-compliant systems with int-conversion errors, which is the expected result. However, the C99 instrumentation used by Red Hat marks the entire build as a failure because of the fact those errors exist at all, even though the build proceeds as normal. Fortunately, the operating systems supported by ksh (afaik) all have proper, POSIX-compliant versions of poll() and thus don't need this test. To work around the flawed instrumentation, the unnecessary test has been jettisoned.
Note that the changes to ksh listed above do not fix all the errors. The remaining implicit function errors are caused by the incomplete exception list in redhat-rpm-config, which doesn't include the necessary functions ksh tests for. That must be fixed in redhat-rpm-config, not ksh. A patch which adds the required exceptions to the Lua script can be found in the following comment:
#664 (comment)
After applying the patch, the Lua script (which can be run via
/usr/lib/rpm/redhat/report-gcc-errors.sh) will ignore the implicit function errors that are expected to occur in the feature tests, which (after fixing conf.tab and _lib_poll) should allow the build to succeed with Red Hat's instrumentation.