Patchwork romcc changes for mingw32 environment

login
register
about
Submitter Patrick Georgi
Date 2009-11-20 21:03:28
Message ID <4B070420.2000503@georgi-clan.de>
Download mbox | patch
Permalink /patch/571/
State Accepted
Headers show

Comments

Patrick Georgi - 2009-11-20 21:03:28
Hi,

I succeeded in building the entire SerialICE tool set (host and target) 
on mingw32. Most changes are in the repository already, but there are 
two issues in romcc without a clean fix so far, so I thought I'll post 
the patch for discussion to have it out in public.
As they're romcc related, I also added the coreboot list.

1. romcc seems to access memory after free. There's an xfree() call 
removed in that patch, which is a thinly disguised free() call. With 
that one removed, free_basic_block(..) works happily, with it present, I 
get a segfault. Win32 seems to invalidate free'd memory by writing a 
pattern (0xfeeefeee) there, and that trips up the next pointer deref.

There's probably a "sane" fix to that.

2. romcc uses open/read/close to read included files. For some reason, 
read always returns 0 bytes, so that's an endless loop. I kept the 
open/stat/close part, and replaced the read with fread (and fopen/fclose 
to support it).

This might be solved in a cleaner way.


Patrick
Peter Stuge - 2009-11-20 21:26:30
Patrick Georgi wrote:
> 2. romcc uses open/read/close to read included files. For some
> reason, read always returns 0 bytes, so that's an endless loop. I
> kept the open/stat/close part, and replaced the read with fread
> (and fopen/fclose to support it).
>
> This might be solved in a cleaner way.

Maybe you can use fdopen() to tur the fd into FILE * once size is
known?

When done, just fclose() is sufficient, close() is not needed (and
will fail, since fclose() on fdopen()ed fds will close()). Confused
yet? :)


//Peter
Patrick Georgi - 2009-11-20 22:24:20
Am 20.11.2009 22:26, schrieb Peter Stuge:
> Maybe you can use fdopen() to tur the fd into FILE * once size is
> known?
>
> When done, just fclose() is sufficient, close() is not needed (and
> will fail, since fclose() on fdopen()ed fds will close()). Confused
> yet? :)
>    
Stefan reminded me on how cbfstool does it: fopen, fseek to the end, 
ftell, fseek to the beginning. I don't want to use a relatively obscure 
call like fdopen() when the aim is portability, but thanks for the idea.

Only leaves the free(..) call removal around.


Patrick

Patch

Index: util/romcc.c
===================================================================
--- util/romcc.c	(Revision 58)
+++ util/romcc.c	(Arbeitskopie)
@@ -224,6 +224,7 @@ 
 	off_t size, progress;
 	ssize_t result;
 	struct stat stats;
+	FILE* file;
 	
 	if (!filename) {
 		*r_size = 0;
@@ -234,6 +235,7 @@ 
 	}
 	xchdir(dirname);
 	fd = open(filename, O_RDONLY);
+	file = fopen(filename, "rb");
 	xchdir(cwd);
 	if (fd < 0) {
 		die("Cannot open '%s' : %s\n",
@@ -250,8 +252,13 @@ 
 	buf[size] = '\n'; /* Make certain the file is newline terminated */
 	buf[size+1] = '\0'; /* Null terminate the file for good measure */
 	progress = 0;
+	result = close(fd);
+	if (result < 0) {
+		die("Close of %s failed: %s\n",
+			filename, strerror(errno));
+	}
 	while(progress < size) {
-		result = read(fd, buf + progress, size - progress);
+		result = fread(buf + progress, 1, size - progress, file);
 		if (result < 0) {
 			if ((errno == EINTR) ||	(errno == EAGAIN))
 				continue;
@@ -260,11 +267,7 @@ 
 		}
 		progress += result;
 	}
-	result = close(fd);
-	if (result < 0) {
-		die("Close of %s failed: %s\n",
-			filename, strerror(errno));
-	}
+	fclose(file);
 	return buf;
 }
 
@@ -15148,7 +15151,6 @@ 
 		}
 	}
 	memset(block, -1, sizeof(*block));
-	xfree(block);
 }
 
 static void free_basic_blocks(struct compile_state *state,