Re: [Rpm-maint] [rpm-software-management/rpm] Compress annobin notes (#751)

2019-06-19 Thread Panu Matilainen
Merged #751 into master.

-- 
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/751#event-2423869845___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Compress annobin notes (#751)

2019-06-19 Thread Panu Matilainen
pmatilai approved this pull request.

Much better, thanks.



-- 
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/751#pullrequestreview-251642515___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Compress annobin notes (#751)

2019-06-17 Thread nickclifton
Hi Colin,

> Can you keep the commit message title to < 80 chars? And move the text from 
> the PR description into the commit message as well.

Sorry about that, I will remember for next time.

> Seems OK to me but: why don't the compilers do this by default?

Because at the time that the notes are created it is not possible
to determine which ones will actually be needed in the final executable.
(This is because of things like link-once sections, hot/cold partitioning
and so on).  The duplicate-eliminating and note-combining steps performed
by "objcopy --merge-notes" could in theory be performed by the linker, but
this has the problem that there are three linkers (ld.bfd, ld.gold, lld)
which would need to be modified, potentially introducing a lot more bugs.
Plus by putting the code into objcopy it means that binaries produced by
older linkers can still be processed.

Cheers
  Nick




-- 
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/751#issuecomment-502677576___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Compress annobin notes (#751)

2019-06-17 Thread nickclifton
Hi Mark,

> I don't think this should be part of add_minidebug (). It is something
> that would need to happen even if we don't run add_minidebug (). Also
> add_minidebug () runs after stripping/splitting the main ELF file into
> a .debug file (which also gets a copy of all notes).
> 
> I noticed older binutils objcopy don't seem to know about --merge-
> notes. And produce large errors when trying to run.
> 
> So I think this should be something like this, in do_file (), before
> the binary is actually stripped.

Thanks very much for the review and corrections.  As you might guess I am
no rpm expert, so I wholeheartedly agree with the changes that you have 
proposed.

Cheers
  Nick


-- 
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/751#issuecomment-502675464___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Compress annobin notes (#751)

2019-06-17 Thread Mark Wielaard
Hi Nick,

On Fri, 2019-06-14 at 09:29 -0700, nickclifton wrote:
> This is a request to add support for compressing annobin notes found
> in executable binaries built on Fedora and RHEL systems.
> 
> The annobin project adds a note section to binary files describing
> the security hardening features of how they were
> built.  Unfortunately these notes can get quite large, especially for
> projects that use lots of object files.  The objcopy program from the
> binutils package has an option to reduce the size of these notes by
> eliminating empties and merging duplicates.  If the binary does not
> contain any annobin notes then the objcopy will take no noticeable
> amount of time.  In fact even if the file does contain annobin notes
> the merging process is relatively fast and it is unlikely to add any
> significant amount of time to the overall build process.
> [...]
> diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
> index d75da1108..14d9ed901 100755
> --- a/scripts/find-debuginfo.sh
> +++ b/scripts/find-debuginfo.sh
> @@ -296,6 +296,8 @@ add_minidebug()
>xz "$mini_debuginfo"
>mini_debuginfo="${mini_debuginfo}.xz"
>objcopy --add-section .gnu_debugdata="$mini_debuginfo" "$binary"
> +  # Compress any annobin notes in the original binary.
> +  objcopy --merge-notes "$binary"
>rm -f "$dynsyms" "$funcsyms" "$keep_symbols" "$mini_debuginfo"
>  }
>  
> 

I don't think this should be part of add_minidebug (). It is something
that would need to happen even if we don't run add_minidebug (). Also
add_minidebug () runs after stripping/splitting the main ELF file into
a .debug file (which also gets a copy of all notes).

I noticed older binutils objcopy don't seem to know about --merge-
notes. And produce large errors when trying to run.

So I think this should be something like this, in do_file (), before
the binary is actually stripped.

diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
index d75da11..b6a343e 100755
--- a/scripts/find-debuginfo.sh
+++ b/scripts/find-debuginfo.sh
@@ -405,6 +405,10 @@ do_file()
 fi
   fi
 
+  # Compress any annobin notes in the original binary.
+  # Ignore any errors, since older objcopy don't support --merge-notes
+  objcopy --merge-notes "$f" 2>/dev/null || true
+
   # A binary already copied into /usr/lib/debug doesn't get stripped,
   # just has its file names collected and adjusted.
   case "$dn" in

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Compress annobin notes (#751)

2019-06-14 Thread Colin Walters
Can you keep the commit message title to < 80 chars?  And move the text from 
the PR description into the commit message as well.

Seems OK to me but: why don't the compilers do this by default?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/751#issuecomment-502178666___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


[Rpm-maint] [rpm-software-management/rpm] Compress annobin notes (#751)

2019-06-14 Thread nickclifton
This is a request to add support for compressing annobin notes found in 
executable binaries built on Fedora and RHEL systems.

The annobin project adds a note section to binary files describing the security 
hardening features of how they were built.  Unfortunately these notes can get 
quite large, especially for projects that use lots of object files.  The 
objcopy program from the binutils package has an option to reduce the size of 
these notes by eliminating empties and merging duplicates.  If the binary does 
not contain any annobin notes then the objcopy will take no noticeable amount 
of time.  In fact even if the file does contain annobin notes the merging 
process is relatively fast and it is unlikely to add any significant amount of 
time to the overall build process.
You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/751

-- Commit Summary --

  * Add a step to the add_minidebug() function that compresses annobin notes 
(if any) in the original binary.

-- File Changes --

M scripts/find-debuginfo.sh (2)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/751.patch
https://github.com/rpm-software-management/rpm/pull/751.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/751
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint