freemint / m68k-atari-mint-gcc

Fork of GNU's gcc with support for the m68k-atari-mint target
https://github.com/freemint/m68k-atari-mint-gcc/wiki
Other
27 stars 7 forks source link

Drop leading underscore by default #25

Closed vinriviere closed 1 year ago

vinriviere commented 1 year ago

When designing the new m68k-atari-mintelf toolchain, I configured gcc to use a leading underscore for user-visible assembler symbols. This includes function names and global variables. This was done using #define USER_LABEL_PREFIX "_" in config/m68k/mint.h

Leading underscore was the default with the m68k-atari-mint toolchain. Actually, leading underscore was common for gcc in the a.out era. But leading underscore was dropped on ELF targets.

So using leading underscore on a new ELF target is very nonstandard. I used that voluntarily, because I wanted m68k-atari-mintelf to be an immediate alternative to build any m68k-atari-mint assembler sources. But this was a temporary compromise, as, ultimately (I mean a few years), the mintelf targets should not use the leading underscore, like other ELF ones.

But something changed recently. I didn't notice that @th-otto had added support for non-leading underscore for the MiNTLib and GemLib. And also for the FreeMiNT kernel. On the other hand, EmuTOS explicitly enables -fleading-underscore when compiled with the standard m68k-elf, and it works well for it, as it is a special self-contained case.

So, is there still a good reason to keep leading underscore support ? Finally, I don't think so. What about completely dropping leading underscore support right now in m68k-atari-mintelf toolchain? We have an opportunity to go forward.

th-otto commented 1 year ago

EmuTOS explicitly enables -fleading-underscore when compiled with the standard m68k-elf, and it works well for it, as it is a special self-contained case.

That was needed for m68k-elf because otherwise the symbol names in assembler sources don't match the compiler-generated ones (in both directions).

After applying the patches i posted recently, either compiler would work, whether using underscore or not. But it will require some work to do similar fixes in other projects, mostly depending on how much assembler code they use.

I personally would prefer to drop that leading underscore, as it is surely very non-standard. It might also give problems with some packages that actually contain some optimized 68k assembler sources, like gmp. OTOH, still using an underscore might increase the acceptence of a new toolchain.

Atleast in all our free projects we are now free to choose. For mintlib, gemlib etc. i addded that support a long time ago, for my mixed elf/a.out toolchains (using elf object file format, but still producing the old a.out executable format). For freemint, i've done that a few days ago.

mikrosk commented 1 year ago

I have no objections against removing it either. It's a new toolchain after all.

Just one question: if I understood this correctly, the underscores were used to avoid some clashes with register names? So if we remove the underscore, does it mean I have to see the AT&T syntax when addressing registers?

th-otto commented 1 year ago

If you mean you have to use the % prefix: yes. Because variables like "d0" are now legal user names. But in assembler sources, that can easily be achieved by some defines (see my patches for emutos, in asmdefs.h, or the same thing in freemint). You just have to take look again at any inline asm (but there, the syntax without % was already deprecated long time ago).

vinriviere commented 1 year ago

@th-otto I personally would prefer to drop that leading underscore, as it is surely very non-standard. @mikrosk I have no objections against removing it either. It's a new toolchain after all.

Well, as both of you agree, let's drop that leading underscore. I'm currently patching my toolchain for that, and I will push the patches. Many thanks for @th-otto for having making that possible to happen. I would have bet it would tak more time (years).

Just one question: if I understood this correctly, the underscores were used to avoid some clashes with register names?

As I understand, yes. Various processors have various register names. Without any prefix for external symbols, and without prefix for registers, it would always be possible that a variable name clashes with a register name, either with current cpus or future ones.

So if we remove the underscore, does it mean I have to see the AT&T syntax when addressing registers?

Again, if you mean the % prefix for registers in assembler sources, and %% in C inline assembly, then yes. It would be insane to allow those prefix to be unused by default on a non-underscore target. For projects with many C/asm sources, it would be a big pain to update the toolchain. But there is an easy solution: just compile with -Wa,--register-prefix-optional and it will immediately work. Only important point is to manually take care that all C variables don't use the the same names as registers. Note that some CPU/FPU in the m68k family have some exotic and unknown names, so that might happen.

Another solution is @th-otto's proposition: use a macro in in assembler sources to automatically replace d0 by %d0, and so on. That's also very easy, and you don't have to use any additional compiler option.

th-otto commented 1 year ago

Just compile with -Wa,--register-prefix-optional and it will immediately work. Only important point is to manually take care that all C variables don't use the the same names as registers

That might work in specific projects (in mintlib there were only 2 places with conflicts), but it will not be a general solution. And because the message is issued by the assembler, with the name of a temporary file, it is sometimes not easy to find the place where the variable causing the mismatch originates from.

Note that some CPU/FPU in the m68k family have some exotic and unknown names, so that might happen.

Yes, especially for coldfire (ACR0 etc). Those are currently not redefined in asmdefs.h because they are rarely used.

vinriviere commented 1 year ago

Well, as both of you agree, let's drop that leading underscore. I'm currently patching my toolchain for that, and I will push the patches.

Done. I've pushed my patches to binutils/gcc, and rebuilt my Ubuntu binaries. Cygwin ones will follow. Good bye leading underscore, I won't look back. Many thanks again to @th-otto for having made this possible.

Really, we just need working DWARF-2 exceptions, then our target will be completely up-to-date. Away from obsolescence. Until GCC drops support for m68k or even 32-bit architectures. I hope this won't happen too soon.

BTW, @th-otto, do you also plan to update the CF-Lib for no-underscore? AFAIK, that's the only remaining library required to build the full FreeMiNT source tree. There is absolutely no hurry.

th-otto commented 1 year ago

Are there any problems with cflib? Atleast i don't see any assembler sources there. There are only a few inline asm, but these are already ok.

vinriviere commented 1 year ago

Are there any problems with cflib? Atleast i don't see any assembler sources there. There are only a few inline asm, but these are already ok.

Apologies, cflib is indeed OK. I meant gemma. https://github.com/freemint/gemma/

There are a few assembler sources:

$ find . -name '*.S'
./configtool/startup.S
./kernel32/header.S
./libgemma/startup.S
./src/header.S
./test/startup.S
th-otto commented 1 year ago

That should already be safe, too. It uses symbols.h to redefine the external names. Generally (unless i overlooked something) there should already be support for elf toolchains in all repos here.

But there might be another problem: it also builds a gemma.slb, and currently there is no support yet in the mint kernel to recognize the new format in slb_open(). Of course that should be added, but it may still happen that someone uses the *.slb with an older kernel. Simple solution for this would be to use the stripex tool, for which i already added support. That is already in freemint, because it is used in some usb directories. But AFAIK it is not installed there. Maybe we should move that tool to the mintbin repo?

mikrosk commented 1 year ago

Maybe we should move that tool to the mintbin repo?

Indeed, it's a useful tool.

th-otto commented 1 year ago

It should also be noted that changing that setting causes a bootstrap problem again. You have to configure gcc to build only the c backend, install it, build mintlib & fdlibm with that new compiler and install them, then rebuild gcc with support for c & c++. Of course other libraries like gemlib also have to be rebuild.

I've pushed my patches to binutils/gcc, and rebuilt my Ubuntu binaries

BTW, for -flto to work, you also have to install a symlink (or copy) of the lto plugin of gcc to BFDs plugin directory. In my scripts, i use a versioned name for this purpose (because different compilers produce different LTO format), and my directory looks like this:

$ ls -l bfd-plugins
lrwxrwxrwx liblto_plugin_mintelf.so.10 -> ../../lib64/gcc/m68k-atari-mintelf/10/liblto_plugin.so
lrwxrwxrwx liblto_plugin_mintelf.so.11 -> ../../lib64/gcc/m68k-atari-mintelf/11/liblto_plugin.so
lrwxrwxrwx liblto_plugin_mintelf.so.12 -> ../../lib64/gcc/m68k-atari-mintelf/12/liblto_plugin.so
lrwxrwxrwx liblto_plugin_mintelf.so.13 -> ../../lib64/gcc/m68k-atari-mintelf/13/liblto_plugin.so
lrwxrwxrwx liblto_plugin_mintelf.so.4.6.4 -> ../../lib64/gcc/m68k-atari-mintelf/4.6.4/liblto_plugin.so
lrwxrwxrwx liblto_plugin_mintelf.so.7 -> ../../lib64/gcc/m68k-atari-mintelf/7/liblto_plugin.so
lrwxrwxrwx liblto_plugin_mintelf.so.8 -> ../../lib64/gcc/m68k-atari-mintelf/8/liblto_plugin.so
lrwxrwxrwx liblto_plugin_mintelf.so.9 -> ../../lib64/gcc/m68k-atari-mintelf/9/liblto_plugin.so
vinriviere commented 1 year ago

That should already be safe, too. It uses symbols.h to redefine the external names. Generally (unless i overlooked something) there should already be support for elf toolchains in all repos here.

Ah, fine. I will try to compile gemma with my brand-new non-underscore toolchain.

But there might be another problem: it also builds a gemma.slb, and currently there is no support yet in the mint kernel to recognize the new format in slb_open(). Of course that should be added,

Ah. Yes. We should open a new issue specifically for slb support.

but it may still happen that someone uses the *.slb with an older kernel.

Generally speaking, the answer for that kind of issue is just "update your kernel".

Simple solution for this would be to use the stripex tool, for which i already added support. That is already in freemint, because it is used in some usb directories. But AFAIK it is not installed there. Maybe we should move that tool to the mintbin repo?

In that case, we could say to those people: you want to use new slb? Use a new kernel. You want to keep your old kernel? Then use stripex manually.

Anyway: yes, it would completely make sense to add stripex to mintbin, as it can be useful.

It should also be noted that changing that setting causes a bootstrap problem again. You have to configure gcc to build only the c backend, install it, build mintlib & fdlibm with that new compiler and install them, then rebuild gcc with support for c & c++. Of course other libraries like gemlib also have to be rebuild.

Yes, it has been a real pain to upgrade my Ubuntu PPA because of that issue. As PPA only allows building whole packages from sources, I needed to upload fake mintlib/fdlibm source packages with pre-built binaries in order to be able to build the gcc package. Then I rebuilt normal mintlib/fdlibm packages afterwards.

Fortunately, this situation happen very rarely. Generally keeping an old mintlib/fdlibm package installed is enough to allow building gcc. Even if this isn't clean at all.

BTW, for -flto to work, you also have to install a symlink (or copy) of the lto plugin of gcc to BFDs plugin directory.

Ah, I read something about that symlink, and tried once. IIRC it is only required when calling ld directly. Normally, when the linker is called through the gcc frontend, it automagically works (thanks to the collect2 wrapper?). So I went to the conclusion that the symlink was't a strong requirement. But it can't hurt.

th-otto commented 1 year ago

Ah, fine. I will try to compile gemma with my brand-new non-underscore toolchain.

There were some other issues though, one related to -fno-common, another one from gcc generating a library call to memcpy. Both fixed now.

Anyway: yes, it would completely make sense to add stripex to mintbin, as it can be useful.

Also done now (and bumped version to 0.4). But i have to check other scripts, the workflow here only builds the m68k binaries. For cross-compiling we need the host binaries.

Normally, when the linker is called through the gcc frontend, it automagically works (thanks to the collect2 wrapper?).

IIRC then the linker just goes through all shared libs there and tries them until it finds one that handles the LTO format.

th-otto commented 1 year ago

Generally keeping an old mintlib/fdlibm package installed is enough to allow building gcc. Even if this isn't clean at all.

In this case this does not work. You even have to install mintlib before trying to compile fdlibm, otherwise the configure script bails out because "the compiler cannot create executables".

BTW, the ability to build gdb now, creates another circular dependency, because it is part of binutils, but requires zlib and maybe a few others like zstd.

So if someone wants to build gcc itself, i have updated all required libraries and put again in https://tho-otto.de/snapshots/gdb/

vinriviere commented 1 year ago

That should already be safe, too. It uses symbols.h to redefine the external names. Generally (unless i overlooked something) there should already be support for elf toolchains in all repos here.

Sorry to insist about gemma. But I precisely noticed the file below. I should have mentioned it in the first post. https://github.com/freemint/gemma/blob/3a609887588bebbd367a92343da56aa19f85f949/libgemma/startup.S#L19-L21

That's libgemma/startup.S. The __startup symbol is global and referenced by include/slb/gemma.h. Unless I'm mistaken, symbols.h and C_SYMBOL_NAME(_startup) should be used there.

th-otto commented 1 year ago

Oops, right. Strangely i don't get link errors. Hm.

After a quick look it seems that this symbol is only referenced by thread_fork() and thread_overlay(). That means, by usercode. But then, the choosen name is a bit too general imho.

Anyway, it's fixed now.

BTW, i already fixed a few warnings in my last commits, but one remains: fselect.c:117:17: warning: 'strcat' accessing 257 or more bytes at offsets 3312 and 3056 may overlap 1 byte at offset 3312 [-Wrestrict] 117 | strcat(proc->fsel_outname, proc->fsel_file);

I don't understand why. fsel_outname and fsel_file are different members, why do i get a restrict warning there?

vinriviere commented 1 year ago

I've rebuilt my Cygwin packages: http://vincent.riviere.free.fr/soft/m68k-atari-mintelf/

No more leading underscore in my stuff. Except in the old m68k-atari-mint GCC 4.6.4 which will stay as-is.

Can we close this issue?

th-otto commented 1 year ago

Yes, i think so. Still thinking about that strcat warning, but thats another issue.

mikrosk commented 1 year ago

But there is an easy solution: just compile with -Wa,--register-prefix-optional and it will immediately work.

Actually, this is not so true. This flag is passed to as also when compiling C sources, so then other stuff gets broken, i.e.

/bin/sh ./libtool --mode=compile m68k-atari-mintelf-gcc -O2 -fomit-frame-pointer -m68000 -Wa,--register-prefix-optional  -I./include -D_GNU_SOURCE=1 -Wall -c ./src/video/xbios/SDL_xbios_tveille.c  -o build/SDL_xbios_tveille.lo
libtool: compile:  m68k-atari-mintelf-gcc -O2 -fomit-frame-pointer -m68000 -Wa,--register-prefix-optional -I./include -D_GNU_SOURCE=1 -Wall -c ./src/video/xbios/SDL_xbios_tveille.c -o build/SDL_xbios_tveille.o
/tmp/ccopU5pU.s: Assembler messages:
/tmp/ccopU5pU.s:42: Error: operands mismatch -- statement `move.l %d0,status' ignored
/tmp/ccopU5pU.s:54: Error: syntax error -- statement `move.b status+3,55(%a0)' ignored
/tmp/ccopU5pU.s:59: Error: symbol `status' is already defined
make[1]: *** [build-deps:1412: build/SDL_xbios_tveille.lo] Error 1
make[1]: Leaving directory '/home/mikro/atari/projects/_deps/SDL-1.2-main'

The file itself is very simple:

#include <mint/cookie.h>

#include "SDL_xbios.h"
#include "SDL_xbios_tveille.h"

static tveille_t *cookie_veil = NULL;
static int status;

int SDL_XBIOS_TveillePresent(_THIS)
{
    long dummy;

    cookie_veil = NULL;
    if (Getcookie(C_VeiL, &dummy) == C_FOUND) {
        cookie_veil = (tveille_t *) dummy;
    }

    return (cookie_veil != NULL);
}

void SDL_XBIOS_TveilleDisable(_THIS)
{
    if (cookie_veil) {
        status = cookie_veil->enabled;
        cookie_veil->enabled = 0xff;
    }
}

void SDL_XBIOS_TveilleEnable(_THIS)
{
    if (cookie_veil) {
        cookie_veil->enabled = status;
    }
}

And since assembly files are compiled using gcc as well, one can't pass ASFLAGS='--register-prefix-optional' to ./configure anyway.

I'm starting to think whether we shouldn't revert this change.

th-otto commented 1 year ago

For SDL, i already have a patch: https://github.com/th-otto/rpmint/blob/master/patches/sdl/sdl-1.2.16-asm.patch

vinriviere commented 1 year ago

/tmp/ccopU5pU.s:42: Error: operands mismatch -- statement `move.l %d0,status' ignored

Yes. As I already told, using -Wa,--register-prefix-optional by default along with -fleading-underscore is insane, as C variable names can collide with register names. Particularly, there are many obscure register names supported by gas/m68k. "status" is one of them (even if I have no idea what it is). Using such configuration by default (inside gcc) would be foolish, as it would regularly expose to such issues.

In case of a specific user project, the right solution is to manually remove the underscore to all global labels in assembler sources, once for all. If you prefer, you can use something like the C_SYMBOL_NAME() macro, as Thorsten did in MiNTLib and co., to keep the sources compilable with or without underscore.

In case of your actual issue, there are easy solutions. First, it must be understood that's a collision between variable name and register name. Then choose a strategy, at your convenience:

1) In C files, rename all "status" occurrences to "status2" for example.

2) Or do it transparently by compiling with -Dstatus=status2

Regarding to SDL, this is currently an issue because the SDL library is not provided with the experimental toolchain. But, at least for my own toolchain, as soon as the ABI is stabilized, I will provide the ready-to-use SDL library again, properly patched, and users won't notice anything.

I'm starting to think whether we shouldn't revert this change.

Among all things, removal of that leading underscore is the most controversial change, as it does require source changes for users mixing C and asm (as most Atari hackers do). But switching to an ELF target is a good opportunity to do that (even if my initial plan, now defeated, was "no source change"). Personally, I won't go back for my own toolchain. But as usual, you are free to build your own gcc with the leading underscore, if that fits your needs.

mikrosk commented 1 year ago

Thanks for the detailed answer. I think we should provide a git repository with instructions like this, Thorsten's symbol.h with some examples, it's a very confusing topic.

vinriviere commented 1 year ago

I think we should provide a git repository with instructions like this, Thorsten's symbol.h with some examples, it's a very confusing topic.

Definitely, yes. If we want people to move forward, we have to explain. Issues and solutions. I think that a wiki page would be perfect. Anything else (GitHub repository, website, etc.) would also do the job, provided that people know where to find that information.

mikrosk commented 1 year ago

Just finished my "transition" in various projects I have been using - key is Thorsten's symbol.h, without it I would have sooner or later given up.

So closing, we can go back to the topic of documentation when appropriate.