Skip to content

Add Checked Blocks/Pragmas to Olden/em3d, Olden/health, Olden/mst #36

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

Merged
merged 5 commits into from
May 17, 2017

Conversation

lenary
Copy link
Collaborator

@lenary lenary commented May 17, 2017

This adds Checked blocks and pragmas to as much code in the following benchmarks as possible:

  • Olden/em3d
  • Olden/health
  • Olden/mst

In almost all cases I've tried to keep the number of changed lines to a reasonable minimum.

Local benchmark headers have had a #pragma BOUNDS_CHECKED ON at the top, and importantly, #pragma BOUNDS_CHECKED OFF at the bottom, so that we don't have to reorder any other headers.

With logging, I've tried to keep chatting invocations on one line, rather than having the checked block start on the previous line.

There's a bug on Mac OS X where NULL is a macro that expands (via __DARWIN_NULL) to ((void*) 0) which is annoying, so I added a hack to stdlib_checked.h to get rid of the error. This seemed a reasonable approach, but I've not seen the equivalent errors on Linux yet.

This caught two unintended uses of unchecked pointers in Olden/em3d, which is good. I've fixed them in this PR.

There's still an issue with bounds interop types and calloc, but I've not seen the same error with any other functions, which is fantastic (all printf's are inside unchecked blocks as we don't yet support null-terminated strings)

@lenary lenary added this to the Preliminary Evaluation milestone May 17, 2017
@lenary lenary requested a review from dtarditi May 17, 2017 06:32
@msftclas
Copy link

@lenary,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@lenary
Copy link
Collaborator Author

lenary commented May 17, 2017

With the fixes to checkedc/checkedc-clang#302, this changes all pass the static analysis, and there are no more errors with calloc.

Copy link
Contributor

@dtarditi dtarditi left a comment

Choose a reason for hiding this comment

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

Looks good!

@dtarditi dtarditi merged commit 9c100c8 into microsoft:master May 17, 2017
@lenary lenary deleted the convert_bms_ashe2_checkedblk branch June 6, 2017 05:32
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