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

Preprocessor constants not properly handled. #650

Closed
Machiry opened this issue Aug 1, 2019 · 4 comments
Closed

Preprocessor constants not properly handled. #650

Machiry opened this issue Aug 1, 2019 · 4 comments
Assignees
Labels
bug This labels issues that are bugs.

Comments

@Machiry
Copy link
Member

Machiry commented Aug 1, 2019

[This is a BUG]

Looks like checkedc-clang does not support preprocessor constants:

Consider the following code:

#include<stdio_checked.h>
void log_write(_Ptr<const char> func) {
    char pre[256];
    snprintf(pre, sizeof(pre), "%s", func);
    printf("%s", pre);
}
int main(void) {
   log_write("This Works");
   log_write(__func__);
}
$ clang -c pre.c 
pre.c:9:14: error: expression has unknown bounds, cast to ptr<T> expects source to have bounds
   log_write(__func__);
             ^~~~~~~~
1 error generated.

Note that, log_write("This Works"); checks out.

@mgrang
Copy link

mgrang commented Aug 14, 2019

Reduced test case:

void foo() {
  _Ptr<const char> func = __func__;
}

@mgrang mgrang added bug This labels issues that are bugs. and removed bug This labels issues that are bugs. labels Aug 14, 2019
@mgrang mgrang self-assigned this Aug 22, 2019
@mgrang mgrang added the bug This labels issues that are bugs. label Aug 22, 2019
@mgrang
Copy link

mgrang commented Aug 22, 2019

This is fixed in #673.
@Machiry Could you please verify that the above PR fixes this issue?

@Machiry
Copy link
Member Author

Machiry commented Aug 23, 2019

It works. Thank you for fixing this.

@mgrang
Copy link

mgrang commented Sep 5, 2019

Added runtime tests checkedc/checkedc#380.

@mgrang mgrang closed this as completed Sep 14, 2019
Machiry pushed a commit to Machiry/checkedc-clang that referenced this issue Jan 21, 2022
…#650)

* Cleanup ScopeVisitor:
  - Better naming for fields
  - Make references to AVarBoundsInfo fields constant
  - Expose result of visitor through constant set reference instead of by
    mutating sets passed as arguments.
  - Remove loop of singleton set.

* Cleanup getReachableBoundKeys
  - Rename SBVar -> FromProgramVar
  - Get rid of unnecessary set in constant finding loop

* Cleanup StructAccessVisitor
  - Make two fields private. One of these was never used publicly, the
    other is exposed through a getter.
  - Slightly simpler logic for handling variable declarations.
    `isLocalVarDecl` returns false for ParmVarDecls, so it's OK to update
    IsGlobal with the result of this function for ParmVarDecl and
    VarDecl.
  - Added note about a bug on structure access such as
    `( 0 ? global_struct_var : local_struct_var).array`.

* Cleanup CollectDeclsVisitor
  - Make set fields private and expose through const reference.
  - Update comments

* Make some methods of ConstraintsGraph const
  - getNeighbors, getSuccessors, getPredecessors, and findNode are
    trivially const.
  - visitBreadthFirst mutates the BFSCache map, but this acceptable
    because it is cache for the actual graph data, and the underlying
    data is not mutated. To let this method be checked `BFSCache` is
    explicitly qualified with mutable.

* Cleanup performFlowAnalysis
  - Avoid re-using AvarBoundsInference instance.
  - Avoid re-using set in inner most inference loop
  - Move some code into performWorkListInference since it was always
    called before/after this function.

* Cleanup performWorkListInference
  - Remove `Changed` loop variable and instead terminate outer loop when
    `WorkList` is empty.
  - Replace inner while loop with a for loop to make it clearer that it
    the loop is just iterating over each element of `WorkList`.

* Cleanup predictBounds
  - Move variable declarations closer to use.
  - Declare variables with explicit types where this helps me understand
    the code.
  - Add some comments.

* Cleanup inferFromPotentialBounds
  - rename Handled to HasInferredBound
  - Use llvm::any_of instead of loop

* Cleanup computeArrPointers
  - Avoid duplicating code in main for loop
  - Add comment expressing concern about special-casing on return-type
    null terminated arrays.
  - Delete some intermediate sets that didn't need to exist.

* Delete commented out method tryGetBoundsKeyVar
  - A nearly identical un-commented version of this method still exists. The
    only difference is that getVariable is used to get a ConstraintVariable
    instead of getExprConstraintVars.

* Cleanup handleAllocatorCall
  - remove unused Fname parameter from isAllocatorCall
  - getCalledFunctionName can only be called on CallExprs, so the type of
    its parameter can be changed from Expr to CallExpr.
  - Introduce function hasValidBoundsKey that can be used instead of
    calling tryGetValidBoundsKey with unused BoundsKey reference.

* Cleanup LocalVarABVisitor
  - Rename addUsedParmVarDecl -> addNonLengthParameter
  - Delete some unused local variables
  - Add a comment about some odd code

* Cleanup ComparisonVisitor
  - Fix typo in class name
  - Remove unused field
  - Expose result through const reference to field instead of mutating
    constructor argument.

* Cleanup LengthVarInference
  - Remove unused fields
  - Avoid needing to `new` and `delete` some fields.

* Add some array bounds debugging utilities
  - Dump context sensitive and reverse context sensitive graphs in
    addition to standard graph.
  - Add dumpBounds method to write current bounds solution state to
    standard error. This can be called from the debugger while stepping
    through execution.

* ProgramVar cleanup
  - Store constant value of variable explicitly
  - Assertions to check that non-constant variable is not used as
    constant
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This labels issues that are bugs.
Projects
None yet
Development

No branches or pull requests

2 participants