Date: Mon, 27 Oct 1997 15:41:26 -0800 (PST)
Message-Id: <199710272341.PAA12338@adit.ap.net>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
To: djgpp AT delorie DOT com
From: Nate Eldredge <eldredge AT ap DOT net>
Subject: Suggestions for startup code
Precedence: bulk

I was looking through the sources to some of the startup code today, and I
noticed a couple of potential problems that I though perhaps should be
addressed.

1. In `src/libc/crt0/crt1.c', in function setup_core_selector(), the _dos_ds
selector is created. I glean from the spec that the newly allocated selector
has a base and limit of 0. On the next line, however, the limit is set to
-1, so that _dos_ds can span the entire address space. I suppose this isn't
too much of a safety issue, since using _dos_ds can screw over DOS anyway.
But according to FAQ section 18.6, some OS's, such as Windows NT and Dosemu,
will refuse to honor a request to make such a huge segment. In this case, I
assume, _dos_ds would be left with a limit of 0, and any attempt to use it
would seg out. I would think that a better alternative would be to try again
with a more reasonable limit, perhaps 1M. Otherwise, access to the low 1M
would break on these systems, and things such as graphics would fail to work.

2. In `src/libc/crt0/mcount.c'. It seems, from looking at generated
assembly, that mcount(), the function which counts calls to profiled
functions, gets an argument in the %edx register. But the value is fetched
using inline assembly from C, and several lines into the code. IMHO, this is
dangerous. There's nothing to stop the compiler from using %edx for
something else before that line gets a shot at it. Admittedly, the current
version of GCC doesn't, but future versions might. I think it would be a
better idea to put an assembly wrapper around the function. Perhaps just this:

.global _mcount
_mcount:
        pushl %edx
        call real_mcount
        popl %edx  # smaller than addl $4,%esp
        ret

Then the existing mcount() would be changed to real_mcount, and have these args:

void real_mcount(MTABE **cache, int _to);

since `cache' is the variable into which %edx is moved. Since the address of
_to is used to peek around on the stack, all references to `&_to' in the
function would be changed to `&_to+1' (since it is now 1 stack slot lower
than otherwise).

Just my $0.02. If anyone has any other ideas, let me know.

Nate Eldredge
eldredge AT ap DOT net