Skip to content

Commit

Permalink
Validate that _Nt_checked type array initializers are null terminated (
Browse files Browse the repository at this point in the history
…#531)

Related Issue: #397

Added a function to validate _Nt_checked type array initializers. 

If the type of the declaration is _Nt_checked, the last item in the initializer list is validated to make sure it is null (nullptr, NULL, 0 or '\0' depending on what's appropriate).

If the type of the initializer is a string literal, the size of the initializer string is validated to be within the size of the _Nt_checked array that is initialized.

If the type of the initializer is record type (struct, typedef struct, union), has _Nt_checked typed field(s) within the initialized data structure, their initializers are validated as well.

Testing:
Added checkedc tests for _Nt_checked initializers.
Automated tests: DevTest Debug X64 Linux, DevTest Debug x64 Windows, DevTest Debug X86 Windows, DevTest Release Linux X64 LNT
  • Loading branch information
Prabhuk committed Jul 24, 2018
1 parent 7332857 commit 8ee16f3
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 0 deletions.
4 changes: 4 additions & 0 deletions include/clang/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -4408,6 +4408,10 @@ class InitListExpr : public Expr {
/// idiomatic?
bool isIdiomaticZeroInitializer(const LangOptions &LangOpts) const;

/// Does the given InitListExpr contain an explicit null terminator?
/// DeclArraySize - Size of the array that is initialized
bool isNullTerminated(ASTContext &C, unsigned DeclArraySize) const;

SourceLocation getLBraceLoc() const { return LBraceLoc; }
void setLBraceLoc(SourceLocation Loc) { LBraceLoc = Loc; }
SourceLocation getRBraceLoc() const { return RBraceLoc; }
Expand Down
2 changes: 2 additions & 0 deletions include/clang/AST/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -1923,6 +1923,7 @@ class Type : public ExtQualsTypeCommonBase {
HasIntWithBounds,
HasUncheckedPointer
};

/// \brief check whether an array, or an object of struct/union type contains a checked value
CheckedValueKind containsCheckedValue(bool InCheckedScope) const;

Expand Down Expand Up @@ -2370,6 +2371,7 @@ class PointerType : public Type, public llvm::FoldingSetNode {
otherQuals.isAddressSpaceSupersetOf(thisQuals);
}

bool isNTChecked() const { return getKind() == CheckedPointerKind::NtArray; }
bool isChecked() const { return getKind() != CheckedPointerKind::Unchecked; }
bool isUnchecked() const { return getKind() == CheckedPointerKind::Unchecked; }
bool isSugared() const { return false; }
Expand Down
6 changes: 6 additions & 0 deletions include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -9569,6 +9569,12 @@ def err_bounds_type_annotation_lost_checking : Error<

def err_initializer_expected_for_ptr : Error<
"automatic variable %0 with _Ptr type must have initializer">;

def err_initializer_not_null_terminated_for_nt_checked : Error<
"null terminator expected in _Nt_checked array initializer">;

def err_initializer_not_null_terminated_for_nt_checked_string_literal : Error<
"initializer-string for _Nt_checked char array is too long">;

def err_initializer_expected_for_integer_with_bounds_expr : Error<
"integer variable %0 with a bounds expression must have an initializer">;
Expand Down
1 change: 1 addition & 0 deletions include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -1972,6 +1972,7 @@ class Sema {
SourceLocation EqualLoc = SourceLocation());
void ActOnUninitializedDecl(Decl *dcl);
void ActOnInitializerError(Decl *Dcl);
bool ValidateNTCheckedType(ASTContext &C, QualType VDeclType, Expr *Init);

void ActOnPureSpecifier(Decl *D, SourceLocation PureSpecLoc);
void ActOnCXXForRangeDecl(Decl *D);
Expand Down
24 changes: 24 additions & 0 deletions lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2001,6 +2001,30 @@ bool InitListExpr::isIdiomaticZeroInitializer(const LangOptions &LangOpts) const
return Lit && Lit->getValue() == 0;
}

bool InitListExpr::isNullTerminated(ASTContext &C, unsigned DeclArraySize) const {
assert(isSemanticForm() && "Null terminator check must be performed "
"after semantic initialization of all "
"sub-objects are made explicit");

if (getNumInits() == 0) {
return true;
}

if (getNumInits() == 1 && getInit(0) && isa<StringLiteral>(getInit(0))) {
const StringLiteral *InitializerString = cast<StringLiteral>(getInit(0));
if (DeclArraySize < InitializerString->getLength())
{
const char *StringConstant = InitializerString->getString().data();
return (StringConstant[DeclArraySize - 1] == '\0');
}
return true;
}

const Expr *LastItem = getInit(getNumInits() - 1);
Expr::NullPointerConstantKind E = LastItem->isNullPointerConstant(C, Expr::NPC_ValueDependentIsNull);
return E != NPCK_NotNull;
}

SourceLocation InitListExpr::getLocStart() const {
if (InitListExpr *SyntacticForm = getSyntacticForm())
return SyntacticForm->getLocStart();
Expand Down
120 changes: 120 additions & 0 deletions lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11065,6 +11065,16 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit,
getCurFunction()->markSafeWeakUse(Init);
}

// Checked C: All initializers to _Nt_checked type arrays must be null
// terminated
if (!VDecl->isInvalidDecl()) {
if (!ValidateNTCheckedType(RealDecl->getASTContext(),
VDecl->getType().getCanonicalType(), Init)) {
VDecl->setInvalidDecl();
return;
}
}

// The initialization is usually a full-expression.
//
// FIXME: If this is a braced initialization of an aggregate, it is not
Expand Down Expand Up @@ -11262,6 +11272,116 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit,
CheckCompleteVariableDeclaration(VDecl);
}

/// Checked C: Validate that the _Nt_checked type array initializers must be
/// null terminated
/// VDeclType - CanonicalType of the declared variable that is initialized
/// Init - initializer expression for a declaration with VDeclType
bool Sema::ValidateNTCheckedType(ASTContext &Ctx, QualType VDeclType,
Expr *Init) {
if (!Init) {
return true;
}
const Type *current = VDeclType.getTypePtr();
switch (current->getTypeClass()) {
case Type::Pointer: {
// Pointers to _Nt_checked types (nt_array_ptr) are enforced to point to
// null-terminated values, by static type checking and checking of casts.
return true;
}
case Type::ConstantArray: {
if (current->isNtCheckedArrayType()) {
const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(VDeclType);
const auto DeclaredArraySize = CAT->getSize().getRawData();
InitListExpr *InitEx = dyn_cast<InitListExpr>(Init);
StringLiteral *InitializerString = nullptr;

// Initializer-string enclosed in {} e.g. {"test"}
if (InitEx && InitEx->getNumInits() == 1 && InitEx->getInit(0) &&
isa<StringLiteral>(InitEx->getInit(0))) {
InitializerString = cast<StringLiteral>(InitEx->getInit(0));
}

// Initializer-string
if (Init && isa<StringLiteral>(Init)) {
InitializerString = cast<StringLiteral>(Init);
}

if (InitializerString) {
if (*DeclaredArraySize < InitializerString->getLength()) {
const char *StringConstant = InitializerString->getString().data();
if (StringConstant[*DeclaredArraySize - 1] != '\0') {
Diag(
Init->getLocStart(),
diag::err_initializer_not_null_terminated_for_nt_checked_string_literal)
<< Init->getSourceRange();
return false;
}
}
} else if (InitListExpr *E = dyn_cast<InitListExpr>(Init)) {
if (!E->isNullTerminated(Ctx, *DeclaredArraySize)) {
const Expr *LastItem = E->getInit(E->getNumInits() - 1);
Diag(LastItem->getLocStart(),
diag::err_initializer_not_null_terminated_for_nt_checked)
<< LastItem->getSourceRange();
return false;
}
}
}
return true;
}
// Use RecordType to process Struct/Union
case Type::Elaborated:
case Type::Record: {
RecordDecl *RD;
if (isa<ElaboratedType>(current)) {
const ElaboratedType *ET = cast<ElaboratedType>(current);
TagDecl *TG = ET->getAsTagDecl();
if (!TG) {
return true;
}
if (!isa<RecordDecl>(TG)) { // Enum types
return true;
}
RD = cast<RecordDecl>(TG);
} else {
const RecordType *RT = cast<RecordType>(current);
RD = RT->getDecl();
}
if (InitListExpr *ILE = dyn_cast<InitListExpr>(Init)) {
if (RD->isInvalidDecl()) {
return false;
}
// if this is a struct/union type, we must iterate over all its members
unsigned index = 0;
bool FieldValidationFailed = false;
for (FieldDecl *FD : RD->fields()) {
// Recurse until all _Nt_checked type fields are discovered and
// validated
if (ILE->getNumInits() <= index) {
break;
}
Expr *CurrInit = ILE->getInit(index++);
if (FD->getType()->isIntegerType() ||
FD->getType()->isUncheckedPointerType()) {
continue;
}
// Recursive call to handle members of the RecordDecl fields
if (!ValidateNTCheckedType(Ctx, FD->getType().getCanonicalType(),
CurrInit)) {
FieldValidationFailed = true;
}
}
if (FieldValidationFailed) {
return false;
}
}
return true;
}
default:
return true;
}
}

/// ActOnInitializerError - Given that there was an error parsing an
/// initializer for the given declaration, try to return to some form
/// of sanity.
Expand Down

0 comments on commit 8ee16f3

Please sign in to comment.