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

Migrate code to C99 #777

Open
McDutchie opened this issue Aug 8, 2024 · 1 comment
Open

Migrate code to C99 #777

McDutchie opened this issue Aug 8, 2024 · 1 comment
Labels
TODO Things to be done before releasing

Comments

@McDutchie
Copy link

We now require C90, but the C99 standard is 25 years old. It's time. As discussed in #772 (reply in thread), moving to C99 can fix undefined behaviour (UB) that the ast and ksh code base is currently suffering from:

  1. Lots of uses of the struct hack in libast and ksh, for example, the final dolval/argval members of struct dolnod and struct argnod which are central to command arguments processing in ksh. It works in practice because C90 had no alternative and compiler implementers just made it work and allowed out-of-bounds array access regardless of what the standard said. But C99 does have a good construct for exactly that use case (flexible array members), so we should use it. There is no guarantee the struct hack will still work in practice 25 years from now. The change should be made very carefully as flexible array members have a size of zero, so the often-used sizeof of these structs will change.
  2. Undefined behaviour in many places with regards to float values and their typecasts to/from integer values, e.g., this typecast which has UB if the float value is infinite or is not a number (NaN), a scenario which can easily occur in practice. To fix issues like these we need fpclassify(3) and/or isnan(3), isinf(3), etc., which were all introduced in C99. The libast code does feature tests for these now, so we could implement the fixes conditionally, but it's 2024 and it's better just to require them at this point.
  3. More to be found?
@McDutchie McDutchie added the TODO Things to be done before releasing label Aug 8, 2024
@McDutchie
Copy link
Author

Alternatively, we can keep C90 compatibility with the struct hack for now since it won't realistically be a problem anytime soon, if ever.

But problem 2, the undefined float behaviour, is actual breakage occurring right now. So at minimum we should modify the libast/features/float tests to error out if fpclassify(3) and friends are not present and working, and then use them to fix this. (Somewhat related issue: #771)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO Things to be done before releasing
Projects
None yet
Development

No branches or pull requests

1 participant