Skip to content

Merge from 3C 2021-02 #993

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 1,546 commits into from
Mar 15, 2021
Merged

Conversation

mattmccutchen-cci
Copy link
Member

Here's a sneak peek at what's coming. You can see what we have left to do on our GitHub project. If you feel like going ahead and looking at this PR and you spot any problems that are not already on our project, please let me know here.

As of now, we're able to run the tests on both Windows X64 and Windows X86 (a release build because of the crash in the debug build; it looks like you're doing the same). Assuming that keeps working, we'll verify that the 3C tests pass in both of those configurations before declaring the PR ready for review, so hopefully your tests will pass too (modulo any existing failures you're still working on). Nevertheless, please run them before merging this PR. We won't be responsible for rushing to fix failures on your master branch after merging, though of course you can mark any failing tests UNSUPPORTED: system-windows yourselves and then file issues with us to get them fixed.

Because our previous PR (#930) was squash-merged, many of our commits have not yet entered your history even though the changes have; we've applied a workaround to ensure that this does not result in this PR merging incorrectly. Here are the commits that actually represent new changes; you'll see that the content diff is identical. To prevent the problem from happening again on our next PR, please remember to merge this PR with a merge commit per our agreement, rather than squash-merging as you do for routine PRs.

john-h-kastner and others added 30 commits October 22, 2020 14:44
Instead of considering a pointer "originally checked" if any one of the
pointer levels in it is checked in the source, characterize each pointer
level individually.
Major refactoring of array bounds code and some performance improvements
This reverts commit c554433, reversing
changes made to 0f8d6dc.
Fixing array length inference performance issue
Revert "Fixing array length inference performance issue"
Major refactoring of array bounds code and some performance improvements
john-h-kastner and others added 14 commits February 24, 2021 11:58
)

Fixes a error in rewriting for function declaration such as

    typedef void foo(int*);
    foo bar;
    void bar(int *a){};

Previously `FunctionDeclBuilder` only replaced the parameter declaration
of `bar` because nothing in the return type needed to be rewritten. This
caused the rewriting of the first declaration to be missing the return
type. With this change, `FunctionDeclBuilder` detects that `bar` is
declared using a typedef type, and generates replacement text for the
entire declaration instead of only the parameter list. This change does not
implement proper constraint generation for these function, so the typedef foo
is not rewritten. 

Reconstruct a parameter declaration if we can't get its original source
due to a macro.

Co-authored-by: Matt McCutchen (Correct Computation) <[email protected]>
…#453)

The one in liberal_itypes_ptr.c conflicted with the system headers on
Windows X64. We haven't seen a problem with the one in allocator.c yet,
but let's remove it too for consistency.

Fixes #451.
The type va_list is defined as a reference type on windows. This wasn't handled
by 3C since references are typically only allowed in C++. By skipping past the
reference type we are able to generate correct constraints for variable
argument functions on windows.
…456)

This may significantly speed up the build.
…cations. (#458)

Avoids a crash when an implicit `struct _GUID` is injected with an
invalid source location on Windows (fixes #448).
This was the cause of the `definedType.c` failure on Windows X86, so we
can now re-enable that test. We also have a dedicated test for the bug
that should work in our main Linux environment.

Fixes #345.
The removed conditional stopped constraint variables being created for fields of
structures that were defined in macros. The original reason for adding this is
unclear, but removing it seems to be correct.
The if statement that tries to replace an existing cast with a new cast
assumed that the new cast was a c-style cast and not a CheckedC
bounds cast. If the branch was taken when inserting a bounds cast,
the leading '_' was cut off, resulting in invalid code. This commit
adds a condition that ensures this does not happen.
…nings. (#461)

While we're here:

- Add note about -allow-unwritable-changes to the unwritable change
  error, and change the existing note to have no location for
  consistency with the others.

- Fix a highly visible typo in a `3c` error message.
@mattmccutchen-cci mattmccutchen-cci marked this pull request as ready for review March 1, 2021 14:20
@sulekhark
Copy link
Contributor

sulekhark commented Mar 5, 2021

Thank you for this PR! The changes look good to me. Here are two minor review comments:

  • Some files in clang/test/3C dir contain long lines that go beyond the 80-column recommendation by llvm (other than RUN
    and CHECK lines), and/or trailing white space at the end of lines (the llvm recommendation is to avoid it). They are:
    1. Numerous test files (probably) modified by the fix that added the "static" keyword to the test cases.
    2. The new test files unsafefptrarg*.c
    3. The files processor.py, test_generator.py, prototype_success*.c
    It is possible that I might have missed listing some files above.
  • Recommendation only: I noticed that clang/include/clang/3C/ProgramInfo.h uses std::map and clang/lib/3C.cpp uses
    std::vector. Llvm recommends that: "llvm::DenseMap should almost always be used instead of std::map or
    std::unordered_map, and llvm::SmallVector should usually be used instead of std::vector" .

@mattmccutchen-cci
Copy link
Member Author

Thanks for the review.

@mattmccutchen-cci
Copy link
Member Author

The clang/test/3C formatting is done.

@sulekhark
Copy link
Contributor

Thank you! LGTM. I have started the tests.

@mattmccutchen-cci
Copy link
Member Author

It looks like the tests have passed. If you're ready to merge this PR, please remember to "Create a merge commit", not "Squash and merge".

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.

9 participants