Skip to content

Commit

Permalink
Fix various bugs uncovered in the wild
Browse files Browse the repository at this point in the history
Signed-off-by: Anthony Burzillo <aburzillo@bloomberg.net>
  • Loading branch information
burz authored and ruoso committed Mar 14, 2019
1 parent 7e8b90e commit 83e5271
Show file tree
Hide file tree
Showing 10 changed files with 267 additions and 32 deletions.
5 changes: 5 additions & 0 deletions cmake/clangmetatool-get-clang-std-include.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/bin/bash

VERSION=$($1/../bin/clang --version | head -n 1 | cut -d '(' -f 1 | cut -d ' ' -f 3 | cut -d '-' -f 1)

echo $(realpath $1/../lib*/clang/$VERSION/include)
10 changes: 8 additions & 2 deletions include/clangmetatool/propagation/propagation_result.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,16 @@ template <typename ResultType> class PropagationResult {
return result < rhs.result;
}
bool operator==(const PropagationResult<ResultType> &rhs) const {
return unresolved == rhs.unresolved && result == rhs.result;
if (unresolved && rhs.unresolved) {
return true;
} else if (unresolved == rhs.unresolved) {
return result == rhs.result;
}

return false;
}
bool operator!=(const PropagationResult<ResultType> &rhs) const {
return unresolved != rhs.unresolved || result != rhs.result;
return !(*this == rhs);
}
};

Expand Down
16 changes: 11 additions & 5 deletions src/propagation/constant_cstring_propagator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,15 @@ namespace clangmetatool {
namespace propagation {
namespace {

// Given a type, determine if it is a char*
bool isCharPtrType(const clang::Type *T) {
return T->isPointerType() && T->getPointeeType().getTypePtr()->isCharType();
// Given an expression, determine if it is a char*
bool isCharPtrType(const clang::Expr *E) {
auto QT = E->getType();

if (QT.isNull() || !QT.getTypePtr()->isPointerType()) return false;

auto QT2 = QT.getTypePtr()->getPointeeType();

return !QT2.isNull() && QT2.getTypePtr()->isCharType();
}

// Utility class to visit the statements of a block and update the
Expand All @@ -25,7 +31,7 @@ class CStringVisitor : public PropagationVisitor<CStringVisitor, std::string> {
// false if this is not possible
bool evalExprToString(std::string &result, const clang::Expr *E) {
// We only care about char types
if (isCharPtrType(E->getType().getTypePtr())) {
if (isCharPtrType(E)) {
clang::Expr::EvalResult ER;

if (E->isEvaluatable(context) && E->EvaluateAsRValue(ER, context)) {
Expand Down Expand Up @@ -100,7 +106,7 @@ class CStringVisitor : public PropagationVisitor<CStringVisitor, std::string> {
if (clang::Stmt::DeclRefExprClass == base->getStmtClass()) {
auto DR = reinterpret_cast<const clang::DeclRefExpr *>(base);

if (isCharPtrType(DR->getType().getTypePtr())) {
if (isCharPtrType(DR)) {
// If the variable is a char*, mark it as UNRESOLVED
addToMap(DR->getNameInfo().getAsString(), {}, CE->getLocEnd());
}
Expand Down
57 changes: 37 additions & 20 deletions src/propagation/constant_integer_propagator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,22 @@ namespace {

// Given a qualified type, determine if it is an int
inline bool isIntegerType(const clang::QualType &QT) {
return QT.getTypePtr()->isIntegerType();
return !QT.isNull() &&
QT.getTypePtr()->isIntegerType();
}

// Given a type, determine if its a pointer to non-const int
inline bool isPtrToMutableIntegerType(const clang::QualType &QT) {
return QT.getTypePtr()->isPointerType() &&
return !QT.isNull() &&
QT.getTypePtr()->isPointerType() &&
!QT.getTypePtr()->getPointeeType().isConstQualified() &&
isIntegerType(QT.getTypePtr()->getPointeeType());
}

// Given a type determine if it is a reference to non-const int
inline bool isRefToMutableIntegerType(const clang::QualType &QT) {
return QT.getTypePtr()->isReferenceType() &&
return !QT.isNull() &&
QT.getTypePtr()->isReferenceType() &&
!QT.getNonReferenceType().isConstQualified() &&
isIntegerType(QT.getNonReferenceType());
}
Expand Down Expand Up @@ -111,25 +114,39 @@ class IntegerVisitor
// Only types of function calls considered are those that take mutable refs
// or (const or non-const) pointers to mutable ints
void VisitCallExpr(const clang::CallExpr *CE) {
auto callArgTypes = CE->getDirectCallee()->parameters();
int idx = 0;
for (auto A : CE->arguments()) {
auto base = A->IgnoreImpCasts();
const clang::DeclRefExpr *DR = nullptr;

if (clang::Stmt::DeclRefExprClass == base->getStmtClass()) {
DR = reinterpret_cast<const clang::DeclRefExpr *>(base);

} else if (clang::Stmt::UnaryOperatorClass == base->getStmtClass()) {
auto UO = reinterpret_cast<const clang::UnaryOperator *>(base);
DR = reinterpret_cast<const clang::DeclRefExpr *>(UO->getSubExpr());
}
auto DC = CE->getDirectCallee();
if (nullptr != DC) {
auto callArgTypes = DC->parameters();
int idx = 0;
for (auto A : CE->arguments()) {
auto base = A->IgnoreImpCasts();
const clang::DeclRefExpr *DR = nullptr;

if (clang::Stmt::DeclRefExprClass == base->getStmtClass()) {
DR = reinterpret_cast<const clang::DeclRefExpr *>(base);

} else if (clang::Stmt::UnaryOperatorClass == base->getStmtClass()) {
auto SE = reinterpret_cast<const clang::UnaryOperator *>(base)->getSubExpr();

if (clang::Stmt::DeclRefExprClass == SE->getStmtClass()) {
DR = reinterpret_cast<const clang::DeclRefExpr *>(SE);
}
}

if (allowsMutation(callArgTypes[idx]->getType())) {
// If the variable is a int*, mark it as UNRESOLVED
addToMap(DR->getNameInfo().getAsString(), {}, CE->getLocStart());
bool allowsM;
// If we are into the variadic arguments to a function
if (idx >= DC->getNumParams()) {
allowsM = allowsMutation(base->getType());
} else {
allowsM = allowsMutation(callArgTypes[idx]->getType());
}

if (nullptr != DR && allowsM) {
// If the variable is a int*, mark it as UNRESOLVED
addToMap(DR->getNameInfo().getAsString(), {}, CE->getLocEnd());
}
++idx;
}
++idx;
}
}
};
Expand Down
12 changes: 7 additions & 5 deletions t/020-function-args-integer-propagation.t.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,13 @@ class MyTool {
const char* expectedResult =
"main >>>>>>>>>>>>>>>>>>>>>>>>>>\n"
" ** v1\n"
" - 9:3 '0' (Changed by code)\n"
" - 17:3 '1' (Changed by code)\n"
" - 21:3 '<UNRESOLVED>' (Changed by code)\n"
" - 23:3 '2' (Changed by code)\n"
" - 26:3 '<UNRESOLVED>' (Changed by code)\n"
" - 11:3 '0' (Changed by code)\n"
" - 19:3 '1' (Changed by code)\n"
" - 23:10 '<UNRESOLVED>' (Changed by code)\n"
" - 25:3 '2' (Changed by code)\n"
" - 28:9 '<UNRESOLVED>' (Changed by code)\n"
" - 30:3 '3' (Changed by code)\n"
" - 33:10 '<UNRESOLVED>' (Changed by code)\n"
"main <<<<<<<<<<<<<<<<<<<<<<<<<<\n";

EXPECT_STREQ(expectedResult, stream.str().c_str());
Expand Down
166 changes: 166 additions & 0 deletions t/027-c-style-variadic-integer-propagation.t.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
#include "clangmetatool-testconfig.h"

#include <sstream>
#include <string>
#include <vector>
#include <utility>

#include <clang/ASTMatchers/ASTMatchers.h>
#include <clang/ASTMatchers/ASTMatchFinder.h>
#include <clang/Frontend/FrontendAction.h>
#include <clang/Tooling/Core/Replacement.h>
#include <clang/Tooling/CommonOptionsParser.h>
#include <clang/Tooling/Tooling.h>
#include <clang/Tooling/Refactoring.h>
#include <llvm/Support/CommandLine.h>
#include <clangmetatool/meta_tool_factory.h>
#include <clangmetatool/meta_tool.h>
#include <clangmetatool/propagation/constant_integer_propagator.h>

#include <gtest/gtest.h>

namespace {

using namespace clang::ast_matchers;

using FindVarDeclsDatum = std::pair<const clang::FunctionDecl*, const clang::DeclRefExpr*>;
using FindVarDeclsData = std::vector<FindVarDeclsDatum>;

class FindVarDeclsCallback : public MatchFinder::MatchCallback {
private:
clang::CompilerInstance* ci;
FindVarDeclsData* data;

public:
FindVarDeclsCallback(clang::CompilerInstance* ci, FindVarDeclsData* data)
: ci(ci), data(data) {
}

virtual void run(const MatchFinder::MatchResult& r) override {
const clang::FunctionDecl* f = r.Nodes.getNodeAs<clang::FunctionDecl>("func");

const clang::DeclRefExpr* d = r.Nodes.getNodeAs<clang::DeclRefExpr>("declRef");

data->push_back({f, d});
}
};

class FindVarDecls {
private:
FindVarDeclsData data;

StatementMatcher matcher =
(declRefExpr
(hasDeclaration
(varDecl
(hasAnyName
(
"a",
"b",
"c",
"d",
"e"
)
)
),
hasAncestor(
functionDecl().bind("func")
)
)
).bind("declRef");

FindVarDeclsCallback callback;

public:
FindVarDecls(clang::CompilerInstance* ci,
MatchFinder* f)
: callback(ci, &data)
{
f->addMatcher(matcher, &callback);
}

FindVarDeclsData* getData() { return &data; }
};

class MyTool {
private:
clang::CompilerInstance* ci;
FindVarDecls fvd;
clangmetatool::propagation::ConstantIntegerPropagator cip;

public:
MyTool(clang::CompilerInstance* ci, MatchFinder *f)
: ci(ci), fvd(ci, f), cip(ci) {
}

void postProcessing
(std::map<std::string, clang::tooling::Replacements> &replacementsMap) {
FindVarDeclsData* decls = fvd.getData();

for(auto decl : *decls) {
cip.runPropagation(decl.first, decl.second);
}

std::ostringstream stream;

cip.dump(stream);

const char* expectedResult =
"main >>>>>>>>>>>>>>>>>>>>>>>>>>\n"
" ** a\n"
" - 6:3 '0' (Changed by code)\n"
" ** b\n"
" - 7:3 '1' (Changed by code)\n"
" - 12:22 '<UNRESOLVED>' (Changed by code)\n"
" ** c\n"
" - 8:3 '2' (Changed by code)\n"
" - 12:22 '<UNRESOLVED>' (Changed by code)\n"
" ** d\n"
" - 9:3 '3' (Changed by code)\n"
" ** e\n"
" - 10:3 '4' (Changed by code)\n"
" - 12:22 '<UNRESOLVED>' (Changed by code)\n"
"main <<<<<<<<<<<<<<<<<<<<<<<<<<\n";

EXPECT_STREQ(expectedResult, stream.str().c_str());
}
};

} // namespace anonymous

TEST(propagation_ConstantIntegerPropagation, functionArgs) {
llvm::cl::OptionCategory MyToolCategory("my-tool options");
int argc = 4;
const char* argv[] = {
"foo",
CMAKE_SOURCE_DIR "/t/data/027-c-style-variadic-integer-propagation/main.cpp",
"--extra-arg-before=-I" CLANG_STD_INCLUDE_DIR,
"--",
// Test C++ for confirming that this works on a superset
"-xc++"
};
clang::tooling::CommonOptionsParser optionsParser
(argc, argv, MyToolCategory);
clang::tooling::RefactoringTool tool
(optionsParser.getCompilations(), optionsParser.getSourcePathList());
clangmetatool::MetaToolFactory<clangmetatool::MetaTool<MyTool>>
raf(tool.getReplacements());
int r = tool.runAndSave(&raf);
ASSERT_EQ(0, r);
}

// ----------------------------------------------------------------------------
// Copyright 2018 Bloomberg Finance L.P.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// ----------------------------- END-OF-FILE ----------------------------------
9 changes: 9 additions & 0 deletions t/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
find_package(Threads REQUIRED)
find_package(GTest REQUIRED)

execute_process(
COMMAND
"${PROJECT_SOURCE_DIR}/cmake/clangmetatool-get-clang-std-include.sh"
"${CLANG_INCLUDE_DIRS}"
OUTPUT_VARIABLE CLANG_STD_INCLUDE_DIR
OUTPUT_STRIP_TRAILING_WHITESPACE
)

configure_file(
include/clangmetatool-testconfig.h.in
include/clangmetatool-testconfig.h
Expand Down Expand Up @@ -34,6 +42,7 @@ foreach(
024-find-cxx-member-calls
025-find-mixed-calls
026-macro-propagation
027-c-style-variadic-integer-propagation
)

add_executable(${TEST}.t ${TEST}.t.cpp)
Expand Down
7 changes: 7 additions & 0 deletions t/data/020-function-args-integer-propagation/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ int boo(const int*);
int bar(int*);
int baz(const int&);
int qux(int&);
int dud(const int* const);
int qax(int* const);

int main(int argc, char* argv[]) {
int v1 = 0;
Expand All @@ -25,5 +27,10 @@ int main(int argc, char* argv[]) {
baz(v1);
qux(v1);

v1 = 3;

dud(&v1);
qax(&v1);

return 0;
}
15 changes: 15 additions & 0 deletions t/data/027-c-style-variadic-integer-propagation/main.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#include <stdarg.h>

int foo(int, int*, int&, ...);

int main(int argc, char* argv[]) {
int a = 0;
int b = 1;
int c = 2;
int d = 3;
int e = 4;

foo(a, &b, c, d, &e);

return 0;
}
2 changes: 2 additions & 0 deletions t/include/clangmetatool-testconfig.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@
#define CMAKE_SOURCE_DIR "@CMAKE_SOURCE_DIR@"
#define CMAKE_BINARY_DIR "@CMAKE_BINARY_DIR@"

#define CLANG_STD_INCLUDE_DIR "@CLANG_STD_INCLUDE_DIR@"

#endif

0 comments on commit 83e5271

Please sign in to comment.