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

Add support for relative bounds expressions #37

Closed
secure-sw-dev-bot opened this issue Jan 16, 2022 · 5 comments
Closed

Add support for relative bounds expressions #37

secure-sw-dev-bot opened this issue Jan 16, 2022 · 5 comments
Labels

Comments

@secure-sw-dev-bot
Copy link

This issue was copied from checkedc/checkedc-clang#37


The Checked C specification has relative bounds expressions. We need to extend the IR to represent relative bounds expressions, add parsing support for that, and add typechecking for that.

@secure-sw-dev-bot
Copy link
Author

Comment from @jijoongmoon:

I'm trying to implement relative bounds expressions. In order to do that, I have several questions about it.

. Most of the example codes in the specification, I can see the relative expression with types such as rel_align(char). But I'm supposed that type is not the only one for the variable of relative expression.

. This relative expression could be after the bounds and span expressions. count and byte_count is suppose to have the alignment as type size and one byte for each. But we need to have alignment for the range bounds expression with relative expression. So, I tried to use private TypeSourceInfo * in RangeBoundsExpr class. Do you think which on is ok better to create another class for relative expression or use private variable?

. Handling the exceptions like this bounds(none + arr, arr + len) rel_align(char) = arr; , When I'm reading the clang code, current checkedc implementation suppose to this as NullaryBoundsExpr and SkipUntil the r_paran of bounds expression. if I implement the rel_align after this bounds expression, the parser also should go to the r_paran of rel_aling expression. Am I right?

. Talking about the ASTDumper for relative alignment, I think it is ok something like " rel_align : char " after the dump Bounds Kind. How about it?

@secure-sw-dev-bot
Copy link
Author

Comment from @dtarditi:

  • Yes, in general, a type name could appear in the rel_align clause. For example, a C programmer might create a typedef and use the typedef name in the rel_align clause.
  • I would recommend that you create a class to hold the relative alignment information, and then have the RangeBoundsExpr point to an instance of this class. The reason is that relative alignment will be rare, and having the information in a separate class that is created only on "demand" will save memory overall. We also need to worry about rel_align_value clauses, which would hold an integer.
  • I would implement the parsing as:
    -- Try parsing the bounds expression. If something goes wrong, skip to the rparen.
    -- Then try parsing the rel_align clause. If something goes wrong, skip to the r_paren of the relative align expression.
  • Yes, your suggestion for the ASTDumper sounds good.

@secure-sw-dev-bot
Copy link
Author

Comment from @jijoongmoon:

I tested with check-clang and check-checkedc. In case of check-checkedc, there is no error message except the tests in samples directory.
And there are two errors in check-clang case.
one is unexpected error in Modules/explicit-build-extra-files.cpp and I confirmed the python configuration is cause of this error. I tested the check-clang with another computer, there is no unexpected error.
and the other one is CheckedC/dynamic-checks/array-subscript-code-gen.c. I think this unexpected error is caused by the difference between x86_64. It is 8bytes rather than 4bytes. In the test code, there is align 4.

@secure-sw-dev-bot
Copy link
Author

Comment from @lenary:

@jijoongmoon if you pull the most recent version of checkedc-llvm, the error about the samples directory should go away.

As for platform-specific tests, I haven't yet added a way to add platform-specific tests to check-clang. Please can you file a new bug in the checkedc-clang repo with the output from the failure of CheckedC/dynamic-checks/array-subscript-code-gen.c?

@secure-sw-dev-bot
Copy link
Author

Comment from @dtarditi:

Jijoong Moon has completed this work, so closing this issue. Thanks Jijoong!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant