Fix system-dependent code in src/gc/system.c

[whiteknight]

See also #271 for some background about this problem on Sparc64/OpenBSD.

src/gc/system.c contains a number of system-specific routines for reading pointer values out of processor registers and storing them onto the stack for a later stackwalk. There are some system-dependent routines in this file separated by preprocessor flags, each working a little bit differently.

We need to clean this section up. It needs to use the configure system to generate platform-specific code, and definitely needs to avoid hand-assembled machine instructions.

[jkeenan]

Replying to whiteknight:

For reference, here is (by my reading) the relevant section of src/gc/system.c:

void
trace_system_areas(PARROT_INTERP)
{
    ASSERT_ARGS(trace_system_areas)
    {
#if defined(__sparc)
        /* Flush the register windows. For sparc systems, we use hand-coded
           assembly language to create a small function that flushes the
           register windows. Store the code in a union with a double to
           ensure proper memory alignment. */
        /* TT #271: This needs to be fixed in a variety of ways */
        static union {
            unsigned int insns[4];
            double align_hack[2];
        } u = { {
#  ifdef __sparcv9
                            0x81580000, /* flushw */
#  else
                            0x91d02003, /* ta ST_FLUSH_WINDOWS */
#  endif
                            0x81c3e008, /* retl */
                            0x01000000  /* nop */
        } };

        /* Turn the array of machine code values above into a function pointer.
           Call the new function pointer to flush the register windows. */
        static void (*fn_ptr)(void) = (void (*)(void))&u.align_hack[0];
        fn_ptr();

#elif defined(__ia64__)


#  if defined(__hpux)
        ucontext_t ucp;
        void *current_regstore_top;

        getcontext(&ucp);
        _Asm_flushrs();

#    if defined(_LP64)
        current_regstore_top = (void*)(uint64_t)_Asm_mov_from_ar(_AREG_BSP);
#    else
        current_regstore_top = (void*)(uint32_t)_Asm_mov_from_ar(_AREG_BSP);
#    endif

        size_t base = 0;
        struct pst_vm_status buf;
        int i = 0;

        while (pstat_getprocvm(&buf, sizeof (buf), 0, i++) == 1) {
            if (buf.pst_type == PS_RSESTACK) {
                base = (size_t)buf.pst_vaddr;
                break;
            }
        }

#  else /* !__hpux */
        /* On IA64 Linux systems, we use the function getcontext() to get the
           current processor context. This function is located in <ucontext.h>,
           included above. */
        struct ucontext ucp;
        void *current_regstore_top;

        /* Trace the memory block for the register backing stack, which
           is separate from the normal system stack. The register backing
           stack starts at memory address 0x80000FFF80000000 and ends at
           current_regstore_top. */
        size_t base = 0x80000fff80000000;

        getcontext(&ucp);

        /* flush_reg_store() is defined in config/gen/platforms/ia64/asm.s.
           it calls the flushrs opcode to perform the register flush, and
           returns the address of the register backing stack. */
        current_regstore_top = flush_reg_store();

#  endif /* __hpux */

        trace_mem_block(interp, base,
                (size_t)current_regstore_top);

#else /* !__ia64__ */

#  ifdef PARROT_HAS_HEADER_SETJMP
        /* A jump buffer that is used to store the current processor context.
           local variables like this are created on the stack. */
        Parrot_jump_buff env;

        /* Zero the Parrot_jump_buff, otherwise you will trace stale objects.
           Plus, optimizing compilers won't be so quick to optimize the data
           away if we're passing pointers around. */
        memset(&env, 0, sizeof (env));

        /* this should put registers in env, which then get marked in
         * trace_system_stack below
         */
        setjmp(env);
#  endif

#endif /* __ia64__ */
    }

    /* With the processor context accounted for above, we can trace the
       system stack here. */
    trace_system_stack(interp);
}

Can anyone offer suggestions as to how this could be moved into the configuration system?

Thank you very much.

kid51

[whiteknight]

see config/gen/platform/ia64/asm.s for an example of a platform-specific assembly-language file that's included in the build conditional on the configuration. Does that mechanism extend to other platforms as well?

[jkeenan]

Replying to whiteknight:

see config/gen/platform/ia64/asm.s for an example of a platform-specific assembly-language file that's included in the build conditional on the configuration. Does that mechanism extend to other platforms as well?

Since the code we want to refactor is C code, I wonder if a better model is something like config/gen/platform/darwin/memalign.c?

[jkeenan]

Replying to jkeenan:

Replying to whiteknight:

see config/gen/platform/ia64/asm.s for an example of a platform-specific assembly-language file that's included in the build conditional on the configuration. Does that mechanism extend to other platforms as well?

Since the code we want to refactor is C code, I wonder if a better model is something like config/gen/platform/darwin/memalign.c?

whiteknight: Any update?

Thanks.

kid51

[whiteknight]

This code has not been cleaned up significantly as I had suggested. I am going to drop the priority/severity of this ticket since we are obviously doing fine without implementing these changes. We will start to see a little bit more pain when we want to add new systems and architectures to the list of Parrot's supported platforms, but we can deal with the issue then.

[whiteknight]

This is a nice cleanup task, but unless somebody is very keen to tackle it I don't think this ticket is worth pursuing. I'm closing this ticket, but I'll keep some comments and TODO notes in the code to help direct future hackers who are interested in that code.