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

byterun: do not alias function arguments to sigprocmask #1752

Merged
merged 2 commits into from May 2, 2018

Conversation

Projects
None yet
4 participants
@avsm
Copy link
Member

avsm commented May 2, 2018

Recent gcc versions (e.g. as found in Fedora 28) check that restrict-qualified parameters do not alias each other in function calls. One such example that triggers a compile error is sigprocmask in the bytecode runtime.

The error this changeset fixes while compiling on Fedora 28 is:

signals.c: In function 'caml_execute_signal':
signals.c:152:33: error: passing argument 3 to restrict-qualified parameter aliases with argument 2 [-Werror=restrict]
   sigprocmask(SIG_BLOCK, &sigs, &sigs);
byterun: do not alias function arguments to sigprocmask
Recent gcc versions (e.g. as found in Fedora 28) check that
restrict-qualified parameters do not alias each other in
function calls. One such example that triggers a compile
error is sigprocmask in the bytecode runtime.

The error this changeset fixes is:

```
signals.c: In function 'caml_execute_signal':
signals.c:152:33: error: passing argument 3 to restrict-qualified parameter aliases with argument 2 [-Werror=restrict]
   sigprocmask(SIG_BLOCK, &sigs, &sigs);
```
@gasche

gasche approved these changes May 2, 2018

Copy link
Member

gasche left a comment

Looks correct to me, I think this is good to merge after the CI passes. I would be interested in another pair of eyeball before deciding to backport into 4.07.

@gasche

This comment has been minimized.

Copy link
Member

gasche commented May 2, 2018

@avsm wouldn't you want to add a Changes entry? I think that this could go into the 4.07 section.

@avsm

This comment has been minimized.

Copy link
Member Author

avsm commented May 2, 2018

@gasche thanks for the reminder; added!

@stedolan

This comment has been minimized.

Copy link
Contributor

stedolan commented May 2, 2018

Looks correct to me too.

@gasche gasche merged commit 8e582c7 into ocaml:trunk May 2, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

gasche added a commit that referenced this pull request May 2, 2018

Merge pull request #1752 from avsm/restrict-safe
byterun: do not alias function arguments to sigprocmask
(cherry picked from commit 8e582c7)
@gasche

This comment has been minimized.

Copy link
Member

gasche commented May 2, 2018

Cherry-picked in 4.07 in a4332cb

@xavierleroy

This comment has been minimized.

Copy link
Contributor

xavierleroy commented May 2, 2018

Well spotted! When the code was written the restrict qualifier didn't exist :-) I had no idea that (some implementations of) sigprocmask could object to having the same signal set as input and output. Good to know.

avsm added a commit to avsm/ocaml that referenced this pull request May 2, 2018

h0nzZik added a commit to h0nzZik/k5 that referenced this pull request Oct 30, 2018

h0nzZik added a commit to h0nzZik/k5 that referenced this pull request Oct 30, 2018

Revert "See ocaml/ocaml#1752"
This reverts commit aecb4d0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.