m68k: Elf2Mac failing on computed gotos

[ftrvxmtrx]

The minimal repro I came up with, along with a workaround (putting the table into a .text section):

// m68k-apple-macos-gcc THIS_FILE.c
int main(){
        //__attribute__((section(".text.cursed")))
        static const void * const m[] = {&&a};
        goto *m[0];
a:      return 0;
}

Without the attribute I get the following:

Invalid ref from .data:20 to .code00002()+10240
Assertion failed: sym.section.get() == this (/home/ftrvx/w/_/retro68/Elf2Mac/Section.cc: FixRelocs: 238)
collect2: fatal error: ld terminated with signal 6 [Aborted]
compilation terminated.

[ftrvxmtrx]

A much bigger example continuously failed to link for m68k with -Wl,--gc-sections unless I would either disable computed gotos, or used the hack above. Another solution was to use -Wl,--mac-single, which ofc results in a somewhat bigger final executable.

[autc04]

Confirmed.

So what we're asking for here is to have pointers to code stored in the data section, at static initialization time.
CODE resources are loaded on demand, so pointers to code are just not possible at this moment.

For function pointers, the runtime system solves this by providing jump table entry in the data section (actually, the CODE 0 resource, but that gets loaded right next to DATA, and both can be addressed relative to processor register A5).

Now it should be possible to just allocate jump table entries for each global label referenced like that, but going through a jump table entry requires an extra jump, so it is slower than a regular computed goto. And given that computed gotos are mostly used for performance, this might not even be worth the effort, and one should just put the tables into the text section instead.

Opinions?

[ftrvxmtrx]

I agree, @autc04 - to me it makes very little sense to have computed goto table anywhere but in the same section as the code that uses it. I am not even sure what'd be the reason to do otherwise.

[autc04]

I wonder if there's a way to get the compiler to enforce or automate this, so that we don't need a platform-specific #ifdef for this...

[ftrvxmtrx]

Perhaps this is relevant?

Target Hook: section * TARGET_ASM_SELECT_SECTION (tree exp, int reloc, unsigned HOST_WIDE_INT align)
    Return the section into which exp should be placed.
You can assume that exp is either a VAR_DECL node or a
constant of some sort. reloc indicates whether the initial
value of exp requires link-time relocations. Bit 0 is set
when variable contains local relocations only, while bit 1
is set for global relocations. align is the constant
alignment in bits.

    The default version of this function takes care of
putting read-only variables in readonly_data_section.

https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gccint/Sections.html#index-TARGET_005fASM_005fSELECT_005fSECTION

My understanding is that for retro68-specific target this could (maybe) used to fit the computed gotos in the same function section somehow.

[ftrvxmtrx]

Correction: this must be TARGET_ASM_UNIQUE_SECTION for cases where -ffunction-sections is used. Added a debug_tree and this is about what I get with computed gotos:

 <var_decl 0x7f7ab8104090 m
    type <array_type 0x7f7ab80f3738
        type <pointer_type 0x7f7ab80f3498 type <void_type 0x7f7ab80161f8 void>
            readonly sizes-gimplified unsigned SI
            size <integer_cst 0x7f7ab80025a0 constant 32>
            unit-size <integer_cst 0x7f7ab80025b8 constant 4>
            align:16 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7f7ab80f3498>
        sizes-gimplified DI
        size <integer_cst 0x7f7ab8002600 constant 64>
        unit-size <integer_cst 0x7f7ab8002618 constant 8>
        align:16 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7f7ab80f3738
        domain <integer_type 0x7f7ab80f3690 type <integer_type 0x7f7ab800f000 sizetype>
            sizes-gimplified SI size <integer_cst 0x7f7ab80025a0 32> unit-size <integer_cst 0x7f7ab80025b8 4>
            align:16 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7f7ab80f3690 precision:32 min <integer_cst 0x7f7ab80025d0 0> max <integer_cst 0x7f7ab80026f0 1>>>
    readonly addressable asm_written used static read DI 1.c:3:35 size <integer_cst 0x7f7ab8002600 64> unit-size <integer_cst 0x7f7ab8002618 8>
    align:16 warn_if_not_align:0 context <function_decl 0x7f7ab80ea200 main> initial <constructor 0x7f7ab80f61c8>
    (mem/u/c:DI (symbol_ref:SI ("m.0") [flags 0x2] <var_decl 0x7f7ab8104090 m>) [2 m+0 S8 A16]) chain <label_decl 0x7f7ab85f8100 a>>

Looks like checking whether it's an array to labels is not too hard, and then it's a matter of returning a custom section for this specific declaration.

[ftrvxmtrx]

I did a quick test with set_decl_section_name(decl, s);, s being what I'd put in the __attribute__((section(..., and indeed it linked my project without the attribute. It also runs correctly. I can probably get out a patch at some point later, unless somebody beats me to it.