From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark Mitchell To: ian@zembu.com Cc: binutils@sourceware.cygnus.com Subject: Re: PATCH for 64-bit MIPS ELF buglets Date: Mon, 12 Jul 1999 13:37:00 -0000 Message-id: <19990712134003P.mitchell@codesourcery.com> References: <19990712191858.6381.qmail@daffy.airs.com> <19990712124625I.mitchell@codesourcery.com> <19990712200136.7955.qmail@daffy.airs.com> X-SW-Source: 1999-q3/msg00150.html >>>>> "Ian" == Ian Lance Taylor writes: Ian> I'm afraid that there isn't any documentation on those Ian> relocations. I just wrote them so that they worked. I can Ian> probably answer any questions you have. Oh, dear. OK. Ian> The mips16 jump instruction is weird because the address is Ian> stored in a permuted format so that it can be picked up by Ian> the 16 bit instruction decoder. Thanks for the hint. Ian> Some of the mips16 stub support seems to have disappeared as Ian> well. That all needs to get restored too. I'll do my best. In retrospect, using R_MIPS_64 for two different relocations was probably not an ideal design choice; in the future, we should probably try to use non-standard relocations names for non-standard relocations. (As you did for R_MIPS_16*.) Of course, sometimes we have to deal with other tools/ABIs that do things in less than ideal ways. :-) Ian> I find a case like omitting mips16 support pretty scary, Ian> because there isn't much testing for this sort of thing, so Ian> it is likely to get caught months later by somebody who has Ian> no idea what has happened to the code. The scariest thing is that there is no testing of this functionality. That's what makes it so easy to break. The more people work on binutils, the lower will be the percentage of people with your skill and experience and the harder it will be for you to verify every change. In GCC, we tend to break these kinds of corner cases/obscure platforms relatively often. I don't think that's because we're bad engineers; it's just what happens. We use release cycles for stabilization. On the C++ side, we add regression tests for almost every bug fix. That practice has really made a huge difference, and markedly reduces the amount of "this program used to work with 1.x but doesn't with 2.x" kind of bug reports we see. Perhaps it would be worthwhile for someone to produce some .o's that could become part of the testsuite. If ABI's are stable, then .o's should continue to work, and there could actually be tests comparing the output of a link to a fixed image. (Of course, there'd have to be some fudging for the things that *are* allowed to change, but it seems like we could probably things up to at least test the basic functionality.) For example, in this case, testing that R_MIPS16_* didn't cause an assertion failure/abort would probably have worked. Better would be to put the relocation target at an absolute address (via a linker script) and then verify that the relocated contents are correct. It should be relatively simple to right a program (using BFD) that opens an executable image, and examines the value at a fixed location in a particular section. I bet that regression tests using such a framework would do a *ton* towards avoiding problems like this in the future. Note that I'm not suggesting you spend your time on this; I'm sure you're too busy. But, perhaps Cygnus, or someone else with a strong interest in these under-tested platforms, would be motivated to work on such platform-specific test-suites. We would certainly be willing to work on this, if anyone is interested in contracting the work. Ian> I assumed that you broke apart the functions without changing Ian> the functionality; had I noticed that you were actually Ian> changing what they did, I would not have approved the patch. I'm sorry the patch description was unclear. The way in which the relocation values was calculated was not conducive to the multiple-relocs-at-same-location bit in the MIPS ABI, so some considerable changes were necessitated. -- Mark Mitchell mark@codesourcery.com CodeSourcery, LLC http://www.codesourcery.com