Issue 21636003: Add a PT_NOTE section to mark build_id.

Issue 21636003: Add a PT_NOTE section to mark build_id. (Closed)

Created:
7 years, 4 months ago by bradn

Modified:
7 years, 4 months ago

Reviewers:
Mark Seaborn, Roland McGrath

CC:
native-client-reviews_googlegroups.com

Base URL:
http://git.chromium.org/native_client/nacl-glibc.git@master

Visibility:
Public.

More Reviews

Description

Add a PT_NOTE section to mark build_id. Adding a PT_NOTE section to mark the location of the build_id note will allow us to extract it from dl_iterate_phdrs at runtime. This in turn will facilitate providing module information with minidumps. BUG=https://code.google.com/p/nativeclient/issues/detail?id=3607 TEST=None R=mcgrathr@chromium.org, mseaborn@chromium.org Committed: https://git.chromium.org/gitweb?p=native_client/nacl-glibc.git;a=commit;h=668027f

Patch Set 1 #

Total comments: 1

Patch Set 2 : changed names #

Patch Set 3 : fixup fixup_ldso #

Patch Set 4 : fix trybots #

Total comments: 6

Created: 7 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+21 lines, -9 lines)			Patch
M	elf/nacl_fixup_ldso.py	View	1 2	1 chunk	+2 lines, -2 lines	2 comments	Download
M	nacl/dyn-link/ldscripts/elf64_nacl.x	View	1 2 3	2 chunks	+3 lines, -1 line	2 comments	Download
M	nacl/dyn-link/ldscripts/elf64_nacl.xs	View	1 2 3	2 chunks	+3 lines, -1 line	0 comments	Download
M	nacl/dyn-link/ldscripts/elf64_nacl.x.static	View	1 2 3	2 chunks	+4 lines, -2 lines	2 comments	Download
M	nacl/dyn-link/ldscripts/elf_nacl.x	View	1 2 3	2 chunks	+3 lines, -1 line	0 comments	Download
M	nacl/dyn-link/ldscripts/elf_nacl.xs	View	1 2 3	2 chunks	+3 lines, -1 line	0 comments	Download
M	nacl/dyn-link/ldscripts/elf_nacl.x.static	View	1 2 3	2 chunks	+3 lines, -1 line	0 comments	Download

Messages

Total messages: 10 (0 generated)

Expand Messages | Collapse Messages

Mark Seaborn

LGTM, but please make sure this passes the toolchain trybots before committing. https://codereview.chromium.org/21636003/diff/1/nacl/dyn-link/ldscripts/elf64_nacl.x File nacl/dyn-link/ldscripts/elf64_nacl.x ...

7 years, 4 months ago (2013-08-02 00:31:11 UTC) #2

bradn

PTAL This script blew up on the bots. I assume we just had the number ...

7 years, 4 months ago (2013-08-02 21:20:24 UTC) #5

Mark Seaborn

LGTM with tweaks below. https://codereview.chromium.org/21636003/diff/22001/elf/nacl_fixup_ldso.py File elf/nacl_fixup_ldso.py (right): https://codereview.chromium.org/21636003/diff/22001/elf/nacl_fixup_ldso.py#newcode47 elf/nacl_fixup_ldso.py:47: # Drop PT_DYNAMIC, PT_GNU_STACK and ...

7 years, 4 months ago (2013-08-07 16:43:07 UTC) #8

bradn

Committed patchset #4 manually as r668027f (presubmit successful).

7 years, 4 months ago (2013-08-07 18:18:51 UTC) #9

bradn

7 years, 4 months ago (2013-08-07 18:30:02 UTC) #10

Message was sent while issue was closed.

Oops, committed before going back to email and seeing your reply (just saw
rolands).

Picking up the leftovers in:
https://codereview.chromium.org/22527004

https://codereview.chromium.org/21636003/diff/22001/elf/nacl_fixup_ldso.py
File elf/nacl_fixup_ldso.py (right):

https://codereview.chromium.org/21636003/diff/22001/elf/nacl_fixup_ldso.py#ne...
elf/nacl_fixup_ldso.py:47: # Drop PT_DYNAMIC, PT_GNU_STACK and PT_TLS.
On 2013/08/07 16:43:07, Mark Seaborn wrote:
> We should no longer need to do this.  See service_runtime/elf_util.c.  Could
you
> simplify this by dropping this code?  (Optional.)

Done.

https://codereview.chromium.org/21636003/diff/22001/nacl/dyn-link/ldscripts/e...
File nacl/dyn-link/ldscripts/elf64_nacl.x (right):

https://codereview.chromium.org/21636003/diff/22001/nacl/dyn-link/ldscripts/e...
nacl/dyn-link/ldscripts/elf64_nacl.x:57: .dummy : {} : seg_rodata
On 2013/08/07 16:43:07, Mark Seaborn wrote:
> Make this line up with below?  Add spaces before first ":".
> 
> Add a comment: ":seg_rodata is on .dummy rather than on .hash to avoid
breaking
> the linker script munging that glibc's Makerules does using sed."
> 
> Same in other files.

Done.

https://codereview.chromium.org/21636003/diff/22001/nacl/dyn-link/ldscripts/e...
File nacl/dyn-link/ldscripts/elf64_nacl.x.static (right):

https://codereview.chromium.org/21636003/diff/22001/nacl/dyn-link/ldscripts/e...
nacl/dyn-link/ldscripts/elf64_nacl.x.static:56: .hash           : { *(.hash) }
On 2013/08/07 16:43:07, Mark Seaborn wrote:
> Don't add space at end of line here

Done.

Expand Messages | Collapse Messages