diff mbox series

Replace ROUND with ALIGN_UP by p_align [BZ #22370]

Message ID 20171031125622.GA18905@gmail.com
State New
Headers show
Series Replace ROUND with ALIGN_UP by p_align [BZ #22370] | expand

Commit Message

H.J. Lu Oct. 31, 2017, 12:56 p.m. UTC 
Alignment of notes in PT_NOTE segment is specified by the p_align field
in the PT_NOTE segment header.  This patch replaces ROUND with ALIGN_UP
by p_align to scan notes in PT_NOTE segment.

Tested on i686 and x86-64.  Any comments?

H.J.
---
	[BZ #22370]
	* dl-hwcaps.c (ROUND): Removed.
	(_dl_important_hwcaps): Replace ROUND with ALIGN_UP by p_align.
	* elf/dl-load.c (ROUND): Removed.
	(open_verify): Replace ROUND with ALIGN_UP by p_align.
	* elf/readelflib.c (ROUND): Removed.
	(process_elf_file): Replace ROUND with ALIGN_UP by p_align.
---
 elf/dl-hwcaps.c  | 9 +++++----
 elf/dl-load.c    | 6 +++---
 elf/readelflib.c | 6 +++---
 3 files changed, 11 insertions(+), 10 deletions(-)

Comments

Andreas Schwab Oct. 31, 2017, 1:49 p.m. UTC | #1 
On Okt 31 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:

> Alignment of notes in PT_NOTE segment is specified by the p_align field
> in the PT_NOTE segment header.

I don't think it makes sense to alter the note contents due to p_align,
it should only control the padding between each note.

Andreas.
H.J. Lu Oct. 31, 2017, 1:57 p.m. UTC | #2 
On Tue, Oct 31, 2017 at 6:49 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Okt 31 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>
>> Alignment of notes in PT_NOTE segment is specified by the p_align field
>> in the PT_NOTE segment header.
>
> I don't think it makes sense to alter the note contents due to p_align,
> it should only control the padding between each note.
>

Isn't padding between each note determined by alignment of note?
Linker creates one PT_NOTE segment for notes with the same alignment:

There are 38 section headers, starting at offset 0x493f30:

Section Headers:
  [Nr] Name              Type            Address          Off    Size
 ES Flg Lk Inf Al
  [ 0]                   NULL            0000000000000000 000000
000000 00      0   0  0
  [ 1] .note.gnu.property NOTE            00000000004001c8 0001c8
000020 00   A  0   0  8
  [ 2] .note.ABI-tag     NOTE            00000000004001e8 0001e8
000020 00   A  0   0  4
  [ 3] .note.gnu.build-id NOTE            0000000000400208 000208
000024 00   A  0   0  4
readelf: Warning: [ 4]: Link field (0) should index a symtab section.
  [ 4] .rela.plt         RELA            0000000000400230 000230
000210 18  AI  0  23  8
  [ 5] .init             PROGBITS        0000000000400440 000440
000017 00  AX  0   0  4
  [ 6] .plt              PROGBITS        0000000000400460 000460
000160 00  AX  0   0 16
  [ 7] .text             PROGBITS        00000000004005c0 0005c0
0730e0 00  AX  0   0 16
  [ 8] __libc_freeres_fn PROGBITS        00000000004736a0 0736a0
000aa7 00  AX  0   0 16
  [ 9] __libc_thread_freeres_fn PROGBITS        0000000000474150
074150 00015d 00  AX  0   0 16
  [10] .fini             PROGBITS        00000000004742b0 0742b0
000009 00  AX  0   0  4
  [11] .rodata           PROGBITS        00000000004742c0 0742c0
01c138 00   A  0   0 32
  [12] __libc_subfreeres PROGBITS        00000000004903f8 0903f8
000048 00   A  0   0  8
  [13] __libc_IO_vtables PROGBITS        0000000000490440 090440
0006a8 00   A  0   0 32
  [14] __libc_atexit     PROGBITS        0000000000490ae8 090ae8
000008 00   A  0   0  8
  [15] __libc_thread_subfreeres PROGBITS        0000000000490af0
090af0 000010 00   A  0   0  8
  [16] .eh_frame         PROGBITS        0000000000490b00 090b00
009cd0 00   A  0   0  8
  [17] .gcc_except_table PROGBITS        000000000049a7d0 09a7d0
0000a1 00   A  0   0  1
  [18] .tdata            PROGBITS        000000000069ab40 09ab40
000020 00 WAT  0   0  8
  [19] .tbss             NOBITS          000000000069ab60 09ab60
000040 00 WAT  0   0  8
  [20] .init_array       INIT_ARRAY      000000000069ab60 09ab60
000010 08  WA  0   0  8
  [21] .fini_array       FINI_ARRAY      000000000069ab70 09ab70
000010 08  WA  0   0  8
  [22] .data.rel.ro      PROGBITS        000000000069ab80 09ab80
000464 00  WA  0   0 32
  [23] .got.plt          PROGBITS        000000000069b000 09b000
0000c8 08  WA  0   0  8
  [24] .data             PROGBITS        000000000069b0e0 09b0e0
001ae8 00  WA  0   0 32
  [25] .bss              NOBITS          000000000069cbe0 09cbc8
0014e8 00  WA  0   0 32
  [26] __libc_freeres_ptrs NOBITS          000000000069e0c8 09cbc8
000028 00  WA  0   0  8
  [27] .comment          PROGBITS        0000000000000000 09cbc8
000029 01  MS  0   0  1
  [28] .debug_aranges    PROGBITS        0000000000000000 09cc00
004450 00      0   0 16
  [29] .debug_info       PROGBITS        0000000000000000 0a1050
261a05 00      0   0  1
  [30] .debug_abbrev     PROGBITS        0000000000000000 302a55
039aa2 00      0   0  1
  [31] .debug_line       PROGBITS        0000000000000000 33c4f7
0680f6 00      0   0  1
  [32] .debug_str        PROGBITS        0000000000000000 3a45ed
011e67 01  MS  0   0  1
  [33] .debug_loc        PROGBITS        0000000000000000 3b6454
0b41e9 00      0   0  1
  [34] .debug_ranges     PROGBITS        0000000000000000 46a640
016d00 00      0   0 16
  [35] .symtab           SYMTAB          0000000000000000 481340
00ba30 18     36 880  8
  [36] .strtab           STRTAB          0000000000000000 48cd70
006ffc 00      0   0  1
  [37] .shstrtab         STRTAB          0000000000000000 493d6c
0001c1 00      0   0  1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
  L (link order), O (extra OS processing required), G (group), T (TLS),
  C (compressed), x (unknown), o (OS specific), E (exclude),
  l (large), p (processor specific)

Elf file type is EXEC (Executable file)
Entry point 0x4015a0
There are 7 program headers, starting at offset 64

Program Headers:
  Type           Offset   VirtAddr           PhysAddr
FileSiz  MemSiz   Flg Align
  LOAD           0x000000 0x0000000000400000 0x0000000000400000
0x09a871 0x09a871 R E 0x200000
  LOAD           0x09ab40 0x000000000069ab40 0x000000000069ab40
0x002088 0x0035b0 RW  0x200000
  NOTE           0x0001c8 0x00000000004001c8 0x00000000004001c8
0x000020 0x000020 R   0x8
  NOTE           0x0001e8 0x00000000004001e8 0x00000000004001e8
0x000044 0x000044 R   0x4
  TLS            0x09ab40 0x000000000069ab40 0x000000000069ab40
0x000020 0x000060 R   0x8
  GNU_STACK      0x000000 0x0000000000000000 0x0000000000000000
0x000000 0x000000 RW  0x10
  GNU_RELRO      0x09ab40 0x000000000069ab40 0x000000000069ab40
0x0004c0 0x0004c0 R   0x1

 Section to Segment mapping:
  Segment Sections...
   00     .note.gnu.property .note.ABI-tag .note.gnu.build-id
.rela.plt .init .plt .text __libc_freeres_fn __libc_thread_freeres_fn
.fini .rodata __libc_subfreeres __libc_IO_vtables __libc_atexit
__libc_thread_subfreeres .eh_frame .gcc_except_table
   01     .tdata .init_array .fini_array .data.rel.ro .got.plt .data
.bss __libc_freeres_ptrs
   02     .note.gnu.property
   03     .note.ABI-tag .note.gnu.build-id
   04     .tdata .tbss
   05
   06     .tdata .init_array .fini_array .data.rel.ro
H.J. Lu Nov. 11, 2017, 12:26 a.m. UTC | #3 
On Tue, Oct 31, 2017 at 6:57 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Oct 31, 2017 at 6:49 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> On Okt 31 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>>
>>> Alignment of notes in PT_NOTE segment is specified by the p_align field
>>> in the PT_NOTE segment header.
>>
>> I don't think it makes sense to alter the note contents due to p_align,
>> it should only control the padding between each note.
>>
>
> Isn't padding between each note determined by alignment of note?
> Linker creates one PT_NOTE segment for notes with the same alignment:
>
> There are 38 section headers, starting at offset 0x493f30:
>
> Section Headers:
>   [Nr] Name              Type            Address          Off    Size
>  ES Flg Lk Inf Al
>   [ 0]                   NULL            0000000000000000 000000
> 000000 00      0   0  0
>   [ 1] .note.gnu.property NOTE            00000000004001c8 0001c8
> 000020 00   A  0   0  8
>   [ 2] .note.ABI-tag     NOTE            00000000004001e8 0001e8
> 000020 00   A  0   0  4
>   [ 3] .note.gnu.build-id NOTE            0000000000400208 000208
> 000024 00   A  0   0  4
> readelf: Warning: [ 4]: Link field (0) should index a symtab section.
>   [ 4] .rela.plt         RELA            0000000000400230 000230
> 000210 18  AI  0  23  8
>   [ 5] .init             PROGBITS        0000000000400440 000440
> 000017 00  AX  0   0  4
>   [ 6] .plt              PROGBITS        0000000000400460 000460
> 000160 00  AX  0   0 16
>   [ 7] .text             PROGBITS        00000000004005c0 0005c0
> 0730e0 00  AX  0   0 16
>   [ 8] __libc_freeres_fn PROGBITS        00000000004736a0 0736a0
> 000aa7 00  AX  0   0 16
>   [ 9] __libc_thread_freeres_fn PROGBITS        0000000000474150
> 074150 00015d 00  AX  0   0 16
>   [10] .fini             PROGBITS        00000000004742b0 0742b0
> 000009 00  AX  0   0  4
>   [11] .rodata           PROGBITS        00000000004742c0 0742c0
> 01c138 00   A  0   0 32
>   [12] __libc_subfreeres PROGBITS        00000000004903f8 0903f8
> 000048 00   A  0   0  8
>   [13] __libc_IO_vtables PROGBITS        0000000000490440 090440
> 0006a8 00   A  0   0 32
>   [14] __libc_atexit     PROGBITS        0000000000490ae8 090ae8
> 000008 00   A  0   0  8
>   [15] __libc_thread_subfreeres PROGBITS        0000000000490af0
> 090af0 000010 00   A  0   0  8
>   [16] .eh_frame         PROGBITS        0000000000490b00 090b00
> 009cd0 00   A  0   0  8
>   [17] .gcc_except_table PROGBITS        000000000049a7d0 09a7d0
> 0000a1 00   A  0   0  1
>   [18] .tdata            PROGBITS        000000000069ab40 09ab40
> 000020 00 WAT  0   0  8
>   [19] .tbss             NOBITS          000000000069ab60 09ab60
> 000040 00 WAT  0   0  8
>   [20] .init_array       INIT_ARRAY      000000000069ab60 09ab60
> 000010 08  WA  0   0  8
>   [21] .fini_array       FINI_ARRAY      000000000069ab70 09ab70
> 000010 08  WA  0   0  8
>   [22] .data.rel.ro      PROGBITS        000000000069ab80 09ab80
> 000464 00  WA  0   0 32
>   [23] .got.plt          PROGBITS        000000000069b000 09b000
> 0000c8 08  WA  0   0  8
>   [24] .data             PROGBITS        000000000069b0e0 09b0e0
> 001ae8 00  WA  0   0 32
>   [25] .bss              NOBITS          000000000069cbe0 09cbc8
> 0014e8 00  WA  0   0 32
>   [26] __libc_freeres_ptrs NOBITS          000000000069e0c8 09cbc8
> 000028 00  WA  0   0  8
>   [27] .comment          PROGBITS        0000000000000000 09cbc8
> 000029 01  MS  0   0  1
>   [28] .debug_aranges    PROGBITS        0000000000000000 09cc00
> 004450 00      0   0 16
>   [29] .debug_info       PROGBITS        0000000000000000 0a1050
> 261a05 00      0   0  1
>   [30] .debug_abbrev     PROGBITS        0000000000000000 302a55
> 039aa2 00      0   0  1
>   [31] .debug_line       PROGBITS        0000000000000000 33c4f7
> 0680f6 00      0   0  1
>   [32] .debug_str        PROGBITS        0000000000000000 3a45ed
> 011e67 01  MS  0   0  1
>   [33] .debug_loc        PROGBITS        0000000000000000 3b6454
> 0b41e9 00      0   0  1
>   [34] .debug_ranges     PROGBITS        0000000000000000 46a640
> 016d00 00      0   0 16
>   [35] .symtab           SYMTAB          0000000000000000 481340
> 00ba30 18     36 880  8
>   [36] .strtab           STRTAB          0000000000000000 48cd70
> 006ffc 00      0   0  1
>   [37] .shstrtab         STRTAB          0000000000000000 493d6c
> 0001c1 00      0   0  1
> Key to Flags:
>   W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
>   L (link order), O (extra OS processing required), G (group), T (TLS),
>   C (compressed), x (unknown), o (OS specific), E (exclude),
>   l (large), p (processor specific)
>
> Elf file type is EXEC (Executable file)
> Entry point 0x4015a0
> There are 7 program headers, starting at offset 64
>
> Program Headers:
>   Type           Offset   VirtAddr           PhysAddr
> FileSiz  MemSiz   Flg Align
>   LOAD           0x000000 0x0000000000400000 0x0000000000400000
> 0x09a871 0x09a871 R E 0x200000
>   LOAD           0x09ab40 0x000000000069ab40 0x000000000069ab40
> 0x002088 0x0035b0 RW  0x200000
>   NOTE           0x0001c8 0x00000000004001c8 0x00000000004001c8
> 0x000020 0x000020 R   0x8
>   NOTE           0x0001e8 0x00000000004001e8 0x00000000004001e8
> 0x000044 0x000044 R   0x4
>   TLS            0x09ab40 0x000000000069ab40 0x000000000069ab40
> 0x000020 0x000060 R   0x8
>   GNU_STACK      0x000000 0x0000000000000000 0x0000000000000000
> 0x000000 0x000000 RW  0x10
>   GNU_RELRO      0x09ab40 0x000000000069ab40 0x000000000069ab40
> 0x0004c0 0x0004c0 R   0x1
>
>  Section to Segment mapping:
>   Segment Sections...
>    00     .note.gnu.property .note.ABI-tag .note.gnu.build-id
> .rela.plt .init .plt .text __libc_freeres_fn __libc_thread_freeres_fn
> .fini .rodata __libc_subfreeres __libc_IO_vtables __libc_atexit
> __libc_thread_subfreeres .eh_frame .gcc_except_table
>    01     .tdata .init_array .fini_array .data.rel.ro .got.plt .data
> .bss __libc_freeres_ptrs
>    02     .note.gnu.property
>    03     .note.ABI-tag .note.gnu.build-id
>    04     .tdata .tbss
>    05
>    06     .tdata .init_array .fini_array .data.rel.ro
>
>

Hi Andreas,

All notes in a PT_NOTE segment have the same alignment,  Otherwise,
one can't parse PT_NOTE segment.  I will check in my patch next week
if there are no objections.
Andreas Schwab Nov. 11, 2017, 12:39 p.m. UTC | #4 
On Nov 10 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:

> All notes in a PT_NOTE segment have the same alignment,  Otherwise,
> one can't parse PT_NOTE segment.

But the ABI says that the name and data size is always aligned to the
word size.  That is independent of the padding between notes.

Andreas.
H.J. Lu Nov. 11, 2017, 2:35 p.m. UTC | #5 
On Sat, Nov 11, 2017 at 4:39 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Nov 10 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>
>> All notes in a PT_NOTE segment have the same alignment,  Otherwise,
>> one can't parse PT_NOTE segment.
>
> But the ABI says that the name and data size is always aligned to the
> word size.  That is independent of the padding between notes.
>

The gABI says:

In 64-bit objects (files with e_ident[EI_CLASS] equal to ELFCLASS64),
each entry is an array of 8-byte words in the format of the target
processor. In 32-bit objects (files with e_ident[EI_CLASS] equal to
ELFCLASS32), each entry is an array of 4-byte words in the format of
the target processor.

Since .note.ABI-tag and .note.gnu.build-id notes are aligned to 4
bytes in 64-bit object while .note.gnu.property notes are aligned
to 8 bytes in 64-bit object, we have to check p_align in program
header for note alignment in PT_NOTE segments.
Andreas Schwab Nov. 11, 2017, 2:49 p.m. UTC | #6 
On Nov 11 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:

> The gABI says:
>
> In 64-bit objects (files with e_ident[EI_CLASS] equal to ELFCLASS64),
> each entry is an array of 8-byte words in the format of the target
> processor. In 32-bit objects (files with e_ident[EI_CLASS] equal to
> ELFCLASS32), each entry is an array of 4-byte words in the format of
> the target processor.
>
> Since .note.ABI-tag and .note.gnu.build-id notes are aligned to 4
> bytes in 64-bit object while .note.gnu.property notes are aligned
> to 8 bytes in 64-bit object, we have to check p_align in program
> header for note alignment in PT_NOTE segments.

I don't see any reference to the segment alignment in the quote above.

Andreas.
H.J. Lu Nov. 11, 2017, 3 p.m. UTC | #7 
On Sat, Nov 11, 2017 at 6:49 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Nov 11 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>
>> The gABI says:
>>
>> In 64-bit objects (files with e_ident[EI_CLASS] equal to ELFCLASS64),
>> each entry is an array of 8-byte words in the format of the target
>> processor. In 32-bit objects (files with e_ident[EI_CLASS] equal to
>> ELFCLASS32), each entry is an array of 4-byte words in the format of
>> the target processor.
>>
>> Since .note.ABI-tag and .note.gnu.build-id notes are aligned to 4
>> bytes in 64-bit object while .note.gnu.property notes are aligned
>> to 8 bytes in 64-bit object, we have to check p_align in program
>> header for note alignment in PT_NOTE segments.
>
> I don't see any reference to the segment alignment in the quote above.
>

The full text is

---
Note Section

Sometimes a vendor or system builder needs to mark an object file with
special information that other programs will check for conformance,
compatibility, etc. Sections of type SHT_NOTE and program header
elements of type PT_NOTE can be used for this purpose. The note
information in sections and program header elements holds a variable
amount of entries. In 64-bit objects (files with e_ident[EI_CLASS]
equal to ELFCLASS64), each entry is an array of 8-byte words in the
format of the target processor. In 32-bit objects (files with
e_ident[EI_CLASS] equal to ELFCLASS32), each entry is an array of
4-byte words in the format of the target processor. Labels appear
below to help explain note information organization, but they are not
part of the specification.
---

You tell me what note alignment in PT_NOTE segment is.
Andreas Schwab Nov. 11, 2017, 3:07 p.m. UTC | #8 
On Nov 11 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:

> The full text is
>
> ---
> Note Section
>
> Sometimes a vendor or system builder needs to mark an object file with
> special information that other programs will check for conformance,
> compatibility, etc. Sections of type SHT_NOTE and program header
> elements of type PT_NOTE can be used for this purpose. The note
> information in sections and program header elements holds a variable
> amount of entries. In 64-bit objects (files with e_ident[EI_CLASS]
> equal to ELFCLASS64), each entry is an array of 8-byte words in the
> format of the target processor. In 32-bit objects (files with
> e_ident[EI_CLASS] equal to ELFCLASS32), each entry is an array of
> 4-byte words in the format of the target processor. Labels appear
> below to help explain note information organization, but they are not
> part of the specification.
> ---
>
> You tell me what note alignment in PT_NOTE segment is.

I still don't see any reference to the segment alignment.

Andreas.
H.J. Lu Nov. 11, 2017, 3:24 p.m. UTC | #9 
On Sat, Nov 11, 2017 at 7:07 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Nov 11 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>
>> The full text is
>>
>> ---
>> Note Section
>>
>> Sometimes a vendor or system builder needs to mark an object file with
>> special information that other programs will check for conformance,
>> compatibility, etc. Sections of type SHT_NOTE and program header
>> elements of type PT_NOTE can be used for this purpose. The note
>> information in sections and program header elements holds a variable
>> amount of entries. In 64-bit objects (files with e_ident[EI_CLASS]
>> equal to ELFCLASS64), each entry is an array of 8-byte words in the
>> format of the target processor. In 32-bit objects (files with
>> e_ident[EI_CLASS] equal to ELFCLASS32), each entry is an array of
>> 4-byte words in the format of the target processor. Labels appear
>> below to help explain note information organization, but they are not
>> part of the specification.
>> ---
>>
>> You tell me what note alignment in PT_NOTE segment is.
>
> I still don't see any reference to the segment alignment.
>
> Andreas.
>

If .note.ABI-tag and .note.gnu.build-id notes followed the gABI, they should
have been aligned to 8 bytes in 64-bit objects and we could have assumed
all notes in PT_NOTE segments in 64-bit objects are aligned to 8 bytes.  But
.note.ABI-tag and .note.gnu.build-id notes don't follow the gABI.

p_align in program header is the the maximum alignment of notes in PT_NOTE
segment and linker groups notes with the same alignment in one PT_NOTE
segment.  p_align of PT_NOTE program header is the note alignment.
Andreas Schwab Nov. 11, 2017, 5:05 p.m. UTC | #10 
On Nov 11 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:

> If .note.ABI-tag and .note.gnu.build-id notes followed the gABI, they should
> have been aligned to 8 bytes in 64-bit objects and we could have assumed
> all notes in PT_NOTE segments in 64-bit objects are aligned to 8 bytes.  But
> .note.ABI-tag and .note.gnu.build-id notes don't follow the gABI.

How comes this has always been working?

Andreas.
H.J. Lu Nov. 12, 2017, 1:29 a.m. UTC | #11 
On Sat, Nov 11, 2017 at 9:05 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Nov 11 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>
>> If .note.ABI-tag and .note.gnu.build-id notes followed the gABI, they should
>> have been aligned to 8 bytes in 64-bit objects and we could have assumed
>> all notes in PT_NOTE segments in 64-bit objects are aligned to 8 bytes.  But
>> .note.ABI-tag and .note.gnu.build-id notes don't follow the gABI.
>
> How comes this has always been working?
>

Before

commit 62753d2c09108550650ab83a69e99ca28d8bde3b
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Aug 18 08:49:07 2017 -0700

    Add NT_GNU_PROPERTY_TYPE_0 macros

    Add macros used in GNU .note.gnu.property notes (NT_GNU_PROPERTY_TYPE_0).

            * elf/elf.h (NT_GNU_PROPERTY_TYPE_0): New.
            (NOTE_GNU_PROPERTY_SECTION_NAME): Likewise.

glibc never saw other notes.  .note.gnu.property note uses the same GNU note
format as .note.ABI-tag and .note.gnu.build-id notes, but it is
aligned to 8 bytes in
64-bit objects.  My patch fixes glibc so that it can parse GNU notes
aligned to either
4 bytes or 8 bytes.
Andreas Schwab Nov. 12, 2017, 1:51 p.m. UTC | #12 
On Nov 11 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:

> glibc never saw other notes.  .note.gnu.property note uses the same GNU note
> format as .note.ABI-tag and .note.gnu.build-id notes, but it is
> aligned to 8 bytes in
> 64-bit objects.

Wouldn't it be better for .note.gnu.property to follow existing
practice?  It seems like everyone assumes word alignment for notes.
Also note that Elf64_Nhdr consists of odd number of words, which makes
the note data unaligned for xwords, causing the next note to be
unaligned relative to the segment alignment.

Andreas.
H.J. Lu Nov. 12, 2017, 2:56 p.m. UTC | #13 
On Sun, Nov 12, 2017 at 5:51 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Nov 11 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>
>> glibc never saw other notes.  .note.gnu.property note uses the same GNU note
>> format as .note.ABI-tag and .note.gnu.build-id notes, but it is
>> aligned to 8 bytes in
>> 64-bit objects.
>
> Wouldn't it be better for .note.gnu.property to follow existing
> practice?  It seems like everyone assumes word alignment for notes.

.note.gnu.property supports address/size related properties, which are
64 bits in 64-bit objects.  It is natural to follow the gABI.

> Also note that Elf64_Nhdr consists of odd number of words, which makes
> the note data unaligned for xwords, causing the next note to be
> unaligned relative to the segment alignment.
>

Data after Elf64_Nhdr is the "name" field which is a 4-byte string, "GNU".
There is no misalignment.  The .note.gnu.property note is designed in
such a way that properties can be 64 bits without misalignment in 64-bit
objects:

https://sourceware.org/ml/gnu-gabi/2016-q4/msg00000.html
Andreas Schwab Nov. 12, 2017, 4:03 p.m. UTC | #14 
On Nov 12 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:

> Data after Elf64_Nhdr is the "name" field which is a 4-byte string, "GNU".
> There is no misalignment.

That's not what your patch does.  You are aligning both the name length
and the data length to a 8 byte boundary, making the note size
unaligned.

Andreas.
H.J. Lu Nov. 15, 2017, 11:03 p.m. UTC | #15 
On Sun, Nov 12, 2017 at 8:03 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Nov 12 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>
>> Data after Elf64_Nhdr is the "name" field which is a 4-byte string, "GNU".
>> There is no misalignment.
>
> That's not what your patch does.  You are aligning both the name length
> and the data length to a 8 byte boundary, making the note size
> unaligned.
>

You are right.  Both glibc and binutils get this wrong.  I opened a
binutils bug:

https://sourceware.org/bugzilla/show_bug.cgi?id=22444

I will fix it first and take care of glibc next.

Thanks.
diff mbox series

Patch

diff --git a/elf/dl-hwcaps.c b/elf/dl-hwcaps.c
index 92f2eb45ce..de271a6fc8 100644
--- a/elf/dl-hwcaps.c
+++ b/elf/dl-hwcaps.c
@@ -67,6 +67,7 @@  _dl_important_hwcaps (const char *platform, size_t platform_len, size_t *sz,
 	  {
 	    const ElfW(Addr) start = (phdr[i].p_vaddr
 				      + GLRO(dl_sysinfo_map)->l_addr);
+	    const ElfW(Addr) align = phdr[i].p_align;
 	    /* The standard ELF note layout is exactly as the anonymous struct.
 	       The next element is a variable length vendor name of length
 	       VENDORLEN (with a real length rounded to ElfW(Word)), followed
@@ -80,7 +81,6 @@  _dl_important_hwcaps (const char *platform, size_t platform_len, size_t *sz,
 	    } *note = (const void *) start;
 	    while ((ElfW(Addr)) (note + 1) - start < phdr[i].p_memsz)
 	      {
-#define ROUND(len) (((len) + sizeof (ElfW(Word)) - 1) & -sizeof (ElfW(Word)))
 		/* The layout of the type 2, vendor "GNU" note is as follows:
 		   .long <Number of capabilities enabled by this note>
 		   .long <Capabilities mask> (as mask >> _DL_FIRST_EXTRA).
@@ -92,7 +92,8 @@  _dl_important_hwcaps (const char *platform, size_t platform_len, size_t *sz,
 		    && note->datalen > 2 * sizeof (ElfW(Word)) + 2)
 		  {
 		    const ElfW(Word) *p = ((const void *) (note + 1)
-					   + ROUND (sizeof "GNU"));
+					   + ALIGN_UP (sizeof "GNU",
+						       sizeof (ElfW(Word))));
 		    cnt += *p++;
 		    ++p;	/* Skip mask word.  */
 		    dsocaps = (const char *) p; /* Pseudo-string "<b>name"  */
@@ -100,8 +101,8 @@  _dl_important_hwcaps (const char *platform, size_t platform_len, size_t *sz,
 		    break;
 		  }
 		note = ((const void *) (note + 1)
-			+ ROUND (note->vendorlen) + ROUND (note->datalen));
-#undef ROUND
+			+ ALIGN_UP (note->vendorlen, align)
+			+ ALIGN_UP (note->datalen, align));
 	      }
 	    if (dsocaps != NULL)
 	      break;
diff --git a/elf/dl-load.c b/elf/dl-load.c
index 1220183ce2..8fdfadd41e 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1683,6 +1683,7 @@  open_verify (const char *name, int fd,
 	if (ph->p_type == PT_NOTE && ph->p_filesz >= 32 && ph->p_align >= 4)
 	  {
 	    ElfW(Addr) size = ph->p_filesz;
+	    ElfW(Addr) align = ph->p_align;
 
 	    if (ph->p_offset + size <= (size_t) fbp->len)
 	      abi_note = (void *) (fbp->buf + ph->p_offset);
@@ -1696,10 +1697,9 @@  open_verify (const char *name, int fd,
 
 	    while (memcmp (abi_note, &expected_note, sizeof (expected_note)))
 	      {
-#define ROUND(len) (((len) + sizeof (ElfW(Word)) - 1) & -sizeof (ElfW(Word)))
 		ElfW(Addr) note_size = 3 * sizeof (ElfW(Word))
-				       + ROUND (abi_note[0])
-				       + ROUND (abi_note[1]);
+				       + ALIGN_UP (abi_note[0], align)
+				       + ALIGN_UP (abi_note[1], align);
 
 		if (size - 32 < note_size)
 		  {
diff --git a/elf/readelflib.c b/elf/readelflib.c
index 9ad56dcc34..247afa7de2 100644
--- a/elf/readelflib.c
+++ b/elf/readelflib.c
@@ -131,15 +131,15 @@  process_elf_file (const char *file_name, const char *lib, int *flag,
 	      ElfW(Word) *abi_note = (ElfW(Word) *) (file_contents
 						     + segment->p_offset);
 	      ElfW(Addr) size = segment->p_filesz;
+	      ElfW(Addr) align = segment->p_align;
 
 	      while (abi_note [0] != 4 || abi_note [1] != 16
 		     || abi_note [2] != 1
 		     || memcmp (abi_note + 3, "GNU", 4) != 0)
 		{
-#define ROUND(len) (((len) + sizeof (ElfW(Word)) - 1) & -sizeof (ElfW(Word)))
 		  ElfW(Addr) note_size = 3 * sizeof (ElfW(Word))
-					 + ROUND (abi_note[0])
-					 + ROUND (abi_note[1]);
+				         + ALIGN_UP (abi_note[0], align)
+				         + ALIGN_UP (abi_note[1], align);
 
 		  if (size - 32 < note_size || note_size == 0)
 		    {