* [PATCH] lto: fix LTO debug sections copying.
@ 2020-10-05 15:20 Martin Liška
2020-10-05 15:22 ` Martin Liška
0 siblings, 1 reply; 10+ messages in thread
From: Martin Liška @ 2020-10-05 15:20 UTC (permalink / raw)
To: gcc-patches
As seen in the PR, we get to situation where we have a big number
of symbols (~125K) and thus we reach .symtab_shndx section usage.
For .symtab we get the following sh_link:
(gdb) p strtab
$1 = 81997
readelf -S prints:
There are 81999 section headers, starting at offset 0x1f488060:
Section Headers:
[Nr] Name Type Address Off Size ES Flg Lk Inf Al
[ 0] NULL 0000000000000000 000000 01404f 00 81998 0 0
[ 1] .group GROUP 0000000000000000 000040 000008 04 81995 105027 4
...
[81995] .symtab SYMTAB 0000000000000000 d5d9298 2db310 18 81997 105026 8
[81996] .symtab_shndx SYMTAB SECTION INDICES 0000000000000000 d8b45a8 079dd8 04 81995 0 4
...
Apparently the index is starting from 1 and as we skip first section
│ 1118 /* Read the section headers. We skip section 0, which is not a
│ 1119 useful section. */
thus we need to subtract 2.
I run lto.exp and it's fine.
Ready for master?
Thanks,
Martin
libiberty/ChangeLog:
PR lto/97290
* simple-object-elf.c (simple_object_elf_copy_lto_debug_sections): Fix off-by-one error.
---
libiberty/simple-object-elf.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/libiberty/simple-object-elf.c b/libiberty/simple-object-elf.c
index 7c9d492f6a4..ce3e809e1e0 100644
--- a/libiberty/simple-object-elf.c
+++ b/libiberty/simple-object-elf.c
@@ -1380,11 +1380,16 @@ simple_object_elf_copy_lto_debug_sections (simple_object_read *sobj,
/* Read the section index table if present. */
if (symtab_indices_shndx[i - 1] != 0)
{
- unsigned char *sidxhdr = shdrs + (strtab - 1) * shdr_size;
+ unsigned char *sidxhdr = shdrs + (strtab - 2) * shdr_size;
off_t sidxoff = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
sidxhdr, sh_offset, Elf_Addr);
size_t sidxsz = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
sidxhdr, sh_size, Elf_Addr);
+ unsigned int shndx_type
+ = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
+ sidxhdr, sh_type, Elf_Word);
+ if (shndx_type != SHT_SYMTAB_SHNDX)
+ return "Wrong section type of a SYMTAB SECTION INDICES section";
shndx_table = (unsigned *)XNEWVEC (char, sidxsz);
simple_object_internal_read (sobj->descriptor,
sobj->offset + sidxoff,
--
2.28.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] lto: fix LTO debug sections copying.
2020-10-05 15:20 [PATCH] lto: fix LTO debug sections copying Martin Liška
@ 2020-10-05 15:22 ` Martin Liška
2020-10-05 16:09 ` Martin Liška
0 siblings, 1 reply; 10+ messages in thread
From: Martin Liška @ 2020-10-05 15:22 UTC (permalink / raw)
To: gcc-patches
Adding Ian (and Richi) to CC.
On 10/5/20 5:20 PM, Martin Liška wrote:
> As seen in the PR, we get to situation where we have a big number
> of symbols (~125K) and thus we reach .symtab_shndx section usage.
> For .symtab we get the following sh_link:
>
> (gdb) p strtab
> $1 = 81997
>
> readelf -S prints:
>
> There are 81999 section headers, starting at offset 0x1f488060:
>
> Section Headers:
> [Nr] Name Type Address Off Size ES Flg Lk Inf Al
> [ 0] NULL 0000000000000000 000000 01404f 00 81998 0 0
> [ 1] .group GROUP 0000000000000000 000040 000008 04 81995 105027 4
> ...
> [81995] .symtab SYMTAB 0000000000000000 d5d9298 2db310 18 81997 105026 8
> [81996] .symtab_shndx SYMTAB SECTION INDICES 0000000000000000 d8b45a8 079dd8 04 81995 0 4
> ...
>
> Apparently the index is starting from 1 and as we skip first section
>
> │ 1118 /* Read the section headers. We skip section 0, which is not a
> │ 1119 useful section. */
>
> thus we need to subtract 2.
>
> I run lto.exp and it's fine.
> Ready for master?
> Thanks,
> Martin
>
> libiberty/ChangeLog:
>
> PR lto/97290
> * simple-object-elf.c (simple_object_elf_copy_lto_debug_sections): Fix off-by-one error.
> ---
> libiberty/simple-object-elf.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libiberty/simple-object-elf.c b/libiberty/simple-object-elf.c
> index 7c9d492f6a4..ce3e809e1e0 100644
> --- a/libiberty/simple-object-elf.c
> +++ b/libiberty/simple-object-elf.c
> @@ -1380,11 +1380,16 @@ simple_object_elf_copy_lto_debug_sections (simple_object_read *sobj,
> /* Read the section index table if present. */
> if (symtab_indices_shndx[i - 1] != 0)
> {
> - unsigned char *sidxhdr = shdrs + (strtab - 1) * shdr_size;
> + unsigned char *sidxhdr = shdrs + (strtab - 2) * shdr_size;
> off_t sidxoff = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
> sidxhdr, sh_offset, Elf_Addr);
> size_t sidxsz = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
> sidxhdr, sh_size, Elf_Addr);
> + unsigned int shndx_type
> + = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
> + sidxhdr, sh_type, Elf_Word);
> + if (shndx_type != SHT_SYMTAB_SHNDX)
> + return "Wrong section type of a SYMTAB SECTION INDICES section";
> shndx_table = (unsigned *)XNEWVEC (char, sidxsz);
> simple_object_internal_read (sobj->descriptor,
> sobj->offset + sidxoff,
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] lto: fix LTO debug sections copying.
2020-10-05 15:22 ` Martin Liška
@ 2020-10-05 16:09 ` Martin Liška
2020-10-05 16:34 ` Ian Lance Taylor
0 siblings, 1 reply; 10+ messages in thread
From: Martin Liška @ 2020-10-05 16:09 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 95 bytes --]
Hi.
The previous patch was not correct. This one should be.
Ready for master?
Thanks,
Martin
[-- Attachment #2: 0001-lto-fix-LTO-debug-sections-copying.patch --]
[-- Type: text/x-patch, Size: 3704 bytes --]
From a96f7ae39b5d56ce886edf1bfb9ca6475a857652 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Mon, 5 Oct 2020 18:03:08 +0200
Subject: [PATCH] lto: fix LTO debug sections copying.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
readelf -S prints:
There are 81999 section headers, starting at offset 0x1f488060:
Section Headers:
[Nr] Name Type Address Off Size ES Flg Lk Inf Al
[ 0] NULL 0000000000000000 000000 01404f 00 81998 0 0
[ 1] .group GROUP 0000000000000000 000040 000008 04 81995 105027 4
...
[81995] .symtab SYMTAB 0000000000000000 d5d9298 2db310 18 81997 105026 8
[81996] .symtab_shndx SYMTAB SECTION INDICES 0000000000000000 d8b45a8 079dd8 04 81995 0 4
[81997] .strtab STRTAB 0000000000000000 d92e380 80460c 00 0 0 1
...
Looking at the documentation:
Table 7–15 ELF sh_link and sh_info Interpretation
sh_type - sh_link
SHT_SYMTAB - The section header index of the associated string table.
SHT_SYMTAB_SHNDX - The section header index of the associated symbol table.
As seen, sh_link of a SHT_SYMTAB always points to a .strtab and readelf
confirms that.
So we need to use reverse mapping taken from
[81996] .symtab_shndx SYMTAB SECTION INDICES 0000000000000000 d8b45a8 079dd8 04 81995 0 4
where sh_link points to 81995.
libiberty/ChangeLog:
PR lto/97290
* simple-object-elf.c (simple_object_elf_copy_lto_debug_sections):
Use sh_link of a .symtab_shndx section.
---
libiberty/simple-object-elf.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/libiberty/simple-object-elf.c b/libiberty/simple-object-elf.c
index 7c9d492f6a4..37e73348cb7 100644
--- a/libiberty/simple-object-elf.c
+++ b/libiberty/simple-object-elf.c
@@ -1191,7 +1191,7 @@ simple_object_elf_copy_lto_debug_sections (simple_object_read *sobj,
unsigned int sh_link;
sh_link = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
shdr, sh_link, Elf_Word);
- symtab_indices_shndx[sh_link - 1] = i;
+ symtab_indices_shndx[sh_link - 1] = i - 1;
/* Always discard the extended index sections, after
copying it will not be needed. This way we don't need to
update it and deal with the ordering constraints of
@@ -1372,19 +1372,22 @@ simple_object_elf_copy_lto_debug_sections (simple_object_read *sobj,
{
unsigned entsize = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
shdr, sh_entsize, Elf_Addr);
- unsigned strtab = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
- shdr, sh_link, Elf_Word);
size_t prevailing_name_idx = 0;
unsigned char *ent;
unsigned *shndx_table = NULL;
/* Read the section index table if present. */
if (symtab_indices_shndx[i - 1] != 0)
{
- unsigned char *sidxhdr = shdrs + (strtab - 1) * shdr_size;
+ unsigned char *sidxhdr = shdrs + symtab_indices_shndx[i - 1] * shdr_size;
off_t sidxoff = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
sidxhdr, sh_offset, Elf_Addr);
size_t sidxsz = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
sidxhdr, sh_size, Elf_Addr);
+ unsigned int shndx_type
+ = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
+ sidxhdr, sh_type, Elf_Word);
+ if (shndx_type != SHT_SYMTAB_SHNDX)
+ return "Wrong section type of a SYMTAB SECTION INDICES section";
shndx_table = (unsigned *)XNEWVEC (char, sidxsz);
simple_object_internal_read (sobj->descriptor,
sobj->offset + sidxoff,
--
2.28.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] lto: fix LTO debug sections copying.
2020-10-05 16:09 ` Martin Liška
@ 2020-10-05 16:34 ` Ian Lance Taylor
2020-10-06 7:01 ` Martin Liška
0 siblings, 1 reply; 10+ messages in thread
From: Ian Lance Taylor @ 2020-10-05 16:34 UTC (permalink / raw)
To: Martin Liška; +Cc: gcc-patches, Richard Biener
On Mon, Oct 5, 2020 at 9:09 AM Martin Liška <mliska@suse.cz> wrote:
>
> The previous patch was not correct. This one should be.
>
> Ready for master?
I don't understand why this code uses symtab_indices_shndx at all.
There should only be one SHT_SYMTAB_SHNDX section. There shouldn't be
any need for the symtab_indices_shndx vector.
But in any case this patch looks OK.
Thanks.
Ian
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] lto: fix LTO debug sections copying.
2020-10-05 16:34 ` Ian Lance Taylor
@ 2020-10-06 7:01 ` Martin Liška
2020-10-06 8:00 ` Richard Biener
0 siblings, 1 reply; 10+ messages in thread
From: Martin Liška @ 2020-10-06 7:01 UTC (permalink / raw)
To: Ian Lance Taylor; +Cc: gcc-patches, Richard Biener
On 10/5/20 6:34 PM, Ian Lance Taylor wrote:
> On Mon, Oct 5, 2020 at 9:09 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> The previous patch was not correct. This one should be.
>>
>> Ready for master?
>
> I don't understand why this code uses symtab_indices_shndx at all.
> There should only be one SHT_SYMTAB_SHNDX section. There shouldn't be
> any need for the symtab_indices_shndx vector.
Well, the question is if we can have multiple .symtab sections in one ELF
file? Theoretically yes, so we should also handle SHT_SYMTAB_SHNDX sections.
Note that the original usage of the SHT_SYMTAB_SHNDX section was motivated
by PR81968 which is about Solaris ld.
>
> But in any case this patch looks OK.
Waiting for a feedback from Richi.
Thanks,
Martin
>
> Thanks.
>
> Ian
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] lto: fix LTO debug sections copying.
2020-10-06 7:01 ` Martin Liška
@ 2020-10-06 8:00 ` Richard Biener
2020-10-06 10:20 ` Martin Liška
0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2020-10-06 8:00 UTC (permalink / raw)
To: Martin Liška, Rainer Orth; +Cc: Ian Lance Taylor, gcc-patches
On Tue, Oct 6, 2020 at 9:01 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 10/5/20 6:34 PM, Ian Lance Taylor wrote:
> > On Mon, Oct 5, 2020 at 9:09 AM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> The previous patch was not correct. This one should be.
> >>
> >> Ready for master?
> >
> > I don't understand why this code uses symtab_indices_shndx at all.
> > There should only be one SHT_SYMTAB_SHNDX section. There shouldn't be
> > any need for the symtab_indices_shndx vector.
>
> Well, the question is if we can have multiple .symtab sections in one ELF
> file? Theoretically yes, so we should also handle SHT_SYMTAB_SHNDX sections.
> Note that the original usage of the SHT_SYMTAB_SHNDX section was motivated
> by PR81968 which is about Solaris ld.
It wasn't my code but I suppose this way the implementation was
"easiest". There
should be exactly one symtab / shndx section. Rainer authored this support.
> >
> > But in any case this patch looks OK.
I also think the patch looks OK. Rainer?
Richard.
> Waiting for a feedback from Richi.
>
> Thanks,
> Martin
>
> >
> > Thanks.
> >
> > Ian
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] lto: fix LTO debug sections copying.
2020-10-06 8:00 ` Richard Biener
@ 2020-10-06 10:20 ` Martin Liška
2020-10-06 14:23 ` Jakub Jelinek
2020-10-06 17:34 ` Ian Lance Taylor
0 siblings, 2 replies; 10+ messages in thread
From: Martin Liška @ 2020-10-06 10:20 UTC (permalink / raw)
To: Richard Biener, Rainer Orth; +Cc: Ian Lance Taylor, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]
On 10/6/20 10:00 AM, Richard Biener wrote:
> On Tue, Oct 6, 2020 at 9:01 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 10/5/20 6:34 PM, Ian Lance Taylor wrote:
>>> On Mon, Oct 5, 2020 at 9:09 AM Martin Liška <mliska@suse.cz> wrote:
>>>>
>>>> The previous patch was not correct. This one should be.
>>>>
>>>> Ready for master?
>>>
>>> I don't understand why this code uses symtab_indices_shndx at all.
>>> There should only be one SHT_SYMTAB_SHNDX section. There shouldn't be
>>> any need for the symtab_indices_shndx vector.
>>
>> Well, the question is if we can have multiple .symtab sections in one ELF
>> file? Theoretically yes, so we should also handle SHT_SYMTAB_SHNDX sections.
>> Note that the original usage of the SHT_SYMTAB_SHNDX section was motivated
>> by PR81968 which is about Solaris ld.
>
> It wasn't my code but I suppose this way the implementation was
> "easiest". There
> should be exactly one symtab / shndx section. Rainer authored this support.
If we expect at maximum one SHT_SYMTAB_SHNDX section section, then I'm suggesting
an updated version of the patch. It's what Ian offered.
Thoughts?
Martin
>
>>>
>>> But in any case this patch looks OK.
>
> I also think the patch looks OK. Rainer?
>
> Richard.
>
>> Waiting for a feedback from Richi.
>>
>> Thanks,
>> Martin
>>
>>>
>>> Thanks.
>>>
>>> Ian
>>>
>>
[-- Attachment #2: 0001-lto-fix-LTO-debug-sections-copying.patch --]
[-- Type: text/x-patch, Size: 5175 bytes --]
From bb259b4dc2a79ef45d449896d05855122ecc2ef9 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Mon, 5 Oct 2020 18:03:08 +0200
Subject: [PATCH] lto: fix LTO debug sections copying.
readelf -S prints:
There are 81999 section headers, starting at offset 0x1f488060:
Section Headers:
[Nr] Name Type Address Off Size ES Flg Lk Inf Al
[ 0] NULL 0000000000000000 000000 01404f 00 81998 0 0
[ 1] .group GROUP 0000000000000000 000040 000008 04 81995 105027 4
...
[81995] .symtab SYMTAB 0000000000000000 d5d9298 2db310 18 81997 105026 8
[81996] .symtab_shndx SYMTAB SECTION INDICES 0000000000000000 d8b45a8 079dd8 04 81995 0 4
[81997] .strtab STRTAB 0000000000000000 d92e380 80460c 00 0 0 1
...
Expect only at maximum one .symtab_shndx section.
libiberty/ChangeLog:
PR lto/97290
* simple-object-elf.c (simple_object_elf_copy_lto_debug_sections):
Expect only one .symtab_shndx section.
---
libiberty/simple-object-elf.c | 28 +++++++++++-----------------
1 file changed, 11 insertions(+), 17 deletions(-)
diff --git a/libiberty/simple-object-elf.c b/libiberty/simple-object-elf.c
index 7c9d492f6a4..6dc5c60a842 100644
--- a/libiberty/simple-object-elf.c
+++ b/libiberty/simple-object-elf.c
@@ -1109,7 +1109,7 @@ simple_object_elf_copy_lto_debug_sections (simple_object_read *sobj,
unsigned new_i;
unsigned *sh_map;
unsigned first_shndx = 0;
- unsigned int *symtab_indices_shndx;
+ unsigned int symtab_shndx = 0;
shdr_size = (ei_class == ELFCLASS32
? sizeof (Elf32_External_Shdr)
@@ -1151,9 +1151,6 @@ simple_object_elf_copy_lto_debug_sections (simple_object_read *sobj,
pfnret = XNEWVEC (int, shnum);
pfnname = XNEWVEC (const char *, shnum);
- /* Map of symtab to index section. */
- symtab_indices_shndx = XCNEWVEC (unsigned int, shnum - 1);
-
/* First perform the callbacks to know which sections to preserve and
what name to use for those. */
for (i = 1; i < shnum; ++i)
@@ -1188,10 +1185,9 @@ simple_object_elf_copy_lto_debug_sections (simple_object_read *sobj,
shdr, sh_type, Elf_Word);
if (sh_type == SHT_SYMTAB_SHNDX)
{
- unsigned int sh_link;
- sh_link = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
- shdr, sh_link, Elf_Word);
- symtab_indices_shndx[sh_link - 1] = i;
+ if (symtab_shndx != 0)
+ return "Multiple SYMTAB SECTION INDICES sections";
+ symtab_shndx = i - 1;
/* Always discard the extended index sections, after
copying it will not be needed. This way we don't need to
update it and deal with the ordering constraints of
@@ -1323,7 +1319,6 @@ simple_object_elf_copy_lto_debug_sections (simple_object_read *sobj,
*err = 0;
XDELETEVEC (names);
XDELETEVEC (shdrs);
- XDELETEVEC (symtab_indices_shndx);
return "ELF section name out of range";
}
@@ -1341,7 +1336,6 @@ simple_object_elf_copy_lto_debug_sections (simple_object_read *sobj,
{
XDELETEVEC (names);
XDELETEVEC (shdrs);
- XDELETEVEC (symtab_indices_shndx);
return errmsg;
}
@@ -1362,7 +1356,6 @@ simple_object_elf_copy_lto_debug_sections (simple_object_read *sobj,
XDELETEVEC (buf);
XDELETEVEC (names);
XDELETEVEC (shdrs);
- XDELETEVEC (symtab_indices_shndx);
return errmsg;
}
@@ -1372,19 +1365,22 @@ simple_object_elf_copy_lto_debug_sections (simple_object_read *sobj,
{
unsigned entsize = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
shdr, sh_entsize, Elf_Addr);
- unsigned strtab = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
- shdr, sh_link, Elf_Word);
size_t prevailing_name_idx = 0;
unsigned char *ent;
unsigned *shndx_table = NULL;
/* Read the section index table if present. */
- if (symtab_indices_shndx[i - 1] != 0)
+ if (symtab_shndx != 0)
{
- unsigned char *sidxhdr = shdrs + (strtab - 1) * shdr_size;
+ unsigned char *sidxhdr = shdrs + symtab_shndx * shdr_size;
off_t sidxoff = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
sidxhdr, sh_offset, Elf_Addr);
size_t sidxsz = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
sidxhdr, sh_size, Elf_Addr);
+ unsigned int shndx_type
+ = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
+ sidxhdr, sh_type, Elf_Word);
+ if (shndx_type != SHT_SYMTAB_SHNDX)
+ return "Wrong section type of a SYMTAB SECTION INDICES section";
shndx_table = (unsigned *)XNEWVEC (char, sidxsz);
simple_object_internal_read (sobj->descriptor,
sobj->offset + sidxoff,
@@ -1543,7 +1539,6 @@ simple_object_elf_copy_lto_debug_sections (simple_object_read *sobj,
{
XDELETEVEC (names);
XDELETEVEC (shdrs);
- XDELETEVEC (symtab_indices_shndx);
return errmsg;
}
@@ -1585,7 +1580,6 @@ simple_object_elf_copy_lto_debug_sections (simple_object_read *sobj,
XDELETEVEC (shdrs);
XDELETEVEC (pfnret);
XDELETEVEC (pfnname);
- XDELETEVEC (symtab_indices_shndx);
XDELETEVEC (sh_map);
return NULL;
--
2.28.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] lto: fix LTO debug sections copying.
2020-10-06 10:20 ` Martin Liška
@ 2020-10-06 14:23 ` Jakub Jelinek
2020-10-06 17:34 ` Ian Lance Taylor
1 sibling, 0 replies; 10+ messages in thread
From: Jakub Jelinek @ 2020-10-06 14:23 UTC (permalink / raw)
To: Martin Liška; +Cc: Richard Biener, Rainer Orth, gcc-patches
On Tue, Oct 06, 2020 at 12:20:14PM +0200, Martin Liška wrote:
> On 10/6/20 10:00 AM, Richard Biener wrote:
> > On Tue, Oct 6, 2020 at 9:01 AM Martin Liška <mliska@suse.cz> wrote:
> > >
> > > On 10/5/20 6:34 PM, Ian Lance Taylor wrote:
> > > > On Mon, Oct 5, 2020 at 9:09 AM Martin Liška <mliska@suse.cz> wrote:
> > > > >
> > > > > The previous patch was not correct. This one should be.
> > > > >
> > > > > Ready for master?
> > > >
> > > > I don't understand why this code uses symtab_indices_shndx at all.
> > > > There should only be one SHT_SYMTAB_SHNDX section. There shouldn't be
> > > > any need for the symtab_indices_shndx vector.
> > >
> > > Well, the question is if we can have multiple .symtab sections in one ELF
> > > file? Theoretically yes, so we should also handle SHT_SYMTAB_SHNDX sections.
> > > Note that the original usage of the SHT_SYMTAB_SHNDX section was motivated
> > > by PR81968 which is about Solaris ld.
> >
> > It wasn't my code but I suppose this way the implementation was
> > "easiest". There
> > should be exactly one symtab / shndx section. Rainer authored this support.
>
> If we expect at maximum one SHT_SYMTAB_SHNDX section section, then I'm suggesting
> an updated version of the patch. It's what Ian offered.
gABI says on
SHT_SYMTAB/SHT_DYNSYM:
Currently, an object file may have only one section of each type, but this restriction may be relaxed in the future.
SHT_SYMTAB_SHNDX:
This section is associated with a symbol table section and is required if any of the section header indexes referenced by that symbol table contain the escape value SHN_XINDEX.
So, I guess only at most one SHT_SYMTAB_SHNDX can appear in ET_REL objects
which we are talking about, and at most two SHT_SYMTAB_SHNDX in
ET_EXEC/ET_DYN (though only in the very unlikely case that the binary/dso
contains more than 65536-epsilon sections and both .symtab and .dynsym need
to refer to those. One would need to play with linker scripts to convince
ld.bfd to create many sections in ET_EXEC/ET_DYN.
Jakub
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] lto: fix LTO debug sections copying.
2020-10-06 10:20 ` Martin Liška
2020-10-06 14:23 ` Jakub Jelinek
@ 2020-10-06 17:34 ` Ian Lance Taylor
2020-10-07 6:56 ` Martin Liška
1 sibling, 1 reply; 10+ messages in thread
From: Ian Lance Taylor @ 2020-10-06 17:34 UTC (permalink / raw)
To: Martin Liška; +Cc: Richard Biener, Rainer Orth, gcc-patches
On Tue, Oct 6, 2020 at 3:20 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 10/6/20 10:00 AM, Richard Biener wrote:
> > On Tue, Oct 6, 2020 at 9:01 AM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> On 10/5/20 6:34 PM, Ian Lance Taylor wrote:
> >>> On Mon, Oct 5, 2020 at 9:09 AM Martin Liška <mliska@suse.cz> wrote:
> >>>>
> >>>> The previous patch was not correct. This one should be.
> >>>>
> >>>> Ready for master?
> >>>
> >>> I don't understand why this code uses symtab_indices_shndx at all.
> >>> There should only be one SHT_SYMTAB_SHNDX section. There shouldn't be
> >>> any need for the symtab_indices_shndx vector.
> >>
> >> Well, the question is if we can have multiple .symtab sections in one ELF
> >> file? Theoretically yes, so we should also handle SHT_SYMTAB_SHNDX sections.
> >> Note that the original usage of the SHT_SYMTAB_SHNDX section was motivated
> >> by PR81968 which is about Solaris ld.
> >
> > It wasn't my code but I suppose this way the implementation was
> > "easiest". There
> > should be exactly one symtab / shndx section. Rainer authored this support.
>
> If we expect at maximum one SHT_SYMTAB_SHNDX section section, then I'm suggesting
> an updated version of the patch. It's what Ian offered.
This is OK with me with one minor change.
> + return "Multiple SYMTAB SECTION INDICES sections";
I think simply "More than one SHT_SYMTAB_SHNDX section". SYMTAB
SECTION INDICES doesn't mean anything to me, and at least people can
do a web search for SHT_SYMTAB_SHNDX.
Thanks.
Ian
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] lto: fix LTO debug sections copying.
2020-10-06 17:34 ` Ian Lance Taylor
@ 2020-10-07 6:56 ` Martin Liška
0 siblings, 0 replies; 10+ messages in thread
From: Martin Liška @ 2020-10-07 6:56 UTC (permalink / raw)
To: Ian Lance Taylor; +Cc: Richard Biener, Rainer Orth, gcc-patches
On 10/6/20 7:34 PM, Ian Lance Taylor wrote:
> On Tue, Oct 6, 2020 at 3:20 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 10/6/20 10:00 AM, Richard Biener wrote:
>>> On Tue, Oct 6, 2020 at 9:01 AM Martin Liška <mliska@suse.cz> wrote:
>>>>
>>>> On 10/5/20 6:34 PM, Ian Lance Taylor wrote:
>>>>> On Mon, Oct 5, 2020 at 9:09 AM Martin Liška <mliska@suse.cz> wrote:
>>>>>>
>>>>>> The previous patch was not correct. This one should be.
>>>>>>
>>>>>> Ready for master?
>>>>>
>>>>> I don't understand why this code uses symtab_indices_shndx at all.
>>>>> There should only be one SHT_SYMTAB_SHNDX section. There shouldn't be
>>>>> any need for the symtab_indices_shndx vector.
>>>>
>>>> Well, the question is if we can have multiple .symtab sections in one ELF
>>>> file? Theoretically yes, so we should also handle SHT_SYMTAB_SHNDX sections.
>>>> Note that the original usage of the SHT_SYMTAB_SHNDX section was motivated
>>>> by PR81968 which is about Solaris ld.
>>>
>>> It wasn't my code but I suppose this way the implementation was
>>> "easiest". There
>>> should be exactly one symtab / shndx section. Rainer authored this support.
>>
>> If we expect at maximum one SHT_SYMTAB_SHNDX section section, then I'm suggesting
>> an updated version of the patch. It's what Ian offered.
>
> This is OK with me with one minor change.
>
>> + return "Multiple SYMTAB SECTION INDICES sections";
>
> I think simply "More than one SHT_SYMTAB_SHNDX section". SYMTAB
> SECTION INDICES doesn't mean anything to me, and at least people can
> do a web search for SHT_SYMTAB_SHNDX.
Ok, so I decided to install the first version of the patch.
Thanks for review,
Martin
>
> Thanks.
>
> Ian
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-10-07 6:56 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-05 15:20 [PATCH] lto: fix LTO debug sections copying Martin Liška
2020-10-05 15:22 ` Martin Liška
2020-10-05 16:09 ` Martin Liška
2020-10-05 16:34 ` Ian Lance Taylor
2020-10-06 7:01 ` Martin Liška
2020-10-06 8:00 ` Richard Biener
2020-10-06 10:20 ` Martin Liška
2020-10-06 14:23 ` Jakub Jelinek
2020-10-06 17:34 ` Ian Lance Taylor
2020-10-07 6:56 ` Martin Liška
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).