Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always pass -fno-strict-aliasing during compilation #714

Merged
merged 3 commits into from
Mar 23, 2024

Conversation

JohnoKing
Copy link

The ksh93 codebase is incompatible with strict aliasing, yet currently the default flags enable -fstrict-aliasing via the -Os optimization flag. Additionally, manually passing a higher level optimization flag via CCFLAGS (e.g. -O3) will result in strict aliasing becoming inadvertently enabled unless -fno-strict-aliasing is explicitly passed to disable it (cf. the Fedora and OpenSUSE build scripts for ksh*).

This commit implements a probe that will detect the existence of -fno-strict-aliasing and use it via ${mam_cc_FLAGS}, bypassing CCFLAGS.

References:
https://src.fedoraproject.org/rpms/ksh/blob/rawhide/f/ksh.spec#_49
https://build.opensuse.org/package/view_file/shells/ksh/ksh.spec?expand=1

The ksh93 codebase is incompatible with strict aliasing, yet currently
the default flags enable -fstrict-aliasing via the -Os optimization
flag. Additionally, manually passing a higher level optimization flag
via CCFLAGS (e.g. -O3) will result in strict aliasing becoming
inadvertently enabled unless -fno-strict-aliasing is explicitly passed
to disable it (cf. the Fedora and OpenSUSE build scripts for ksh[*]).

This commit implements a probe that will detect the existence of
-fno-strict-aliasing and use it via ${mam_cc_FLAGS}, bypassing
CCFLAGS.

[*]: https://src.fedoraproject.org/rpms/ksh/blob/rawhide/f/ksh.spec#_49
     https://build.opensuse.org/package/view_file/shells/ksh/ksh.spec?expand=1
@McDutchie
Copy link

I have been compiling and running ksh with -Os with no problems for years. Is it actually true that this code base is incompatible with strict aliasing? Which bits are incompatible, and can we fix them?

I need to learn more about what this means, but I need to find the time to do that first.

@JohnoKing
Copy link
Author

Most of the strict aliasing violations in the codebase are undetected because -Wstrict-aliasing is set to the lowest setting by default, for the purpose of avoiding false positives. However, raising the level to -Wstrict-aliasing=2 is enough to catch many examples of actual strict aliasing violations, such as this one:

/home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/print.c: In function 'B_echo':
/home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/print.c:119:51: warning: dereferencing type-punned pointer might break strict-aliasing rules [-Wstrict-aliasing]
  119 |                 return b_print(0,argv,(Shbltin_t*)&prdata);
      |                                                   ^~~~~~~
/home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/print.c:139:43: warning: dereferencing type-punned pointer might break strict-aliasing rules [-Wstrict-aliasing]
  139 |         return b_print(0,argv,(Shbltin_t*)&prdata);
      |                                           ^~~~~~~
/home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/print.c: In function 'b_printf':
/home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/print.c:150:44: warning: dereferencing type-punned pointer might break strict-aliasing rules [-Wstrict-aliasing]
  150 |         return b_print(-1,argv,(Shbltin_t*)&prdata);
      |                                            ^~~~~~~

struct print prdata;
prdata.options = sh_optecho+5;
prdata.raw = prdata.echon = 0;

return b_print(0,argv,(Shbltin_t*)&prdata);

struct print *pp = (struct print*)context;

Here, prdata, which is struct print, is cast to a Shbltin_t so that it can be passed as an argument to b_print. Many more of the warnings produced by -Wstrict-aliasing=2 are likely indicative of strict aliasing violations, and that's not even the highest warning level!

Build log with the strict aliasing warning level set to 1: log.txt

@McDutchie McDutchie merged commit e60c727 into ksh93:dev Mar 23, 2024
McDutchie pushed a commit that referenced this pull request Mar 23, 2024
The ksh93 codebase is incompatible with strict aliasing, yet
currently the default flags enable -fstrict-aliasing via the -Os
optimization flag. Additionally, manually passing a higher level
optimization flag via CCFLAGS (e.g. -O3) will result in strict
aliasing becoming inadvertently enabled unless -fno-strict-aliasing
is explicitly passed to disable it (cf. the Fedora and OpenSUSE
build scripts for ksh[*]).

This commit implements a probe that will detect the existence of
-fno-strict-aliasing and use it via ${mam_cc_FLAGS}, bypassing
CCFLAGS.

[*]: https://src.fedoraproject.org/rpms/ksh/blob/rawhide/f/ksh.spec#_49
     https://build.opensuse.org/package/view_file/shells/ksh/ksh.spec?expand=1
McDutchie pushed a commit that referenced this pull request Mar 23, 2024
The ksh93 codebase is incompatible with strict aliasing, yet
currently the default flags enable -fstrict-aliasing via the -Os
optimization flag. Additionally, manually passing a higher level
optimization flag via CCFLAGS (e.g. -O3) will result in strict
aliasing becoming inadvertently enabled unless -fno-strict-aliasing
is explicitly passed to disable it (cf. the Fedora and OpenSUSE
build scripts for ksh[*]).

This commit implements a probe that will detect the existence of
-fno-strict-aliasing and use it via ${mam_cc_FLAGS}, bypassing
CCFLAGS.

[*]: https://src.fedoraproject.org/rpms/ksh/blob/rawhide/f/ksh.spec#_49
     https://build.opensuse.org/package/view_file/shells/ksh/ksh.spec?expand=1
@JohnoKing JohnoKing deleted the avoid-strict-aliasing branch March 24, 2024 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants