Code generation bug for 32-bit compact memory model

[Leif Ekblad]

I eventually found a code-generator bug for 32-bit compact memory model
after all.

C code:
"If (NULL != Handler)"
Handler is a procedure pointer.

Code generated:

"mov eax,cs
mov edx,dword ptr [edx].Handler
test edx,edx
jne fail
test ax,ax
je ok"

So, the code generator does this comparison using 48-bit far pointers (which
is correct for data pointers but not function pointers) instead of only
using offset comparison. The results is that Handler is never NULL because
cs is never NULL. The part that compares selectors is not applicable to
compact memory model, since all procedures are near.

Anybody has an idea where this might be located in the code-generator?

Leif Ekblad

[Hans-Bernhard Bröker]

On 06.10.2011 19:07, Leif Ekblad wrote:
> I eventually found a code-generator bug for 32-bit compact memory model
> after all.
>
> C code:
> "If (NULL != Handler)"
> Handler is a procedure pointer.

That's actually a bug in your code. The standard NULL macro is _not_ a
function pointer. It's a universal data pointer, but data pointers and
function pointers are separate classes of objects in C. Comparisons
between function pointers and data pointers are undefined by the language.

You really should have written

if (0 != Handler)

or just

if (Handler)

> So, the code generator does this comparison using 48-bit far pointers (which
> is correct for data pointers but not function pointers)

And it's correct in doing so.

[Leif Ekblad]

Hans-Bernhard Bröker:

> That's actually a bug in your code. The standard NULL macro is _not_ a
> function pointer. It's a universal data pointer, but data pointers and
> function pointers are separate classes of objects in C. Comparisons
> between function pointers and data pointers are undefined by the language.
>
> You really should have written
>
> if (0 != Handler)
>
> or just
>
> if (Handler)

The problem is that it was not I that wrote the code. This is part of
Intel's ACPI
implementation that was written to be portable between many different
compilers
and operating systems. That either means that nobody has compiled it for
compact memory model, or that other C compilers handle it differently.

Besides, I don't think it is valid to extend a function pointer in compact
memory
model, that uses offset only, to a far pointer when there is no explicit
cast to a
far pointer.

If such comparisons are undefined by the language, they should be handled in
a graceful way. Nobody that writes such code means that the near function
pointer
should be extended to a far pointer, and then comparing it with 0. That
comparison
would always be false (at least in protected mode where CS can never be
zero).

Leif Ekblad

[Peter C. Chapin]

On 2011-10-06 17:25, Leif Ekblad wrote:

> If such comparisons are undefined by the language, they should be
> handled in a graceful way. Nobody that writes such code means that
> the near function pointer should be extended to a far pointer, and
> then comparing it with 0. That comparison would always be false (at
> least in protected mode where CS can never be zero).

It is true that if something is undefined by the language we are free to
define it in some way that is presumably sensible for our case. As a
word of caution we should be careful about making a change that is
sensible in one context but nonsense in another. Even if everything is
technically undefined and thus still strictly correct from a standards
point of view, shuffling a surprise from one place to another does not
necessarily put us ahead.

I'm not saying this is the case here. I'm just speaking in general terms.

Peter

[Leif Ekblad]

Peter C. Chapin:

> It is true that if something is undefined by the language we are free to
> define it in some way that is presumably sensible for our case. As a word
> of caution we should be careful about making a change that is sensible in
> one context but nonsense in another. Even if everything is technically
> undefined and thus still strictly correct from a standards point of view,
> shuffling a surprise from one place to another does not necessarily put us
> ahead.

I fixed all the instances of comparing a function pointer to NULL in
the ACPICA code, but there is a big problem with this behavior.
Most of today's software is written for a flat memory model, where
size of a function pointer is equal to the size of a data pointer, and
the CS register is ignored. That means that people testing their code
with a flat memory model will not notice any problem, and thus
will not fix the issue by using 0 instead of NULL. When I then
compile this code with a compact memory model it fails. That
is a big problem for me.

Leif Ekblad

[Paul S. Person]

Perhaps I'm confused, but I thought one of the advantages of using
NULL or 0 in a comparison was that the compiler would do whatever was
necessary to make it work properly. Or am I only semi-confused, that
is, does that only apply to data and data pointers?
--
"'If God foreknew that this would happen,
it will happen.'"

[Paul S. Person]

On Thu, 6 Oct 2011 23:25:30 +0200, "Leif Ekblad" <le...@rdos.net>
wrote:

I may be wrong, but is it not the case that, given a selector <s>, the
pointer <s>:0 is valid? That is, is it not true that the first byte in
the segment defined by the selector (I can only hope I get the
terminology right) is accessible? And so, that a function (or data
item) could, in fact, start there?

If so, then comparing offsets won't work -- even in Flat model. But
perhaps I am wrong and the first byte or bytes are not valid targets
for the offset. Perhaps the old DOS (or was that originally CP/M?)
prefix lives on, even in 32-bit protected mode, even in Flat model!

[Hans-Bernhard Bröker]

On 10.10.2011 18:50, Paul S. Person wrote:

> Perhaps I'm confused, but I thought one of the advantages of using
> NULL or 0 in a comparison was that the compiler would do whatever was
> necessary to make it work properly.

That advantage applies completely only in the case of using an integer
zero. NULL (particularly in C compilers) has a strong tendency of being
defined as ((void *)0), which makes it a data pointer.

The case where that makes a difference is when you use NULL in
connection to function pointers. Function pointers and data pointers
don't mix. In a nutshell, C assumes a Harvard architecture, and that's
a fact surprisingly many, even experienced C programmers aren't aware of
at all.

[Leif Ekblad]

Paul S. Person:

> I may be wrong, but is it not the case that, given a selector <s>, the
> pointer <s>:0 is valid? That is, is it not true that the first byte in
> the segment defined by the selector (I can only hope I get the
> terminology right) is accessible? And so, that a function (or data
> item) could, in fact, start there?

Yes, it is valid from the point of view of segmentation.

> If so, then comparing offsets won't work -- even in Flat model. But
> perhaps I am wrong and the first byte or bytes are not valid targets
> for the offset. Perhaps the old DOS (or was that originally CP/M?)
> prefix lives on, even in 32-bit protected mode, even in Flat model!

Flat memory model uses paging as the primary protection scheme.
Most implementations make the first pages (and sometimes more) inaccessible.
This is to catch null-pointers. OWs CLIB also has a similar feature, but
places
an INT 3 first in the executable for small-code memory models just to catch
invalid calls.

The problem here is that in compact memory model, all function pointers
are near, and assumed to be within a single code-segment. The function null
pointer is offset 0. That is probably why OW extends the function pointer to
48-bits by adding current CS. This would be perfectly valid if comparing
near
pointers with far pointers, but it is not valid when it is compared to a
48-bit
null pointer, as this comparison is always not-equal, and useless.

Leif Ekblad