Hi Bart.

After a lot of effort, UCX64 and cc64 are now running
on real hardware, self-compiling/hosting. Pictures
and more technical information available here:

https://groups.io/g/hercules-380/message/2696

It took a Chinese computer with a Chinese BIOS (written
in actual Chinese characters) with a Zhaoxin Chinese
(ie not AMD or Intel) x64 CPU to find this bug that
wasn't showing on Virtualbox or qemu.

I made this commit:

https://sourceforge.net/p/pdos/gitcode/ci/81d0fa1b901803ae3198bd8df0cd9c8756e08cd4/

C:\devel\pdos\cc64>git show 81d0fa1b901803ae3198bd8df0cd9c8756e08cd4
commit 81d0fa1b901803ae3198bd8df0cd9c8756e08cd4 (HEAD -> master, origin/master, origin/HEAD)
Author: Paul Edwards <mutazilah@gmail.com>
Date: Wed Sep 20 15:42:07 2023 +0800

initialize a variable found to be a wild pointer

diff --git a/cc64/cc64.c b/cc64/cc64.c
index 525e5f2b..4dcbc4db 100644
--- a/cc64/cc64.c
+++ b/cc64/cc64.c
@@ -22449,6 +22449,7 @@ void cc_libmcl_genmc(i64 opcode,struct cc_libmcl_opndrec * a,struct cc_libmcl_op
struct cc_libmcl_mclrec * m;
struct cc_libmcl_mclrec * oldm;
m = (struct cc_libmcl_mclrec *)(mlib_pcm_alloc((i64)32));
+ (*m).nextmcl = NULL; /* paul added this - should allocz be used instead? */
(*m).lineno = cc_decls_clineno;
(*m).opcode = (u64)(opcode);
(*m).a = a;

C:\devel\pdos\cc64>

Any comment on my comment?

I don't know if there are similar issues in other
parts of the code, but if they exist they're
probably not very important.

Thanks. Paul.

On 20/09/2023 11:39, Paul Edwards wrote:
> Hi Bart.
>
> After a lot of effort, UCX64 and cc64 are now running
> on real hardware, self-compiling/hosting. Pictures
> and more technical information available here:
>
> https://groups.io/g/hercules-380/message/2696
>
>
> It took a Chinese computer with a Chinese BIOS (written
> in actual Chinese characters) with a Zhaoxin Chinese
> (ie not AMD or Intel) x64 CPU to find this bug that
> wasn't showing on Virtualbox or qemu.
>
>
> I made this commit:
>
> https://sourceforge.net/p/pdos/gitcode/ci/81d0fa1b901803ae3198bd8df0cd9c8756e08cd4/
>
> C:\devel\pdos\cc64>git show 81d0fa1b901803ae3198bd8df0cd9c8756e08cd4
> commit 81d0fa1b901803ae3198bd8df0cd9c8756e08cd4 (HEAD -> master, origin/master, origin/HEAD)
> Author: Paul Edwards <mutazilah@gmail.com>
> Date: Wed Sep 20 15:42:07 2023 +0800
>
> initialize a variable found to be a wild pointer
>
> diff --git a/cc64/cc64.c b/cc64/cc64.c
> index 525e5f2b..4dcbc4db 100644
> --- a/cc64/cc64.c
> +++ b/cc64/cc64.c
> @@ -22449,6 +22449,7 @@ void cc_libmcl_genmc(i64 opcode,struct cc_libmcl_opndrec * a,struct cc_libmcl_op
> struct cc_libmcl_mclrec * m;
> struct cc_libmcl_mclrec * oldm;
> m = (struct cc_libmcl_mclrec *)(mlib_pcm_alloc((i64)32));
> + (*m).nextmcl = NULL; /* paul added this - should allocz be used instead? */
> (*m).lineno = cc_decls_clineno;
> (*m).opcode = (u64)(opcode);
> (*m).a = a;
>
> C:\devel\pdos\cc64>
>
>
> Any comment on my comment?

Yes, that was an oversight. Either pcm_allocz should be called or, more
efficient as most fields are initialised anyway, just set that extra
field as you have done.

Although under tests, that is consistently all zeros, and I'm not sure
why. Calling malloc() here instead will immediately return garbage.
pcm_alloc() will allocate space from a large pool, but that was itself
allocated with malloc.

This is an interesting little program:

#include <stdio.h>
#include <stdlib.h>

int main(void) {
enum {N=1024};
long long int sum=0;
unsigned char* p = malloc(N);

for (int i=0; i<N; ++i) sum+=*p++;

printf("Sum of bytes: %lld\n", sum);
}

This allocates memory with malloc then adds up all the byte values.

When N is small, like 1024, it will give results of around 80,000. For
16K, it's 3000, and at 512K, it's 700.

But at 1MB (N=1048576), it's always exactly zero. So perhaps it grabs
memory direct from the OS or something, which happens to be zeroed.

The memory pools used by pcm_alloc() in cc64.c are 2MB in size.

Still, the code should be fixed and not rely on such accidents. (Perhaps
allocated memory should be randomised during testing.)

(Newer versions of both this code and the alloc functions work
differently, for example the pool is explicitly zeroed, which for
allocations that are never freed, means they do not need individual
zeroing.)

On Wednesday, September 20, 2023 at 8:08:33 PM UTC+8, Bart wrote:

> Yes, that was an oversight. Either pcm_allocz should be called or, more
> efficient as most fields are initialised anyway, just set that extra
> field as you have done.

Ok, thanks, I'll leave it as-is then.

> But at 1MB (N=1048576), it's always exactly zero. So perhaps it grabs
> memory direct from the OS or something, which happens to be zeroed.
>
> The memory pools used by pcm_alloc() in cc64.c are 2MB in size.

Ok, so that's why it works under Windows. I switched to UEFI
(sort of - effectively).

> Still, the code should be fixed and not rely on such accidents. (Perhaps
> allocated memory should be randomised during testing.)

This commit helped find the problem - I got x'99':

https://sourceforge.net/p/pdos/gitcode/ci/9231911169702297ff25b818c40733fcb35d97a3/

BFN. Paul.