Skip to content

Commit 5999194

Browse files
john-h-kastnermattmccutchen-cci
john-h-kastner
and
mattmccutchen-cci
authored
Fix failing 3C test for Windows (checkedc#956)
* Include stddef.h where size_t is needed * Fix io redirection issues for windows * Add more null checks to extractBaseType This should resolve tests failing on windows due to null type location * Add `#include <cctype>` to a file that uses `std::tolower`. Hope this fixes a Windows build failure reported by Mandeep. * Add null check to getSourceRange This avoids an assertion fail on some windows header files. * Change IO redirections to work on windows * Disable 3C extstructfields test on Windows again. Windows doesn't have sigaction, and we couldn't find a reasonable alternative. * Add -base-dir to 3c commands in *rewrite_header tests. This is the same workaround used in many other tests for a Windows-specific problem where 3C thinks it isn't allowed to write a file because (1) it isn't under the base dir (which defaults to something under the build directory) and (2) the backslash in the canonicalized file path doesn't match the hard-coded forward slash in the RUN command. We should probably fix this silly problem a better way, but first we may as well apply the workaround we already have. While I'm here, replace --output-postfix with -output-postfix for consistency with the other tests. * rewrite_header.c: Remove %S\ from replacement of sed substitution. `#include "rewrite_header.checked.h"` will work just fine to include a file in the same directory, and %S caused a problem on Windows when it expanded to a file path including "\3C" and the \3 was interpreted as a regular expression backreference. * rewrite_header: Avoid `sed -i`. The GnuWin32 `sed -i` has a bug where it leaves its temporary file behind, and there's no known solution other than to not use it. * Fix root_cause.c test for windows A conditional in Constraints.cpp checked if the first character of a file path was "/" to determine if the path was absolute, but on windows, an absolute path will start with "C:\". Co-authored-by: Matt McCutchen (Correct Computation) <[email protected]>
1 parent 5b34358 commit 5999194

17 files changed

+61
-54
lines changed

‎clang/include/clang/3C/ConstraintVariables.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,11 +228,19 @@ class PointerVariableConstraint : public ConstraintVariable {
228228
void addArrayAnnotations(std::stack<std::string> &CheckedArrs,
229229
std::deque<std::string> &EndStrs) const;
230230

231-
// Utility used by the constructor to extract string representation of the
232-
// base type that preserves macros where possible.
231+
// Utility used by the constructor to obtain a string representation of a
232+
// declaration's base type. To preserve macros, this we first try to take
233+
// the type directly from source code. Where that is not possible, the type
234+
// is regenerated from the type in the clang AST.
233235
static std::string extractBaseType(DeclaratorDecl *D, QualType QT,
234236
const Type *Ty, const ASTContext &C);
235237

238+
// Try to extract string representation of the base type for a declaration
239+
// from the source code. If the base type cannot be extracted from source, an
240+
// empty string is returned instead.
241+
static std::string tryExtractBaseType(DeclaratorDecl *D, QualType QT,
242+
const Type *Ty, const ASTContext &C);
243+
236244
// Flag to indicate that this constraint is a part of function prototype
237245
// e.g., Parameters or Return.
238246
bool PartOfFuncPrototype;

‎clang/include/clang/3C/RewriteUtils.h

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,14 @@ class FunctionDeclReplacement
8888

8989
SourceRange getSourceRange(SourceManager &SM) const override {
9090
TypeSourceInfo *TSInfo = Decl->getTypeSourceInfo();
91-
if (!TSInfo)
91+
FunctionTypeLoc TypeLoc;
92+
if (TSInfo) {
93+
auto TSInfoLoc = TSInfo->getTypeLoc();
94+
TypeLoc = getBaseTypeLoc(TSInfoLoc).getAs<clang::FunctionTypeLoc>();
95+
}
96+
if (!TSInfo || TypeLoc.isNull())
9297
return SourceRange(Decl->getBeginLoc(),
9398
getFunctionDeclarationEnd(Decl, SM));
94-
FunctionTypeLoc TypeLoc =
95-
getBaseTypeLoc(TSInfo->getTypeLoc()).getAs<clang::FunctionTypeLoc>();
96-
97-
assert("FunctionDecl doesn't have function type?" && !TypeLoc.isNull());
9899

99100
// Function pointer are funky, and require special handling to rewrite the
100101
// return type.

‎clang/lib/3C/ArrayBoundsInferenceConsumer.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "clang/3C/ConstraintResolver.h"
1616
#include "clang/Analysis/Analyses/Dominators.h"
1717
#include "clang/Analysis/CFG.h"
18+
#include <cctype>
1819
#include <sstream>
1920

2021
static std::set<std::string> LengthVarNamesPrefixes = {"len", "count", "size",

‎clang/lib/3C/ConstraintVariables.cpp

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -391,11 +391,10 @@ PointerVariableConstraint::PointerVariableConstraint(
391391
}
392392
}
393393

394-
std::string PointerVariableConstraint::extractBaseType(DeclaratorDecl *D,
395-
QualType QT,
396-
const Type *Ty,
397-
const ASTContext &C) {
398-
std::string BaseTypeStr;
394+
std::string PointerVariableConstraint::tryExtractBaseType(DeclaratorDecl *D,
395+
QualType QT,
396+
const Type *Ty,
397+
const ASTContext &C) {
399398
bool FoundBaseTypeInSrc = false;
400399
if (!QT->isOrContainsCheckedType() && !Ty->getAs<TypedefType>() && D &&
401400
D->getTypeSourceInfo()) {
@@ -406,32 +405,33 @@ std::string PointerVariableConstraint::extractBaseType(DeclaratorDecl *D,
406405
TL = getBaseTypeLoc(TL).getAs<FunctionTypeLoc>();
407406
// FunctionDecl that doesn't have function type? weird
408407
if (TL.isNull())
409-
FoundBaseTypeInSrc = false;
410-
else
411-
TL = TL.getAs<clang::FunctionTypeLoc>().getReturnLoc();
408+
return "";
409+
TL = TL.getAs<clang::FunctionTypeLoc>().getReturnLoc();
412410
} else {
413411
FoundBaseTypeInSrc = D->getType() == QT;
414412
}
415-
TypeLoc BaseLoc = getBaseTypeLoc(TL);
416-
if (!BaseLoc.getAs<TypedefTypeLoc>().isNull()) {
417-
FoundBaseTypeInSrc = false;
418-
} else {
419-
// Only use this type if the type passed as a parameter to this
420-
// constructor agrees with the actual type of the declaration.
421-
SourceRange SR = BaseLoc.getSourceRange();
422-
if (FoundBaseTypeInSrc && SR.isValid()) {
423-
BaseTypeStr = getSourceText(SR, C);
424-
425-
// getSourceText returns the empty string when there's a pointer level
426-
// inside a macro. Not sure how to handle this, so fall back to tyToStr.
427-
if (BaseTypeStr.empty())
428-
FoundBaseTypeInSrc = false;
429-
} else
430-
FoundBaseTypeInSrc = false;
413+
if (!TL.isNull()) {
414+
TypeLoc BaseLoc = getBaseTypeLoc(TL);
415+
// Only proceed if the base type location is not null, amd it is not a
416+
// typedef type location.
417+
if (!BaseLoc.isNull() && BaseLoc.getAs<TypedefTypeLoc>().isNull()) {
418+
SourceRange SR = BaseLoc.getSourceRange();
419+
if (FoundBaseTypeInSrc && SR.isValid())
420+
return getSourceText(SR, C);
421+
}
431422
}
432423
}
424+
425+
return "";
426+
}
427+
428+
std::string PointerVariableConstraint::extractBaseType(DeclaratorDecl *D,
429+
QualType QT,
430+
const Type *Ty,
431+
const ASTContext &C) {
432+
std::string BaseTypeStr = tryExtractBaseType(D, QT, Ty, C);
433433
// Fall back to rebuilding the base type based on type passed to constructor
434-
if (!FoundBaseTypeInSrc)
434+
if (BaseTypeStr.empty())
435435
BaseTypeStr = tyToStr(Ty);
436436

437437
return BaseTypeStr;

‎clang/lib/3C/Constraints.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -611,7 +611,7 @@ Geq *Constraints::createGeq(Atom *Lhs, Atom *Rhs, const std::string &Rsn,
611611
if (PL != nullptr && PL->valid()) {
612612
// Make this invalid, if the source location is not absolute path
613613
// this is to avoid crashes in clangd.
614-
if (PL->getFileName().c_str()[0] != '/')
614+
if (!llvm::sys::path::is_absolute(PL->getFileName()))
615615
PL = nullptr;
616616
}
617617
assert("Shouldn't be constraining WILD >= VAR" && Lhs != getWild());

‎clang/lib/3C/Utils.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,7 @@ bool isZeroBoundsExpr(BoundsExpr *BE, const ASTContext &C) {
411411
}
412412

413413
TypeLoc getBaseTypeLoc(TypeLoc T) {
414+
assert(!T.isNull() && "Can't get base location from Null.");
414415
while (!T.getNextTypeLoc().isNull() &&
415416
(!T.getAs<ParenTypeLoc>().isNull() ||
416417
T.getTypePtr()->isPointerType() || T.getTypePtr()->isArrayType()))

‎clang/test/3C/3d-allocation.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// UNSUPPORTED: system-windows
21
// RUN: 3c -alltypes -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s
32
// RUN: 3c -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s
43
// RUN: 3c -addcr %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null -

‎clang/test/3C/basic.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// UNSUPPORTED: system-windows
21
// RUN: 3c -alltypes %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s
32
// RUN: 3c %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s
43
// RUN: 3c %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null -

‎clang/test/3C/definedType.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
// UNSUPPORTED: system-windows
21
// RUN: 3c -addcr -alltypes %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s
32
// RUN: 3c -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s
43
// RUN: 3c -addcr %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null -
54
// RUN: 3c -alltypes -output-postfix=checked %s
65
// RUN: 3c -alltypes %S/definedType.checked.c -- | count 0
76
// RUN: rm %S/definedType.checked.c
87

9-
#define size_t unsigned long
8+
#include <stddef.h>
109
_Itype_for_any(T) void *malloc(size_t size) : itype(_Array_ptr<T>) byte_count(size);
1110

1211
// From issue 204

‎clang/test/3C/dont_rewrite_header.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
// UNSUPPORTED: system-windows
2-
// RUN: 3c -alltypes -addcr --output-postfix=checked %S/dont_rewrite_header.c %S/dont_rewrite_header.h
3-
// RUN: FileCheck -match-full-lines -check-prefixes="CHECK" %S/dont_rewrite_header.checked.c < %S/dont_rewrite_header.checked.c
1+
// RUN: 3c -base-dir=%S -alltypes -addcr -output-postfix=checked %S/dont_rewrite_header.c %S/dont_rewrite_header.h
2+
// RUN: FileCheck -match-full-lines -check-prefixes="CHECK" %S/dont_rewrite_header.checked.c --input-file %S/dont_rewrite_header.checked.c
43
// RUN: test ! -f %S/dont_rewrite_header.checked.h
54
// RUN: %clang -c -fcheckedc-extension -x c -o /dev/null %S/dont_rewrite_header.checked.c
6-
// RUN: 3c -alltypes -addcr --output-postfix=checked2 %S/dont_rewrite_header.checked.c %S/dont_rewrite_header.h
5+
// RUN: 3c -base-dir=%S -alltypes -addcr --output-postfix=checked2 %S/dont_rewrite_header.checked.c %S/dont_rewrite_header.h
76
// RUN: test ! -f %S/dont_rewrite_header.checked.checked2.h -a ! -f %S/dont_rewrite_header.checked.checked2.c
87
// RUN: rm %S/dont_rewrite_header.checked.c
98

0 commit comments

Comments
 (0)