dciabrin / ngdevkit

Open source development for Neo-Geo
GNU Lesser General Public License v3.0
262 stars 26 forks source link

Failing to swap bytes of p1 file: "cannot reverse bytes: length of section .padding must be evenly divisible by 2" #17

Closed city41 closed 4 years ago

city41 commented 5 years ago

I have been working on a game using this dev kit for a while now. I am now getting this error

/home/matt/dev/ngdevkit/local/bin/m68k-neogeo-elf-objcopy: cannot reverse bytes: length of section .padding must be evenly divisible by 2
Makefile:61: recipe for target 'prom' failed

When I dump the rom.elf file, sure enough the .padding section has an odd number of bytes

Contents of section .padding:
 0468f ffffffff ffffffff ffffffff ffffffff  ................
 0469f ffffffff ffffffff ffffffff ffffffff  ................
 046af ffffffff ffffffff ffffffff ffffffff  ................

... many bytes skipped ...

 7ffdf ffffffff ffffffff ffffffff ffffffff  ................
 7ffef ffffffff ffffffff ffffffff ffffffff  ................
 7ffff ff                                   .               
Contents of section .comment:
 0000 4743433a 2028474e 55292035 2e352e30  GCC: (GNU) 5.5.0
 0010 00                                   .               
Contents of section .debug_info:
 0000 000000ad 00040000 00000401 00000045  ...............E

I have found telling objcopy to strip the padding section seems to get past this and work on gngeo. But it completely does not work at all on NeoSD. I also encounter some bugs in gngeo which feel like might be due to not having .padding in the rom.

This is totally blocking me, so I'm researching and experimenting to try and find a fix. Figured I'd post here too in case anyone else has hit this.

I also tried swapping the bytes using tools that aren't elf aware, they just swap all bytes in the file. That has not gone well :)

city41 commented 5 years ago

I am having some luck with

prom: $(ELF) rom
    $(OBJCOPY) -O binary -S -R .comment $< rom.stripped
    (cd ../swaptool && ts-node index.ts -i ../src/rom.stripped -d ../src/rom/202-p1.p1)

Where swaptool is just a dumb program I wrote that swaps byte pairs. Basically telling objcopy to strip out all the unneeded stuff then swapping remaining bytes blindly afterwards.

I guess it's possible the padding section can be whatever (I still don't understand its purpose) and objcopy's --reverse-bytes was used when it can only do the job when rom.elf just happens to have even padding? In other words, reversing the bytes with objcopy might not be the right way to go?

The approach outlined in this comment works on gngeo and NeoSD.

dciabrin commented 5 years ago

Wow, bad error indeed... let me see if I can come up with something that doesn't require additional tool, I don't remember whether my neogeo linkscript is causing the generation of the .padding section or if it's a common section. But either way I'm almost sure we could strip that section and be done with it.

Can you check on your rom whether $(OBJCOPY) -O binary -S -R .comment -R .padding works better?

city41 commented 5 years ago

Actually I think I am wrong that removing .padding broke the rom on NeoSD. I believe that was another issue. So it's possible just killing .padding altogether is the way to go. I will test tonight.

city41 commented 5 years ago

Did some tests and I am finding when I remove .padding from the p rom by adding -R .padding to the objcopy command, it works just fine on gngeo, but does not work correctly on real hardware with the NeoSD. I get garbled graphics as if the tiles in my C ROMs and S ROM are messed up. But I suspect some pointers in the P ROM might be off? causing the game to load the wrong tiles?

If I build the exact same game, except leave in .padding, then it works as expected in both gngeo and on real hardware. In this case I am using my cheesy little swaptool to swap the bytes as outlined above.

image

city41 commented 5 years ago

looks like padding is added in the link script https://github.com/dciabrin/ngdevkit/blob/master/runtime/ngdevkit.ld#L103

dciabrin commented 5 years ago

For garbled gfx, you might want to have a look at #14 and check if the solution over here has any impact on your rom.

Right now I fail to see why removing the last section of the ROM would have an impact on sprites :/, would need to look into it and try to generate a p-ROM that exhibit your problem.

I'll report my findings as soon as I get some spare time, I still hope we can fix the issue with a tweak in the link script rather.

city41 commented 5 years ago

I am generating my C ROMs using a tool I wrote. I also made this, https://city41.github.io/neospriteviewer/, which is helpful to validate C ROMs, and I am pretty confident the C ROMs are built correctly.

I don't think it's a C ROM issue. As when .padding is no longer present, my S ROM tiles are also messed up. But I totally agree removing padding leading to this issue is very strange.

I have no idea what the actual issue is. I'm very new to native dev like this, reading up on ld scripts right now.

This isn't urgent for me. I hope to learn enough to contribute rather than just post problems :)

city41 commented 5 years ago

One theory is the NeoSD can not handle such tiny P ROMs. They may be assuming a P ROM is always at least 512kb. I think that's the smallest P ROM size of commercial Neo Geo games.

I posted on the TerraOnion forums asking about this.

city41 commented 5 years ago

The p rom needs to be a multiple of 64kb for the NeoSD. So that is very likely the issue.

So yeah, maybe just get rid of .padding?

city41 commented 5 years ago

I removed .padding from ngdevkit.ld in my fork and returned to having objcopy swap the bytes. No problems so far, everything works great on gngeo and real hardware. I am padding out the p rom to 64kb with my neosd converter tool.

dciabrin commented 4 years ago

I removed .padding from ngdevkit.ld in my fork and returned to having objcopy swap the bytes. No problems so far, everything works great on gngeo and real hardware. I am padding out the p rom to 64kb with my neosd converter tool.

Ok by re-reading objcopy's manual, I saw that the semantics of reversing bytes is to work on each section individually, rather than on the output file. So indeed, since the linkscript doesn't enforce all section sizes to be even, this can cause problems as you mentioned... I now use dd to swap bytes. It's generic enough that it will work on all OS, and it leaves the linkscript untouched, so I think that it's a good compromise.

Also, it should fit the NeoSD use case as I believe that the default linkscript aims at padding to 128KB, which should be good enough.

Don't hesitate to reopen if the problem persists, Thanks!