Overlap error not generated for sections w/o content

[Galit Heller]

Hi,

We use the following linker script excerpt to define the stack for CR16C
embedded systems:

.stack : { . += 0x600; }

since in our tool chain the input (ELF) files aren't comprised of .stack
sections.

If the linker script is erroneously written, so that the .stack load
address is set to a value which overlaps that of another output section,
no error is generated, since the SEC_ALLOC flag isn't set for .stack. E.g.

.data 0x1000 : { *(.data) }
.stack 0x1000 : { . += 600; }

does not cause an error nor a warning. Whereas if an output section data
expression (e.g. BYTE(0);) is used:

.stack 0x1000 : { BYTE(0); . += 0x600; }

then the following error is generated, and thus users get indication of a
problematic situation:

section .stack [0001000 -> 0001600] overlaps section .data [0001000 -> 0001200]

Should a linker hook function (e.g. ldemul_after_allocation) be used to
generate an error in such a case, or is there a chance that this behavior will
be changed ?

Regards,
Galit.

[Tom Rix]

This is a patch for gas on the m68hc11-elf. Built on RH 8 and tested with
m68hc11-sim.

The problem is reproduced using the the current testsuite. This line from
m68hc11.exp caused a failure :

gas_m68hc11_error "-m68hc12" "movb 2,x,bar,y\nbar=300\n" \
"Offset out of 5-bit range for movw/movb insn: 300"

The problem in tc-m68hc11, is that the movb/movw are incorrectly being
allowed to relax. The only valid movb/movw IDX addressing size is 5 bit.
The symbol "bar" is being relaxed to a 16 bit size.

The fix is to generate a normal fixup when the movb/movw insn's are being
built.

The error checking tests, related to this problem, in m68hc11.exp have
increased and a new dump test to cover the good cases of movb/movw has been
created.

I do not believe I have write permissions so could someone else check this
in?

Tom

[Nick Clifton]

Hi Tom,

Post by Tom Rix
This is a patch for gas on the m68hc11-elf. Built on RH 8 and tested with
m68hc11-sim.

Thanks very much for submitting this, and thanks even more for
including a new test to check the patch.

Post by Tom Rix
I do not believe I have write permissions so could someone else check this
in?

I have done so - one point though. In the future please could you
submit the ChangeLog entries as plain text rather than context diffs
as they very rarely apply cleanly.

Cheers
Nick

bfd/ChangeLog
2004-01-11 Tom Rix <***@worldnet.att.net>

* reloc.c: New 5 bit reloc, BFD_RELOC_M68HC12_5B, for m68hc12 movb/movw.
* bfd-in2.h, libbfd.h: Rebuilt.

gas/ChangeLog
2004-01-11 Tom Rix <***@worldnet.att.net>

* config/tc-m68hc11.c (build_indexed_byte): movb and movw can not
be relaxed, use fixup.
(md_apply_fix3): Use 5 bit reloc from movb and movw fixup.

gas/testsuite/ChangeLog
2004-01-11 Tom Rix <***@worldnet.att.net>

* gas/m68hc11/movb.s: Add m68hc12 movb and movw dump test.
* gas/m68hc11/movb.d: Likewise.
* gas/m68hc11/m68hc11.exp: Likewise. Add more movb failure tests.

Applied.

[Alan Modra]

Post by Galit Heller
If the linker script is erroneously written, so that the .stack load
address is set to a value which overlaps that of another output section,
no error is generated, since the SEC_ALLOC flag isn't set for .stack. E.g.

I think that's a linker bug. Clearly, addresses of SEC_ALLOC sections
matter whether they are SEC_LOAD or not. Hmm, looking at ChangeLog
entries, I see Nick made some changes in this area back in 1999.
http://sources.redhat.com/ml/binutils/1999-11/msg00137.html

Possibly the following is a better patch to fix the problem Nick found.
Nick, how's your memory?

* ldlang.c (IGNORE_SECTION): Don't ignore SEC_ALLOC && !SEC_LOAD
sections. Do ignore SEC_NEVER_LOAD sections.
(lang_size_sections_1): Remove test made redundant with the above.

Index: ld/ldlang.c
===================================================================
RCS file: /cvs/src/src/ld/ldlang.c,v
retrieving revision 1.132
diff -u -p -r1.132 ldlang.c
--- ld/ldlang.c 3 Jan 2004 12:39:07 -0000 1.132
+++ ld/ldlang.c 12 Jan 2004 01:07:50 -0000
@@ -2762,8 +2762,8 @@ size_input_section (lang_statement_union
}

#define IGNORE_SECTION(bfd, s) \
- (((bfd_get_section_flags (bfd, s) & (SEC_ALLOC | SEC_LOAD)) \
- != (SEC_ALLOC | SEC_LOAD)) \
+ (((bfd_get_section_flags (bfd, s) & (SEC_ALLOC | SEC_NEVER_LOAD)) \
+ != SEC_ALLOC) \
|| bfd_section_size (bfd, s) == 0)

/* Check to see if any allocated sections overlap with other allocated
@@ -2930,8 +2930,6 @@ lang_size_sections_1
region, and some non default memory regions were
defined, issue an error message. */
if (!IGNORE_SECTION (output_bfd, os->bfd_section)
- && (bfd_get_section_flags (output_bfd, os->bfd_section)
- & SEC_NEVER_LOAD) == 0
&& ! link_info.relocatable
&& check_regions
&& strcmp (os->region->name, DEFAULT_MEMORY_REGION) == 0

[Galit Heller]

Hi Alan,

Post by Alan Modra

Post by Galit Heller
If the linker script is erroneously written, so that the .stack load
address is set to a value which overlaps that of another output section,
no error is generated, since the SEC_ALLOC flag isn't set for .stack. E.g.

I think that's a linker bug. Clearly, addresses of SEC_ALLOC sections
matter whether they are SEC_LOAD or not. Hmm, looking at ChangeLog
entries, I see Nick made some changes in this area back in 1999.
http://sources.redhat.com/ml/binutils/1999-11/msg00137.html
Possibly the following is a better patch to fix the problem Nick found.
Nick, how's your memory?
* ldlang.c (IGNORE_SECTION): Don't ignore SEC_ALLOC && !SEC_LOAD
sections. Do ignore SEC_NEVER_LOAD sections.
(lang_size_sections_1): Remove test made redundant with the above.

This patch is certainly beneficial, in that error is reported when e.g.
a .bss section causes overlap. Without the patch there is no indication of
this situation.

However, the case which originally triggered my question still stands:

when an output section is created via the statement:

.section_name : { . += 600; }

then .section_name does not have SEC_ALLOC flag set (and thus no overlap
error is reported for this section).

This could be considered unintuitive, since the only obvious reason for a
statement in which dot is incremented, and nothing else happens, would be
to allocate space for the specified output section. Since there isn't an
"ALLOC" output section type in the linker script syntax, the user cannot
indicate that this section should indeed be taken into account when
allocating memory and reporting overlap.

Thanks,
Galit.

[Alan Modra]

Post by Galit Heller
.section_name : { . += 600; }
then .section_name does not have SEC_ALLOC flag set (and thus no overlap
error is reported for this section).

We can't really do anything about this. !SEC_ALLOC && !SEC_LOAD
sections are typically used for debug sections. If we started checking
these, they would all clash.

[Galit Heller]

Post by Alan Modra

Post by Galit Heller
.section_name : { . += 600; }
then .section_name does not have SEC_ALLOC flag set (and thus no overlap
error is reported for this section).

We can't really do anything about this. !SEC_ALLOC && !SEC_LOAD
sections are typically used for debug sections. If we started checking
these, they would all clash.

Of course. The question is whether SEC_ALLOC could be automatically set
for a section in which dot is incremented, as is done in lang_size_sections_1
for output sections whose statement consists of an output section data
expression
e.g. .section_name : { BYTE(0); } .

I realize that the latter case is a more clear cut indication of the section
needing to be allocated.

Thanks,
Galit.

[Nick Clifton]

Hi Alan, Hi Galit,

Post by Alan Modra
I think that's a linker bug. Clearly, addresses of SEC_ALLOC sections
matter whether they are SEC_LOAD or not. Hmm, looking at ChangeLog
entries, I see Nick made some changes in this area back in 1999.
http://sources.redhat.com/ml/binutils/1999-11/msg00137.html
Possibly the following is a better patch to fix the problem Nick found.
Nick, how's your memory?

A bit vague, but I do recall working on this bit fo code.

Post by Alan Modra
* ldlang.c (IGNORE_SECTION): Don't ignore SEC_ALLOC && !SEC_LOAD
sections. Do ignore SEC_NEVER_LOAD sections.
(lang_size_sections_1): Remove test made redundant with the above.

This looks like a very good change, so please do commit it.

: > From Alan:
: >
: > We can't really do anything about this. !SEC_ALLOC && !SEC_LOAD
: > sections are typically used for debug sections. If we started checking
: > these, they would all clash.
:
: From Galit:
:
: Of course. The question is whether SEC_ALLOC could be automatically set
: for a section in which dot is incremented, as is done in lang_size_sections_1
: for output sections whose statement consists of an output section data
: expression
: e.g. .section_name : { BYTE(0); } .
:
: I realize that the latter case is a more clear cut indication of the section
: needing to be allocated.

I agree however. Either the section should become allocated or else a
warning should be issued that advancing dot has no effect inside an
unallocated section. How about a patch like this:

Cheers
Nick

2004-01-12 Nick Clifton <***@redhat.com>

* ldlang.c (lang_size_sections_1): If dot is advanced, then
assume that the section should be allocated.


Index: ld/ldlang.c
===================================================================
RCS file: /cvs/src/src/ld/ldlang.c,v
retrieving revision 1.132
diff -c -3 -p -r1.132 ldlang.c
*** ld/ldlang.c 3 Jan 2004 12:39:07 -0000 1.132
--- ld/ldlang.c 12 Jan 2004 13:27:04 -0000
*************** lang_size_sections_1
*** 3207,3212 ****
--- 3207,3218 ----
s = s->header.next;
}

+ /* If dot is advanced, this implies that the section should
+ have space allocated to it, unless the user has explicitly
+ stated that the section should never be loaded. */
+ if (!(output_section_statement->flags & (SEC_NEVER_LOAD | SEC_ALLOC)))
+ output_section_statement->bfd_section->flags |= SEC_ALLOC;
+
dot = newdot;
}
}

[Galit Heller]

Hi,

Post by Nick Clifton
I agree however. Either the section should become allocated or else a
warning should be issued that advancing dot has no effect inside an
Cheers
Nick
* ldlang.c (lang_size_sections_1): If dot is advanced, then
assume that the section should be allocated.

Works for me.

BTW, it seems that the change mentioned in:

http://sources.redhat.com/ml/binutils/2003-10/msg00213.html

and which is related to region checking, was not made.

Thanks,
Galit.

[Nick Clifton]

Hi Galit,

Post by Galit Heller

Post by Nick Clifton
I agree however. Either the section should become allocated or else a
warning should be issued that advancing dot has no effect inside an
Cheers
Nick
* ldlang.c (lang_size_sections_1): If dot is advanced, then
assume that the section should be allocated.

Works for me.

Good.

Post by Galit Heller
http://sources.redhat.com/ml/binutils/2003-10/msg00213.html
and which is related to region checking, was not made.

That would be because we have only just talked about it and I expect
that Alan is busy working....

Cheers
Nick

[Nick Clifton]

Hi Galit,

Post by Nick Clifton

Post by Galit Heller
http://sources.redhat.com/ml/binutils/2003-10/msg00213.html
and which is related to region checking, was not made.

That would be because we have only just talked about it and I expect
that Alan is busy working....

Oops - Sorry I was looking at the wrong URL.

Yes you are right - this change seems to have become lost in the fog.
I will take care of it and check it in shortly.

Cheers
Nick

[Alan Modra]

Post by Nick Clifton
Hi Galit,

Post by Nick Clifton

Post by Galit Heller
http://sources.redhat.com/ml/binutils/2003-10/msg00213.html
and which is related to region checking, was not made.

That would be because we have only just talked about it and I expect
that Alan is busy working....

Oops - Sorry I was looking at the wrong URL.
Yes you are right - this change seems to have become lost in the fog.
I will take care of it and check it in shortly.

Yes, I dropped it. However, the most recent change to IGNORE_SECTIONS
means we are OK now. The original condition might look different from
what we have now, but it turns out that SEC_LOAD sections are always
SEC_ALLOC.

[Alan Modra]

Re: http://sourceware.org/ml/binutils/2004-01/msg00179.html

I was looking at this code the other day, and puzzled for a while over
why we want to test output_section_statement->flags & SEC_ALLOC here.
I figure it's just there to save an unnecessary write to memory, but
the "flags" are different so it isn't obviously correct.

* ldlang.c (lang_size_sections_1): Simplify SEC_NEVER_LOAD test.

Index: ld/ldlang.c
===================================================================
RCS file: /cvs/src/src/ld/ldlang.c,v
retrieving revision 1.277
diff -u -p -r1.277 ldlang.c
--- ld/ldlang.c 16 Jan 2008 00:31:18 -0000 1.277
+++ ld/ldlang.c 16 Jan 2008 00:34:45 -0000
@@ -4702,8 +4702,7 @@ lang_size_sections_1
should have space allocated to it, unless the
user has explicitly stated that the section
should never be loaded. */
- if (!(output_section_statement->flags
- & (SEC_NEVER_LOAD | SEC_ALLOC)))
+ if (!(output_section_statement->flags & SEC_NEVER_LOAD))
output_section_statement->bfd_section->flags |= SEC_ALLOC;
}
dot = newdot;