search

checkedc / checkedc-llvm-project

This repo contains a version of clang that is modified to support Checked C. Checked C is an extension to C that lets programmers write C code with bounds checking and improved type-safety.
13 stars 19 forks source link

Update 3C conversion tool to latest changes from Correct Computation #1192

Closed dtarditi closed 2 years ago

dtarditi commented 2 years ago

This PR updates the 3C conversion tool for Checked C to the latest changes at https://github.com/correctcomputation/checkedc-clang. The specific commit hash that we are updating to is https://github.com/correctcomputation/checkedc-clang/commit/79804ee066f04e44583dd6df2dbe883b466aef5d

@mwhicks1, does this look good to you?

mwhicks1 commented 2 years ago

Quick sanity check: Maybe make sure our benchmarks all work and give the same answers as pre-PR, and likewise the regression tests? Just want to make sure the new Checked C compiler stuff we merged with didn't change anything.

mwhicks1 commented 2 years ago

We should also merge https://github.com/correctcomputation/checkedc here; it has a bunch of updates to checked headers that may be material to the tests you've run.

Machiry commented 2 years ago

Results of the benchmarks on the CC branch and the 3c-jan-2022-update are same. Attached are the files. BenchmarkResults.zip

Note: There are minor differences in running time, which is expected.

mwhicks1 commented 2 years ago

Great! Let’s merge.

mattmccutchen-cci commented 2 years ago

So I see no problem with merging this PR now.

I know we want to update https://github.com/secure-sw-dev/checkedc from https://github.com/correctcomputation/checkedc also, but that doesn't block this PR because the differences in the checked headers do not affect the 3C regression tests and nobody should be using https://github.com/secure-sw-dev/checkedc for anything that is affected by the differences right now. (The checkedc-{vsftpd,thttpd,icecast} repositories were recently moved to this organization, but their readmes all explicitly say to use https://github.com/correctcomputation/checkedc for now.)

One thing to be aware of is that when this PR is merged, there is a risk of unqualified "closing keywords" in the 3C commit messages in the PR unintentionally closing issues in this repository, just as previous PRs did in Microsoft's repository. (As soon as we learned about the problem in July 2021, we made an effort to stop adding more unqualified closing keywords to 3C, but we may have made some mistakes, and some unqualified closing keywords may remain from between the previous PR and when we learned about the problem.) We should just check for and reopen any affected issues after the merge. It looks like I have write permission on this repository, so I can do that myself when the time comes. Update 2022-02-09: I checked, and there were no unintentionally closed issues because none of the numbers in unqualified closing keywords in the PR corresponded to open issues in this repository.

dtarditi commented 2 years ago

When I ran testing using the clang regression test suite, there was one 3C failure on Windows. The test 3C/multiple_tu.c caused 3C to crash. This is new and did not happen before the merge. I am investigating. At first glance, it looks like an issue with incorrect handling of a file name string.

mattmccutchen-cci commented 2 years ago

When I ran testing using the clang regression test suite, there was one 3C failure on Windows. The test 3C/multiple_tu.c caused 3C to crash. This is new and did not happen before the merge. I am investigating. At first glance, it looks like an issue with incorrect handling of a file name string.

I'd be happy to help with this if you like since I know a bit about the 3C code involved. We no longer have easy access to a Windows machine powerful enough to build 3C, but I could log in remotely to yours. Or if you use git bisect to find the change that introduced the problem, I might be able to guess the cause.