diff mbox series

[v3,03/13] Rewrite abi-note.S in C.

Message ID 6fbbbbbca662497f180ea5356d50a4f16a3b5716.1589552054.git.szabolcs.nagy@arm.com
State Superseded
Headers
Series aarch64: branch protection support |

Commit Message

Szabolcs Nagy May 15, 2020, 2:40 p.m. UTC 
  Using C code allows the compiler to add target specific object file
markings based on CFLAGS.

The arm specific abi-note.S is removed and similar object file fix
up will be avoided on AArch64 with standard branch-prtection.
---
 csu/{abi-note.S => abi-note.c} | 28 ++++++++++++++++++----------
 sysdeps/arm/abi-note.S         |  8 --------
 2 files changed, 18 insertions(+), 18 deletions(-)
 rename csu/{abi-note.S => abi-note.c} (88%)
 delete mode 100644 sysdeps/arm/abi-note.S

Comments

Florian Weimer May 18, 2020, 3:28 p.m. UTC | #1 
* Szabolcs Nagy:

> +/* Note: Custom type is used as ElfW(Nhdr) is wrong on 64 bit targets.  */

Are you sure about that?

typedef struct
{
  Elf32_Word n_namesz;			/* Length of the note's name.  */
  Elf32_Word n_descsz;			/* Length of the note's descriptor.  */
  Elf32_Word n_type;			/* Type of the note.  */
} Elf32_Nhdr;

typedef struct
{
  Elf64_Word n_namesz;			/* Length of the note's name.  */
  Elf64_Word n_descsz;			/* Length of the note's descriptor.  */
  Elf64_Word n_type;			/* Type of the note.  */
} Elf64_Nhdr;

The types are the same:

typedef uint32_t Elf32_Word;
typedef uint32_t Elf64_Word;

I admit this is super-confusing.

Thanks,
Florian
Szabolcs Nagy May 20, 2020, 10:27 a.m. UTC | #2 
The 05/18/2020 17:28, Florian Weimer wrote:
> * Szabolcs Nagy:
> 
> > +/* Note: Custom type is used as ElfW(Nhdr) is wrong on 64 bit targets.  */
> 
> Are you sure about that?
> 
> typedef struct
> {
>   Elf32_Word n_namesz;			/* Length of the note's name.  */
>   Elf32_Word n_descsz;			/* Length of the note's descriptor.  */
>   Elf32_Word n_type;			/* Type of the note.  */
> } Elf32_Nhdr;
> 
> typedef struct
> {
>   Elf64_Word n_namesz;			/* Length of the note's name.  */
>   Elf64_Word n_descsz;			/* Length of the note's descriptor.  */
>   Elf64_Word n_type;			/* Type of the note.  */
> } Elf64_Nhdr;
> 
> The types are the same:
> 
> typedef uint32_t Elf32_Word;
> typedef uint32_t Elf64_Word;
> 
> I admit this is super-confusing.

aah i missed this, then the struct can be

#include <elf.h>

__attribute__ ((used, aligned (4), section (".note.ABI-tag")))
static const struct
{
  ElfW(Nhdr) nhdr;
  char name[4];
  int32_t desc[4];
} __abi_tag = {
  { .n_namesz = 4, .n_descsz = 16, .n_type = 1 },
  "GNU",
  { __ABI_TAG_OS, __ABI_TAG_VERSION }
};

i think the aligned attribute is still needed in case on
some target uint32_t has smaller alignemnt than 4.

i can change this if it is considered to be better
or just remove the incorrect comment.
Florian Weimer May 20, 2020, 10:34 a.m. UTC | #3 
* Szabolcs Nagy:

> The 05/18/2020 17:28, Florian Weimer wrote:
>> * Szabolcs Nagy:
>> 
>> > +/* Note: Custom type is used as ElfW(Nhdr) is wrong on 64 bit targets.  */
>> 
>> Are you sure about that?
>> 
>> typedef struct
>> {
>>   Elf32_Word n_namesz;			/* Length of the note's name.  */
>>   Elf32_Word n_descsz;			/* Length of the note's descriptor.  */
>>   Elf32_Word n_type;			/* Type of the note.  */
>> } Elf32_Nhdr;
>> 
>> typedef struct
>> {
>>   Elf64_Word n_namesz;			/* Length of the note's name.  */
>>   Elf64_Word n_descsz;			/* Length of the note's descriptor.  */
>>   Elf64_Word n_type;			/* Type of the note.  */
>> } Elf64_Nhdr;
>> 
>> The types are the same:
>> 
>> typedef uint32_t Elf32_Word;
>> typedef uint32_t Elf64_Word;
>> 
>> I admit this is super-confusing.
>
> aah i missed this, then the struct can be
>
> #include <elf.h>
>
> __attribute__ ((used, aligned (4), section (".note.ABI-tag")))
> static const struct
> {
>   ElfW(Nhdr) nhdr;
>   char name[4];
>   int32_t desc[4];
> } __abi_tag = {
>   { .n_namesz = 4, .n_descsz = 16, .n_type = 1 },
>   "GNU",
>   { __ABI_TAG_OS, __ABI_TAG_VERSION }
> };
>
> i think the aligned attribute is still needed in case on
> some target uint32_t has smaller alignemnt than 4.

Agreed, I think it's needed for m68k.

> i can change this if it is considered to be better
> or just remove the incorrect comment.

I slightly prefer using the type from <elf.h>.

Thanks,
Florian
diff mbox series

Patch

diff --git a/csu/abi-note.S b/csu/abi-note.c
similarity index 88%
rename from csu/abi-note.S
rename to csu/abi-note.c
index 2b4b5f8824..cd45f17345 100644
--- a/csu/abi-note.S
+++ b/csu/abi-note.c
@@ -53,6 +53,7 @@  offset	length	contents
    identify the earliest release of that OS that supports this ABI.
    See abi-tags (top level) for details. */
 
+#include <stdint.h>
 #include <config.h>
 #include <abi-tag.h>		/* OS-specific ABI tag value */
 
@@ -60,13 +61,20 @@  offset	length	contents
    name begins with `.note' and creates a PT_NOTE program header entry
    pointing at it. */
 
-	.section ".note.ABI-tag", "a"
-	.p2align 2
-	.long 1f - 0f		/* name length */
-	.long 3f - 2f		/* data length */
-	.long  1		/* note type */
-0:	.asciz "GNU"		/* vendor name */
-1:	.p2align 2
-2:	.long __ABI_TAG_OS	/* note data: the ABI tag */
-	.long __ABI_TAG_VERSION
-3:	.p2align 2		/* pad out section */
+/* Note: Custom type is used as ElfW(Nhdr) is wrong on 64 bit targets.  */
+
+__attribute__ ((used, aligned (4), section (".note.ABI-tag")))
+static const struct
+{
+  int32_t namesz;
+  int32_t descsz;
+  int32_t type;
+  char name[4];
+  int32_t desc[4];
+} __abi_tag = {
+  4,
+  16,
+  1,
+  "GNU",
+  { __ABI_TAG_OS, __ABI_TAG_VERSION }
+};
diff --git a/sysdeps/arm/abi-note.S b/sysdeps/arm/abi-note.S
deleted file mode 100644
index 07bd4c4619..0000000000
--- a/sysdeps/arm/abi-note.S
+++ /dev/null
@@ -1,8 +0,0 @@ 
-/* Tag_ABI_align8_preserved: This code preserves 8-byte
-   alignment in any callee.  */
-	.eabi_attribute 25, 1
-/* Tag_ABI_align8_needed: This code may require 8-byte alignment from
-   the caller.  */
-	.eabi_attribute 24, 1
-
-#include <csu/abi-note.S>