ps2dev / ps2toolchain

This program will automatically build and install a compiler and other tools used in the creation of homebrew software for the Sony PlayStation® 2 videogame system.
BSD 2-Clause "Simplified" License
236 stars 71 forks source link

dvp-as doesn't resolve symbols #30

Closed aap closed 6 years ago

aap commented 7 years ago

After I got MPG working I tried to assemble a less trivial file than I used for testing and it looks like there's some problems with symbol resolution. This test:

MPG 0, *
.vu
loop:
    NOP B loop
    NOP NOP
.EndMPG

produces test2.dsm:6: Error: attempt to get value of unresolved symbol `loop'

I will try to fix it.

aap commented 7 years ago

It is this block of code in gas/symbols.c that causes the problems:

          else if (finalize_syms && final_seg == expr_section
                   && seg_left != expr_section)
            {
              /* If the symbol is an expression symbol, do similarly
                 as for undefined and common syms above.  Handles
                 "sym +/- expr" where "expr" cannot be evaluated
                 immediately, and we want relocations to be against
                 "sym", eg. because it is weak.  */
              symp->sy_value.X_op = O_symbol;
              symp->sy_value.X_add_symbol = add_symbol;
              symp->sy_value.X_add_number = final_val;
              symp->sy_value.X_op_symbol = add_symbol;
              final_seg = seg_left;
              final_val += symp->sy_frag->fr_address + left;
              resolved = symbol_resolved_p (add_symbol);
              symp->sy_resolving = 0;
              goto exit_dont_set_value;
            }

Setting s->sy_value.X_op to O_symbol and skipping S_SET_VALUE causes S_GET_VALUE to see this symbol as unresolved. Older version of gas don't have this block and set a constant value for the symbol with S_SET_VALUE instead (S_SET_VALUE sets s->sy_value.X_op to O_constant). I'm not sure how this should be fixed as I'm not deep enough into the gas code. Removing this block certainly makes it work but I'm sure there's a reason it was added....

aap commented 7 years ago

This actually looks like a bug in gas to me. Which is surprising since I'd think somebody would have noticed it. When a symbol expression is 'absolute + constant', the assembler should probably generate an absolute constant like it used to in earlier versions, but because seg_left == absolute_section, the code above is - if i'm not mistaken unnecessarily - executed. So it looks to me like the test should actually be

          else if (finalize_syms && final_seg == expr_section
                   && seg_left != absolute_section
                   && seg_left != expr_section)

I should probably ask on the binutils mailing list about it.

sp193 commented 6 years ago

Did you ever get an answer to this question?

aap commented 6 years ago

Damn, forgot to answer. But my reply is boring anyway: I simply never asked them.

sp193 commented 6 years ago

Ah. Well, thanks anyway.

I'll leave this issue open, since it is an active issue that we have no solution to (yet).

madebr commented 6 years ago

https://github.com/madebr/ps2toolchain/commit/2dee26295fddb6a210fddaf44d43262538bf8927 fixes this by patching binutils.

sp193 commented 6 years ago

If you have tested it, please consider making a pull request! Thanks.

mirh commented 6 years ago

With mainline linux going to be a thing in the somewhat near future, please remember me why we stick with 15yo toolchains?

sp193 commented 6 years ago

We don't have any new toolchain that is complete, which does support the features of the R5900.

mirh commented 6 years ago

Yeah, what I wanted to entail was.. why is nobody submitting patches upstream? I have seen @jur working on it in 2013, and even yourself seemed pretty enthusiastic some time ago.

sp193 commented 6 years ago

Oh so you've noticed! You might have also encountered various members of the PS2 world, who have once undertaken great journeys to bring back a modern version of GCC. Usually, they find a lot of pitstops, struggling to deal with the weird architecture that no other platform was. @Jur managed to come up with something that the GNU folks could agree with, possibly putting an end to all that. However, as he wrote, his work only led to the r5900 being a target that is supported by GNU tools... but has no support for anything r5900-specific (it supports no features that no other MIPS doesn't support).

I have to agree with what @jur once wrote - it sometimes felt like they're ignoring this platform. The GNU tools are difficult things to work with, with the lack of documentation, long code...

Perhaps because the GNU folks are just so overwhelmed or what, but I also found it really difficult to get them to accept patches or to give help. libgcc had (and likely still has) a bug, when it comes to r5900 support - we cannot enable hardware floating-point support properly. I did try to submit a patch (after help from their folks there), but even a small patch for existing code (previously submitted by @Jur) seemed to have fallen through the cracks... The patch was originally from @Jur, who tried submitting an earlier version of it before. So it's not even a new patch.

Some of their folks were really supportive, while some others were hoping that we could find our own ways. I gave up eventually, after not managing to get the GCC internals to work properly for complete MMI support (hence no advantage over our current tools). Anyway, my lack of experience did not give me the confidence that the final product would be of great quality.

But that being said, it's all just my opinion. I was also very discouraged by how I couldn't fix it all up. If you (or anyone) wants to continue from where we left off, please do!

mirh commented 6 years ago

I mean, have you ever tried to check them on IRC? I can see why having to figure out the world via ML leaves people with a looot of haste. Also #linux-mips perhaps.

mirh commented 5 years ago

https://github.com/qemu/qemu/commit/c96292036a17857d62b8b5d3c8752bac3d6b7193 If it could perk up somebody..