Re: Changes to gas/write.c and others break the ia32 assembler

["H . J . Lu" <hjl at lucon dot org>]

On Wed, Aug 23, 2000 at 07:19:44AM -0300, Alexandre Oliva wrote:
> On Aug 23, 2000, "H . J . Lu" <hjl@lucon.org> wrote:
> 
> > This patch seems to work for me on ia32. Alexandre, I am afraid all
> > those platforms where the original behavior is changed due to your
> > patch are broken.
> 
> As I wrote in the original patch, I tested it minimally, and it
> appeared to work, but I'd have appreciated if maintainers of each
> affected port could double-check it.
> 

There is no need for double-check since you changed the original
behavior. Right now the ia32 assembler generates the different output
on the same input. It is not ok for an assembler unless there is a
real bug. In that case, you should provide a testcase.


> > 	* write.c (TC_FIX_ADJUSTABLE): Remove the duplicate.
> 
> Wrong.  It's not a duplicate, it's a default.

I saw it in write.c:

#ifndef TC_FIX_ADJUSTABLE
#define TC_FIX_ADJUSTABLE(fix) 1
#endif
         
#ifndef TC_LINKRELAX_FIXUP
#define TC_LINKRELAX_FIXUP(SEG) 1
#endif

#ifndef TC_FIX_ADJUSTABLE
#define TC_FIX_ADJUSTABLE(fix) 1
#endif
         
Can you tell me why you need 2 TC_FIX_ADJUSTABLE?

> 
> > 	* config/tc-i386.h (TC_FIX_ADJUSTABLE): Restore it to the
> > 	original behavior.
> 
> This might be the way to go short-term, but if tc_fix_adjustable()
> says a reloc can be fixed up by the assembler, there should be no
> reason why it shouldn't do it.  It is probably tc_fix_adjustable()
> that needs tweaking.  Redefining TC_FIX_ADJUSTABLE to 0 is just
> papering over the problem.
> 

As I said, please restore the original behavior. I don't care how.
I want the same output on the same input. Having worked on mixing
PE, COFF and ELF object files to generate the ELF executable, I know
it is very tricky. Don't be fooled by the obvious.

> 
> > -#if defined (OBJ_ELF) || defined (OBJ_COFF) || defined (TE_PE)
> > +#if (defined (OBJ_MAYBE_ELF) || defined (OBJ_ELF) || defined (OBJ_COFF)) && !defined (TE_PE)
> 
> The original #if in write.c was:
> 
> #if !defined (TC_I386) || !(defined (OBJ_ELF) || defined (OBJ_COFF)) || defined (TE_PE)
> 
> so I indeed goofed in TE_PE (I missed the closing paren).  But
> OBJ_MAYBE_ELF was not there for sure.

As I said, OBJ_MAYBE_ELF is needed for multi-format assembler. You
may also need to check OBJ_MAYBE_COFF. Alan has warned the danger of
touching relocation code in assembler. He knows what he was talking
about. He put in the support for ELF/a.out/COFF in one ia32 assembler.

I don't see there is an easy way to fix it and verify the fix. I will
back out your patch in my local tree if it is resolved today or
tomorrow.


H.J.