Skip to content

Commit

Permalink
Check for unstable types in IPC messages.
Browse files Browse the repository at this point in the history
We want IPC messages to be stable so that 32-bit and 64-bit processes
can talk to each other. This change adds a check to find-bad-constructs
Clang plugin to detect usage of the following unstable types in IPC
messages:

1. Types: long / unsigned long (but not typedefs to)
2. Typedefs: intmax_t, uintmax_t, intptr_t, uintptr_t, wint_t, size_t,
   rsize_t, ssize_t, ptrdiff_t, dev_t, off_t, clock_t, time_t, suseconds_t
   (including typedefs to)
3. Any template referencing the above (e.g. std::vector<size_t>)

BUG=581409

Review URL: https://codereview.chromium.org/1810243002

Cr-Commit-Position: refs/heads/master@{#385469}
  • Loading branch information
dskiba authored and Commit bot committed Apr 6, 2016
1 parent b3ed853 commit 2bc8946
Show file tree
Hide file tree
Showing 12 changed files with 1,069 additions and 4 deletions.
2 changes: 1 addition & 1 deletion ipc/ipc_message_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@
#define IPC_SYNC_MESSAGE_ROUTED(msg_class, in, out) \
IPC_MESSAGE_DECL(msg_class, ROUTED, IPC_TUPLE in, IPC_TUPLE out)

#define IPC_TUPLE(...) std::tuple<__VA_ARGS__>
#define IPC_TUPLE(...) IPC::CheckedTuple<__VA_ARGS__>::Tuple

#define IPC_MESSAGE_DECL(msg_name, kind, in_tuple, out_tuple) \
struct IPC_MESSAGE_EXPORT msg_name##_Meta { \
Expand Down
14 changes: 14 additions & 0 deletions ipc/ipc_message_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,26 @@ struct IPC_EXPORT LogData {
struct NoParams {
};

// Specializations are checked by 'IPC checker' part of find-bad-constructs
// Clang plugin (see WriteParam() below for the details).
template <typename... Ts>
struct CheckedTuple {
typedef std::tuple<Ts...> Tuple;
};

template <class P>
static inline void GetParamSize(base::PickleSizer* sizer, const P& p) {
typedef typename SimilarTypeTraits<P>::Type Type;
ParamTraits<Type>::GetSize(sizer, static_cast<const Type&>(p));
}

// This function is checked by 'IPC checker' part of find-bad-constructs
// Clang plugin to make it's not called on the following types:
// 1. long / unsigned long (but not typedefs to)
// 2. intmax_t, uintmax_t, intptr_t, uintptr_t, wint_t,
// size_t, rsize_t, ssize_t, ptrdiff_t, dev_t, off_t, clock_t,
// time_t, suseconds_t (including typedefs to)
// 3. Any template referencing types above (e.g. std::vector<size_t>)
template <class P>
static inline void WriteParam(base::Pickle* m, const P& p) {
typedef typename SimilarTypeTraits<P>::Type Type;
Expand Down
3 changes: 2 additions & 1 deletion tools/clang/plugins/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
set(plugin_sources
ChromeClassTester.cpp
FindBadConstructsAction.cpp
FindBadConstructsConsumer.cpp)
FindBadConstructsConsumer.cpp
CheckIPCVisitor.cpp)

if(WIN32)
# Clang doesn't support loadable modules on Windows. Unfortunately, building
Expand Down
288 changes: 288 additions & 0 deletions tools/clang/plugins/CheckIPCVisitor.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,288 @@
// Copyright (c) 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "CheckIPCVisitor.h"

using namespace clang;

namespace chrome_checker {

namespace {

const char kWriteParamBadType[] =
"[chromium-ipc] IPC::WriteParam() is called on blacklisted type '%0'%1.";

const char kTupleBadType[] =
"[chromium-ipc] IPC tuple references banned type '%0'%1.";

const char kWriteParamBadSignature[] =
"[chromium-ipc] IPC::WriteParam() is expected to have two arguments.";

const char kNoteSeeHere[] =
"see here";

} // namespace

CheckIPCVisitor::CheckIPCVisitor(CompilerInstance& compiler)
: compiler_(compiler), context_(nullptr) {
auto& diagnostics = compiler_.getDiagnostics();
error_write_param_bad_type_ = diagnostics.getCustomDiagID(
DiagnosticsEngine::Error, kWriteParamBadType);
error_tuple_bad_type_ = diagnostics.getCustomDiagID(
DiagnosticsEngine::Error, kTupleBadType);
error_write_param_bad_signature_ = diagnostics.getCustomDiagID(
DiagnosticsEngine::Error, kWriteParamBadSignature);
note_see_here_ = diagnostics.getCustomDiagID(
DiagnosticsEngine::Note, kNoteSeeHere);

blacklisted_typedefs_ = llvm::StringSet<>({
"intmax_t",
"uintmax_t",
"intptr_t",
"uintptr_t",
"wint_t",
"size_t",
"rsize_t",
"ssize_t",
"ptrdiff_t",
"dev_t",
"off_t",
"clock_t",
"time_t",
"suseconds_t"
});
}

void CheckIPCVisitor::BeginDecl(Decl* decl) {
decl_stack_.push_back(decl);
}

void CheckIPCVisitor::EndDecl() {
decl_stack_.pop_back();
}

void CheckIPCVisitor::VisitTemplateSpecializationType(
TemplateSpecializationType* spec) {
ValidateCheckedTuple(spec);
}

void CheckIPCVisitor::VisitCallExpr(CallExpr* call_expr) {
ValidateWriteParam(call_expr);
}

bool CheckIPCVisitor::ValidateWriteParam(const CallExpr* call_expr) {
const FunctionDecl* callee_decl = call_expr->getDirectCallee();
if (!callee_decl ||
callee_decl->getQualifiedNameAsString() != "IPC::WriteParam") {
return true;
}

return ValidateWriteParamSignature(call_expr) &&
ValidateWriteParamArgument(call_expr->getArg(1));
}

// Checks that IPC::WriteParam() has expected signature.
bool CheckIPCVisitor::ValidateWriteParamSignature(
const CallExpr* call_expr) {
if (call_expr->getNumArgs() != 2) {
compiler_.getDiagnostics().Report(
call_expr->getExprLoc(), error_write_param_bad_signature_);
return false;
}
return true;
}

// Checks that IPC::WriteParam() argument type is allowed.
// See CheckType() for specifics.
bool CheckIPCVisitor::ValidateWriteParamArgument(const Expr* arg_expr) {
if (auto* parent_fn_decl = GetParentDecl<FunctionDecl>()) {
auto template_kind = parent_fn_decl->getTemplatedKind();
if (template_kind != FunctionDecl::TK_NonTemplate &&
template_kind != FunctionDecl::TK_FunctionTemplate) {
// Skip all specializations - we don't check WriteParam() on dependent
// types (typedef info gets lost), and we checked all non-dependent uses
// earlier (when we checked the template itself).
return true;
}
}

QualType arg_type;

arg_expr = arg_expr->IgnoreImplicit();
if (auto* cast_expr = dyn_cast<ExplicitCastExpr>(arg_expr)) {
arg_type = cast_expr->getTypeAsWritten();
} else {
arg_type = arg_expr->getType();
}

CheckDetails details;
if (CheckType(arg_type, &details)) {
return true;
}

ReportCheckError(details,
arg_expr->getExprLoc(),
error_write_param_bad_type_);

return false;
}

// Checks that IPC::CheckedTuple<> is specialized with allowed types.
// See CheckType() above for specifics.
bool CheckIPCVisitor::ValidateCheckedTuple(
const TemplateSpecializationType* spec) {
TemplateDecl* decl = spec->getTemplateName().getAsTemplateDecl();
if (!decl || decl->getQualifiedNameAsString() != "IPC::CheckedTuple") {
return true;
}

bool valid = true;
for (unsigned i = 0; i != spec->getNumArgs(); ++i) {
const TemplateArgument& arg = spec->getArg(i);
CheckDetails details;
if (CheckTemplateArgument(arg, &details)) {
continue;
}

valid = false;

auto* parent_decl = GetParentDecl<Decl>();
ReportCheckError(
details,
parent_decl ? parent_decl->getLocStart() : SourceLocation(),
error_tuple_bad_type_);
}

return valid;
}

template <typename T>
const T* CheckIPCVisitor::GetParentDecl() const {
for (auto i = decl_stack_.rbegin(); i != decl_stack_.rend(); ++i) {
if (auto* parent = dyn_cast_or_null<T>(*i)) {
return parent;
}
}
return nullptr;
}


bool CheckIPCVisitor::IsBlacklistedType(QualType type) const {
return context_->hasSameUnqualifiedType(type, context_->LongTy) ||
context_->hasSameUnqualifiedType(type, context_->UnsignedLongTy);
}

bool CheckIPCVisitor::IsBlacklistedTypedef(const TypedefNameDecl* tdef) const {
return blacklisted_typedefs_.find(tdef->getName()) !=
blacklisted_typedefs_.end();
}

// Checks that integer type is allowed (not blacklisted).
bool CheckIPCVisitor::CheckIntegerType(QualType type,
CheckDetails* details) const {
bool seen_typedef = false;
while (true) {
details->exit_type = type;

if (auto* tdef = dyn_cast<TypedefType>(type)) {
if (IsBlacklistedTypedef(tdef->getDecl())) {
return false;
}
details->typedefs.push_back(tdef);
seen_typedef = true;
}

QualType desugared_type =
type->getLocallyUnqualifiedSingleStepDesugaredType();
if (desugared_type == type) {
break;
}

type = desugared_type;
}

return seen_typedef || !IsBlacklistedType(type);
}

// Checks that |type| is allowed (not blacklisted), recursively visiting
// template specializations.
bool CheckIPCVisitor::CheckType(QualType type, CheckDetails* details) const {
if (type->isReferenceType()) {
type = type->getPointeeType();
}
type = type.getLocalUnqualifiedType();

if (details->entry_type.isNull()) {
details->entry_type = type;
}

if (type->isIntegerType()) {
return CheckIntegerType(type, details);
}

while (true) {
if (auto* spec = dyn_cast<TemplateSpecializationType>(type)) {
for (const TemplateArgument& arg: *spec) {
if (!CheckTemplateArgument(arg, details)) {
return false;
}
}
return true;
}

if (auto* record = dyn_cast<RecordType>(type)) {
if (auto* spec = dyn_cast<ClassTemplateSpecializationDecl>(
record->getDecl())) {
const TemplateArgumentList& args = spec->getTemplateArgs();
for (unsigned i = 0; i != args.size(); ++i) {
if (!CheckTemplateArgument(args[i], details)) {
return false;
}
}
}
return true;
}

if (auto* tdef = dyn_cast<TypedefType>(type)) {
details->typedefs.push_back(tdef);
}

QualType desugared_type =
type->getLocallyUnqualifiedSingleStepDesugaredType();
if (desugared_type == type) {
break;
}

type = desugared_type;
}

return true;
}

bool CheckIPCVisitor::CheckTemplateArgument(const TemplateArgument& arg,
CheckDetails* details) const {
return arg.getKind() != TemplateArgument::Type ||
CheckType(arg.getAsType(), details);
}

void CheckIPCVisitor::ReportCheckError(const CheckDetails& details,
SourceLocation loc,
unsigned error) {
DiagnosticsEngine& diagnostics = compiler_.getDiagnostics();

std::string entry_type = details.entry_type.getAsString();
std::string exit_type = details.exit_type.getAsString();

std::string via;
if (entry_type != exit_type) {
via = " via '" + entry_type + "'";
}
diagnostics.Report(loc, error) << exit_type << via;

for (const TypedefType* tdef: details.typedefs) {
diagnostics.Report(tdef->getDecl()->getLocation(), note_see_here_);
}
}

} // namespace chrome_checker
Loading

0 comments on commit 2bc8946

Please sign in to comment.