Skip to content

CheckedRegions understand return types, fixes #263 #357

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.

Sign up for GitHub

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 2 commits into from
Dec 18, 2020
Merged

CheckedRegions understand return types, fixes #263 #357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dea4915
CheckedRegions understnad ret types
Dec 18, 2020
8448c26
Fixing comments
Dec 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 29 additions & 6 deletions clang/lib/3C/CheckedRegions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,18 @@
using namespace llvm;
using namespace clang;


// Check if the compound statement is a function body
// Used in both visitors so abstracted to a function
bool isTopLevel(ASTContext *Context, CompoundStmt *S) {
const auto &Parents = Context->getParents(*S);
if (Parents.empty()) {
return false;
}
// Ensure that our parent is a functiondecl
return Parents[0].get<FunctionDecl>() != nullptr;
}

// CheckedRegionAdder

bool CheckedRegionAdder::VisitCompoundStmt(CompoundStmt *S) {
Expand Down Expand Up @@ -92,12 +104,10 @@ CheckedRegionAdder::findParentCompound(const ast_type_traits::DynTypedNode &N,
return *Min;
}



bool CheckedRegionAdder::isFunctionBody(CompoundStmt *S) {
const auto &Parents = Context->getParents(*S);
if (Parents.empty()) {
return false;
}
return Parents[0].get<FunctionDecl>();
return isTopLevel(Context, S);
}

bool CheckedRegionAdder::isParentChecked(
Expand Down Expand Up @@ -154,13 +164,26 @@ bool CheckedRegionFinder::VisitDoStmt(DoStmt *S) {

bool CheckedRegionFinder::VisitCompoundStmt(CompoundStmt *S) {
// Visit all subblocks, find all unchecked types
bool Localwild = 0;
bool Localwild = false;
for (const auto &SubStmt : S->children()) {
CheckedRegionFinder Sub(Context, Writer, Info, Seen, Map, EmitWarnings);
Sub.TraverseStmt(SubStmt);
Localwild |= Sub.Wild;
}

// If we are a function def, need to check return type
if (isTopLevel(Context, S)) {
const auto &Parents = Context->getParents(*S);
assert(!Parents.empty());
FunctionDecl* Parent = const_cast<FunctionDecl*>(Parents[0].get<FunctionDecl>());
Copy link
Member

Choose a reason for hiding this comment

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

Why does Parent need to be non-const ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because ProgramInfo::getVariable takes a non-const

Copy link
Collaborator

@john-h-kastner john-h-kastner Dec 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think we might be able to make the parameter of getVariable const. We should rarely if ever be mutating this sort of clang AST node object. Perhaps not as part of this PR, but in a later change we could add const annotations to these AST node objects where we can.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I'm happy to go through and try to make that change.

Copy link
Member

Choose a reason for hiding this comment

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

But, per the status meeting: Let's just do this as a separate PR; leave the const_cast for now.

assert(Parent != nullptr);
auto retType = Parent->getReturnType().getTypePtr();
if (retType->isPointerType()) {
CVarOption CV = Info.getVariable(Parent, Context);
Localwild |= isWild(CV) || containsUncheckedPtr(Parent->getReturnType());
}
}

markChecked(S, Localwild);

Wild = false;
Expand Down
7 changes: 7 additions & 0 deletions clang/test/3C/checkedregions.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,10 @@ void baz(void) {
bad();
}
}

int* g() {
//CHECK: int* g(void) {
return 1;
}


10 changes: 5 additions & 5 deletions clang/test/3C/fn_sets.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,19 @@ void foo1(int *z) {
/* Testing Something with a larger set of functions */

int *a() {
//CHECK: int *a(void) _Checked {
//CHECK: int *a(void) {
return 0;
}
int *b() {
//CHECK: int *b(void) _Checked {
//CHECK: int *b(void) {
return 0;
}
int *c() {
//CHECK: int *c(void) _Checked {
//CHECK: int *c(void) {
return 0;
}
int *d() {
//CHECK: int *d(void) _Checked {
//CHECK: int *d(void) {
return 0;
}
int *e() {
Expand All @@ -72,7 +72,7 @@ int *e() {
//CHECK: return (int*) 1;
}
int *i() {
//CHECK: _Ptr<int> i(void) _Checked {
//CHECK: _Ptr<int> i(void) _Checked {
return 0;
}

Expand Down