Bug 207 - Enable debug symbol propagation (-g)
Summary: Enable debug symbol propagation (-g)
Status: RESOLVED FIXED
Alias: None
Product: RLN
Classification: Unclassified
Component: Core (show other bugs)
Version: unspecified
Hardware: All All
: Normal enhancement
Assignee: Shamus Hammons
URL:
Depends on:
Blocks:
 
Reported: 2022-07-25 01:19 CDT by James Jones
Modified: 2022-08-15 21:36 CDT (History)
2 users (show)

See Also:


Attachments
Enables the existing handling of the '-g' option and fixes a few bugs in it. (2.58 KB, patch)
2022-07-25 01:19 CDT, James Jones
Details
Rework OSTAdd()/OSTLookup() to use an array of SYMREC (16.77 KB, patch)
2022-07-26 05:49 CDT, James Jones
Details
Enable debug symbol propagation via the -g command line option (2.58 KB, patch)
2022-07-26 05:50 CDT, James Jones
Details

Note You need to log in before you can comment on or make changes to this bug.
Description James Jones 2022-07-25 01:19:02 CDT
Created attachment 191 [details]
Enables the existing handling of the '-g' option and fixes a few bugs in it.

The existing code was sufficient to pass through and relocate most debug symbols as necessary. It was just disabled for some reason. The only bugs were in the handling of non-text line numbers, which aren't ever used as far as I can tell, and include files, which should be relocated like line numbers to match ALN's behavior.
Comment 1 James Jones 2022-07-25 01:24:52 CDT
Note this was generated on top of my other pending patches, but it applies cleanly without them.

Tested using the associated RMAC source-level debug info generation patch, as well as with the build of GCC from my Jaguar SDK (https://github.com/cubanismo/jaguar-sdk) using some pretty simple binaries (jaghello from the examples dir in my SDK, 42bs's gpufire256 demo) and gdbjag from my SDK, as well as WDB. Output was also manually compared to that of ALN with identical input using the "size" utility (version 2.24 and later) from my jag_utils repo (https://github.com/cubanismo/jag_utils), run using the parameters "size -v0 -sd <file>".

Compared to ALN, the symbol ordering is a bit different, but output should be otherwise identical, and the various debuggers don't seem to care about the ordering.
Comment 2 James Jones 2022-07-26 02:48:33 CDT
Oof, put this one on hold. It seriously breaks relocations.

I'm not sure how this got past my testing. I think what I did was test the RLN patches against MADMAC .o files, and the RMAC changes with ALN, then RMAC + RLN on a simple program (gpufire256) that likely has no relocations, but when I tested the combination of RMAC with -g support + RLN with -g support + relocations, it was obvious there was a problem.

The issue was exposed by this code in the RelocateSegment():

    ssidx = OSTLookup(sym);
    newdata = GetLong(ost + ((ssidx - 1) * 12) + 8);

OSTLookup() returns the symbol's name's index in the output string table. Note the non-obvious assumption here: A symbol's string's index in the output string table is equal to its index in the output symbol table. While this is indeed how OSTLookup() was designed to operate, the assumption breaks down if any output symbol doesn't have an associated entry in the output string table, which is the case for line number debug symbols (See their existing special handling in OSTAdd()).

The assumption here holds up if there are no nameless symbols before all the ones used in relocations, which is the case with MADMAC's and GCC's debug symbol output. I'm still not clear why/if it worked with multiple .o files sorted this way though. Bleh.

I've identified a few possible fixes:

-Kludge in an empty string for each symbol without a name. Then all the existing logic works, but this bloats out the output file by one byte for each line number. Arguably not a big deal for files already bloated by debug symbols, but... I don't like it.

-Maintain a separate output symbol table for debugging symbols and concatenate the two tables in WriteOutputFile(). This has the nice side effect of sorting the symbols similarly to how ALN does: All "normal" symbols followed by all debug symbols. It makes OSTAdd() really messy. I wrote up half an implementation before I realized I was going to have to relocate the second string table when concatenating them, and gave up.

-Rewrite the OST*() functions to build an actual table rather than directly building the output symbol and string tables in-place, and then serialize them down to a single table in WriteOutputFile(). This is the most invasive change, but also the most robust IMHO. It could also support sorting based on symbol type to more closely mimic ALN's behavior.

Anyone else have a preference?
Comment 3 James Jones 2022-07-26 05:49:08 CDT
Created attachment 193 [details]
Rework OSTAdd()/OSTLookup() to use an array of SYMREC
Comment 4 James Jones 2022-07-26 05:50:14 CDT
Created attachment 194 [details]
Enable debug symbol propagation via the -g command line option
Comment 5 James Jones 2022-07-26 06:15:12 CDT
Attachment 193 [details] demonstrates the alternate output symbol table handling I was referring to in my last comment. It wasn't as invasive as I thought it would be, but it's still a somewhat large change as far as RLN goes.

I've tested this on a much larger program with lots of C files now (The copy of the Atari 3D demo renderer in my SDK), and it seems to hold up OK in GDB and WDB.
Comment 6 ggn 2022-07-26 13:13:13 CDT
Everything checks out fine here, nicely done!

Just to double check, we should apply #193 and then #194?
Comment 7 James Jones 2022-07-26 13:22:52 CDT
Correct, apply 193 first if you don't want 194 to break linking of object files containing debug information :-) Other than that, they're independent.
Comment 8 Shamus Hammons 2022-08-15 21:36:43 CDT
Thanks for the patches!  :-)