Skip to content

Conversation

@sunfishcode
Copy link
Member

Previously, utimensat would leave the mtim and/or atim timestamps
uninitialized when the MTIM_NOW or ATIM_NOW were in use, because
that means the respective timestamps are not used.

However, clang now automatically adds noundef to the arguments in
functions like __wasi_path_filestat_set_times, and there are cases
where simplifycfg can see paths where the uninitialized values are
passed to those noundef arguments.

To fix this, change the utimens code to zero out the timestamps when
they aren't in use, to avoid passing uninitialized arguments.

Previously, utimensat would leave the mtim and/or atim timestamps
uninitialized when the `MTIM_NOW` or `ATIM_NOW` were in use, because
that means the respective timestamps are not used.

However, clang now automatically adds `noundef` to the arguments in
functions like `__wasi_path_filestat_set_times`, and there are cases
where simplifycfg can see paths where the uninitialized values are
passed to those `noundef` arguments.

To fix this, change the utimens code to zero out the timestamps when
they aren't in use, to avoid passing uninitialized arguments.
@sbc100
Copy link
Member

sbc100 commented May 24, 2022

Why does clang automatically add noundef to those argument? It would seem incorrect if they are essentially optional.

@sunfishcode
Copy link
Member Author

I'm not current on the latest thinking about uninitialized variables, but I imagine it's considered UB in C to pass uninitialized values as function arguments. Clang 14 appears to automatically add noundef to all C function arguments.

Copy link
Member

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough.

@sunfishcode sunfishcode merged commit 225b285 into main May 24, 2022
@sunfishcode sunfishcode deleted the sunfishcode/utimens-timestamps branch May 24, 2022 22:17
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.

3 participants