adtools / amigaos-cross-toolchain

AmigaOS cross compiler for Linux / MacOSX / Windows
184 stars 48 forks source link

gcc-2.95.3: baserel compiling doesn't put const data into .text section #31

Closed jens-maus closed 8 years ago

jens-maus commented 8 years ago

When using baserel compiling options like -fbaserel32 the GCC 2.95.3 m68k-amigaos cross compiler is not putting const data into the .text section like this is done for the non-baserel compiling case.

However, especially when using baserel compiling for creating a shared library that contains global data (e.g. like AmiSSL) results into some trouble. While other baserel-aware compiler suite for e.g. OS4 put const data correctly into a read-only or .text section, the current GCC 2.95.3 baserel support doesn't do that and thus results into some severe problems when using it for baserel compiling.

One example where this is causing problems is documented here:

https://github.com/jens-maus/amissl/issues/1

There is also a short example source code added demonstrating the problem.

cahirwpz commented 8 years ago

Thank you for detailed bug report. I'm not yet convinced if its gcc or linker script bug. I'll investigate that.

jens-maus commented 8 years ago

Without me knowing the complete internal of GCC I think this is a shortcoming of the baserel support in GCC in general. It might actually boil down to modifying the gcc/config/m68k/amigaos.c file and actually a proper modification of the function defining which symbols to put in the .text section. See here:

https://github.com/cahirwpz/amigaos-cross-toolchain/blob/master/patches/gcc-2.95.3/gcc/config/m68k/amigaos.c#L49-L66

Without knowing the deep internals of GCC I don't dare to modify these functions as this might have other effects I cannot tell. Hopefully you have a bit more experience in that and can actually come up with a nice solution that will allow to fix baserel support for variables defined as "const".

cahirwpz commented 8 years ago

Hmm... I think there's more to that than we initially thought. Consider following code:

int k = 10;
const struct { int a, int *b } s = { 20, &k };

Though s is constant it cannot be put into .text section with -fbaserel mode. That's because it generates relocation (&k) which would change .text section at load-time. That violates requirements for AmigaOS pure executables and cannot be allowed by compiler. So to implement it correctly one should recursively analyse each declaration and check for references.

cahirwpz commented 8 years ago

After the fix it seems to work as expected:

# m68k-amigaos-gcc -mcrt=clib2 -m68020-60 -o issue-31 issue-31.c
# m68k-amigaos-objdump --headers issue-31

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 .text         00004dec  00000000  00000000  00000028  2**2
                  CONTENTS, ALLOC, LOAD, RELOC, CODE
  1 .data         00000210  00000000  00000000  00005f70  2**2
                  CONTENTS, ALLOC, LOAD, RELOC, DATA
  2 .bss          000001b8  00000000  00000000  00006764  2**2
                  ALLOC
# m68k-amigaos-gcc -mcrt=clib2 -m68020-60 -fbaserel32 -o issue-31 issue-31.c
# m68k-amigaos-objdump --headers issue-31

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 .text         00004dcc  00000000  00000000  00000024  2**2
                  CONTENTS, ALLOC, LOAD, RELOC, CODE
  1 .data         00000500  00000000  00000000  00005cf8  2**2
                  CONTENTS, ALLOC, LOAD, RELOC, DATA

... for following source file:

#include <stdio.h>

static const int my_array[1000] = {0};

int main(void)
{
    printf("arr is %d\n", my_array[0]);
    return 0;
}
jens-maus commented 7 years ago

@cahirwpz Thanks for working on better baserel support in gcc 2.95.3. I have verified your observations here with my own test case and could verify that gcc 2.95.3 is finally putting const data into the text section now when using -fbaserel. Great.

However, while AmiSSLv4 (https://github.com/jens-maus/amissl) compiles fine with your modified gcc 2.95.3 version, it seems to break baserel support in AmiSSLv4 somehow. I haven't investigated deeply yet, but here my test binaries crash my UAE environment already when I try to do a simple OpenLibrary("amisslmaster.library", 0); on one of the main AmiSSL libraries. Reverting back to my older gcc 2.95.3 version without your improved const/baserel support it seems to work fine. So could you please take a look at that yourself?

It should be quite straight forward to compile AmiSSLv4 if you have a running gcc 2.95.3 cross compiler. Just perform a "make OS=os3" on the AmiSSLv4 sources and wait until everything compiles/links. then try to execute the "amisslmaster_test" binary the build environment will put into the "build_os3" top-level directory.

Thanks in advance.

cahirwpz commented 7 years ago

@jens-maus I'm trying to switch focus to my other, and neglected, projects now. I'll be back when @cnvogelg fixes vamos to run libnix linked binaries, so I can run DejaGNU tests. I feel like walking on a thin ice without regression testing for GCC. Could you try to narrow down the issue during that time? Oh... and please create another github issue for baserel problems.

jens-maus commented 7 years ago

Ok, however I was hoping you could shortly invest some time in trying to get my issues reproduced and perhaps helping me a little in potentially fixing something in The AmiSSL library init stuff that may be the reason for the crashes now that const data has been moved to the .text section. I would really appreciate some help in this regard. Beside, I will make sure to create a new ticket for these baserel issues if baserel is really the reason.

cahirwpz commented 7 years ago

Please send me both binaries - working and broken – I'll compare them and try to figure out what's wrong. Try to minimize differences in the build environment - i.e. share between toolchains as much as possible (gcc, clib2, etc.). Hopefully you'll narrow down the issue to binutils.