[PATCH] Add support for subsections to the ELF assembler. Fixes PR8717.

Peter Collingbourne peter at pcc.me.uk
Fri Apr 12 17:08:17 PDT 2013


On Wed, Apr 10, 2013 at 08:49:07AM -0400, Rafael EspĂ­ndola wrote:
> On 10 April 2013 02:47, Peter Collingbourne <peter at pcc.me.uk> wrote:
> >
> >   > Please add a test showing this.
> >
> >   To be clear, I'll add a test making sure we don't emit a relocation in this case.
> 
> Please also test that we use a 2 byte jump.

OK.

> >   > Please make sure we reject numbers larger than 8192.
> >
> >   I thought about this, but a seemingly undocumented feature of gas is that it supports larger numbers, as well as negative numbers, so I decided to omit the check (although it's debatable whether we need to be compatible on this).  If we decide not to support this we can probably simplify getSubsectionInsertionPoint as well.
> 
> Do you depend on that feature? If not it is probably best to start
> with the more restrictive option.

No, OK.

> > It seems better to me to be able to deal with the insertion logic in this function rather than having to repetitively deal with insertion throughout the MCObjectStreamers.  In my opinion, repetition would make it easier to make a mistake in one of the streamers and (e.g.) have fragments appended to the fragment list rather than inserted in the right place.
> 
> Why would it be repeated? Instead of
> 
> Fragment *F = new...;
> insert(F);
> 
> you get
> 
> Frament F = new Fragment(....CurInsertionPoint);
> 
> and the compiler makes sure CurInsertionPoint is always passed.

We would in fact need two parameters (the fragment list and the
insertion point), as we won't necessarily be able to derive the
fragment list from the insertion point if it's at the end.

There are also a few places in ELFObjectWriter where we do
need to create unparented MCFragments.

> >> Can we report the error without using report_fatal_error?
> > Yes, but this would be a relatively invasive QOI improvement to handle a relatively rare occurrence; it would probably entail changing ChangeSection and SwitchSection to return an error value, and SwitchSection has a large number of callers that would each need to be audited to make sure they're handling errors correctly.
> 
> OK. We can do this in another patch then.

OK.

Thanks,
-- 
Peter



More information about the llvm-commits mailing list