-However, Ian wrote the following, regarding removing the line below,
-which explains why it is still enabled: --djm
-
-If you put a patch like that into BFD you need to check all the COFF
-linkers. I am fairly certain that patch will break coff-i386 (e.g.,
-SCO); see coff_i386_reloc in coff-i386.c where I worked around the
-problem in a different way. There may very well be a reason that the
-code works as it does.
-
-Hmmm. The first obvious point is that bfd_perform_relocation should
-not have any tests that depend upon the flavour. It's seem like
-entirely the wrong place for such a thing. The second obvious point
-is that the current code ignores the reloc addend when producing
-relocateable output for COFF. That's peculiar. In fact, I really
-have no idea what the point of the line you want to remove is.
-
-A typical COFF reloc subtracts the old value of the symbol and adds in
-the new value to the location in the object file (if it's a pc
-relative reloc it adds the difference between the symbol value and the
-location). When relocating we need to preserve that property.
-
-BFD handles this by setting the addend to the negative of the old
-value of the symbol. Unfortunately it handles common symbols in a
-non-standard way (it doesn't subtract the old value) but that's a
-different story (we can't change it without losing backward
-compatibility with old object files) (coff-i386 does subtract the old
-value, to be compatible with existing coff-i386 targets, like SCO).
-
-So everything works fine when not producing relocateable output. When
-we are producing relocateable output, logically we should do exactly
-what we do when not producing relocateable output. Therefore, your
-patch is correct. In fact, it should probably always just set
-reloc_entry->addend to 0 for all cases, since it is, in fact, going to
-add the value into the object file. This won't hurt the COFF code,
-which doesn't use the addend; I'm not sure what it will do to other
-formats (the thing to check for would be whether any formats both use
-the addend and set partial_inplace).
-
-When I wanted to make coff-i386 produce relocateable output, I ran
-into the problem that you are running into: I wanted to remove that
-line. Rather than risk it, I made the coff-i386 relocs use a special
-function; it's coff_i386_reloc in coff-i386.c. The function
-specifically adds the addend field into the object file, knowing that
-bfd_perform_relocation is not going to. If you remove that line, then
-coff-i386.c will wind up adding the addend field in twice. It's
-trivial to fix; it just needs to be done.
-
-The problem with removing the line is just that it may break some
-working code. With BFD it's hard to be sure of anything. The right
-way to deal with this is simply to build and test at least all the
-supported COFF targets. It should be straightforward if time and disk
-space consuming. For each target:
- 1) build the linker
- 2) generate some executable, and link it using -r (I would
- probably use paranoia.o and link against newlib/libc.a, which
- for all the supported targets would be available in
- /usr/cygnus/progressive/H-host/target/lib/libc.a).
- 3) make the change to reloc.c
- 4) rebuild the linker
- 5) repeat step 2
- 6) if the resulting object files are the same, you have at least
- made it no worse
- 7) if they are different you have to figure out which version is
- right
-*/
+ However, Ian wrote the following, regarding removing the line
+ below, which explains why it is still enabled: --djm
+
+ If you put a patch like that into BFD you need to check all
+ the COFF linkers. I am fairly certain that patch will break
+ coff-i386 (e.g., SCO); see coff_i386_reloc in coff-i386.c
+ where I worked around the problem in a different way. There
+ may very well be a reason that the code works as it does.
+
+ Hmmm. The first obvious point is that bfd_perform_relocation
+ should not have any tests that depend upon the flavour. It's
+ seem like entirely the wrong place for such a thing. The
+ second obvious point is that the current code ignores the
+ reloc addend when producing relocatable output for COFF.
+ That's peculiar. In fact, I really have no idea what the
+ point of the line you want to remove is.
+
+ A typical COFF reloc subtracts the old value of the symbol
+ and adds in the new value to the location in the object file
+ (if it's a pc relative reloc it adds the difference between
+ the symbol value and the location). When relocating we need
+ to preserve that property.
+
+ BFD handles this by setting the addend to the negative of the
+ old value of the symbol. Unfortunately it handles common
+ symbols in a non-standard way (it doesn't subtract the old
+ value) but that's a different story (we can't change it
+ without losing backward compatibility with old object files)
+ (coff-i386 does subtract the old value, to be compatible with
+ existing coff-i386 targets, like SCO).
+
+ So everything works fine when not producing relocatable
+ output. When we are producing relocatable output, logically
+ we should do exactly what we do when not producing
+ relocatable output. Therefore, your patch is correct. In
+ fact, it should probably always just set reloc_entry->addend
+ to 0 for all cases, since it is, in fact, going to add the
+ value into the object file. This won't hurt the COFF code,
+ which doesn't use the addend; I'm not sure what it will do
+ to other formats (the thing to check for would be whether
+ any formats both use the addend and set partial_inplace).
+
+ When I wanted to make coff-i386 produce relocatable output,
+ I ran into the problem that you are running into: I wanted
+ to remove that line. Rather than risk it, I made the
+ coff-i386 relocs use a special function; it's coff_i386_reloc
+ in coff-i386.c. The function specifically adds the addend
+ field into the object file, knowing that bfd_perform_relocation
+ is not going to. If you remove that line, then coff-i386.c
+ will wind up adding the addend field in twice. It's trivial
+ to fix; it just needs to be done.
+
+ The problem with removing the line is just that it may break
+ some working code. With BFD it's hard to be sure of anything.
+ The right way to deal with this is simply to build and test at
+ least all the supported COFF targets. It should be
+ straightforward if time and disk space consuming. For each
+ target:
+ 1) build the linker
+ 2) generate some executable, and link it using -r (I would
+ probably use paranoia.o and link against newlib/libc.a,
+ which for all the supported targets would be available in
+ /usr/cygnus/progressive/H-host/target/lib/libc.a).
+ 3) make the change to reloc.c
+ 4) rebuild the linker
+ 5) repeat step 2
+ 6) if the resulting object files are the same, you have at
+ least made it no worse
+ 7) if they are different you have to figure out which
+ version is right. */
projects / deliverable / binutils-gdb.git / blobdiff