* [PATCH][Binutils][Arm] BFD: Exclude sections with no content from compress check.
@ 2020-04-21 13:41 Tamar Christina
2020-04-21 13:58 ` Alan Modra
2020-04-21 14:29 ` Nick Clifton
0 siblings, 2 replies; 6+ messages in thread
From: Tamar Christina @ 2020-04-21 13:41 UTC (permalink / raw)
To: binutils; +Cc: nd, Richard.Earnshaw, nickc, ramana.radhakrishnan
[-- Attachment #1: Type: text/plain, Size: 1247 bytes --]
Hi All,
The check in bfd_get_full_section_contents is trying to check that we don't
allocate more space for a section than the size of the section is on disk.
Previously we excluded linker created sections since they didn't have a size on
disk. However we also need to exclude sections with no content as well such as
the BSS section. Space for these would not have been allocated by the assembler
and so the check would incorrectly fail.
build on native hardware and regtested on
aarch64-none-elf, aarch64-none-elf (32 bit host),
aarch64-none-linux-gnu, aarch64-none-linux-gnu (32 bit host),
arm-none-eabi, arm-none-eabi (32 bit host),
arm-none-linux-gnueabihf, arm-none-linux-gnueabihf (32 bit host)
Cross-compiled and regtested on
aarch64-none-linux-gnu, aarch64_be-none-linux-gnu,
arm-none-eabi, armeb-none-eabi, arm-wince-pe
and no issues.
Ok for master? and for backport to binutils-2.34?
Thanks,
Tamar
bfd/ChangeLog:
2020-04-21 Tamar Christina <tamar.christina@arm.com>
* compress.c (bfd_get_full_section_contents): Exclude sections with no
content.
gas/ChangeLog:
2020-04-21 Tamar Christina <tamar.christina@arm.com>
* testsuite/gas/arm/pr24753.d: New test.
* testsuite/gas/arm/pr24753.s: New test.
--
[-- Attachment #2: rb12947.patch --]
[-- Type: text/x-diff, Size: 1614 bytes --]
diff --git a/bfd/compress.c b/bfd/compress.c
index ce6bb2beaee689d0eb700f69a08e7eb5fe92ab5c..0f04912cb3723eec77fc3af1a015a1eb8472a9de 100644
--- a/bfd/compress.c
+++ b/bfd/compress.c
@@ -255,6 +255,9 @@ bfd_get_full_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **ptr)
/* PR 24753: Linker created sections can be larger than
the file size, eg if they are being used to hold stubs. */
&& (bfd_section_flags (sec) & SEC_LINKER_CREATED) == 0
+ /* PR 24753: Sections which have no content should also be excluded as
+ they contain no size on disk. */
+ && (bfd_section_flags (sec) & SEC_HAS_CONTENTS) == SEC_HAS_CONTENTS
/* The MMO file format supports its own special compression
technique, but it uses COMPRESS_SECTION_NONE when loading
a section's contents. */
diff --git a/gas/testsuite/gas/arm/pr24753.d b/gas/testsuite/gas/arm/pr24753.d
new file mode 100644
index 0000000000000000000000000000000000000000..01990d1ff517c5c39e8d68c084f97ed897e24382
--- /dev/null
+++ b/gas/testsuite/gas/arm/pr24753.d
@@ -0,0 +1,7 @@
+#skip: *-*-pe *-*-wince *-*-vxworks
+#objdump: -d
+#name: PR24753: Don't error on sections with no content size mismatch with file
+
+.*: +file format .*arm.*
+
+#...
diff --git a/gas/testsuite/gas/arm/pr24753.s b/gas/testsuite/gas/arm/pr24753.s
new file mode 100644
index 0000000000000000000000000000000000000000..5ba33fd29c7c95f02d422ea4f3f6f4159af91856
--- /dev/null
+++ b/gas/testsuite/gas/arm/pr24753.s
@@ -0,0 +1,12 @@
+.text
+.global _start
+_start:
+ nop
+
+.section .text2, "ax", %progbits
+_func:
+ nop
+
+.bss
+.fill 0x8000
+
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][Binutils][Arm] BFD: Exclude sections with no content from compress check.
2020-04-21 13:41 [PATCH][Binutils][Arm] BFD: Exclude sections with no content from compress check Tamar Christina
@ 2020-04-21 13:58 ` Alan Modra
2020-04-21 14:01 ` Tamar Christina
2020-04-21 14:29 ` Nick Clifton
1 sibling, 1 reply; 6+ messages in thread
From: Alan Modra @ 2020-04-21 13:58 UTC (permalink / raw)
To: Tamar Christina; +Cc: binutils, Richard.Earnshaw, nd, ramana.radhakrishnan
On Tue, Apr 21, 2020 at 02:41:52PM +0100, Tamar Christina wrote:
> Hi All,
>
> The check in bfd_get_full_section_contents is trying to check that we don't
> allocate more space for a section than the size of the section is on disk.
>
> Previously we excluded linker created sections since they didn't have a size on
> disk. However we also need to exclude sections with no content as well such as
> the BSS section. Space for these would not have been allocated by the assembler
> and so the check would incorrectly fail.
>
> build on native hardware and regtested on
> aarch64-none-elf, aarch64-none-elf (32 bit host),
> aarch64-none-linux-gnu, aarch64-none-linux-gnu (32 bit host),
> arm-none-eabi, arm-none-eabi (32 bit host),
> arm-none-linux-gnueabihf, arm-none-linux-gnueabihf (32 bit host)
>
> Cross-compiled and regtested on
> aarch64-none-linux-gnu, aarch64_be-none-linux-gnu,
> arm-none-eabi, armeb-none-eabi, arm-wince-pe
>
> and no issues.
>
> Ok for master? and for backport to binutils-2.34?
OK for both, except
> + && (bfd_section_flags (sec) & SEC_HAS_CONTENTS) == SEC_HAS_CONTENTS
the line is more than 80 chars, so we usually would write
&& (bfd_section_flags (sec) & SEC_HAS_CONTENTS) != 0
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH][Binutils][Arm] BFD: Exclude sections with no content from compress check.
2020-04-21 13:58 ` Alan Modra
@ 2020-04-21 14:01 ` Tamar Christina
0 siblings, 0 replies; 6+ messages in thread
From: Tamar Christina @ 2020-04-21 14:01 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils, Richard Earnshaw, nd, Ramana Radhakrishnan
Thanks Alan,
Sorry I hadn't noticed the long line, I'll correct and commit.
Thanks,
Tamar
> -----Original Message-----
> From: Alan Modra <amodra@gmail.com>
> Sent: Tuesday, April 21, 2020 2:59 PM
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: binutils@sourceware.org; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; nd <nd@arm.com>; Ramana Radhakrishnan
> <Ramana.Radhakrishnan@arm.com>
> Subject: Re: [PATCH][Binutils][Arm] BFD: Exclude sections with no content
> from compress check.
>
> On Tue, Apr 21, 2020 at 02:41:52PM +0100, Tamar Christina wrote:
> > Hi All,
> >
> > The check in bfd_get_full_section_contents is trying to check that we
> > don't allocate more space for a section than the size of the section is on
> disk.
> >
> > Previously we excluded linker created sections since they didn't have
> > a size on disk. However we also need to exclude sections with no
> > content as well such as the BSS section. Space for these would not
> > have been allocated by the assembler and so the check would incorrectly
> fail.
> >
> > build on native hardware and regtested on
> > aarch64-none-elf, aarch64-none-elf (32 bit host),
> > aarch64-none-linux-gnu, aarch64-none-linux-gnu (32 bit host),
> > arm-none-eabi, arm-none-eabi (32 bit host),
> > arm-none-linux-gnueabihf, arm-none-linux-gnueabihf (32 bit host)
> >
> > Cross-compiled and regtested on
> > aarch64-none-linux-gnu, aarch64_be-none-linux-gnu,
> > arm-none-eabi, armeb-none-eabi, arm-wince-pe
> >
> > and no issues.
> >
> > Ok for master? and for backport to binutils-2.34?
>
> OK for both, except
>
> > + && (bfd_section_flags (sec) & SEC_HAS_CONTENTS) ==
> > +SEC_HAS_CONTENTS
> the line is more than 80 chars, so we usually would write
> && (bfd_section_flags (sec) & SEC_HAS_CONTENTS) != 0
>
> --
> Alan Modra
> Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][Binutils][Arm] BFD: Exclude sections with no content from compress check.
2020-04-21 13:41 [PATCH][Binutils][Arm] BFD: Exclude sections with no content from compress check Tamar Christina
2020-04-21 13:58 ` Alan Modra
@ 2020-04-21 14:29 ` Nick Clifton
2020-04-21 15:50 ` Tamar Christina
1 sibling, 1 reply; 6+ messages in thread
From: Nick Clifton @ 2020-04-21 14:29 UTC (permalink / raw)
To: Tamar Christina, binutils; +Cc: nd, Richard.Earnshaw, ramana.radhakrishnan
Hi Tamar,
> Cross-compiled and regtested on
> aarch64-none-linux-gnu, aarch64_be-none-linux-gnu,
> arm-none-eabi, armeb-none-eabi, arm-wince-pe
Since your patch affects generic code, please could you widen your testing ?
At least include x86_64-pc-linux-gnu and maybe a toolchain configured with
--enable-targets=all.
The added code also references PR 24753, but this thread has not be included
in that PR. Ideally it would be nice if you could reopen the PR and add
your patch there, so that there is a permanent record of the addition which
is easy to locate based on the comment. Also, if you do this, please reference
the PR in the changelog message.
Cheers
Nick
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH][Binutils][Arm] BFD: Exclude sections with no content from compress check.
2020-04-21 14:29 ` Nick Clifton
@ 2020-04-21 15:50 ` Tamar Christina
2020-04-21 16:51 ` Nick Clifton
0 siblings, 1 reply; 6+ messages in thread
From: Tamar Christina @ 2020-04-21 15:50 UTC (permalink / raw)
To: nickc, binutils; +Cc: nd, Richard Earnshaw, Ramana Radhakrishnan
Hi Nick,
>
> > Cross-compiled and regtested on
> > aarch64-none-linux-gnu, aarch64_be-none-linux-gnu,
> > arm-none-eabi, armeb-none-eabi, arm-wince-pe
>
> Since your patch affects generic code, please could you widen your testing ?
> At least include x86_64-pc-linux-gnu and maybe a toolchain configured with -
> -enable-targets=all.
Ah I have tested x86_64-pc-linux-gnu and it's clean, I have also tried one configured with --enable-targets=all
and while the build seems to have taken longer I don't see the number of tests actually increasing.
I am just doing ../binutils-gdb-fsf/configure --prefix=(readlink -f .)/install; and make -j; and make check -j
Do I need to do something else?
I did however already commit the patch after Alan's approval.
>
> The added code also references PR 24753, but this thread has not be
> included in that PR. Ideally it would be nice if you could reopen the PR and
> add your patch there, so that there is a permanent record of the addition
> which is easy to locate based on the comment. Also, if you do this, please
> reference the PR in the changelog message.
Sorry I noticed after I sent it out that I forgot to add the PR. The PR was added however before I committed it
so the commits are on Bugzilla.
In general should re-open old PRs before committing to them?
Thanks,
Tamar
>
> Cheers
> Nick
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][Binutils][Arm] BFD: Exclude sections with no content from compress check.
2020-04-21 15:50 ` Tamar Christina
@ 2020-04-21 16:51 ` Nick Clifton
0 siblings, 0 replies; 6+ messages in thread
From: Nick Clifton @ 2020-04-21 16:51 UTC (permalink / raw)
To: Tamar Christina, binutils; +Cc: nd, Richard Earnshaw, Ramana Radhakrishnan
Hi Tamar,
> Ah I have tested x86_64-pc-linux-gnu and it's clean, I have also tried one configured with --enable-targets=all
> and while the build seems to have taken longer I don't see the number of tests actually increasing.
>
> I am just doing ../binutils-gdb-fsf/configure --prefix=(readlink -f .)/install; and make -j; and make check -j
>
> Do I need to do something else?
No, that is great - thanks.
> In general should re-open old PRs before committing to them?
No - if you are just adding an extra fix and not reporting a resurrection of the problem, then just adding a comment to the already-closed PR is OK.
Cheers
Nick
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-21 16:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-21 13:41 [PATCH][Binutils][Arm] BFD: Exclude sections with no content from compress check Tamar Christina
2020-04-21 13:58 ` Alan Modra
2020-04-21 14:01 ` Tamar Christina
2020-04-21 14:29 ` Nick Clifton
2020-04-21 15:50 ` Tamar Christina
2020-04-21 16:51 ` Nick Clifton
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).