APE stddef.h ptrdiff_t should it be 8?

APE stddef.h ptrdiff_t should it be 8?

[Trevor Higgins]

I ran into a fustercluck trying to get tcl to run on amd64.

Turns out it is due to APE ptrdiff_t being defined as long and not long 
long (which it is on *nix).

What is the correct definition of ptrdiff_t on amd64?

I declucked tcl by creating my own typedef. It now runs but I am sure 
tcls memory management routines are foobar and I expect problems with it 
when I actually start using it.


-- 
We need another plan

Re: [9front] APE stddef.h ptrdiff_t should it be 8?

[ori]

> I ran into a fustercluck trying to get tcl to run on amd64.

Can you be more specific? What failed?

> Turns out it is due to APE ptrdiff_t being defined as long and not long 
> long (which it is on *nix).
> 
> What is the correct definition of ptrdiff_t on amd64?

The definition should be a vlong, since that's what subtracting
two pointers yields. If you want to submit a patch, the system
dependent location for this is:

	/$objtype/include/ape/stdint.h

Re: [9front] APE stddef.h ptrdiff_t should it be 8?

[Trevor Higgins]

On 03/08/2020 04:55 AM, ori@eigenstate.org wrote:
>> I ran into a fustercluck trying to get tcl to run on amd64.
just random memory corruptions as would be expected.
>> Can you be more specific? What failed?
>>
>> Turns out it is due to APE ptrdiff_t being defined as long and not long
>> long (which it is on *nix).
>>
>> What is the correct definition of ptrdiff_t on amd64?
> The definition should be a vlong, since that's what subtracting
> two pointers yields. If you want to submit a patch, the system
> dependent location for this is:
>
> 	/$objtype/include/ape/stdint.h
Not wishing to be picky, but vlong would not be APE , correct or not? it 
would be typedef long long.
Also this change still requires patching /sys/include/ape/stddef.h to 
prevent attempt to duplicately define the structure
and Probably for good measure in both files:
#ifndef _PTRDIFF_T
#define _PTRDIFF_T
typedef  <arch specific>  ptrdiff_t   /*** long long in amd64 and just 
long in sys/include/ape/stddef.h **/
#endif

Or maybe , ptrdiff_t should be removed from stddef.h


-- 
We need another plan

Re: [9front] APE stddef.h ptrdiff_t should it be 8?

[jamos]

Others on this list of course know these things
much better than me, but this is my understanding:

The header file, as it is different for different
architectures should be in /$objtype/inlcude/ape _instead_
of in the current location. You'd have to have one
header file each objtype to, but it would probably only
be the definition of ptrdiff_t that would differ,
depending on 32 or 64 bit addresses for the arch.
In Plan 9, include files does in general not include other
include files either.

And it should be 'long long' in ape, as vlong isn't defined.

/Jonas

On 2020-03-08 00:14, Trevor Higgins wrote:

> On 03/08/2020 04:55 AM, ori@eigenstate.org wrote: I ran into a 
> fustercluck trying to get tcl to run on amd64.
  just random memory corruptions as would be expected.

>> Can you be more specific? What failed?
>> 
>> Turns out it is due to APE ptrdiff_t being defined as long and not 
>> long
>> long (which it is on *nix).
>> 
>> What is the correct definition of ptrdiff_t on amd64?
> The definition should be a vlong, since that's what subtracting
> two pointers yields. If you want to submit a patch, the system
> dependent location for this is:
> 
> /$objtype/include/ape/stdint.h
  Not wishing to be picky, but vlong would not be APE , correct or not? 
it would be typedef long long.
Also this change still requires patching /sys/include/ape/stddef.h to 
prevent attempt to duplicately define the structure
and Probably for good measure in both files:
#ifndef _PTRDIFF_T
#define _PTRDIFF_T
typedef  <arch specific>  ptrdiff_t   /*** long long in amd64 and just 
long in sys/include/ape/stddef.h **/
#endif

Or maybe , ptrdiff_t should be removed from stddef.h

Re: [9front] APE stddef.h ptrdiff_t should it be 8?

[ori]

> And it should be 'long long' in ape, as vlong isn't defined.

Hm. Thinking about this a bit more, it's both a standards
question *and* an expectations question, and I'm not sure
that our current definition is wrong.

Our size_t is 32 bits, and so is our ptrdiff_t right now.
That means that ptr1 - ptr2 must fit within 4 gigs. But
in standard C, you can only take differences to pointers
within the same object (or to the end of the object).
Since our size_t is 32 bits, and our malloc takes an
int, our ptrdiff_t should be able to hold this difference.

While our implementation with 32 bit size_t and ptrdiff_t
is unexpected for a lot of software, I think it's standards
compliant.

I'd really like to know what went wrong with tcl, and how
to reproduce it.

Re: [9front] APE stddef.h ptrdiff_t should it be 8? [Tcl memory problem]

[Trevor Higgins]

On 03/09/2020 05:06 PM, ori@eigenstate.org wrote:
>
> I'd really like to know what went wrong with tcl, and how
> to reproduce it.
>
Ok, but you need to understand the tcl code to follow along.
Compile with amd64 and run.  It generates a TclPanic due to a pointer 
passed to the StackFree routine not matching the value expected.
This is caused by the macro OFFSET (inline func actually)  in 
TclExecute.c. In tcl8.6 the macro has been renamed wordskip as this is a 
more appropriate name.

At first glance it looks like the macro computes the number of bytes 
needed to align the address to 16 bytes. But it does not. It should 
return either 1 or 2 but it does so only when the address passed in is 
XXXX0 or XXXX8. When an address of XXXC is used, the macro returns 0. 
Any other low order address bit combinations will also be barf.

The zero value causes the Macro MEMSTART to return the wrong address to 
the application. The offset is needed for space to store a marker that 
is used by the StackFree routine. The application writes all over the 
marker and thus the panic.

The second problem is two fold. In TclExcuteByteCodeObj (or therabouts), 
the code keeps track of how much of the stack memory is in use with two 
values that are ptrdiff_t. Except they are pointers to ptrdiff_t.
When the said routine returns the memory back to the stack allocator it 
uses ptr+1 (for some unknown reason). Because ptrdiff* is int* it adds 
four to the address. It needs to add 8 for the code to work properly. 
Why the +1 I do not know, it looks like a "off by 1 hack" but in all 
other cases, the Value returned to StackFree is the same value given by 
StackAlloc.

The second problem is that memory accesses occur outside the allocated 
memory. Without passing properly aligned memory addresses to this 
routine it cannot be investigated further and may go away once the above 
is corrected.

Why tcl needs  the memory to be 128bit aligned I have not looked into. 
But I suspect it is the way the byte code engine converts word to byte 
addresses. In any case, in running some code with some patches in to 
address the above , I still get memory corruptions and 'suicide traps' 
due to the code accessing memory that is not within the application. IE 
the high word of the pointer has become corrupted.

The machine I am using is second hand and I have not used it much for 
anything else, other than installing Linux on one partition , so it 
could be memory problems with this machine. I verified most of the above 
with QEMU but in qemu with the OFFSET fix applied there are no other 
random memory panics. I suspect it just depends on the value of the 
pointers passed from malloc and that under QEMU I am not seeing the ones 
that cause the problem.


-- 
We need another plan

Re: [9front] APE stddef.h ptrdiff_t should it be 8? [Tcl memory problem]

[ori]

> On 03/09/2020 05:06 PM, ori@eigenstate.org wrote:
>>
>> I'd really like to know what went wrong with tcl, and how
>> to reproduce it.
>>
> Ok, but you need to understand the tcl code to follow along.

I tried to get the tcl code from ports, but it seems like
the port for it is a bit broken. Specifically, there's a
bug in the mkfile pointing at 'unix/tclAppInit.c;, which
should probably be just 'tclAppInit.c'. When I patch that,
I get this error:

/sys/ports/dev-lang/tcl/work/./tommath.h:94[stdin:28460] external redeclaration of: mp_digit
	TYPEDEF ULONG /sys/ports/dev-lang/tcl/work/./tommath.h:94[stdin:28460]
	TYPEDEF UINT /sys/ports/dev-lang/tcl/work/./tcl.h:2186[stdin:1995]
/sys/ports/dev-lang/tcl/work/./tommath.h:117[stdin:28483] syntax error, last name: void

Before I go further: how are you getting tcl, and if you
have patches to it, can you share them?

> At first glance it looks like the macro computes the number of bytes 
> needed to align the address to 16 bytes. But it does not. It should 
> return either 1 or 2 but it does so only when the address passed in is 
> XXXX0 or XXXX8.
> <snip>

Yeah. The code is clearly trying to align to 8 bytes, and the way
that we did it is broken on 64 bits. While C doesn't guarantee
much about pointer alignment, most ABIs do. Patch committed.
 
> The second problem is two fold. In TclExcuteByteCodeObj (or therabouts), 
> the code keeps track of how much of the stack memory is in use with two 
> values that are ptrdiff_t.

I can't find this code -- it looks like you're looking at a different
version of TCL than I am.

> <snip> IE the high word of the pointer has become corrupted.

That also sounds like 32/64 bit issues, where the top of pointers is
getting truncated or filled with junk after passing through a 32 bit
integer conversion.

I *suspect* that they're using ptrdiff_t where they should be using
intptr_t, but I don't have the code you're looking at right now.