Bug 140 - Unusual case in COFF writer
Summary: Unusual case in COFF writer
Status: RESOLVED FIXED
Alias: None
Product: RMAC
Classification: Unclassified
Component: Core (show other bugs)
Version: unspecified
Hardware: All All
: Normal normal
Assignee: Shamus Hammons
URL:
Depends on:
Blocks:
 
Reported: 2019-11-06 15:48 CST by ggn
Modified: 2020-03-02 08:06 CST (History)
1 user (show)

See Also:


Attachments
Please go away. Pretty please? (1.72 KB, patch)
2020-03-01 14:28 CST, ggn
Details

Note You need to log in before you can comment on or make changes to this bug.
Description ggn 2019-11-06 15:48:15 CST
While hunting the reason why lachoneus couldn't get the removers lib to assemble correctly using rmac/rln I came across a weird case that caused the linker to produce garbled results.

Inside the library (http://removers.free.fr/softs/archives/rmvlib-1.3.5.tar.gz) and file sound/sound.s one can see the following at the bottom of the file:

	.text
	.globl	_init_module
;; init_module(char *module, int tempo_enabled);
_init_module	equ	mt_init

	.globl	_play_module
;; play_module();
_play_module	equ	mt_music_vbl


(etc etc)

So the code is defining global symbols, and equates them to some known addresses inside the code.

This is all fine and dandy but the coff image writer sees that there's an equ'd symbol and assumes blindly it's an absolute symbol (object.c "if (w1 & EQUATED)"), completely bypassing the part that checks if the equate resides inside text/data/bss. This actually tells the linker to leave the symbol alone (https://www.freebsd.org/cgi/man.cgi?query=a.out&sektion=5&manpath=FreeBSD+5.0-RELEASE - "N_ABS An absolute symbol. The link editor does not update an absolute symbol.") which means no fixups which means kaboom!

My proposed fix is to change the "else" of that if statement to "if (w1 & DEFINED)" (i.e. remove the else, replace with that). So if the assembler comes across an equated symbol it'll give the equated flag. But next if the symbol actually resides in TBD then modify the value again and give it a relocatable segment.

I tried this with that source file and it fixes the offending symbols' attributes to text and manages to link and run fine :). The .o file has been checked against madmac and seems to match in that area (there are a few other symbols that aren't equated in rmac while madmac equates them but I'll defer that investigation if/when it becomes relevant :P).

So I dunno, I gave the issue a lot of consideration and think that this is sane. I haven't run the regression tester yet but this will happen soon. Worst case I can run the source file with the ST version of madmac and trace the code to see what it is doing :P

But anyway, your thoughts and decision at this point would be nice :)
Comment 1 Shamus Hammons 2020-01-02 14:36:35 CST
It seems like it would be OK, but it causes a regression in TalkTalk2.  Have to take a deeper look into it to see what's what.
Comment 2 Shamus Hammons 2020-01-02 16:08:39 CST
Turns out the regression in TalkTalk2 was a false alarm; this has been pushed out to master.
Comment 3 ggn 2020-03-01 14:27:46 CST
Ok, reopening this because we stumbled across this issue again (https://atariage.com/forums/topic/297621-removers-library-aout-binary-emulation-dropped-from-linux-kernel/?do=findComment&comment=4470643). The same pair of equ/globl labels would get exported as EQU in AddBSDSymEntry, so what I did was to change the logic there as well.

Patch follows, let me know what you think!

(also, waiting for confirmation that this fixes all the issues)
Comment 4 ggn 2020-03-01 14:28:27 CST
Created attachment 115 [details]
Please go away. Pretty please?
Comment 5 Shamus Hammons 2020-03-02 08:06:49 CST
/me sprinkles holy water on the coffin and gravesite