From 9f67b45ea0abd003a42b241d86bc763ef51b60bc Mon Sep 17 00:00:00 2001 From: acqn Date: Sat, 18 Jul 2020 23:30:09 +0800 Subject: [PATCH] Fixed returning by value structs/unions <= 4 bytes in size from functions. Larger ones are forbidden for now. --- src/cc65/assignment.c | 144 +++++++++++++++++++----------------------- src/cc65/expr.c | 57 +++++++++++------ src/cc65/loadexpr.c | 2 +- src/cc65/stmt.c | 16 ++++- 4 files changed, 118 insertions(+), 101 deletions(-) diff --git a/src/cc65/assignment.c b/src/cc65/assignment.c index 8e42a338f..2a278f4a5 100644 --- a/src/cc65/assignment.c +++ b/src/cc65/assignment.c @@ -54,6 +54,69 @@ +static int CopyStruct (ExprDesc* LExpr, ExprDesc* RExpr) +/* Copy the struct represented by RExpr to the one represented by LExpr */ +{ + /* If the size is that of a basic type (char, int, long), we will copy + ** the struct using the primary register, otherwise we use memcpy. In + ** the former case, push the address only if really needed. + */ + const Type* ltype = LExpr->Type; + const Type* stype = GetReplacementType (ltype); + int UseReg = (stype != LExpr->Type); + + if (UseReg) { + PushAddr (LExpr); + } else { + ED_MarkExprAsRVal (LExpr); + LoadExpr (CF_NONE, LExpr); + g_push (CF_PTR | CF_UNSIGNED, 0); + } + + /* Get the expression on the right of the '=' into the primary */ + hie1 (RExpr); + + /* Check for equality of the structs */ + if (TypeCmp (ltype, RExpr->Type) < TC_STRICT_COMPATIBLE) { + Error ("Incompatible types"); + } + + /* Do we copy using the primary? */ + if (UseReg) { + + /* Check if the right hand side is an lvalue */ + if (ED_IsLVal (RExpr)) { + /* Just load the value into the primary as the replacement type. */ + LoadExpr (TypeOf (stype) | CF_FORCECHAR, RExpr); + } + + /* Store it into the new location */ + Store (LExpr, stype); + + } else { + + /* Check if the right hand side is an lvalue */ + if (ED_IsLVal (RExpr)) { + /* We will use memcpy. Push the address of the rhs */ + ED_MarkExprAsRVal (RExpr); + LoadExpr (CF_NONE, RExpr); + } + + /* Push the address (or whatever is in ax in case of errors) */ + g_push (CF_PTR | CF_UNSIGNED, 0); + + /* Load the size of the struct into the primary */ + g_getimmed (CF_INT | CF_UNSIGNED | CF_CONST, CheckedSizeOf (ltype), 0); + + /* Call the memcpy function */ + g_call (CF_FIXARGC, Func_memcpy, 4); + } + + return 0; +} + + + void Assignment (ExprDesc* Expr) /* Parse an assignment */ { @@ -79,85 +142,8 @@ void Assignment (ExprDesc* Expr) ** family, allow it here. */ if (IsClassStruct (ltype)) { - - /* Get the size of the left hand side. */ - unsigned Size = SizeOf (ltype); - - /* If the size is that of a basic type (char, int, long), we will copy - ** the struct using the primary register, otherwise we use memcpy. In - ** the former case, push the address only if really needed. - */ - int UseReg = 1; - Type* stype; - switch (Size) { - case SIZEOF_CHAR: stype = type_uchar; break; - case SIZEOF_INT: stype = type_uint; break; - case SIZEOF_LONG: stype = type_ulong; break; - default: stype = ltype; UseReg = 0; break; - } - if (UseReg) { - PushAddr (Expr); - } else { - ED_MarkExprAsRVal (Expr); - LoadExpr (CF_NONE, Expr); - g_push (CF_PTR | CF_UNSIGNED, 0); - } - - /* Get the expression on the right of the '=' into the primary */ - hie1 (&Expr2); - - /* Check for equality of the structs */ - if (TypeCmp (ltype, Expr2.Type) < TC_STRICT_COMPATIBLE) { - Error ("Incompatible types"); - } - - /* Check if the right hand side is an lvalue */ - if (ED_IsLVal (&Expr2)) { - /* We have an lvalue. Do we copy using the primary? */ - if (UseReg) { - /* Just use the replacement type */ - Expr2.Type = stype; - - /* Load the value into the primary */ - LoadExpr (CF_FORCECHAR, &Expr2); - - /* Store it into the new location */ - Store (Expr, stype); - - } else { - - /* We will use memcpy. Push the address of the rhs */ - ED_MarkExprAsRVal (&Expr2); - LoadExpr (CF_NONE, &Expr2); - - /* Push the address (or whatever is in ax in case of errors) */ - g_push (CF_PTR | CF_UNSIGNED, 0); - - /* Load the size of the struct into the primary */ - g_getimmed (CF_INT | CF_UNSIGNED | CF_CONST, CheckedSizeOf (ltype), 0); - - /* Call the memcpy function */ - g_call (CF_FIXARGC, Func_memcpy, 4); - } - - } else { - - /* We have an rvalue. This can only happen if a function returns - ** a struct, since there is no other way to generate an expression - ** that has a struct as an rvalue result. We allow only 1, 2, and 4 - ** byte sized structs, and do direct assignment. - */ - if (UseReg) { - /* Do the store */ - Store (Expr, stype); - } else { - /* Print a diagnostic */ - Error ("Structs of this size are not supported"); - /* Adjust the stack so we won't run in an internal error later */ - pop (CF_PTR); - } - - } + /* Copy the struct by value */ + CopyStruct (Expr, &Expr2); } else if (ED_IsBitField (Expr)) { diff --git a/src/cc65/expr.c b/src/cc65/expr.c index f6aa5aad6..70b28a408 100644 --- a/src/cc65/expr.c +++ b/src/cc65/expr.c @@ -470,6 +470,7 @@ static void FunctionCall (ExprDesc* Expr) int PtrOffs = 0; /* Offset of function pointer on stack */ int IsFastcall = 0; /* True if it's a fast-call function */ int PtrOnStack = 0; /* True if a pointer copy is on stack */ + Type* ReturnType; /* Skip the left paren */ NextToken (); @@ -648,7 +649,19 @@ static void FunctionCall (ExprDesc* Expr) /* The function result is an rvalue in the primary register */ ED_FinalizeRValLoad (Expr); - Expr->Type = GetFuncReturn (Expr->Type); + ReturnType = GetFuncReturn (Expr->Type); + + /* Handle struct specially */ + if (IsTypeStruct (ReturnType)) { + /* If there is no replacement type, then it is just the address */ + if (ReturnType == GetReplacementType (ReturnType)) { + /* Dereference it */ + ED_IndExpr (Expr); + ED_MarkExprAsRVal (Expr); + } + } + + Expr->Type = ReturnType; } @@ -1206,12 +1219,20 @@ static void StructRef (ExprDesc* Expr) return; } - if (IsTypePtr (Expr->Type)) { - /* If we have a struct pointer that is an lvalue and not already in the - ** primary, load its content now. - */ - if (!ED_IsConst (Expr)) { - /* Load into the primary */ + /* A struct is usually an lvalue. If not, it is a struct passed in the + ** primary register, which is usually a result returned from a function. + ** However, it is possible that this rvalue is a result of certain + ** operations on an lvalue, and there are no reasons to disallow that. + ** So we just rely on the check on function returns to catch the errors + ** and dereference the rvalue address of the struct here. + */ + if (IsTypePtr (Expr->Type) || + (ED_IsRVal (Expr) && + ED_IsLocPrimary (Expr) && + Expr->Type == GetReplacementType (Expr->Type))) { + + if (!ED_IsConst (Expr) && !ED_IsLocPrimary (Expr)) { + /* If we have a non-const struct pointer, load its content now */ LoadExpr (CF_NONE, Expr); /* Clear the offset */ @@ -1241,27 +1262,25 @@ static void StructRef (ExprDesc* Expr) FinalType->C |= Q; } - /* A struct is usually an lvalue. If not, it is a struct referenced in the - ** primary register, which is likely to be returned from a function. - */ - if (ED_IsRVal (Expr) && ED_IsLocExpr (Expr) && !IsTypePtr (Expr->Type)) { + if (ED_IsRVal (Expr) && ED_IsLocPrimary (Expr) && !IsTypePtr (Expr->Type)) { unsigned Flags = 0; unsigned BitOffs; /* Get the size of the type */ - unsigned Size = SizeOf (Expr->Type); + unsigned StructSize = SizeOf (Expr->Type); + unsigned FieldSize = SizeOf (Field->Type); /* Safety check */ - CHECK (Field->V.Offs + Size <= SIZEOF_LONG); + CHECK (Field->V.Offs + FieldSize <= StructSize); /* The type of the operation depends on the type of the struct */ - switch (Size) { - case 1: Flags = CF_CHAR | CF_UNSIGNED | CF_CONST; break; - case 2: Flags = CF_INT | CF_UNSIGNED | CF_CONST; break; + switch (StructSize) { + case 1: Flags = CF_CHAR | CF_UNSIGNED | CF_CONST; break; + case 2: Flags = CF_INT | CF_UNSIGNED | CF_CONST; break; case 3: /* FALLTHROUGH */ - case 4: Flags = CF_LONG | CF_UNSIGNED | CF_CONST; break; - default: Internal ("Invalid struct size: %u", Size); break; + case 4: Flags = CF_LONG | CF_UNSIGNED | CF_CONST; break; + default: Internal ("Invalid struct size: %u", StructSize); break; } /* Generate a shift to get the field in the proper position in the @@ -1274,7 +1293,7 @@ static void StructRef (ExprDesc* Expr) /* Mask the value. This is unnecessary if the shift executed above ** moved only zeroes into the value. */ - if (BitOffs + Field->V.B.BitWidth != Size * CHAR_BITS) { + if (BitOffs + Field->V.B.BitWidth != FieldSize * CHAR_BITS) { g_and (CF_INT | CF_UNSIGNED | CF_CONST, (0x0001U << Field->V.B.BitWidth) - 1U); } diff --git a/src/cc65/loadexpr.c b/src/cc65/loadexpr.c index 7eef174e2..bc0ee1dd0 100644 --- a/src/cc65/loadexpr.c +++ b/src/cc65/loadexpr.c @@ -116,7 +116,7 @@ void LoadExpr (unsigned Flags, struct ExprDesc* Expr) if (ED_IsBitField (Expr)) { Flags |= (Expr->BitOffs + Expr->BitWidth <= CHAR_BITS) ? CF_CHAR : CF_INT; Flags |= CF_UNSIGNED; - } else { + } else if ((Flags & CF_TYPEMASK) == 0) { Flags |= TypeOf (Expr->Type); } if (ED_NeedsTest (Expr)) { diff --git a/src/cc65/stmt.c b/src/cc65/stmt.c index 657bc9963..a1384b0ed 100644 --- a/src/cc65/stmt.c +++ b/src/cc65/stmt.c @@ -309,7 +309,8 @@ static void WhileStatement (void) static void ReturnStatement (void) /* Handle the 'return' statement */ { - ExprDesc Expr; + ExprDesc Expr; + const Type* ReturnType; NextToken (); if (CurTok.Tok != TOK_SEMI) { @@ -328,7 +329,18 @@ static void ReturnStatement (void) TypeConversion (&Expr, F_GetReturnType (CurrentFunc)); /* Load the value into the primary */ - LoadExpr (CF_NONE, &Expr); + if (IsTypeStruct (Expr.Type) || IsTypeUnion (Expr.Type)) { + /* Handle struct/union specially */ + ReturnType = GetReplacementType (Expr.Type); + if (ReturnType == Expr.Type) { + Error ("Returning %s of this size by value is not supported", GetBasicTypeName (Expr.Type)); + } + LoadExpr (TypeOf (ReturnType), &Expr); + + } else { + /* Load the value into the primary */ + LoadExpr (CF_NONE, &Expr); + } } } else if (!F_HasVoidReturn (CurrentFunc) && !F_HasOldStyleIntRet (CurrentFunc)) {