From 9396d7672ee737e1b50d4acdebe5ac9f51953c24 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Fri, 27 Aug 2021 11:54:37 -0400 Subject: [PATCH 01/15] fix typedef function rewriting errors --- clang/include/clang/3C/Utils.h | 2 ++ clang/lib/3C/ConstraintVariables.cpp | 41 ++++++++++++++++++++++++---- clang/lib/3C/Utils.cpp | 8 ++++-- 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/clang/include/clang/3C/Utils.h b/clang/include/clang/3C/Utils.h index 441833b9bd4c..8bcccc7596f5 100644 --- a/clang/include/clang/3C/Utils.h +++ b/clang/include/clang/3C/Utils.h @@ -181,6 +181,8 @@ bool isInSysHeader(clang::Decl *D); std::string getSourceText(const clang::SourceRange &SR, const clang::ASTContext &C); +std::string getSourceText(const clang::CharSourceRange &SR, + const clang::ASTContext &C); // Find the longest common subsequence. unsigned longestCommonSubsequence(const char *Str1, const char *Str2, diff --git a/clang/lib/3C/ConstraintVariables.cpp b/clang/lib/3C/ConstraintVariables.cpp index 6d5751a4c423..f14a7037389e 100644 --- a/clang/lib/3C/ConstraintVariables.cpp +++ b/clang/lib/3C/ConstraintVariables.cpp @@ -547,12 +547,21 @@ std::string PointerVariableConstraint::tryExtractBaseType(DeclaratorDecl *D, QualType QT, const Type *Ty, const ASTContext &C) { - bool FoundBaseTypeInSrc = false; - if (D && !TSI) + // Implicit parameters declarations from typedef function declarations will + // still have valid and non-empty source ranges, but implicit declarations + // aren't written in the source, so extracting the base type from this range + // gives incorrect type strings. For example, the base type for the implicit + // parameter for `foo_decl` in `typedef void foo(int*); foo foo_decl;` would + // be extracted as "foo_decl", when it should be "int". + if (!D || D->isImplicit()) + return ""; + + if (!TSI) TSI = D->getTypeSourceInfo(); - if (!QT->isOrContainsCheckedType() && !Ty->getAs() && D && TSI) { + if (!QT->isOrContainsCheckedType() && !Ty->getAs() && TSI) { // Try to extract the type from original source to preserve defines TypeLoc TL = TSI->getTypeLoc(); + bool FoundBaseTypeInSrc = false; if (isa(D)) { FoundBaseTypeInSrc = D->getAsFunction()->getReturnType() == QT; TL = getBaseTypeLoc(TL).getAs(); @@ -2211,8 +2220,30 @@ FVComponentVariable::FVComponentVariable(const QualType &QT, // parameters similarly to how we do it for pointers in regular function // declarations. if (D && D->getType() == QT) { - SourceRange SR = D->getSourceRange(); - SourceDeclaration = SR.isValid() ? getSourceText(SR, C) : ""; + SourceRange DRange = D->getSourceRange(); + if (DRange.isValid()) { + const SourceManager &SM = C.getSourceManager(); + SourceLocation DLoc = D->getLocation(); + CharSourceRange CSR; + if (SM.isBeforeInTranslationUnit(DRange.getEnd(), DLoc)) { + // It's not clear to me why, but the end of the SourceRange for the + // declaration can come before the SourceLocation for the declaration. + // This can result in SourceDeclaration failing to capture the whole + // declaration string, causing rewriting errors. In this case it is also + // necessary to use CharSourceRange::getCharRange to work around some + // cases where CharSourceRange::getTokenRange (the default behavior of + // getSourceText when passed a SourceRange) would not get the full + // declaration. For example: a parameter declared without a name such as + // `_Ptr` was captured as `_Ptr`. + CSR = CharSourceRange::getCharRange(DRange.getBegin(), DLoc); + } else { + CSR = CharSourceRange::getTokenRange(DRange); + } + + SourceDeclaration = getSourceText(CSR , C); + } else { + SourceDeclaration = ""; + } } } diff --git a/clang/lib/3C/Utils.cpp b/clang/lib/3C/Utils.cpp index 69cb1cfdb28b..09208228ad9a 100644 --- a/clang/lib/3C/Utils.cpp +++ b/clang/lib/3C/Utils.cpp @@ -387,11 +387,15 @@ bool isInSysHeader(clang::Decl *D) { std::string getSourceText(const clang::SourceRange &SR, const clang::ASTContext &C) { + return getSourceText(CharSourceRange::getTokenRange(SR), C); +} + +std::string getSourceText(const clang::CharSourceRange &SR, + const clang::ASTContext &C) { assert(SR.isValid() && "Invalid Source Range requested."); auto &SM = C.getSourceManager(); auto LO = C.getLangOpts(); - llvm::StringRef Srctxt = - Lexer::getSourceText(CharSourceRange::getTokenRange(SR), SM, LO); + llvm::StringRef Srctxt = Lexer::getSourceText(SR, SM, LO); return Srctxt.str(); } From 3025472d874044b84dc798757b7b1028d03f9045 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Fri, 27 Aug 2021 11:57:28 -0400 Subject: [PATCH 02/15] Changes to add itypes on undefined functions --- clang/lib/3C/ConstraintVariables.cpp | 5 +- clang/lib/3C/DeclRewriter.cpp | 4 +- clang/lib/3C/ProgramInfo.cpp | 7 +- clang/test/3C/add_bounds.c | 2 +- clang/test/3C/amper.c | 13 ++- clang/test/3C/basic_checks.c | 4 +- clang/test/3C/bodiless.c | 11 +-- clang/test/3C/calloc.c | 3 +- clang/test/3C/cast.c | 4 +- clang/test/3C/fp.c | 12 ++- clang/test/3C/function_typedef.c | 10 +- clang/test/3C/generalize.c | 2 +- clang/test/3C/itype_undef.c | 129 ++++++++++++++++++++++++++ clang/test/3C/itypes_for_extern.c | 4 +- clang/test/3C/liberal_itypes_ptr.c | 8 +- clang/test/3C/liberal_itypes_ptrptr.c | 4 +- clang/test/3C/no_casts.c | 21 ++++- clang/test/3C/simple_locals.c | 7 +- clang/test/3C/struct_init_list.c | 5 +- clang/test/3C/valist.c | 9 +- 20 files changed, 216 insertions(+), 48 deletions(-) create mode 100644 clang/test/3C/itype_undef.c diff --git a/clang/lib/3C/ConstraintVariables.cpp b/clang/lib/3C/ConstraintVariables.cpp index f14a7037389e..113ddccccd3f 100644 --- a/clang/lib/3C/ConstraintVariables.cpp +++ b/clang/lib/3C/ConstraintVariables.cpp @@ -2257,7 +2257,8 @@ void FVComponentVariable::equateWithItype(ProgramInfo &I, "for which 3C currently does not support re-solving." : ReasonUnchangeable; bool HasBounds = ExternalConstraint->srcHasBounds(); - bool HasItype = ExternalConstraint->srcHasItype(); + bool SrcChecked = ExternalConstraint->isOriginallyChecked() || + ExternalConstraint->srcHasItype(); // If the type cannot change at all (ReasonUnchangeable2 is set), then we // constrain both the external and internal types to not change. Otherwise, if // the variable has bounds, then we don't want the checked (external) portion @@ -2265,7 +2266,7 @@ void FVComponentVariable::equateWithItype(ProgramInfo &I, // allow the internal type to change so that the type can change from an itype // to fully checked. bool MustConstrainInternalType = !ReasonUnchangeable2.empty(); - if (HasItype && (MustConstrainInternalType || HasBounds)) { + if (SrcChecked && (MustConstrainInternalType || HasBounds)) { ExternalConstraint->equateWithItype(I, ReasonUnchangeable2, PSL); if (ExternalConstraint != InternalConstraint) linkInternalExternal(I, false); diff --git a/clang/lib/3C/DeclRewriter.cpp b/clang/lib/3C/DeclRewriter.cpp index bd37b53c044f..8dc93a1dec34 100644 --- a/clang/lib/3C/DeclRewriter.cpp +++ b/clang/lib/3C/DeclRewriter.cpp @@ -567,8 +567,8 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) { // If this is an external function, there is no need to rewrite the // declaration. We cannot change the signature of external functions. - if (!FDConstraint->hasBody()) - return true; + //if (!FDConstraint->hasBody()) + // return true; // RewriteParams and RewriteReturn track if we will need to rewrite the // parameter and return type declarations on this function. They are first diff --git a/clang/lib/3C/ProgramInfo.cpp b/clang/lib/3C/ProgramInfo.cpp index 836586287090..440bfa1b473b 100644 --- a/clang/lib/3C/ProgramInfo.cpp +++ b/clang/lib/3C/ProgramInfo.cpp @@ -439,19 +439,14 @@ bool ProgramInfo::link() { // everything about it. // Some global symbols we don't need to constrain to wild, like // malloc and free. Check those here and skip if we find them. + // FIXME: do this properly if (!G->hasBody()) { const FVComponentVariable *Ret = G->getCombineReturn(); Ret->getInternal()->constrainToWild(CS, Rsn); - if (!Ret->getExternal()->srcHasItype() && - !Ret->getExternal()->isGeneric()) - Ret->getExternal()->constrainToWild(CS, Rsn); for (unsigned I = 0; I < G->numParams(); I++) { const FVComponentVariable *Param = G->getCombineParam(I); Param->getInternal()->constrainToWild(CS, Rsn); - if (!Param->getExternal()->srcHasItype() && - !Param->getExternal()->isGeneric()) - Param->getExternal()->constrainToWild(CS, Rsn); } } } diff --git a/clang/test/3C/add_bounds.c b/clang/test/3C/add_bounds.c index 478cdabe4b5d..6d01be4b2181 100644 --- a/clang/test/3C/add_bounds.c +++ b/clang/test/3C/add_bounds.c @@ -16,7 +16,7 @@ _Array_ptr baz(_Array_ptr b) { return b; } -void buz(int *); +void buz(void *); void fiz(int * a : itype(_Array_ptr) count(n), int n); void fuz(int * a : itype(_Array_ptr)) { //CHECK_ALL: void fuz(int * a : itype(_Array_ptr) count(4)) { diff --git a/clang/test/3C/amper.c b/clang/test/3C/amper.c index c8188bc4be8e..26f30e52433a 100644 --- a/clang/test/3C/amper.c +++ b/clang/test/3C/amper.c @@ -40,10 +40,19 @@ void baz(void) { } extern int xfunc(int *arg); +//CHECK: extern int xfunc(int *arg : itype(_Ptr)); int (*fp)(int *); -//CHECK: _Ptr fp = ((void *)0); +//CHECK: _Ptr))> fp = ((void *)0); -void addrof(void) { fp = &xfunc; } +extern int xvoid_func(void *arg); +//CHECK: extern int xvoid_func(void *arg); +int (*void_fp)(void *); +//CHECK: _Ptr void_fp = ((void *)0); + +void addrof(void) { + fp = &xfunc; + void_fp = &xvoid_func; +} void bif(int **x) { //CHECK: void bif(_Ptr<_Ptr> x) _Checked { diff --git a/clang/test/3C/basic_checks.c b/clang/test/3C/basic_checks.c index b94ec1e49916..17595f99fac2 100644 --- a/clang/test/3C/basic_checks.c +++ b/clang/test/3C/basic_checks.c @@ -54,7 +54,7 @@ int *id(int *a) { return a; } //CHECK: _Ptr id(_Ptr a) { return a; } extern int *fry(void); -//CHECK: extern int *fry(void); +//CHECK: extern int *fry(void) : itype(_Ptr); void fret_driver(void) { int a = 0; @@ -66,7 +66,7 @@ void fret_driver(void) { //CHECK-NEXT: int a = 0; //CHECK-NEXT: _Ptr b = &a; //CHECK-NEXT: _Ptr c = id(b); -//CHECK-NEXT: int *d = fry(); +//CHECK-NEXT: _Ptr d = fry(); typedef int *(*fooptr)(int *, int); //CHECK: typedef _Ptr<_Ptr (_Ptr, int)> fooptr; diff --git a/clang/test/3C/bodiless.c b/clang/test/3C/bodiless.c index be5234042025..3296954fb92f 100644 --- a/clang/test/3C/bodiless.c +++ b/clang/test/3C/bodiless.c @@ -22,7 +22,8 @@ void test3() { int *a = foo3(); } int *foo4(void); void test4() { int *a = foo4(); } -//CHECK: void test4() { int *a = foo4(); } +//CHECK: int *foo4(void) : itype(_Ptr); +//CHECK: void test4() { _Ptr a = foo4(); } int *foo5(void) : itype(_Ptr); void test5() { int *a = foo5(); } @@ -30,13 +31,9 @@ void test5() { int *a = foo5(); } extern int *foo6(void); void test6() { int *a = foo6(); } -//CHECK: void test6() { int *a = foo6(); } +//CHECK: extern int *foo6(void) : itype(_Ptr); +//CHECK: void test6() { _Ptr a = foo6(); } extern int *foo7(void) : itype(_Ptr); void test7() { int *a = foo7(); } //CHECK: void test7() { _Ptr a = foo7(); } - -// parameters are not defined and therefore unchecked -extern int *foo8() : itype(_Ptr); -void test8() { int *a = foo8(); } -//CHECK: void test8() { int *a = foo8(); } diff --git a/clang/test/3C/calloc.c b/clang/test/3C/calloc.c index 9afeb4e444d3..6e1776971a19 100644 --- a/clang/test/3C/calloc.c +++ b/clang/test/3C/calloc.c @@ -8,7 +8,8 @@ #include void func(int *x : itype(_Array_ptr)); -//CHECK: void func(int *x : itype(_Array_ptr)); +//CHECK_NOALL: void func(int *x : itype(_Array_ptr)); +//CHECK_ALL: void func(int *x : itype(_Array_ptr) count(5)); void foo(int *w) { //CHECK: void foo(_Ptr w) { diff --git a/clang/test/3C/cast.c b/clang/test/3C/cast.c index deba20691f52..49d03b643070 100644 --- a/clang/test/3C/cast.c +++ b/clang/test/3C/cast.c @@ -45,10 +45,10 @@ void test_generic_casts(void) { } // Check that casts to generics use the local type variables -int *mk_ptr(void); +void *mk_ptr(void); _For_any(T) void free_ptr(_Ptr p_ptr) {} void work (void) { - int *node = mk_ptr(); // wild because extern unavailable + int *node = mk_ptr(); // wild because void pointer free_ptr(node); // CHECK: free_ptr(_Assume_bounds_cast<_Ptr>(node)); } diff --git a/clang/test/3C/fp.c b/clang/test/3C/fp.c index 3b7e0d528f87..de29d20e9e67 100644 --- a/clang/test/3C/fp.c +++ b/clang/test/3C/fp.c @@ -6,13 +6,21 @@ // RUN: 3c -base-dir=%t.checked -alltypes %t.checked/fp.c -- | diff %t.checked/fp.c - int f(int *p); -//CHECK: int f(int *p); +//CHECK: int f(int *p : itype(_Ptr)); void bar() { int (*fp)(int *p) = f; - //CHECK: _Ptr fp = f; + //CHECK: _Ptr))> fp = f; f((void *)0); } +int void_f(void *p); +//CHECK: int void_f(void *p); +void void_bar() { + int (*fp)(void *p) = void_f; + //CHECK: _Ptr fp = void_f; + void_f((void *)0); +} + int mul_by_2(int x) { //CHECK: int mul_by_2(int x) _Checked { return x * 2; diff --git a/clang/test/3C/function_typedef.c b/clang/test/3C/function_typedef.c index 678ebff8d616..8a56115c7ee6 100644 --- a/clang/test/3C/function_typedef.c +++ b/clang/test/3C/function_typedef.c @@ -11,9 +11,9 @@ typedef void foo(int *); foo foo_impl; -void foo_imp(int *a) {} -//CHECK: foo foo_impl; -//CHECK: void foo_imp(_Ptr a) _Checked {} +void foo_impl(int *a) {} +//CHECK: void foo_impl(_Ptr a); +//CHECK: void foo_impl(_Ptr a) _Checked {} typedef int *bar(); bar bar_impl; @@ -26,3 +26,7 @@ baz baz_impl; int *baz_impl(int *a) { return 0; } //CHECK: _Ptr baz_impl(_Ptr a); //CHECK: _Ptr baz_impl(_Ptr a) _Checked { return 0; } + +typedef void biz(int *); +biz biz_impl; +//CHECK: void biz_impl(int * : itype(_Ptr)); diff --git a/clang/test/3C/generalize.c b/clang/test/3C/generalize.c index 9a9badc7fc3a..cbae8d9b0438 100644 --- a/clang/test/3C/generalize.c +++ b/clang/test/3C/generalize.c @@ -65,7 +65,7 @@ void double_ptr(void** dp) {} // externs are not converted to generics void elsewhere(void *x, int *y); -// CHECK: void elsewhere(void *x, int *y); +// CHECK: void elsewhere(void *x, int *y : itype(_Ptr)); // existing generics are not rewritten _For_any(T) void forany(T* t, _Ptr p, void * v) {} diff --git a/clang/test/3C/itype_undef.c b/clang/test/3C/itype_undef.c new file mode 100644 index 000000000000..1c1f92f1b449 --- /dev/null +++ b/clang/test/3C/itype_undef.c @@ -0,0 +1,129 @@ +// RUN: rm -rf %t* +// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s +// RUN: 3c -base-dir=%S -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s +// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null - +// RUN: 3c -base-dir=%S -alltypes -output-dir=%t.checked %s -- +// RUN: 3c -base-dir=%t.checked -alltypes %t.checked/itype_undef.c -- | diff %t.checked/itype_undef.c - + +// Basic checks for adding itypes on undefined functions. +void test0(int *a); +//CHECK: void test0(int *a : itype(_Ptr)); + +int *test1(); +//CHECK: int *test1(void) : itype(_Ptr); + +void caller() { +//CHECK: void caller() _Checked { + int *a = 0; + //CHECK: _Ptr a = 0; + test0(a); + + // This block is left unchecked because it calls a function without either a + // defintion or a prototype even though a `void` prototype is added durring + // conversion. Checked region insertion could be improved to recognize this + // as safe. + { + // CHECK: _Unchecked { + int *b = test1(); + //CHECK: _Ptr b = test1(); + } +} + +// Check for undef functions with existing checked types/itypes. +void test2(int* a : itype(_Ptr)); +int *test3(void) : itype(_Ptr); +void test4(_Ptr a); +_Ptr test5(void); +int *test6(void) : count(10); +void test7(int * : count(10)); +//CHECK: void test2(int* a : itype(_Ptr)); +//CHECK: int *test3(void) : itype(_Ptr); +//CHECK: void test4(_Ptr a); +//CHECK: _Ptr test5(void); +//CHECK: int *test6(void) : count(10); +//CHECK: void test7(int * : count(10)); + +// Void pointers should still be fully unchecked unless there is an existing +// checked type/itype. +void test_void0(void *); +void *test_void1(); +//CHECK: void test_void0(void *); +//CHECK: void *test_void1(); +void void_caller() { + void * a = 0; + test_void0(a); + void *b = test_void1(); +} + +void test_void_itype(void * : itype(_Array_ptr)); +void test_void_count(void * : byte_count(0)); +void test_void_checked(_Array_ptr); +//CHECK: void test_void_itype(void * : itype(_Array_ptr)); +//CHECK: void test_void_count(void * : byte_count(0)); +//CHECK: void test_void_checked(_Array_ptr); + +// Pointer type and bounds inference should still work. +int *test_arr0(int n); +//CHECK_ALL: int *test_arr0(int n) : itype(_Array_ptr) count(n); +//CHECK_NOALL: int *test_arr0(int n) : itype(_Ptr); + +void test_arr1(int *a, int n); +//CHECK: void test_arr1(int *a : itype(_Ptr), int n); + +void arr_caller(int s) { +//CHECK_ALL: void arr_caller(int s) _Checked { + // Returns variables use the least solution, so this will cause test_arr0 to + // solve to _Array_ptr. + int *b = test_arr0(s); + //CHECK_ALL: _Array_ptr b : count(s) = test_arr0(s); + //CHECK_NOALL: int *b = test_arr0(s); + for (int i = 0; i < s; i++) + b[i]; + + // Parameter variables use the greatest solution, so test_arr1 will still + // solve to _Ptr. It might be a good idea to change the solution used for + // undefined functions so that we can infer _Array_ptr here and infer it a + // bound. + int *c = 0; + //CHECK_ALL: _Array_ptr c : count(s) = 0; + //CHECK_NOALL: int *c = 0; + test_arr1(c, s); + for (int i = 0; i < s; i++) + c[i]; +} + +// If all uses of the typedef are checked, the unchecked internal constraint +// variable of the undefined function should not make the typedef unchecked. + +typedef int *td0; +void typedef0(td0 a); +void typedef_caller0() { +//CHECK: typedef _Ptr td0; +//CHECK: void typedef0(int *a : itype(td0)); +//CHECK: void typedef_caller0() _Checked { + td0 b = 0; + typedef0(b); +} + +// If the typedef is otherwise unchecked, we should still give a useful +// checked itype to the typedef'ed parameter. + +typedef int *td1; +void typedef1(td1 a); +void typedef_caller1() { +//CHECK: typedef int *td1; +//CHECK: void typedef1(td1 a : itype(_Ptr)); +//CHECK: void typedef_caller1() { + td1 b = 1; + typedef1(b); +} + +// As expected, function typedefs aren't handled well, but they should generate +// valid code. +typedef void fn_typedef0(int *); +fn_typedef0 fntd_decl0; +//CHECK: void fntd_decl0(int * : itype(_Ptr)); + +typedef int *fn_typedef1(); +fn_typedef1 fntd_decl1; +//CHECK: int *fntd_decl1(void) : itype(_Ptr); diff --git a/clang/test/3C/itypes_for_extern.c b/clang/test/3C/itypes_for_extern.c index ee5dede2c5f9..901193fc0acc 100644 --- a/clang/test/3C/itypes_for_extern.c +++ b/clang/test/3C/itypes_for_extern.c @@ -16,8 +16,8 @@ static void static_foo(int *a) {} //CHECK: static void static_foo(_Ptr a) _Checked {} // Don't give a function an itype if it wouldn't normally be checked -void undef_foo(int *a); -//CHECK: void undef_foo(int *a); +void undef_foo(void *a); +//CHECK: void undef_foo(void *a); int *bar() { return 0; } //CHECK: int *bar(void) : itype(_Ptr) _Checked { return 0; } diff --git a/clang/test/3C/liberal_itypes_ptr.c b/clang/test/3C/liberal_itypes_ptr.c index 74eeabfcaad9..22994a4806b6 100644 --- a/clang/test/3C/liberal_itypes_ptr.c +++ b/clang/test/3C/liberal_itypes_ptr.c @@ -122,14 +122,14 @@ void bounds_call(void *p) { // CHECK: bounds_fn(p); } -char *unused_return_unchecked(); +void *unused_return_unchecked(); char *unused_return_checked() { return 0; } char *unused_return_itype() { return 1; } -char **unused_return_unchecked_ptrptr(); -//CHECK: char *unused_return_unchecked(); +void **unused_return_unchecked_ptrptr(); +//CHECK: void *unused_return_unchecked(); //CHECK: _Ptr unused_return_checked(void) _Checked { return 0; } //CHECK: char *unused_return_itype(void) : itype(_Ptr) _Checked { return 1; } -//CHECK: char **unused_return_unchecked_ptrptr(); +//CHECK: void **unused_return_unchecked_ptrptr(); void dont_cast_unused_return() { unused_return_unchecked(); diff --git a/clang/test/3C/liberal_itypes_ptrptr.c b/clang/test/3C/liberal_itypes_ptrptr.c index e08a7a716ea1..54f2e20e8017 100644 --- a/clang/test/3C/liberal_itypes_ptrptr.c +++ b/clang/test/3C/liberal_itypes_ptrptr.c @@ -46,8 +46,8 @@ void ptrptr_caller() { //CHECK: ptrptr(_Assume_bounds_cast<_Ptr>(*g)); } -void ptrptr_wild(int **y); -//CHECK: void ptrptr_wild(int **y); +void ptrptr_wild(void **y); +//CHECK: void ptrptr_wild(void **y); int **ptrptr_ret() { return 0; } //CHECK: _Ptr ptrptr_ret(void) { return 0; } void ptrptr_wild_caller() { diff --git a/clang/test/3C/no_casts.c b/clang/test/3C/no_casts.c index a9b17e730a25..21a692544317 100644 --- a/clang/test/3C/no_casts.c +++ b/clang/test/3C/no_casts.c @@ -1,20 +1,37 @@ // RUN: rm -rf %t* -// RUN: 3c -base-dir=%S %s -- | diff %s - -// RUN: 3c -base-dir=%S %s -- | %clang -c -fcheckedc-extension -x c -o %t.unused - +// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s +// RUN: 3c -base-dir=%S -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s +// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null - +// RUN: 3c -base-dir=%S -alltypes -output-dir=%t.checked %s -- +// RUN: 3c -base-dir=%t.checked -alltypes %t.checked/no_casts.c -- | diff %t.checked/no_casts.c - void foo(char *a); +//CHECK: void foo(char *a : itype(_Ptr)); void bar(int *a); +//CHECK: void bar(int *a : itype(_Ptr)); void baz(int a[1]); +//CHECK_NOALL: void baz(int a[1]); +//CHECK_ALL: void baz(int *a : itype(int _Checked[1])); +// TODO: Checked C's clang seems to treat `int *a : itype(int _Checked[1])` +// the same as `int a[1] : itype(int _Checked[1])`, so the current +// rewriting is acceptable. It still would be better to keep the original +// type as `int[1]`. int *wild(); +//CHECK: int *wild(void) : itype(_Ptr); void test() { foo("test"); + //CHECK: foo("test"); int x; bar(&x); + //CHECK: bar(&x); baz((int[1]){1}); + //CHECK_NOALL: baz((int[1]){1}); + //CHECK_ALL: baz((int _Checked[1]){1}); bar(wild()); + //CHECK: bar(wild()); } diff --git a/clang/test/3C/simple_locals.c b/clang/test/3C/simple_locals.c index e7a5c4ffe69a..519131c5b31c 100644 --- a/clang/test/3C/simple_locals.c +++ b/clang/test/3C/simple_locals.c @@ -80,7 +80,7 @@ void gg(void) { #define ONE 1 int goo(int *, int); -//CHECK: int goo(int *, int); +//CHECK: int goo(int * : itype(_Ptr), int); struct blah { int a; @@ -213,6 +213,7 @@ void dknbhd(void) { //CHECK-NEXT: int *d = *c; extern void dfefwefrw(int **); +//CHECK: extern void dfefwefrw(int ** : itype(_Ptr<_Ptr>)); void cvxqqef(void) { int a = 0; @@ -223,9 +224,9 @@ void cvxqqef(void) { dfefwefrw(&b); } -//CHECK: void cvxqqef(void) { +//CHECK: void cvxqqef(void) _Checked { //CHECK-NEXT: int a = 0; -//CHECK-NEXT: int *b = &a; +//CHECK-NEXT: _Ptr b = &a; //CHECK-NEXT: _Ptr c = &a; // Check that constraints involving arrays work. diff --git a/clang/test/3C/struct_init_list.c b/clang/test/3C/struct_init_list.c index a50d227dfd8c..478de7f8a410 100644 --- a/clang/test/3C/struct_init_list.c +++ b/clang/test/3C/struct_init_list.c @@ -8,13 +8,14 @@ struct foo { int (*fp)(int *p); - // CHECK: _Ptr fp; + // CHECK: _Ptr))> fp; }; extern int xfunc(int *arg); +// CHECK: extern int xfunc(int *arg : itype(_Ptr)); int func(int *q) { - // CHECK: int func(int *q) { + // CHECK: int func(int *q : itype(_Ptr)) { return *q; } diff --git a/clang/test/3C/valist.c b/clang/test/3C/valist.c index 8283d3ba2d1e..1659b70a702a 100644 --- a/clang/test/3C/valist.c +++ b/clang/test/3C/valist.c @@ -12,10 +12,15 @@ extern void luaC_checkGC(lua_State *); extern void lua_unlock(lua_State *); extern const char *luaO_pushvfstring(lua_State *L, const char *fmt, va_list argp); +//CHECK: extern void lua_lock(lua_State * : itype(_Ptr)); +//CHECK: extern void luaC_checkGC(lua_State * : itype(_Ptr)); +//CHECK: extern void lua_unlock(lua_State * : itype(_Ptr)); +//CHECK: extern const char *luaO_pushvfstring(lua_State *L : itype(_Ptr), const char *fmt : itype(_Ptr), va_list argp) : itype(_Ptr); + const char *lua_pushfstring(lua_State *L, const char *fmt, ...) { - //CHECK: const char *lua_pushfstring(lua_State *L : itype(_Ptr), const char *fmt : itype(_Ptr), ...) : itype(_Ptr) { + //CHECK: _Ptr lua_pushfstring(_Ptr L, _Ptr fmt, ...) { const char *ret; - //CHECK: const char *ret; + //CHECK: _Ptr ret = ((void *)0); va_list argp; lua_lock(L); va_start(argp, fmt); From 987611fc2498e0f298b70766b072bab01e30dda0 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Mon, 30 Aug 2021 13:13:40 -0400 Subject: [PATCH 03/15] Hide undefined function rewriting behind flag --- clang/include/clang/3C/3CGlobalOptions.h | 2 ++ clang/lib/3C/DeclRewriter.cpp | 11 +++++++-- clang/lib/3C/ProgramInfo.cpp | 29 +++++++++++++++++++----- clang/tools/3c/3CStandalone.cpp | 14 ++++++++++++ 4 files changed, 48 insertions(+), 8 deletions(-) diff --git a/clang/include/clang/3C/3CGlobalOptions.h b/clang/include/clang/3C/3CGlobalOptions.h index ab2748ea6e12..e092c126683b 100644 --- a/clang/include/clang/3C/3CGlobalOptions.h +++ b/clang/include/clang/3C/3CGlobalOptions.h @@ -67,6 +67,8 @@ struct _3COptions { bool AllowRewriteFailures; bool ItypesForExtern; + + bool InferTypesForUndefs; }; // NOLINTNEXTLINE(readability-identifier-naming) diff --git a/clang/lib/3C/DeclRewriter.cpp b/clang/lib/3C/DeclRewriter.cpp index 8dc93a1dec34..6645b50d5f80 100644 --- a/clang/lib/3C/DeclRewriter.cpp +++ b/clang/lib/3C/DeclRewriter.cpp @@ -567,8 +567,15 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) { // If this is an external function, there is no need to rewrite the // declaration. We cannot change the signature of external functions. - //if (!FDConstraint->hasBody()) - // return true; + // Under the flag -infer-types-for-undef, however, undefined functions do need + // to be rewritten. If the rest of the 3c inference and rewriting code is + // correct, short-circuiting here shouldn't be necessary; the rest of the + // logic in this function should successfully not rewrite undefined functions + // when -infer-types-for-undef is not passed. This assumption could be + // transformed into an assertion if we're confident it won't fail in too many + // places. + if (!_3COpts.InferTypesForUndefs && !FDConstraint->hasBody()) + return true; // RewriteParams and RewriteReturn track if we will need to rewrite the // parameter and return type declarations on this function. They are first diff --git a/clang/lib/3C/ProgramInfo.cpp b/clang/lib/3C/ProgramInfo.cpp index 440bfa1b473b..7abcc3335bfa 100644 --- a/clang/lib/3C/ProgramInfo.cpp +++ b/clang/lib/3C/ProgramInfo.cpp @@ -439,14 +439,22 @@ bool ProgramInfo::link() { // everything about it. // Some global symbols we don't need to constrain to wild, like // malloc and free. Check those here and skip if we find them. - // FIXME: do this properly if (!G->hasBody()) { const FVComponentVariable *Ret = G->getCombineReturn(); Ret->getInternal()->constrainToWild(CS, Rsn); + if (!_3COpts.InferTypesForUndefs && + !Ret->getExternal()->srcHasItype() && + !Ret->getExternal()->isGeneric()) + Ret->getExternal()->constrainToWild(CS, Rsn); + for (unsigned I = 0; I < G->numParams(); I++) { const FVComponentVariable *Param = G->getCombineParam(I); Param->getInternal()->constrainToWild(CS, Rsn); + if (!_3COpts.InferTypesForUndefs && + !Param->getExternal()->srcHasItype() && + !Param->getExternal()->isGeneric()) + Param->getExternal()->constrainToWild(CS, Rsn); } } } @@ -474,11 +482,20 @@ bool ProgramInfo::link() { if (!G->hasBody()) { - if (!G->getExternalReturn()->isGeneric()) - G->getExternalReturn()->constrainToWild(CS, Rsn); - for (unsigned I = 0; I < G->numParams(); I++) - if (!G->getExternalParam(I)->isGeneric()) - G->getExternalParam(I)->constrainToWild(CS, Rsn); + const FVComponentVariable *Ret = G->getCombineReturn(); + Ret->getInternal()->constrainToWild(CS, Rsn); + if (!_3COpts.InferTypesForUndefs && + !Ret->getExternal()->srcHasItype() && + !Ret->getExternal()->isGeneric()) + Ret->getExternal()->constrainToWild(CS, Rsn); + for (unsigned I = 0; I < G->numParams(); I++) { + const FVComponentVariable *Param = G->getCombineParam(I); + Param->getInternal()->constrainToWild(CS, Rsn); + if (!_3COpts.InferTypesForUndefs && + !Param->getExternal()->srcHasItype() && + !Param->getExternal()->isGeneric()) + Param->getExternal()->constrainToWild(CS, Rsn); + } } } } diff --git a/clang/tools/3c/3CStandalone.cpp b/clang/tools/3c/3CStandalone.cpp index 164a18749114..a5d396ad763e 100644 --- a/clang/tools/3c/3CStandalone.cpp +++ b/clang/tools/3c/3CStandalone.cpp @@ -236,6 +236,19 @@ static cl::opt OptItypesForExtern( "unsafe."), cl::init(false), cl::cat(_3CCategory)); +static cl::opt OptInferTypesForUndef( + "infer-types-for-undefs", + cl::desc("Enable type inference for undefined functions. Under this flag, " + "types for undefined functions are inferred according to the same " + "rules as defined functions with the caveat that an undefined " + "function will only solve to an itype and not a fully checked type. " + "Because 3c is not able to examine the body the function, the " + "inferred pointer types and array bounds are not necessarily " + "accurate which may lead to inaccurate inference elsewhere in the " + "program. It is important to manually review type inferred for " + "undefined functions for this reason." ), + cl::init(false), cl::cat(_3CCategory)); + #ifdef FIVE_C static cl::opt OptRemoveItypes( "remove-itypes", @@ -302,6 +315,7 @@ int main(int argc, const char **argv) { CcOptions.AllowUnwritableChanges = OptAllowUnwritableChanges; CcOptions.AllowRewriteFailures = OptAllowRewriteFailures; CcOptions.ItypesForExtern = OptItypesForExtern; + CcOptions.InferTypesForUndefs = OptInferTypesForUndef; #ifdef FIVE_C CcOptions.RemoveItypes = OptRemoveItypes; From d589534b72fb4e98202f87ea35d86179afd555c5 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Mon, 30 Aug 2021 13:40:51 -0400 Subject: [PATCH 04/15] Revert test changes from "Changes to add itypes on undefined functions" This reverts part of commit e8f5ff5af24cb8b8d6d969754f27484f702678dc. --- clang/test/3C/add_bounds.c | 2 +- clang/test/3C/amper.c | 13 ++----------- clang/test/3C/basic_checks.c | 4 ++-- clang/test/3C/bodiless.c | 11 +++++++---- clang/test/3C/calloc.c | 3 +-- clang/test/3C/cast.c | 4 ++-- clang/test/3C/fp.c | 12 ++---------- clang/test/3C/function_typedef.c | 10 +++------- clang/test/3C/generalize.c | 2 +- clang/test/3C/itype_undef.c | 10 +++++----- clang/test/3C/itypes_for_extern.c | 4 ++-- clang/test/3C/liberal_itypes_ptr.c | 8 ++++---- clang/test/3C/liberal_itypes_ptrptr.c | 4 ++-- clang/test/3C/no_casts.c | 21 ++------------------- clang/test/3C/simple_locals.c | 7 +++---- clang/test/3C/struct_init_list.c | 5 ++--- clang/test/3C/valist.c | 9 ++------- 17 files changed, 43 insertions(+), 86 deletions(-) diff --git a/clang/test/3C/add_bounds.c b/clang/test/3C/add_bounds.c index 6d01be4b2181..478cdabe4b5d 100644 --- a/clang/test/3C/add_bounds.c +++ b/clang/test/3C/add_bounds.c @@ -16,7 +16,7 @@ _Array_ptr baz(_Array_ptr b) { return b; } -void buz(void *); +void buz(int *); void fiz(int * a : itype(_Array_ptr) count(n), int n); void fuz(int * a : itype(_Array_ptr)) { //CHECK_ALL: void fuz(int * a : itype(_Array_ptr) count(4)) { diff --git a/clang/test/3C/amper.c b/clang/test/3C/amper.c index 26f30e52433a..c8188bc4be8e 100644 --- a/clang/test/3C/amper.c +++ b/clang/test/3C/amper.c @@ -40,19 +40,10 @@ void baz(void) { } extern int xfunc(int *arg); -//CHECK: extern int xfunc(int *arg : itype(_Ptr)); int (*fp)(int *); -//CHECK: _Ptr))> fp = ((void *)0); +//CHECK: _Ptr fp = ((void *)0); -extern int xvoid_func(void *arg); -//CHECK: extern int xvoid_func(void *arg); -int (*void_fp)(void *); -//CHECK: _Ptr void_fp = ((void *)0); - -void addrof(void) { - fp = &xfunc; - void_fp = &xvoid_func; -} +void addrof(void) { fp = &xfunc; } void bif(int **x) { //CHECK: void bif(_Ptr<_Ptr> x) _Checked { diff --git a/clang/test/3C/basic_checks.c b/clang/test/3C/basic_checks.c index 17595f99fac2..b94ec1e49916 100644 --- a/clang/test/3C/basic_checks.c +++ b/clang/test/3C/basic_checks.c @@ -54,7 +54,7 @@ int *id(int *a) { return a; } //CHECK: _Ptr id(_Ptr a) { return a; } extern int *fry(void); -//CHECK: extern int *fry(void) : itype(_Ptr); +//CHECK: extern int *fry(void); void fret_driver(void) { int a = 0; @@ -66,7 +66,7 @@ void fret_driver(void) { //CHECK-NEXT: int a = 0; //CHECK-NEXT: _Ptr b = &a; //CHECK-NEXT: _Ptr c = id(b); -//CHECK-NEXT: _Ptr d = fry(); +//CHECK-NEXT: int *d = fry(); typedef int *(*fooptr)(int *, int); //CHECK: typedef _Ptr<_Ptr (_Ptr, int)> fooptr; diff --git a/clang/test/3C/bodiless.c b/clang/test/3C/bodiless.c index 3296954fb92f..be5234042025 100644 --- a/clang/test/3C/bodiless.c +++ b/clang/test/3C/bodiless.c @@ -22,8 +22,7 @@ void test3() { int *a = foo3(); } int *foo4(void); void test4() { int *a = foo4(); } -//CHECK: int *foo4(void) : itype(_Ptr); -//CHECK: void test4() { _Ptr a = foo4(); } +//CHECK: void test4() { int *a = foo4(); } int *foo5(void) : itype(_Ptr); void test5() { int *a = foo5(); } @@ -31,9 +30,13 @@ void test5() { int *a = foo5(); } extern int *foo6(void); void test6() { int *a = foo6(); } -//CHECK: extern int *foo6(void) : itype(_Ptr); -//CHECK: void test6() { _Ptr a = foo6(); } +//CHECK: void test6() { int *a = foo6(); } extern int *foo7(void) : itype(_Ptr); void test7() { int *a = foo7(); } //CHECK: void test7() { _Ptr a = foo7(); } + +// parameters are not defined and therefore unchecked +extern int *foo8() : itype(_Ptr); +void test8() { int *a = foo8(); } +//CHECK: void test8() { int *a = foo8(); } diff --git a/clang/test/3C/calloc.c b/clang/test/3C/calloc.c index 6e1776971a19..9afeb4e444d3 100644 --- a/clang/test/3C/calloc.c +++ b/clang/test/3C/calloc.c @@ -8,8 +8,7 @@ #include void func(int *x : itype(_Array_ptr)); -//CHECK_NOALL: void func(int *x : itype(_Array_ptr)); -//CHECK_ALL: void func(int *x : itype(_Array_ptr) count(5)); +//CHECK: void func(int *x : itype(_Array_ptr)); void foo(int *w) { //CHECK: void foo(_Ptr w) { diff --git a/clang/test/3C/cast.c b/clang/test/3C/cast.c index 49d03b643070..deba20691f52 100644 --- a/clang/test/3C/cast.c +++ b/clang/test/3C/cast.c @@ -45,10 +45,10 @@ void test_generic_casts(void) { } // Check that casts to generics use the local type variables -void *mk_ptr(void); +int *mk_ptr(void); _For_any(T) void free_ptr(_Ptr p_ptr) {} void work (void) { - int *node = mk_ptr(); // wild because void pointer + int *node = mk_ptr(); // wild because extern unavailable free_ptr(node); // CHECK: free_ptr(_Assume_bounds_cast<_Ptr>(node)); } diff --git a/clang/test/3C/fp.c b/clang/test/3C/fp.c index de29d20e9e67..3b7e0d528f87 100644 --- a/clang/test/3C/fp.c +++ b/clang/test/3C/fp.c @@ -6,21 +6,13 @@ // RUN: 3c -base-dir=%t.checked -alltypes %t.checked/fp.c -- | diff %t.checked/fp.c - int f(int *p); -//CHECK: int f(int *p : itype(_Ptr)); +//CHECK: int f(int *p); void bar() { int (*fp)(int *p) = f; - //CHECK: _Ptr))> fp = f; + //CHECK: _Ptr fp = f; f((void *)0); } -int void_f(void *p); -//CHECK: int void_f(void *p); -void void_bar() { - int (*fp)(void *p) = void_f; - //CHECK: _Ptr fp = void_f; - void_f((void *)0); -} - int mul_by_2(int x) { //CHECK: int mul_by_2(int x) _Checked { return x * 2; diff --git a/clang/test/3C/function_typedef.c b/clang/test/3C/function_typedef.c index 8a56115c7ee6..678ebff8d616 100644 --- a/clang/test/3C/function_typedef.c +++ b/clang/test/3C/function_typedef.c @@ -11,9 +11,9 @@ typedef void foo(int *); foo foo_impl; -void foo_impl(int *a) {} -//CHECK: void foo_impl(_Ptr a); -//CHECK: void foo_impl(_Ptr a) _Checked {} +void foo_imp(int *a) {} +//CHECK: foo foo_impl; +//CHECK: void foo_imp(_Ptr a) _Checked {} typedef int *bar(); bar bar_impl; @@ -26,7 +26,3 @@ baz baz_impl; int *baz_impl(int *a) { return 0; } //CHECK: _Ptr baz_impl(_Ptr a); //CHECK: _Ptr baz_impl(_Ptr a) _Checked { return 0; } - -typedef void biz(int *); -biz biz_impl; -//CHECK: void biz_impl(int * : itype(_Ptr)); diff --git a/clang/test/3C/generalize.c b/clang/test/3C/generalize.c index cbae8d9b0438..9a9badc7fc3a 100644 --- a/clang/test/3C/generalize.c +++ b/clang/test/3C/generalize.c @@ -65,7 +65,7 @@ void double_ptr(void** dp) {} // externs are not converted to generics void elsewhere(void *x, int *y); -// CHECK: void elsewhere(void *x, int *y : itype(_Ptr)); +// CHECK: void elsewhere(void *x, int *y); // existing generics are not rewritten _For_any(T) void forany(T* t, _Ptr p, void * v) {} diff --git a/clang/test/3C/itype_undef.c b/clang/test/3C/itype_undef.c index 1c1f92f1b449..a994b5eab415 100644 --- a/clang/test/3C/itype_undef.c +++ b/clang/test/3C/itype_undef.c @@ -1,9 +1,9 @@ // RUN: rm -rf %t* -// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s -// RUN: 3c -base-dir=%S -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s -// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null - -// RUN: 3c -base-dir=%S -alltypes -output-dir=%t.checked %s -- -// RUN: 3c -base-dir=%t.checked -alltypes %t.checked/itype_undef.c -- | diff %t.checked/itype_undef.c - +// RUN: 3c -base-dir=%S --infer-types-for-undefs -alltypes -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s +// RUN: 3c -base-dir=%S --infer-types-for-undefs -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s +// RUN: 3c -base-dir=%S --infer-types-for-undefs -alltypes -addcr %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null - +// RUN: 3c -base-dir=%S --infer-types-for-undefs -alltypes -output-dir=%t.checked %s -- +// RUN: 3c -base-dir=%t.checked --infer-types-for-undefs -alltypes %t.checked/itype_undef.c -- | diff %t.checked/itype_undef.c - // Basic checks for adding itypes on undefined functions. void test0(int *a); diff --git a/clang/test/3C/itypes_for_extern.c b/clang/test/3C/itypes_for_extern.c index 901193fc0acc..ee5dede2c5f9 100644 --- a/clang/test/3C/itypes_for_extern.c +++ b/clang/test/3C/itypes_for_extern.c @@ -16,8 +16,8 @@ static void static_foo(int *a) {} //CHECK: static void static_foo(_Ptr a) _Checked {} // Don't give a function an itype if it wouldn't normally be checked -void undef_foo(void *a); -//CHECK: void undef_foo(void *a); +void undef_foo(int *a); +//CHECK: void undef_foo(int *a); int *bar() { return 0; } //CHECK: int *bar(void) : itype(_Ptr) _Checked { return 0; } diff --git a/clang/test/3C/liberal_itypes_ptr.c b/clang/test/3C/liberal_itypes_ptr.c index 22994a4806b6..74eeabfcaad9 100644 --- a/clang/test/3C/liberal_itypes_ptr.c +++ b/clang/test/3C/liberal_itypes_ptr.c @@ -122,14 +122,14 @@ void bounds_call(void *p) { // CHECK: bounds_fn(p); } -void *unused_return_unchecked(); +char *unused_return_unchecked(); char *unused_return_checked() { return 0; } char *unused_return_itype() { return 1; } -void **unused_return_unchecked_ptrptr(); -//CHECK: void *unused_return_unchecked(); +char **unused_return_unchecked_ptrptr(); +//CHECK: char *unused_return_unchecked(); //CHECK: _Ptr unused_return_checked(void) _Checked { return 0; } //CHECK: char *unused_return_itype(void) : itype(_Ptr) _Checked { return 1; } -//CHECK: void **unused_return_unchecked_ptrptr(); +//CHECK: char **unused_return_unchecked_ptrptr(); void dont_cast_unused_return() { unused_return_unchecked(); diff --git a/clang/test/3C/liberal_itypes_ptrptr.c b/clang/test/3C/liberal_itypes_ptrptr.c index 54f2e20e8017..e08a7a716ea1 100644 --- a/clang/test/3C/liberal_itypes_ptrptr.c +++ b/clang/test/3C/liberal_itypes_ptrptr.c @@ -46,8 +46,8 @@ void ptrptr_caller() { //CHECK: ptrptr(_Assume_bounds_cast<_Ptr>(*g)); } -void ptrptr_wild(void **y); -//CHECK: void ptrptr_wild(void **y); +void ptrptr_wild(int **y); +//CHECK: void ptrptr_wild(int **y); int **ptrptr_ret() { return 0; } //CHECK: _Ptr ptrptr_ret(void) { return 0; } void ptrptr_wild_caller() { diff --git a/clang/test/3C/no_casts.c b/clang/test/3C/no_casts.c index 21a692544317..a9b17e730a25 100644 --- a/clang/test/3C/no_casts.c +++ b/clang/test/3C/no_casts.c @@ -1,37 +1,20 @@ // RUN: rm -rf %t* -// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s -// RUN: 3c -base-dir=%S -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s -// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null - -// RUN: 3c -base-dir=%S -alltypes -output-dir=%t.checked %s -- -// RUN: 3c -base-dir=%t.checked -alltypes %t.checked/no_casts.c -- | diff %t.checked/no_casts.c - +// RUN: 3c -base-dir=%S %s -- | diff %s - +// RUN: 3c -base-dir=%S %s -- | %clang -c -fcheckedc-extension -x c -o %t.unused - void foo(char *a); -//CHECK: void foo(char *a : itype(_Ptr)); void bar(int *a); -//CHECK: void bar(int *a : itype(_Ptr)); void baz(int a[1]); -//CHECK_NOALL: void baz(int a[1]); -//CHECK_ALL: void baz(int *a : itype(int _Checked[1])); -// TODO: Checked C's clang seems to treat `int *a : itype(int _Checked[1])` -// the same as `int a[1] : itype(int _Checked[1])`, so the current -// rewriting is acceptable. It still would be better to keep the original -// type as `int[1]`. int *wild(); -//CHECK: int *wild(void) : itype(_Ptr); void test() { foo("test"); - //CHECK: foo("test"); int x; bar(&x); - //CHECK: bar(&x); baz((int[1]){1}); - //CHECK_NOALL: baz((int[1]){1}); - //CHECK_ALL: baz((int _Checked[1]){1}); bar(wild()); - //CHECK: bar(wild()); } diff --git a/clang/test/3C/simple_locals.c b/clang/test/3C/simple_locals.c index 519131c5b31c..e7a5c4ffe69a 100644 --- a/clang/test/3C/simple_locals.c +++ b/clang/test/3C/simple_locals.c @@ -80,7 +80,7 @@ void gg(void) { #define ONE 1 int goo(int *, int); -//CHECK: int goo(int * : itype(_Ptr), int); +//CHECK: int goo(int *, int); struct blah { int a; @@ -213,7 +213,6 @@ void dknbhd(void) { //CHECK-NEXT: int *d = *c; extern void dfefwefrw(int **); -//CHECK: extern void dfefwefrw(int ** : itype(_Ptr<_Ptr>)); void cvxqqef(void) { int a = 0; @@ -224,9 +223,9 @@ void cvxqqef(void) { dfefwefrw(&b); } -//CHECK: void cvxqqef(void) _Checked { +//CHECK: void cvxqqef(void) { //CHECK-NEXT: int a = 0; -//CHECK-NEXT: _Ptr b = &a; +//CHECK-NEXT: int *b = &a; //CHECK-NEXT: _Ptr c = &a; // Check that constraints involving arrays work. diff --git a/clang/test/3C/struct_init_list.c b/clang/test/3C/struct_init_list.c index 478de7f8a410..a50d227dfd8c 100644 --- a/clang/test/3C/struct_init_list.c +++ b/clang/test/3C/struct_init_list.c @@ -8,14 +8,13 @@ struct foo { int (*fp)(int *p); - // CHECK: _Ptr))> fp; + // CHECK: _Ptr fp; }; extern int xfunc(int *arg); -// CHECK: extern int xfunc(int *arg : itype(_Ptr)); int func(int *q) { - // CHECK: int func(int *q : itype(_Ptr)) { + // CHECK: int func(int *q) { return *q; } diff --git a/clang/test/3C/valist.c b/clang/test/3C/valist.c index 1659b70a702a..8283d3ba2d1e 100644 --- a/clang/test/3C/valist.c +++ b/clang/test/3C/valist.c @@ -12,15 +12,10 @@ extern void luaC_checkGC(lua_State *); extern void lua_unlock(lua_State *); extern const char *luaO_pushvfstring(lua_State *L, const char *fmt, va_list argp); -//CHECK: extern void lua_lock(lua_State * : itype(_Ptr)); -//CHECK: extern void luaC_checkGC(lua_State * : itype(_Ptr)); -//CHECK: extern void lua_unlock(lua_State * : itype(_Ptr)); -//CHECK: extern const char *luaO_pushvfstring(lua_State *L : itype(_Ptr), const char *fmt : itype(_Ptr), va_list argp) : itype(_Ptr); - const char *lua_pushfstring(lua_State *L, const char *fmt, ...) { - //CHECK: _Ptr lua_pushfstring(_Ptr L, _Ptr fmt, ...) { + //CHECK: const char *lua_pushfstring(lua_State *L : itype(_Ptr), const char *fmt : itype(_Ptr), ...) : itype(_Ptr) { const char *ret; - //CHECK: _Ptr ret = ((void *)0); + //CHECK: const char *ret; va_list argp; lua_lock(L); va_start(argp, fmt); From 1e3e262f87a9a039b2627f56520d2008e324b5ea Mon Sep 17 00:00:00 2001 From: John Kastner Date: Mon, 30 Aug 2021 14:20:26 -0400 Subject: [PATCH 05/15] Reduce duplicate code in ProgramInfo::link --- clang/include/clang/3C/ProgramInfo.h | 2 + clang/lib/3C/ProgramInfo.cpp | 110 +++++++++------------------ 2 files changed, 37 insertions(+), 75 deletions(-) diff --git a/clang/include/clang/3C/ProgramInfo.h b/clang/include/clang/3C/ProgramInfo.h index 23dfe71c0712..167b1f8244a6 100644 --- a/clang/include/clang/3C/ProgramInfo.h +++ b/clang/include/clang/3C/ProgramInfo.h @@ -269,6 +269,8 @@ class ProgramInfo : public ProgramVariableAdder { // constraint system for that pointer type. void addVariable(clang::DeclaratorDecl *D, clang::ASTContext *AstContext) override; + + void linkFunction(FunctionVariableConstraint *FV); }; #endif diff --git a/clang/lib/3C/ProgramInfo.cpp b/clang/lib/3C/ProgramInfo.cpp index 7abcc3335bfa..a5b508a9a6bc 100644 --- a/clang/lib/3C/ProgramInfo.cpp +++ b/clang/lib/3C/ProgramInfo.cpp @@ -417,47 +417,8 @@ bool ProgramInfo::link() { // For every global function that is an unresolved external, constrain // its parameter types to be wild. Unless it has a bounds-safe annotation. - for (const auto &U : ExternalFunctionFVCons) { - std::string FuncName = U.first; - FVConstraint *G = U.second; - - // If there was a checked type on a variable in the input program, it - // should stay that way. Otherwise, we shouldn't be adding a checked type - // to an extern function. - std::string Rsn = (G->hasBody() ? "" - : "Unchecked pointer in parameter or " - "return of external function " + - FuncName); - - // Handle the cases where itype parameters should not be treated as their - // unchecked type. - // TODO: Ditto re getting a PSL (in the case in which Rsn is non-empty and - // it is actually used). - G->equateWithItype(*this, Rsn, nullptr); - - // If we've seen this symbol, but never seen a body for it, constrain - // everything about it. - // Some global symbols we don't need to constrain to wild, like - // malloc and free. Check those here and skip if we find them. - if (!G->hasBody()) { - const FVComponentVariable *Ret = G->getCombineReturn(); - Ret->getInternal()->constrainToWild(CS, Rsn); - if (!_3COpts.InferTypesForUndefs && - !Ret->getExternal()->srcHasItype() && - !Ret->getExternal()->isGeneric()) - Ret->getExternal()->constrainToWild(CS, Rsn); - - - for (unsigned I = 0; I < G->numParams(); I++) { - const FVComponentVariable *Param = G->getCombineParam(I); - Param->getInternal()->constrainToWild(CS, Rsn); - if (!_3COpts.InferTypesForUndefs && - !Param->getExternal()->srcHasItype() && - !Param->getExternal()->isGeneric()) - Param->getExternal()->constrainToWild(CS, Rsn); - } - } - } + for (const auto &U : ExternalFunctionFVCons) + linkFunction(U.second); // Repeat for static functions. // @@ -465,44 +426,43 @@ bool ProgramInfo::link() { // error during compilation. They may still be useful as code is developed, // so we treat them as if they are external, and constrain parameters // to wild as appropriate. - for (const auto &U : StaticFunctionFVCons) { - for (const auto &V : U.second) { - - std::string FileName = U.first; - std::string FuncName = V.first; - FVConstraint *G = V.second; - - std::string Rsn = (G->hasBody() ? "" - : "Unchecked pointer in parameter or " - "return of static function " + - FuncName + " in " + FileName); - - // TODO: Ditto re getting a PSL - G->equateWithItype(*this, Rsn, nullptr); - - if (!G->hasBody()) { - - const FVComponentVariable *Ret = G->getCombineReturn(); - Ret->getInternal()->constrainToWild(CS, Rsn); - if (!_3COpts.InferTypesForUndefs && - !Ret->getExternal()->srcHasItype() && - !Ret->getExternal()->isGeneric()) - Ret->getExternal()->constrainToWild(CS, Rsn); - for (unsigned I = 0; I < G->numParams(); I++) { - const FVComponentVariable *Param = G->getCombineParam(I); - Param->getInternal()->constrainToWild(CS, Rsn); - if (!_3COpts.InferTypesForUndefs && - !Param->getExternal()->srcHasItype() && - !Param->getExternal()->isGeneric()) - Param->getExternal()->constrainToWild(CS, Rsn); - } - } - } - } + for (const auto &U : StaticFunctionFVCons) + for (const auto &V : U.second) + linkFunction(V.second); return true; } +void ProgramInfo::linkFunction(FunctionVariableConstraint *FV) { + // If there was a checked type on a variable in the input program, it + // should stay that way. Otherwise, we shouldn't be adding a checked type + // to an undefined function. + std::string Rsn = (FV->hasBody() ? "" : "Unchecked pointer in parameter or " + "return of undefined function " + + FV->getName()); + + // Handle the cases where itype parameters should not be treated as their + // unchecked type. + // TODO: Ditto re getting a PSL (in the case in which Rsn is non-empty and + // it is actually used). + FV->equateWithItype(*this, Rsn, nullptr); + + auto LinkComponent = [this, Rsn](const FVComponentVariable *FVC) { + FVC->getInternal()->constrainToWild(CS, Rsn); + if (!_3COpts.InferTypesForUndefs && + !FVC->getExternal()->srcHasItype() && !FVC->getExternal()->isGeneric()) + FVC->getExternal()->constrainToWild(CS, Rsn); + }; + + // If we've seen this symbol, but never seen a body for it, constrain + // everything about it. + if (!FV->hasBody()) { + LinkComponent(FV->getCombineReturn()); + for (unsigned I = 0; I < FV->numParams(); I++) + LinkComponent(FV->getCombineParam(I)); + } +} + // Populate Variables, VarDeclToStatement, RVariables, and DepthMap with // AST data structures that correspond do the data stored in PDMap and // ReversePDMap. From c019fe6f26b9e3b3a0b063ece43136ae2f7f08c5 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Mon, 30 Aug 2021 14:38:49 -0400 Subject: [PATCH 06/15] Update expected root cause text --- clang/test/3C/implicit_casts_root_cause.c | 2 +- clang/test/3C/root_cause.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/test/3C/implicit_casts_root_cause.c b/clang/test/3C/implicit_casts_root_cause.c index c3f1451c96d4..e9ebadc1a47a 100644 --- a/clang/test/3C/implicit_casts_root_cause.c +++ b/clang/test/3C/implicit_casts_root_cause.c @@ -11,7 +11,7 @@ void test_no_cause() { has_void(c); } -void has_float(float* v); // expected-warning {{Unchecked pointer in parameter or return of external function has_float}} +void has_float(float* v); // expected-warning {{Unchecked pointer in parameter or return of undefined function has_float}} void test_float_cause() { int *b, *c; has_float(b); // expected-warning {{1 unchecked pointer: Cast from int * to float *}} diff --git a/clang/test/3C/root_cause.c b/clang/test/3C/root_cause.c index cbe244955108..318c1f94bbdc 100644 --- a/clang/test/3C/root_cause.c +++ b/clang/test/3C/root_cause.c @@ -6,7 +6,7 @@ // as including stdlib will create numerous root-cause warnings we don't want to deal with // unwritable-expected-warning@+2 {{0 unchecked pointers: Source code in non-writable file}} -// expected-warning@+1 {{Unchecked pointer in parameter or return of external function my_malloc}} +// expected-warning@+1 {{Unchecked pointer in parameter or return of undefined function my_malloc}} _Itype_for_any(T) void *my_malloc(unsigned long size) : itype(_Array_ptr) byte_count(size); @@ -64,7 +64,7 @@ void test1() { extern int *glob; // unwritable-expected-warning@+2 {{0 unchecked pointers: Source code in non-writable file}} -// expected-warning@+1 {{1 unchecked pointer: Unchecked pointer in parameter or return of external function glob_f}} +// expected-warning@+1 {{1 unchecked pointer: Unchecked pointer in parameter or return of undefined function glob_f}} int *glob_f(void); // unwritable-expected-warning@+1 {{0 unchecked pointers: Source code in non-writable file}} From a205b0fe4678f1dbfdb65f45572f1c0e310a828e Mon Sep 17 00:00:00 2001 From: John Kastner Date: Mon, 30 Aug 2021 16:45:49 -0400 Subject: [PATCH 07/15] Undo obsolete change that was needed before other implementation changes --- clang/lib/3C/ConstraintVariables.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang/lib/3C/ConstraintVariables.cpp b/clang/lib/3C/ConstraintVariables.cpp index 113ddccccd3f..f14a7037389e 100644 --- a/clang/lib/3C/ConstraintVariables.cpp +++ b/clang/lib/3C/ConstraintVariables.cpp @@ -2257,8 +2257,7 @@ void FVComponentVariable::equateWithItype(ProgramInfo &I, "for which 3C currently does not support re-solving." : ReasonUnchangeable; bool HasBounds = ExternalConstraint->srcHasBounds(); - bool SrcChecked = ExternalConstraint->isOriginallyChecked() || - ExternalConstraint->srcHasItype(); + bool HasItype = ExternalConstraint->srcHasItype(); // If the type cannot change at all (ReasonUnchangeable2 is set), then we // constrain both the external and internal types to not change. Otherwise, if // the variable has bounds, then we don't want the checked (external) portion @@ -2266,7 +2265,7 @@ void FVComponentVariable::equateWithItype(ProgramInfo &I, // allow the internal type to change so that the type can change from an itype // to fully checked. bool MustConstrainInternalType = !ReasonUnchangeable2.empty(); - if (SrcChecked && (MustConstrainInternalType || HasBounds)) { + if (HasItype && (MustConstrainInternalType || HasBounds)) { ExternalConstraint->equateWithItype(I, ReasonUnchangeable2, PSL); if (ExternalConstraint != InternalConstraint) linkInternalExternal(I, false); From 0ec32e053e6ebdfa9e43178a1ea5785db1405952 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Mon, 30 Aug 2021 16:57:40 -0400 Subject: [PATCH 08/15] Expand test coverage --- clang/test/3C/itype_undef.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/clang/test/3C/itype_undef.c b/clang/test/3C/itype_undef.c index a994b5eab415..e32006226f91 100644 --- a/clang/test/3C/itype_undef.c +++ b/clang/test/3C/itype_undef.c @@ -43,6 +43,39 @@ void test7(int * : count(10)); //CHECK: int *test6(void) : count(10); //CHECK: void test7(int * : count(10)); +void checked_caller() { +//CHECK_NOALL: void checked_caller() { +//CHECK_ALL: void checked_caller() _Checked { + int *a; + //CHECK: _Ptr a = ((void *)0); + test2(a); + + int *b = test3(); + //CHECK: _Ptr b = test3(); + + int *c; + //CHECK: _Ptr c = ((void *)0); + test4(c); + + int *d = test5(); + //CHECK: _Ptr d = test5(); + + int *e = test6(); + //CHECK_NOALL: int *e = test6(); + //CHECK_ALL: _Array_ptr e : count(10) = test6(); + + int *f; + //CHECK_NOALL: int *f; + //CHECK_ALL: _Array_ptr f : count(10) = ((void *)0); + test7(f); + + // Get 3C to infer the correct length for f and e. + for(int i = 0; i < 10; i++) { + f[i]; + e[i]; + } +} + // Void pointers should still be fully unchecked unless there is an existing // checked type/itype. void test_void0(void *); From 080f5439ed09f639d219e896db3d7e01ed5aeaaf Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 31 Aug 2021 13:20:41 -0400 Subject: [PATCH 09/15] Generate itype string from AST when extracting from source fails Fixes an assertion that would fail when attempting to extract the string representation of an itype expression from the original source code if the itype expression was inside a macro. The string representation for itypes in macros is now re-generated from the AST. --- clang/lib/3C/ConstraintVariables.cpp | 11 +++++++++- clang/test/3C/macro_itype.c | 30 ++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 clang/test/3C/macro_itype.c diff --git a/clang/lib/3C/ConstraintVariables.cpp b/clang/lib/3C/ConstraintVariables.cpp index 6d5751a4c423..bf861db2c30e 100644 --- a/clang/lib/3C/ConstraintVariables.cpp +++ b/clang/lib/3C/ConstraintVariables.cpp @@ -280,7 +280,16 @@ PointerVariableConstraint::PointerVariableConstraint( SourceRange R = ITE->getSourceRange(); if (R.isValid()) { ItypeStr = getSourceText(R, C); - assert(ItypeStr.size() > 0); + } + + // ITE->isCompilerGenerated will be true when an itype expression is + // implied a by bounds expression on the declaration. When the itype + // expression is not generated by the compiler, but we failed to extract + // its string representation from the source, build the itype string + // from the AST. + if (!ITE->isCompilerGenerated() && ItypeStr.empty()) { + assert(!InteropType.getAsString().empty()); + ItypeStr = "itype(" + InteropType.getAsString() + ")"; } } } diff --git a/clang/test/3C/macro_itype.c b/clang/test/3C/macro_itype.c new file mode 100644 index 000000000000..021fbd78b3b0 --- /dev/null +++ b/clang/test/3C/macro_itype.c @@ -0,0 +1,30 @@ +// RUN: rm -rf %t* +// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s +// RUN: 3c -base-dir=%S -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s +// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null - +// RUN: 3c -base-dir=%S -alltypes -output-dir=%t.checked %s -- +// RUN: 3c -base-dir=%t.checked -alltypes %t.checked/macro_itype.c -- | diff %t.checked/macro_itype.c - + +// Example encountered while converting libjpeg. This triggered an assertion +// fail because the ItypeStr extracted from the source was empty. +#define macro0 int *a : itype(_Ptr); +struct s { macro0 }; +//CHECK: struct s { macro0 }; + +// Just removing the assertion that failed on the above example caused this to +// rewrite incorectly. The ItypeStr would be left empty, so first paramter +// would be rewritten to `int *b` even though the rewriter intened to give it +// an itype. If the paramter was then passed a checked pointer, there would a +// Checked C compiler error. Ideally, 3C wouldn't need to change the +// declaration of `b` at all, but this is the usual limitation we have with +// rewriting around macros. +#define macro1 : itype(_Ptr) +void fn(int *b macro1, int *c) { +//CHECK: void fn(int *b : itype(_Ptr), _Ptr c) { + b = 1; +} +void caller() { + int *e; + //CHECK: _Ptr e = ((void *)0); + fn(e, 0); +} From cc9024af79c663b78272380cfb554dcc10a9a1c1 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 31 Aug 2021 14:35:43 -0400 Subject: [PATCH 10/15] Comment edits --- clang/lib/3C/ConstraintVariables.cpp | 2 +- clang/test/3C/macro_itype.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clang/lib/3C/ConstraintVariables.cpp b/clang/lib/3C/ConstraintVariables.cpp index bf861db2c30e..a90c8d5103cf 100644 --- a/clang/lib/3C/ConstraintVariables.cpp +++ b/clang/lib/3C/ConstraintVariables.cpp @@ -283,7 +283,7 @@ PointerVariableConstraint::PointerVariableConstraint( } // ITE->isCompilerGenerated will be true when an itype expression is - // implied a by bounds expression on the declaration. When the itype + // implied by a bounds expression on the declaration. When the itype // expression is not generated by the compiler, but we failed to extract // its string representation from the source, build the itype string // from the AST. diff --git a/clang/test/3C/macro_itype.c b/clang/test/3C/macro_itype.c index 021fbd78b3b0..14271ad2fcd7 100644 --- a/clang/test/3C/macro_itype.c +++ b/clang/test/3C/macro_itype.c @@ -12,10 +12,10 @@ struct s { macro0 }; //CHECK: struct s { macro0 }; // Just removing the assertion that failed on the above example caused this to -// rewrite incorectly. The ItypeStr would be left empty, so first paramter -// would be rewritten to `int *b` even though the rewriter intened to give it -// an itype. If the paramter was then passed a checked pointer, there would a -// Checked C compiler error. Ideally, 3C wouldn't need to change the +// rewrite incorrectly. The ItypeStr would be left empty, so first parameter +// would be rewritten to `int *b` even though the rewriter intended to give it +// an itype. If the parameter was then passed a checked pointer, there would be +// a Checked C compiler error. Ideally, 3C wouldn't need to change the // declaration of `b` at all, but this is the usual limitation we have with // rewriting around macros. #define macro1 : itype(_Ptr) From fde7bda5ed905e915b3719c6a6682e423f2faa0a Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 31 Aug 2021 15:14:35 -0400 Subject: [PATCH 11/15] Update clang/tools/3c/3CStandalone.cpp Co-authored-by: Matt McCutchen (Correct Computation) --- clang/tools/3c/3CStandalone.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/clang/tools/3c/3CStandalone.cpp b/clang/tools/3c/3CStandalone.cpp index a5d396ad763e..08a63c4d4979 100644 --- a/clang/tools/3c/3CStandalone.cpp +++ b/clang/tools/3c/3CStandalone.cpp @@ -242,11 +242,16 @@ static cl::opt OptInferTypesForUndef( "types for undefined functions are inferred according to the same " "rules as defined functions with the caveat that an undefined " "function will only solve to an itype and not a fully checked type. " - "Because 3c is not able to examine the body the function, the " - "inferred pointer types and array bounds are not necessarily " - "accurate which may lead to inaccurate inference elsewhere in the " - "program. It is important to manually review type inferred for " - "undefined functions for this reason." ), + "Because 3c is not able to examine the body of the function, the " + "inferred pointer types (and array bounds) may not be consistent " + "with the actual implementation. By default, the Checked C compiler " + "trusts the declared itypes and will not detect a spatial memory " + "safety violation if the function is used in a way that is " + "consistent with the itypes but not the assumptions actually made " + "by the implementation. Thus, if you want to guarantee spatial " + "memory safety, you must manually check the inferred types against " + "your understanding of what the function actually does (or any " + "available documentation)."), cl::init(false), cl::cat(_3CCategory)); #ifdef FIVE_C From 524eb39f8705556dd8bebda7af0da7822d77b677 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 31 Aug 2021 15:38:50 -0400 Subject: [PATCH 12/15] Update test --- clang/test/3C/params_in_macro.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/clang/test/3C/params_in_macro.c b/clang/test/3C/params_in_macro.c index 1850419d6fff..ceaabe58f967 100644 --- a/clang/test/3C/params_in_macro.c +++ b/clang/test/3C/params_in_macro.c @@ -11,14 +11,11 @@ typedef double mydouble; -// TODO: FunctionDeclBuilder::buildDeclVar should be able to handle an itype -// here, but currently the PointerConstraintVariable constructor asserts when it -// fails to retrieve the original source of the itype declaration. #define parms1 volatile mydouble d, void (*f)(void) -#define parms2 int *const y : count(7), _Ptr z +#define parms2 int *const y : count(7), _Ptr z, int *zz : itype(_Ptr) void test(parms1, int *x, parms2) {} -// CHECK: void test(volatile mydouble d, void (*f)(void), _Ptr x, int *const y : count(7), _Ptr z) {} +// CHECK: void test(volatile mydouble d, void (*f)(void), _Ptr x, int *const y : count(7), _Ptr z, int *zz : itype(_Ptr)) {} // Before the bug fix, we got: -// void test(, , _Ptr x, , ) {} +// void test(, , _Ptr x, , , ) {} From 721358df0acd892a16e74db51dfc003f87c514c5 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Wed, 1 Sep 2021 13:41:35 -0400 Subject: [PATCH 13/15] Refernce issue --- clang/test/3C/macro_itype.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/test/3C/macro_itype.c b/clang/test/3C/macro_itype.c index 14271ad2fcd7..fc0a31ce3592 100644 --- a/clang/test/3C/macro_itype.c +++ b/clang/test/3C/macro_itype.c @@ -16,8 +16,7 @@ struct s { macro0 }; // would be rewritten to `int *b` even though the rewriter intended to give it // an itype. If the parameter was then passed a checked pointer, there would be // a Checked C compiler error. Ideally, 3C wouldn't need to change the -// declaration of `b` at all, but this is the usual limitation we have with -// rewriting around macros. +// declaration of `b` at all (see issue correctcomputation/checkedc-clang#694). #define macro1 : itype(_Ptr) void fn(int *b macro1, int *c) { //CHECK: void fn(int *b : itype(_Ptr), _Ptr c) { From 46e5a3deac7c2cef58a53e8cb7cb1bf1477534fe Mon Sep 17 00:00:00 2001 From: John Kastner Date: Thu, 2 Sep 2021 10:41:36 -0400 Subject: [PATCH 14/15] Add test case from correctcomputation/checkedc-clang#594 --- clang/test/3C/macro_itype.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/clang/test/3C/macro_itype.c b/clang/test/3C/macro_itype.c index fc0a31ce3592..fd138524d6fa 100644 --- a/clang/test/3C/macro_itype.c +++ b/clang/test/3C/macro_itype.c @@ -11,6 +11,11 @@ struct s { macro0 }; //CHECK: struct s { macro0 }; +// Example from issue correctcomputation/checkedc-clang#594. +#define PARAM_DECL_WITH_ITYPE int *p : itype(_Ptr) +void foo(PARAM_DECL_WITH_ITYPE); +//CHECK: void foo(PARAM_DECL_WITH_ITYPE); + // Just removing the assertion that failed on the above example caused this to // rewrite incorrectly. The ItypeStr would be left empty, so first parameter // would be rewritten to `int *b` even though the rewriter intended to give it From f75979d12ec23ad563b66be0e9dd5323b4405125 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Wed, 8 Sep 2021 15:27:09 -0400 Subject: [PATCH 15/15] Update comment --- clang/lib/3C/ProgramInfo.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/clang/lib/3C/ProgramInfo.cpp b/clang/lib/3C/ProgramInfo.cpp index a5b508a9a6bc..6cf40e3d9384 100644 --- a/clang/lib/3C/ProgramInfo.cpp +++ b/clang/lib/3C/ProgramInfo.cpp @@ -447,6 +447,12 @@ void ProgramInfo::linkFunction(FunctionVariableConstraint *FV) { // it is actually used). FV->equateWithItype(*this, Rsn, nullptr); + // Used to apply constraints to parameters and returns for function without a + // body. In the default configuration, the function is fully constrained so + // that parameters and returns are considered unchecked. When 3C is run with + // --infer-types-for-undefs, only internal variables are constrained, allowing + // external variables to solve to checked types meaning the parameter will be + // rewritten to an itype. auto LinkComponent = [this, Rsn](const FVComponentVariable *FVC) { FVC->getInternal()->constrainToWild(CS, Rsn); if (!_3COpts.InferTypesForUndefs && @@ -454,8 +460,6 @@ void ProgramInfo::linkFunction(FunctionVariableConstraint *FV) { FVC->getExternal()->constrainToWild(CS, Rsn); }; - // If we've seen this symbol, but never seen a body for it, constrain - // everything about it. if (!FV->hasBody()) { LinkComponent(FV->getCombineReturn()); for (unsigned I = 0; I < FV->numParams(); I++)