Changeset b81fd95 for src

Timestamp:

Jul 29, 2020, 11:29:01 PM (3 years ago)

Author:

Michael Brooks <mlbrooks@…>

Branches:

ADT, arm-eh, enum, forall-pointer-decay, jacob/cs343-translation, master, new-ast, new-ast-unique-expr, pthread-emulation, qualifiedEnum

Children:

0c760db

Parents:

1d17939

Message:

Fix bug where pointer and reference types allow unsound initialization and return. Fixes #189

There are two instances of the same basic change, which is using conversionCost instead of castCost for resolving...
A: an InitExpr?, always; affects variable initializations
B: a CastExpr?, for type-system-generated casts only; affects function returns

Changing the behaviour of the typechecker on initialization (do A) and cast (do B):
src/ResolvExpr/AlternativeFinder.cc
src/SynTree/Expression.h
testsinit1.*

Making type of string literal consistent with how C defines it (accommodate A):
src/Parser/ExpressionNode.cc

Making type system happy with incumbent use of void* (accommodate A):
libcfa/src/concurrency/kernel.cfa
libcfa/src/containers/list.hfa
tests/bugs/66.cfa
tests/avltree/avl1.cfa
tests/concurrent/signal/block.cfa
tests/searchsort.cfa

Making type system happy with incumbent plan-9 downcast (accommodate B):
libcfa/src/containers/list.hfa

Fixing previously incorrect constness of declarations (accommodate A):
tests/exceptions/defaults.cfa
libcfa/src/iostream.hfa

Fixing previously incorrect isGenerated classification of casts that desugaring introduces (accommodate B):
src/Concurrency/Keywords.cc
src/Concurrency/Waitfor.cc

Working around trac #207 (revealed by A):
tests/io2.cfa

Working around trac #208 (speculatively created by B):
libcfa/src/bits/locks.hfa
libcfa/src/concurrency/preemption.cfa

Misc:
tests/exceptions/conditional.cfa (accommodate A)

a _msg function for an exception was declared with wrong return type, so it was not compatible for assignment into the vtable instance

libcfa/src/stdlib.hfa

the compiler now prohibits a prior attempt to call a nonexistent realloc overload; calling alloc_align in its place

Location:

src

Files:

• Concurrency/Keywords.cc (modified) (5 diffs)
• Concurrency/Waitfor.cc (modified) (3 diffs)
• Parser/ExpressionNode.cc (modified) (1 diff)
• ResolvExpr/AlternativeFinder.cc (modified) (2 diffs)
• SynTree/Expression.h (modified) (1 diff)

src/Concurrency/Keywords.cc

@@ -510 +510 @@
                                                 new CastExpr(
                                                         new VariableExpr( func->get_functionType()->get_parameters().front() ),
-                                                        func->get_functionType()->get_parameters().front()->get_type()->stripReferences()->clone()
+                                                        func->get_functionType()->get_parameters().front()->get_type()->stripReferences()->clone(),
+                                                        false
                                                 )
                                         )
@@ -888 +889 @@
                         new SingleInit( new UntypedExpr(
                                 new NameExpr( "get_monitor" ),
-                                {  new CastExpr( new VariableExpr( args.front() ), arg_type ) }
+                                {  new CastExpr( new VariableExpr( args.front() ), arg_type, false ) }
                         ))
                 );
@@ -909 +910 @@
                                         {
                                                 new SingleInit( new AddressExpr( new VariableExpr( monitors ) ) ),
-                                                new SingleInit( new CastExpr( new VariableExpr( func ), generic_func->clone() ) )
+                                                new SingleInit( new CastExpr( new VariableExpr( func ), generic_func->clone(), false ) )
                                         },
                                         noDesignators,
@@ -946 +947 @@
                                         return new SingleInit( new UntypedExpr(
                                                 new NameExpr( "get_monitor" ),
-                                                {  new CastExpr( new VariableExpr( var ), type ) }
+                                                {  new CastExpr( new VariableExpr( var ), type, false ) }
                                         ) );
                                 })
@@ -970 +971 @@
                                                 new SingleInit( new VariableExpr( monitors ) ),
                                                 new SingleInit( new ConstantExpr( Constant::from_ulong( args.size() ) ) ),
-                                                new SingleInit( new CastExpr( new VariableExpr( func ), generic_func->clone() ) )
+                                                new SingleInit( new CastExpr( new VariableExpr( func ), generic_func->clone(), false ) )
                                         },
                                         noDesignators,

src/Concurrency/Waitfor.cc

@@ -384 +384 @@
                                                                 decl_monitor
                                                         )
-                                                )
+                                                ),
+                                                false
                                         );
 
@@ -408 +409 @@
                         new CompoundStmt({
                                 makeAccStatement( acceptables, index, "is_dtor", detectIsDtor( clause.target.function )                                    , indexer ),
-                                makeAccStatement( acceptables, index, "func"   , new CastExpr( clause.target.function, fptr_t )                            , indexer ),
+                                makeAccStatement( acceptables, index, "func"   , new CastExpr( clause.target.function, fptr_t, false )                     , indexer ),
                                 makeAccStatement( acceptables, index, "data"   , new VariableExpr( monitors )                                              , indexer ),
                                 makeAccStatement( acceptables, index, "size"   , new ConstantExpr( Constant::from_ulong( clause.target.arguments.size() ) ), indexer ),
@@ -531 +532 @@
                                                                 decl_mask
                                                         )
-                                                )
+                                                ),
+                                                false
                                         ),
                                         timeout

src/Parser/ExpressionNode.cc

@@ -427 +427 @@
                 if ( str[1] == '8' ) goto Default;                              // utf-8 characters => array of char
                 // lookup type of associated typedef
-                strtype = new TypeInstType( Type::Qualifiers( Type::Const ), "char16_t", false );
+                strtype = new TypeInstType( Type::Qualifiers( ), "char16_t", false );
                 break;
           case 'U':
-                strtype = new TypeInstType( Type::Qualifiers( Type::Const ), "char32_t", false );
+                strtype = new TypeInstType( Type::Qualifiers( ), "char32_t", false );
                 break;
           case 'L':
-                strtype = new TypeInstType( Type::Qualifiers( Type::Const ), "wchar_t", false );
+                strtype = new TypeInstType( Type::Qualifiers( ), "wchar_t", false );
                 break;
           Default:                                                                                      // char default string type
           default:
-                strtype = new BasicType( Type::Qualifiers( Type::Const ), BasicType::Char );
+                strtype = new BasicType( Type::Qualifiers( ), BasicType::Char );
         } // switch
         ArrayType * at = new ArrayType( noQualifiers, strtype,

src/ResolvExpr/AlternativeFinder.cc

@@ -1216 +1216 @@
                         unify( castExpr->result, alt.expr->result, alt.env, needAssertions,
                                 haveAssertions, openVars, indexer );
-                        Cost thisCost = castCost( alt.expr->result, castExpr->result, alt.expr->get_lvalue(),
-                                indexer, alt.env );
+                        Cost thisCost =
+                                castExpr->isGenerated
+                                ? conversionCost( alt.expr->result, castExpr->result, alt.expr->get_lvalue(),   indexer, alt.env )
+                                : castCost( alt.expr->result, castExpr->result, alt.expr->get_lvalue(), indexer, alt.env );
                         PRINT(
                                 std::cerr << "working on cast with result: " << castExpr->result << std::endl;
@@ -1698 +1700 @@
 
                                 // unification run for side-effects
-                                unify( toType, alt.expr->result, newEnv, need, have, openVars, indexer );
+                                bool canUnify = unify( toType, alt.expr->result, newEnv, need, have, openVars, indexer );
+                                (void) canUnify;
                                 // xxx - do some inspecting on this line... why isn't result bound to initAlt.type?
 
-                                Cost thisCost = castCost( alt.expr->result, toType, alt.expr->get_lvalue(),
+                                Cost thisCost = computeConversionCost( alt.expr->result, toType, alt.expr->get_lvalue(),
                                         indexer, newEnv );
+
+                                PRINT(
+                                        Cost legacyCost = castCost( alt.expr->result, toType, alt.expr->get_lvalue(),
+                                                indexer, newEnv );
+                                        std::cerr << "Considering initialization:";
+                                        std::cerr << std::endl << "  FROM: "; alt.expr->result->print(std::cerr);
+                                        std::cerr << std::endl << "  TO: ";   toType          ->print(std::cerr);
+                                        std::cerr << std::endl << "  Unification " << (canUnify ? "succeeded" : "failed");
+                                        std::cerr << std::endl << "  Legacy cost " << legacyCost;
+                                        std::cerr << std::endl << "  New cost " << thisCost;
+                                        std::cerr << std::endl;
+                                )
+                                
                                 if ( thisCost != Cost::infinity ) {
                                         // count one safe conversion for each value that is thrown away

src/SynTree/Expression.h

@@ -206 +206 @@
   public:
         Expression * arg;
-        bool isGenerated = true; // cast generated implicitly by code generation or explicit in program
+
+        // Inidicates cast is introduced by the CFA type system.
+        // true for casts that the resolver introduces to force a return type
+        // false for casts from user code
+        // false for casts from desugaring advanced CFA features into simpler CFA
+        // example
+        //   int * p;     // declaration
+        //   (float *) p; // use, with subject cast
+        // subject cast isGenerated means we are considering an interpretation with a type mismatch
+        // subject cast not isGenerated means someone in charge wants it that way
+        bool isGenerated = true;
 
         CastExpr( Expression * arg, bool isGenerated = true );