Attachment 154 Details for Bug 176 – Behold!

[patch] Behold!

0001-Fix-for-bug-176-During-AddFixup-evaluate-as-much-of-.patch (text/plain), 2.79 KB, created by ggn on 2020-10-12 05:19:46 CDT

(hide)

Description:

Filename:

MIME Type:

Creator: ggn

Created: 2020-10-12 05:19:46 CDT

Size: 2.79 KB

patch

obsolete

>From c325aa1ff80872ec7da4e3e9315f08bd85de6226 Mon Sep 17 00:00:00 2001
>From: ggn <ggn.dbug@gmail.com>
>Date: Mon, 12 Oct 2020 13:16:33 +0300
>Subject: [PATCH] Fix for bug #176: During AddFixup(), evaluate as much of the
> expression as possible to avoid changed symbols during the fixup pass
>
>---
> sect.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 47 insertions(+), 2 deletions(-)
>
>diff --git a/sect.c b/sect.c
>index ae4d750..30b5685 100644
>--- a/sect.c
>+++ b/sect.c
>@@ -294,7 +294,7 @@ int AddFixup(uint32_t attr, uint32_t loc, TOKEN * fexpr)
> 	}
> 
> 	// Allocate space for the fixup + any expression
>-	FIXUP * fixup = malloc(sizeof(FIXUP) + (sizeof(TOKEN) * exprlen));
>+	FIXUP * fixup = malloc(sizeof(FIXUP) + (sizeof(TOKEN) * exprlen)*2);
> 
> 	// Store the relevant fixup information in the FIXUP
> 	fixup->next = NULL;
>@@ -309,8 +309,53 @@ int AddFixup(uint32_t attr, uint32_t loc, TOKEN * fexpr)
> 	// Copy the passed in expression to the FIXUP, if any
> 	if (exprlen > 0)
> 	{
>+		// Here we used to to a plain memcpy and punt on trying to evaluate the expression by then.
>+		// As discussed in bug #176, this can lead to robustness issues because some symbols might
>+		// have changed by the time we perform the relocations (think of a symbol that's SET multiple
>+		// times). So instead we perform a symbol-by-symbol copy and check to see if there are any
>+		// resolved symbols that can be evaluated immediately. Those, we replace with constants.
>+		// Also of note: because "fixup" can be larger than what ExpressionLength() returns
>+		// (due to constants taking up more space than symbols), we allocate twice as RAM as we should
>+		// without expansions just to be on the safe side. The "correct" thing to do would be to
>+		// modify ExpressionLength() to cater for defined symbols and return the exact amount of items.
>+
> 		fixup->expr = (TOKEN *)((uint8_t *)fixup + sizeof(FIXUP));
>-		memcpy(fixup->expr, fexpr, sizeof(TOKEN) * exprlen);
>+		int i;
>+		PTR dstexpr;
>+		dstexpr.u32 = fixup->expr;
>+		SYM *sy;
>+		for (i = 0; i < exprlen; i++)
>+		{
>+			if (*fexpr == SYMBOL)
>+			{
>+				sy = symbolPtr[fexpr[1]];
>+				if (sy->sattr & DEFINED && !(sy->sattr & (TDB| M56KPXYL|M6502)))
>+				{
>+					// Only convert symbols that are defined and are absolute
>+					*dstexpr.u32++ = CONST;
>+					*dstexpr.u64++ = sy->svalue;
>+					fexpr += 2;
>+					i++;
>+				}
>+				else
>+				{
>+					// Just copy the symbol
>+					*dstexpr.u32++ = *fexpr++;
>+					*dstexpr.u32++ = *fexpr++;
>+					i++;
>+				}
>+			}
>+			else if (*fexpr == CONST || *fexpr == FCONST)
>+			{
>+				// Copy constants
>+				*dstexpr.u32++ = *fexpr++;
>+				*dstexpr.u32++ = *fexpr++;
>+				*dstexpr.u32++ = *fexpr++;
>+				i += 2;
>+			}
>+			else
>+				*dstexpr.u32++ = *fexpr++;
>+		}
> 	}
> 
> 	// Finally, put the FIXUP in the current section's linked list
>-- 
>2.25.0.windows.1
>

Actions: View

Attachments on bug 176: 154