z88dk / z88dk

The development kit for over a hundred z80 family machines - c compiler, assembler, linker, libraries.
https://www.z88dk.org
Other
921 stars 175 forks source link

(z80asm) do not link with object files built for a different cpu #2320

Closed pauloscustodio closed 1 year ago

suborb commented 1 year ago

It's not a simple problem - I think we probably want compatibility to parents on the tree (so we can link an 8080 library for the ez80_z80 for example). cbundoc is where we consider +z80 to be now

Untitled Diagram

pauloscustodio commented 1 year ago

Yes, that's true. We need also to take into account the -IXIY flag.

pauloscustodio commented 1 year ago

Feature is implemented, checking for regressions.

Defined behaviour:

This allows, in theory, for a library to contain the same object file assembled for different CPUs and -IXIY options. For that we need to make the -x option update the library instead of re-creating it from scratch. But that change may break Makefile rules that depend on the timestamp of the library file, e.g. update an asm file, update it in the library for one CPU; now the library is up-to-date, but it may have inside other object files for the same asm for different CPUs out of date.

What do you think?

suborb commented 1 year ago
--- Building ixiy crt Library ---
[4181](https://github.com/z88dk/z88dk/actions/runs/5708483393/job/15466196318#step:8:4182)

[4182](https://github.com/z88dk/z88dk/actions/runs/5708483393/job/15466196318#step:8:4183)
make --no-print-directory -C z80_crt0s
[4183](https://github.com/z88dk/z88dk/actions/runs/5708483393/job/15466196318#step:8:4184)
make[1]: Nothing to be done for 'all'.
[4184](https://github.com/z88dk/z88dk/actions/runs/5708483393/job/15466196318#step:8:4185)
make --no-print-directory -C math/fix16
[4185](https://github.com/z88dk/z88dk/actions/runs/5708483393/job/15466196318#step:8:4186)
TYPE=ixiy z88dk-z80asm -d -I/home/runner/work/z88dk/z88dk/lib/config//../ -mz80 -IXIY -DSTANDARDESCAPECHARS -x/home/runner/work/z88dk/z88dk/libsrc//z80iy_crt0 @/home/runner/work/z88dk/z88dk/libsrc//../libsrc//z80.lst
[4186](https://github.com/z88dk/z88dk/actions/runs/5708483393/job/15466196318#step:8:4187)
font/font_4x8/font_4x8.lst:1: error: -IXIY incompatible: file font/font_4x8/obj/z80/x/home/runner/work/z88dk/z88dk/libsrc/_DEVELOPMENT/font/font_4x8/_font_4x8_64_minix.o compiled for IX=IX, incompatible with IX=IY
[4187](https://github.com/z88dk/z88dk/actions/runs/5708483393/job/15466196318#step:8:4188)
  ^---- font/font_4x8/obj/**/*.o
[4188](https://github.com/z88dk/z88dk/actions/runs/5708483393/job/15466196318#step:8:4189)
make: *** [Makefile:2085: z80iy_crt0.lib] Error 1

That is annoying since they're just data - I'll update that branch to build those files for multiple CPUs. However I'm surprised it's failing at library creation time since "object file for a different incompatible CPU or with different -IXIY option inside a library is just skipped in the library search, causing undefined symbols further on.". However it might solve the question you raised!

pauloscustodio commented 1 year ago

However I'm surprised it's failing at library creation time

Yes, at library creation time the compilation options used are compared with the object files, and they must match. It is the same as if building a binary.

The skipping is when linking in libraries.

suborb commented 1 year ago

I’m working through the libsrc errors - I’ve found quite a few.

Most issues so far have been caused by ixiy in the case that the index registers aren’t used.

pauloscustodio commented 1 year ago

Ok, thanks.

suborb commented 1 year ago

The libraries should now build, but I need to create a dedicated z180_crt0.lib.

-mz80 is at "cbundoc" in the diagram, even though crt0 code is at "Documented"

pauloscustodio commented 1 year ago

Does it make sense to create a new cpu z80_strict with just the documented opcodes?

suborb commented 1 year ago

Yes, that would be wonderful!

pauloscustodio commented 1 year ago

Committed to the issue_2320 branch the new CPU z80_strict, and the usage of one single z88dk-z80asm.lib support library instead of one per cpu-ixiy combination.

Github workflow is failing for Ubuntu:

zcc +zx -vn ../graphics/spirograph.c -lndos --math32 -create-app -ospirograph.bin
../graphics/spirograph.c::x::0::0:25: error: undefined symbol: l_f32_sub
  ^---- l_f32_sub
../graphics/spirograph.c::x::0::0:25: error: undefined symbol: cos_fastcall
  ^---- cos_fastcall
../graphics/spirograph.c::x::0::0:25: error: undefined symbol: l_f32_mul
  ^---- l_f32_mul
suborb commented 1 year ago

That one is an easy one to solve - bad naming of the z80n library.

Just want to check one thing (relevant for the maths libraries). If I compile to .o the files needed for the various CPUs and then create a library with -m* it will include all files into a single library file?

If that library is created from the following tree:

obj/z80
obj/z80n
obj/z180
obj/ez80_z80

With a glob wildcard obj/**/*.o will z80asm search for the best CPU match rather than the first CPU match? Eg suppose the z80 files were first in the library and I was linking for ez80_z80 would it preferentially use ez80_z80 then z180 then z80 object files?

Related question - if it does do this, is there an effect on linking time? (In the past we had very slow linking on windows for example).

pauloscustodio commented 1 year ago

With a glob wildcard obj/*/.o will z80asm search for the best CPU match rather than the first CPU match?

Yes, it will get the best CPU match, because the object files are written to the library file from more specific to more generic CPU order. The actual order is fixed in the code: z80n, z80, ez80_z80, z180, z80_strict, 8085, r3k, gbz80, ez80, r2ka and 8080

is there an effect on linking time?

Yes, there will be some impact, as more object files are skipped while searching for symbols. But as the whole library is slurped into memory, the search is memory-only and should be fast. (this slurping into memory was the solution to the windows slow liking times)

To lessen this problem I am currently working on adding a string table to the end of each object file, removing repetitions (e.g. file name) and making the object file smaller. I am also considering adding a hash code to each symbol to make the lookup faster. This is all happening in the issue_2320 branch, so version 18 of the object files is not stable. (the master branch has version 17)

suborb commented 1 year ago

Okay, that's cool - I'll create fat maths libraries once the branch is merged.

Branch is failing on z80asm tests now - so I’ll hand it back to you @pauloscustodio

pauloscustodio commented 1 year ago

Fixed the problems with the tests, now we are getting some undefined library symbols (error_zc, _font_8x8_rom, l_ret)... back to you, @suborb, I think

suborb commented 1 year ago

This is going to hurt. It's a problem with sdcc_iy where the library is compiled with -IXIY but user code isn't.

I can't remember what sdcc_iy is meant to be, sdcc uses ix as the frame register, is the library intended to use iy internally to avoid having to save/restore ix? Help @feilipu ?

feilipu commented 1 year ago

AFAIK, the sdcc_iy library does just that. It uses iy internally. So that ix is used solely by the compiler.

But, I've entirely avoided the index registers in my work so can't give examples.

suborb commented 1 year ago

Thanks for the confirmation. In which case it's an outlier case that I'm not certain how to fix without a lot of pain.

To solve it we need to introduce a new z80asm define INDEX=IX/IY, remove the assembly with -IXIY and then replace all uses of the index registers with that define. Taking special care around __SDCC_IY which deliberately reverses the semantics so -IXIY gets the right index register for ROM calls.

dom@dom-macmini _DEVELOPMENT % find . -type f -name '*.asm' |xargs grep ix | wc
   11766   52660  926212
dom@dom-macmini _DEVELOPMENT % find . -type f -name '*.asm' |xargs grep iy | wc
    2899   12705  234727

Any alternate ideas very welcome!

pauloscustodio commented 1 year ago

There is already a define __SWAP_IXIY__ defined if -IXIY is passed.

AFK at the moment.

A sexta, 4/08/2023, 14:23, suborb @.***> escreveu:

Thanks for the confirmation. In which case it's an outlier case that I'm not certain how to fix without a lot of pain.

To solve it we need to introduce a new z80asm define INDEX=IX/IY, remove the assembly with -IXIY and then replace all uses of the index registers with that define. Taking special care around __SDCC_IY which deliberately reverses the semantics so -IXIY gets the right index register for ROM calls.

@. _DEVELOPMENT % find . -type f -name '.asm' |xargs grep ix | wc 11766 52660 926212 **@.** _DEVELOPMENT % find . -type f -name '.asm' |xargs grep iy | wc 2899 12705 234727

Any alternate ideas very welcome!

— Reply to this email directly, view it on GitHub https://github.com/z88dk/z88dk/issues/2320#issuecomment-1665603079, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAARI5L5UQ4YQEVQMJR6RHLXTTZWLANCNFSM6AAAAAAZNIMJWA . You are receiving this because you were mentioned.Message ID: @.***>

suborb commented 1 year ago

I think what's actually needed is a "soft -IXIY" flag - that does the reversal but doesn't affect the output CPU in the object file. Changing and checking almost 15000 ixy uses is going to lead to bugs.

pauloscustodio commented 1 year ago

I think what's actually needed is a "soft -IXIY" flag

Agree, swap but write object file as not swapped. Any suggestion on the name? -IXIY-soft?

suborb commented 1 year ago

Maybe: -IXIY does the swap by itself, but -mz80_ixiy, -mz180_ixiy etc do the swap and the CPU type?

I've got -mz80_ixiy in zcc already to handle library selection.

pauloscustodio commented 1 year ago

I've just committed f03a1bb87b69fbc026b0bca80917eb51ac4e2927 with the -IXIY-soft option (sorry, did not see your answer before).

Option -IXIY-soft -m -x... creates library only with -IXIY-soft option. Option -m -x... creates library with no swapping and -IXIY. On library search, -IXIY-soft is considered equivalent to no option.

Does this work for zcc?

suborb commented 1 year ago

I think there's a string/memory problem in the branch - the linux build fails, but macOS passes. However, I'm not convinced it should. From my local environment on option_debug.t:

__zx_32col_udgs                 = $8792 ; addr, public, ,  o<8B>^CK<80>, data_clib, console_vars.asm:20
__zx_64col_font                 = $8790 ; addr, public, ,  o<8B>^CK<80>, data_clib, console_vars.asm:18
__zx_print_routine              = $8794 ; addr, public, ,  o<8B>^CK<80>, data_clib, console_vars.asm:23

So there's some use-after-free? going on.

pauloscustodio commented 1 year ago

Correct, a use-after-free bug. Only detected on Linux, because Linux invalidates the allocated memory on free().

suborb commented 1 year ago

With a glob wildcard obj/*/.o will z80asm search for the best CPU match rather than the first CPU match?

Yes, it will get the best CPU match, because the object files are written to the library file from more specific to more generic CPU order. The actual order is fixed in the code: z80n, z80, ez80_z80, z180, z80_strict, 8085, r3k, gbz80, ez80, r2ka and 8080

I was trying to create multi-arch maths library, but I don't think this is possible:

  1. Compile C/.asm files to .o files in obj/[cpu_type]/*.o
  2. Create a library:
z88dk-z80asm -xgenmath -m"*" "obj/**/*.o"
error: CPU incompatible: file obj/ez80_z80/acos.o compiled for ez80_z80, incompatible with z80
pauloscustodio commented 1 year ago

The -m* needs .asm files and assembles them for each CPU and IXIY combination and packs them together in the library.

For the use case you describe, I think we need to let -x allow any object file type inside the library. I can remove the check for object file CPU at library creation.

But libraries are still searched sequentially, which means that "obj/*/.o" will need to be expanded in the correct CPU search order, i.e. obj/z80n/.o obj/z80/.o ...

suborb commented 1 year ago

The check is good - it’s easy to make a mistake: I did find a lot of them during this work.

However it would be good to be able to create a library without those checks being present.

pauloscustodio commented 1 year ago

There's something wrong and I give up for today...

pauloscustodio commented 1 year ago

Fixed -m* to allow your use case.

With -m*:

This way you can type z88dk-z80asm -xgenmath -m"*" "obj/**/*.o" and it should just work. The option -v tells each object file as it is loaded into the library.

Let me know if this suits the needs.

pauloscustodio commented 1 year ago

@suborb, there are now a couple of undefined symbols. Could you have a look? stdio/printf_number.asm:89: error: undefined symbol: l_int2long_s stdio/__printf_number.asm:107: error: undefined symbol: l_long_neg stdio/printf_number.asm:203: error: undefined symbol: l_long_div_u stdio/__printf_print_aligned.asm:25: error: undefined symbol: l_ge stdio/fputc_callee.c::wrapper_fputc_callee_z80::0::1:57: error: undefined symbol: l_jphl

suborb commented 1 year ago

Interesting, the l_ functions are from z80_crt0.lib - not sure how it's ended up with the wrong CPU since it was working beforehand.

Ah, I have z80_crt0.lib compiling with -mz80_strict - maybe that's it

suborb commented 1 year ago

Ah, I have z80_crt0.lib compiling with -mz80_strict - maybe that's it

That is the problem: z80crt0.lib is partially assembled with -mz80 but the l routines are assembled with -mz80_strict.

When creating the library, we use -mz80 and it seems that there's been a change which causes the -mz80_strict modules to be excluded from the library.

So, it looks like the library -m* changed broke something?

pauloscustodio commented 1 year ago

I solved a merge conflct between zcc -mz80 -mz80_strict. Maybe i selected the wrong line?

suborb commented 1 year ago

I solved a merge conflct between zcc -mz80 -mz80_strict. Maybe i selected the wrong line?

The bit of the library building is all done by invoking z80asm. Simple example:

a.asm

SECTION code2

a:
        ld      hl,16384

b.asm:

MODULE b
SECTION code

b:
        ld      hl,32768

Create library:

z88dk-z80asm -mz80 a.asm
z88dk-z80asm -mz80_strict b.asm
z88dk-z80asm -xlib -mz80  *.o
z88dk-z80nm lib.lib
Library file lib.lib at $0000: Z80LMF18
Object  file lib.lib at $0014: Z80RMF18
  Name: a
  CPU:  z80
  Section "": 0 bytes
  Section code2: 3 bytes
  Symbols:

%
suborb commented 1 year ago

I'm guessing it previously worked as a side-effect, so I'll change it back to z80. However, a log would be useful!

suborb commented 1 year ago

Fixed -m* to allow your use case.

I've run into a different issue now:

genmath.lst:3: error: file open: obj/ixiy/c/sccz80/sgn.o
  ^---- obj/ixiy/**/*.o
obj/ixiy/c/sccz80/sgn.o: Too many open files
libc++abi: terminating due to uncaught exception of type std::__1::__fs::filesystem::filesystem_error: filesystem error: in directory_iterator::directory_iterator(...): Too many open files ["obj/ez80_z80"]

% find obj -name '*.o' | wc
     359     359    9002
% ulimit -a
-t: cpu time (seconds)              unlimited
-f: file size (blocks)              unlimited
-d: data seg size (kbytes)          unlimited
-s: stack size (kbytes)             8176
-c: core file size (blocks)         0
-v: address space (kbytes)          unlimited
-l: locked-in-memory size (kbytes)  unlimited
-u: processes                       1333
-n: file descriptors                256

genmath.lst:

obj/z80/**/*.o
obj/z80n/**/*.o
obj/ixiy/**/*.o
obj/ez80_z80/**/*.o

Normally the libraries can have 1000s of files - is -m* keeping files open for some reason?

suborb commented 1 year ago

Another problem, this time with libsrc/fcntl/dummy which builds ndos.lib. It's built with -m*.

Rebuilding the library (with .o files alongside .asm files) results in only 8080 (IXIY) ending up in the final library. Doing a make clean builds it correctly.

This is a regression on the behaviour from a couple of days ago.

suborb commented 1 year ago

I'm guessing it previously worked as a side-effect, so I'll change it back to z80. However, a log would be useful!

Actually, this is turning out to be a bigger issue, eg the m100 is an 8085 machine, but m100_clib.lib previously used to contain a mix of 8080 and 8085 code (rightly or wrongly), with the recent change it just contains 8085 routines.

This also affects +zxn which is mostly z80 code, and only smattering of z80n

pauloscustodio commented 1 year ago

Let me check each of these in turn...

pauloscustodio commented 1 year ago

Interesting, the l_ functions are from z80_crt0.lib - not sure how it's ended up with the wrong CPU since it was working beforehand.

Ah, I have z80_crt0.lib compiling with -mz80_strict - maybe that's it

Was a bug in building libraries; now -x without -m* accepts all objects given as input and packs them together.

Correction: -x without -m* accepts all objects given as input for compatible CPU and packs them together.

pauloscustodio commented 1 year ago

Fixed -m* to allow your use case.

I've run into a different issue now:

genmath.lst:3: error: file open: obj/ixiy/c/sccz80/sgn.o
  ^---- obj/ixiy/**/*.o
obj/ixiy/c/sccz80/sgn.o: Too many open files
libc++abi: terminating due to uncaught exception of type std::__1::__fs::filesystem::filesystem_error: filesystem error: in directory_iterator::directory_iterator(...): Too many open files ["obj/ez80_z80"]

% find obj -name '*.o' | wc
     359     359    9002
% ulimit -a
-t: cpu time (seconds)              unlimited
-f: file size (blocks)              unlimited
-d: data seg size (kbytes)          unlimited
-s: stack size (kbytes)             8176
-c: core file size (blocks)         0
-v: address space (kbytes)          unlimited
-l: locked-in-memory size (kbytes)  unlimited
-u: processes                       1333
-n: file descriptors                256

genmath.lst:

obj/z80/**/*.o
obj/z80n/**/*.o
obj/ixiy/**/*.o
obj/ez80_z80/**/*.o

Normally the libraries can have 1000s of files - is -m* keeping files open for some reason?

Filehandle leak while checking for the correct object file version and bailing out without closing file. fixed.

pauloscustodio commented 1 year ago

I solved a merge conflct between zcc -mz80 -mz80_strict. Maybe i selected the wrong line?

The bit of the library building is all done by invoking z80asm. Simple example:

a.asm

SECTION code2

a:
        ld      hl,16384

b.asm:

MODULE b
SECTION code

b:
        ld      hl,32768

Create library:

z88dk-z80asm -mz80 a.asm
z88dk-z80asm -mz80_strict b.asm
z88dk-z80asm -xlib -mz80  *.o
z88dk-z80nm lib.lib
Library file lib.lib at $0000: Z80LMF18
Object  file lib.lib at $0014: Z80RMF18
  Name: a
  CPU:  z80
  Section "": 0 bytes
  Section code2: 3 bytes
  Symbols:

%

Fixed.

pauloscustodio commented 1 year ago

It's getting late... tomorrow evening I'll have a go with the next issues.

pauloscustodio commented 1 year ago

Another problem, this time with libsrc/fcntl/dummy which builds ndos.lib. It's built with -m*.

Rebuilding the library (with .o files alongside .asm files) results in only 8080 (IXIY) ending up in the final library. Doing a make clean builds it correctly.

This is a regression on the behaviour from a couple of days ago.

The current behaviour is the following:

This is clearly not what is wanted, and is a consequence of z80asm trying to be clever in selecting either an asm file or an object file depending on what exists and on the -d flag. I think that it should only handle the files passed to it and leave the cleverness to the Makefiles. But these checks are there since before my time maintaining it and I did not want to break stuff.

@suborb, I would like to hear your view.

pauloscustodio commented 1 year ago

The tests are now passing, except for the msbuild ones that depend on the nightly built, that still has the wrong libraries.

Did you manage to create the fat libraries? Shall we wait for that before merging?

suborb commented 1 year ago

I took a night off, so I've not done the fat library work - I can do it later.

Regarding -d - I think for m* date checking ought not to be done.

Given that the check is only for date and not for CPU type then it's actually quite "dangerous" for z80asm to have this behaviour so it probably ought to be at least disabled by default if not removed.

pauloscustodio commented 1 year ago

Cleanup of command line parsing done: -d and -m* together are now forbidden, and the assembler always uses either the .o or the .asm files supplied to it; -d is the only exception where either a .o or .asm may be supplied and z80asm selects the .o if up-to-date, or else the .asm.

Now back to you, @suborb:

TYPE=z80 z88dk-z80asm -d -I/Users/runner/work/z88dk/z88dk/lib/config//../ -mz80 -DSTANDARDESCAPECHARS -x/Users/runner/work/z88dk/z88dk/libsrc//z80_crt0 @/Users/runner/work/z88dk/z88dk/libsrc//../libsrc//z80.lst
/Users/runner/work/z88dk/z88dk/libsrc//../libsrc//z80.lst:13: error: CPU incompatible: file stdio/ansi/ansifont_f3.o compiled for z80n, incompatible with z80
suborb commented 1 year ago

Thanks @pauloscustodio - There's a few cases of this so may take me a while to get through them all.

suborb commented 1 year ago

It was fairly painful, but all done - that diagnostic/error was just what was needed - the incorrect files fell into a few camps:

So I'd say there was no actual negative effect of these mistakes.