[PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[H.J. Lu]

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(-)

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)
 		    {
-- 
2.13.6

Re: [PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[Andreas Schwab]

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.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[H.J. Lu]

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.

Re: [PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[H.J. Lu]

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.


-- 
H.J.

Re: [PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[Andreas Schwab]

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.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[H.J. Lu]

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.

-- 
H.J.

Re: [PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[Andreas Schwab]

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.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[H.J. Lu]

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.


-- 
H.J.

Re: [PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[Andreas Schwab]

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.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[H.J. Lu]

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.


-- 
H.J.

Re: [PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[Andreas Schwab]

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.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[H.J. Lu]

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.


-- 
H.J.

Re: [PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[Andreas Schwab]

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.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[H.J. Lu]

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


-- 
H.J.

Re: [PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[Andreas Schwab]

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.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Replace ROUND with ALIGN_UP by p_align [BZ #22370]

[H.J. Lu]

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.

-- 
H.J.