This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: Patch to detect invalid mergeable string sections
- From: Alan Modra <amodra at bigpond dot net dot au>
- To: "Joseph S. Myers" <joseph at codesourcery dot com>, binutils at sourceware dot org
- Date: Mon, 5 Nov 2007 12:42:05 +1030
- Subject: Re: Patch to detect invalid mergeable string sections
- References: <Pine.LNX.4.64.0710301609020.7201@digraph.polyomino.org.uk> <20071030225501.GF5706@bubble.grove.modra.org> <Pine.LNX.4.64.0710311806580.31492@digraph.polyomino.org.uk> <20071101071111.GE11924@bubble.grove.modra.org>
Joseph, I'm reverting most of your patch and instead providing the
missing zero terminator at the end of a string merge section. (My
SLES 10 box system compiler happens to be a 4.1.2 prerelease without
Jakub's fix and I hit your error message on the ld bootstrap tests..)
Since versions of gcc with the bug are widely used, I think ld should
fix the problem, especially since the fix is easy. From Jakub's
description of the problem I'm fairly certain that ld can produce a
valid program.
Apologies for not picking this up earlier when I reviewed your patch.
bfd/
* merge.c (sec_merge_hash_lookup): Revert last change.
(record_section): Likewise.
(_bfd_merge_sections): Likewise.
(_bfd_merged_section_offset): Properly handle NULL secinfo.
(_bfd_add_merge_section): Allocate extra space for a zero
terminator on SEC_STRINGS sections.
ld/testsuite/
* ld-elf/merge3.d, ld-elf/merge3.s: Delete.
Index: bfd/merge.c
===================================================================
RCS file: /cvs/src/src/bfd/merge.c,v
retrieving revision 1.34
diff -u -p -r1.34 merge.c
--- bfd/merge.c 1 Nov 2007 11:45:20 -0000 1.34
+++ bfd/merge.c 5 Nov 2007 02:07:05 -0000
@@ -133,7 +133,6 @@ sec_merge_hash_newfunc (struct bfd_hash_
static struct sec_merge_hash_entry *
sec_merge_hash_lookup (struct sec_merge_hash *table, const char *string,
- const unsigned char *sec_end,
unsigned int alignment, bfd_boolean create)
{
register const unsigned char *s;
@@ -155,8 +154,6 @@ sec_merge_hash_lookup (struct sec_merge_
hash += c + (c << 17);
hash ^= hash >> 2;
++len;
- if (sec_end && s >= sec_end)
- return NULL;
}
hash += len + (len << 17);
}
@@ -164,8 +161,6 @@ sec_merge_hash_lookup (struct sec_merge_
{
for (;;)
{
- if (sec_end && s + table->entsize > sec_end)
- return NULL;
for (i = 0; i < table->entsize; ++i)
if (s[i] != '\0')
break;
@@ -269,9 +264,7 @@ sec_merge_add (struct sec_merge_hash *ta
{
register struct sec_merge_hash_entry *entry;
- entry = sec_merge_hash_lookup (tab, str,
- secinfo->contents + secinfo->sec->size,
- alignment, TRUE);
+ entry = sec_merge_hash_lookup (tab, str, alignment, TRUE);
if (entry == NULL)
return NULL;
@@ -410,7 +403,12 @@ _bfd_add_merge_section (bfd *abfd, void
/* Read the section from abfd. */
- amt = sizeof (struct sec_merge_sec_info) + sec->size - 1;
+ amt = sizeof (struct sec_merge_sec_info) - 1 + sec->size;
+ if (sec->flags & SEC_STRINGS)
+ /* Some versions of gcc may emit a string without a zero terminator.
+ See http://gcc.gnu.org/ml/gcc-patches/2006-06/msg01004.html
+ Allocate space for an extra zero. */
+ amt += sec->entsize;
*psecinfo = bfd_alloc (abfd, amt);
if (*psecinfo == NULL)
goto error_return;
@@ -430,6 +428,8 @@ _bfd_add_merge_section (bfd *abfd, void
secinfo->first_str = NULL;
sec->rawsize = sec->size;
+ if (sec->flags & SEC_STRINGS)
+ memset (secinfo->contents + sec->size, 0, sec->entsize);
if (! bfd_get_section_contents (sec->owner, sec, secinfo->contents,
0, sec->size))
goto error_return;
@@ -443,8 +443,7 @@ _bfd_add_merge_section (bfd *abfd, void
/* Record one section into the hash table. */
static bfd_boolean
-record_section (bfd *abfd,
- struct sec_merge_info *sinfo,
+record_section (struct sec_merge_info *sinfo,
struct sec_merge_sec_info *secinfo)
{
asection *sec = secinfo->sec;
@@ -521,10 +520,6 @@ record_section (bfd *abfd,
return TRUE;
error_return:
- if (bfd_get_error () != bfd_error_no_memory)
- (*_bfd_error_handler)
- (_("%B: unterminated string in section `%A' marked for merging"),
- abfd, sec);
for (secinfo = sinfo->chain; secinfo; secinfo = secinfo->next)
*secinfo->psecinfo = NULL;
return FALSE;
@@ -734,7 +729,7 @@ _bfd_merge_sections (bfd *abfd,
if (remove_hook)
(*remove_hook) (abfd, secinfo->sec);
}
- else if (! record_section (abfd, sinfo, secinfo))
+ else if (! record_section (sinfo, secinfo))
break;
if (secinfo)
@@ -823,7 +818,7 @@ _bfd_merged_section_offset (bfd *output_
secinfo = (struct sec_merge_sec_info *) psecinfo;
if (!secinfo)
- return 0;
+ return offset;
if (offset >= sec->rawsize)
{
@@ -867,7 +862,7 @@ _bfd_merged_section_offset (bfd *output_
{
p = secinfo->contents + (offset / sec->entsize) * sec->entsize;
}
- entry = sec_merge_hash_lookup (secinfo->htab, (char *) p, NULL, 0, FALSE);
+ entry = sec_merge_hash_lookup (secinfo->htab, (char *) p, 0, FALSE);
if (!entry)
{
if (! secinfo->htab->strings)
--
Alan Modra
Australia Development Lab, IBM