[PATCH] define SHT_LLVM_ADDRSIG section rather than error out

[PATCH] define SHT_LLVM_ADDRSIG section rather than error out

[Navin P]

 make elflint ignore it rather error as unsupported type. Other tools like
 readelf , objdump understand this section.

Signed-off-by: Navin P <navinp0304@gmail.com>
---
 libelf/elf.h  | 1 +
 src/elflint.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/libelf/elf.h b/libelf/elf.h
index 6439c1a4..26420b45 100644
--- a/libelf/elf.h
+++ b/libelf/elf.h
@@ -444,6 +444,7 @@ typedef struct
 #define SHT_SYMTAB_SHNDX  18 /* Extended section indeces */
 #define SHT_NUM   19 /* Number of defined types.  */
 #define SHT_LOOS   0x60000000 /* Start OS-specific.  */
+#define SHT_LLVM_ADDRSIG  0x6FFF4C03    /* llvm address sig */
 #define SHT_GNU_ATTRIBUTES 0x6ffffff5 /* Object attributes.  */
 #define SHT_GNU_HASH   0x6ffffff6 /* GNU-style hash table.  */
 #define SHT_GNU_LIBLIST   0x6ffffff7 /* Prelink library list */
diff --git a/src/elflint.c b/src/elflint.c
index ef3e3732..62663800 100644
--- a/src/elflint.c
+++ b/src/elflint.c
@@ -3905,6 +3905,7 @@ section [%2zu] '%s': size not multiple of entry size\n"),
    && shdr->sh_type != SHT_GNU_ATTRIBUTES
    && shdr->sh_type != SHT_GNU_LIBLIST
    && shdr->sh_type != SHT_CHECKSUM
+   && shdr->sh_type != SHT_LLVM_ADDRSIG
    && shdr->sh_type != SHT_GNU_verdef
    && shdr->sh_type != SHT_GNU_verneed
    && shdr->sh_type != SHT_GNU_versym
-- 
2.25.1

Re: [PATCH] define SHT_LLVM_ADDRSIG section rather than error out

[Mark Wielaard]

Hi Navin,

On Fri, 2020-11-13 at 20:45 +0530, Navin P via Elfutils-devel wrote:
>  make elflint ignore it rather error as unsupported type. Other tools
> like
>  readelf , objdump understand this section.

Is there a specification of this section type?

diff --git a/libelf/elf.h b/libelf/elf.h
> index 6439c1a4..26420b45 100644
> --- a/libelf/elf.h
> +++ b/libelf/elf.h
> @@ -444,6 +444,7 @@ typedef struct
>  #define SHT_SYMTAB_SHNDX  18 /* Extended section indeces */
>  #define SHT_NUM   19 /* Number of defined types.  */
>  #define SHT_LOOS   0x60000000 /* Start OS-specific.  */
> +#define SHT_LLVM_ADDRSIG  0x6FFF4C03    /* llvm address sig */
>  #define SHT_GNU_ATTRIBUTES 0x6ffffff5 /* Object attributes.  */
>  #define SHT_GNU_HASH   0x6ffffff6 /* GNU-style hash table.  */
>  #define SHT_GNU_LIBLIST   0x6ffffff7 /* Prelink library list */

elf.h comes from the glibc project.
We should first try to upstream new constants there
(glibc-alpha@sourceware.org)

> diff --git a/src/elflint.c b/src/elflint.c
> index ef3e3732..62663800 100644
> --- a/src/elflint.c
> +++ b/src/elflint.c
> @@ -3905,6 +3905,7 @@ section [%2zu] '%s': size not multiple of entry
> size\n"),
>     && shdr->sh_type != SHT_GNU_ATTRIBUTES
>     && shdr->sh_type != SHT_GNU_LIBLIST
>     && shdr->sh_type != SHT_CHECKSUM
> +   && shdr->sh_type != SHT_LLVM_ADDRSIG
>     && shdr->sh_type != SHT_GNU_verdef
>     && shdr->sh_type != SHT_GNU_verneed
>     && shdr->sh_type != SHT_GNU_versym

Note that for various of these SHT_GNU extensions we actually do have
some extra checks. Do we need to check anything for a section marked
SHT_LLVM_ADDRSIG?

Thanks,

Mark

Re: [PATCH] define SHT_LLVM_ADDRSIG section rather than error out

[Navin P]

On Tue, Nov 17, 2020 at 7:49 PM Mark Wielaard <mark@klomp.org> wrote:
>
> Hi Navin,
>
> On Fri, 2020-11-13 at 20:45 +0530, Navin P via Elfutils-devel wrote:
> >  make elflint ignore it rather error as unsupported type. Other tools
> > like
> >  readelf , objdump understand this section.
>
> Is there a specification of this section type?
>
> diff --git a/libelf/elf.h b/libelf/elf.h
> > index 6439c1a4..26420b45 100644
> > --- a/libelf/elf.h
> > +++ b/libelf/elf.h
> > @@ -444,6 +444,7 @@ typedef struct
> >  #define SHT_SYMTAB_SHNDX  18 /* Extended section indeces */
> >  #define SHT_NUM   19 /* Number of defined types.  */
> >  #define SHT_LOOS   0x60000000 /* Start OS-specific.  */
> > +#define SHT_LLVM_ADDRSIG  0x6FFF4C03    /* llvm address sig */
> >  #define SHT_GNU_ATTRIBUTES 0x6ffffff5 /* Object attributes.  */
> >  #define SHT_GNU_HASH   0x6ffffff6 /* GNU-style hash table.  */
> >  #define SHT_GNU_LIBLIST   0x6ffffff7 /* Prelink library list */
>
> elf.h comes from the glibc project.
> We should first try to upstream new constants there
> (glibc-alpha@sourceware.org)
>
> > diff --git a/src/elflint.c b/src/elflint.c
> > index ef3e3732..62663800 100644
> > --- a/src/elflint.c
> > +++ b/src/elflint.c
> > @@ -3905,6 +3905,7 @@ section [%2zu] '%s': size not multiple of entry
> > size\n"),
> >     && shdr->sh_type != SHT_GNU_ATTRIBUTES
> >     && shdr->sh_type != SHT_GNU_LIBLIST
> >     && shdr->sh_type != SHT_CHECKSUM
> > +   && shdr->sh_type != SHT_LLVM_ADDRSIG
> >     && shdr->sh_type != SHT_GNU_verdef
> >     && shdr->sh_type != SHT_GNU_verneed
> >     && shdr->sh_type != SHT_GNU_versym
>
> Note that for various of these SHT_GNU extensions we actually do have
> some extra checks. Do we need to check anything for a section marked
> SHT_LLVM_ADDRSIG?
>
We can do two things here
a) Recognize the section exists but ignore its contents which is what i
do. This needn't be the correct approach.
You may need to check the contents to sht_llvm_addrsig but that is
lot of work after the format has been frozen.
https://www.mail-archive.com/netdev@vger.kernel.org/msg348254.html
readelf output
[22] .llvm_addrsig     LOOS+0xfff4c03  0000000000000000 ...

b) If we don't want to recognize SHT_LLVM_ADDRSIG
you can strip section from object file by objcopy -R .llvm_addrsig size.o
conditionally  based on clang compiler.

Re: [PATCH] define SHT_LLVM_ADDRSIG section rather than error out

[Timm Bäder]

On 18/11/2020 06:34, Navin P via Elfutils-devel wrote:
>>> diff --git a/src/elflint.c b/src/elflint.c
>>> index ef3e3732..62663800 100644
>>> --- a/src/elflint.c
>>> +++ b/src/elflint.c
>>> @@ -3905,6 +3905,7 @@ section [%2zu] '%s': size not multiple of entry
>>> size\n"),
>>>      && shdr->sh_type != SHT_GNU_ATTRIBUTES
>>>      && shdr->sh_type != SHT_GNU_LIBLIST
>>>      && shdr->sh_type != SHT_CHECKSUM
>>> +   && shdr->sh_type != SHT_LLVM_ADDRSIG
>>>      && shdr->sh_type != SHT_GNU_verdef
>>>      && shdr->sh_type != SHT_GNU_verneed
>>>      && shdr->sh_type != SHT_GNU_versym
>>
>> Note that for various of these SHT_GNU extensions we actually do have
>> some extra checks. Do we need to check anything for a section marked
>> SHT_LLVM_ADDRSIG?
>>
> We can do two things here
> a) Recognize the section exists but ignore its contents which is what i
> do. This needn't be the correct approach.
> You may need to check the contents to sht_llvm_addrsig but that is
> lot of work after the format has been frozen.
> https://www.mail-archive.com/netdev@vger.kernel.org/msg348254.html
> readelf output
> [22] .llvm_addrsig     LOOS+0xfff4c03  0000000000000000 ...
> 
> b) If we don't want to recognize SHT_LLVM_ADDRSIG
> you can strip section from object file by objcopy -R .llvm_addrsig size.o
> conditionally  based on clang compiler.
> 

Hey Navin and Mark,

any update on this? I see that SHT_LLVM_ADDRSIG is still not in upstream
glibc. Are you working on that, Navin?

As for the checks, I'm not sure we can do anything here since elfutils
can't know whether a symbol is rightfully marked as address-significant.



Thanks,
Timm


-- 
Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric 
Shander

Re: [PATCH] define SHT_LLVM_ADDRSIG section rather than error out

[Mark Wielaard]

Hi Timm,

On Thu, 2021-03-04 at 14:44 +0100, Timm Bäder wrote:
> any update on this? I see that SHT_LLVM_ADDRSIG is still not in upstream
> glibc. Are you working on that, Navin?
> 
> As for the checks, I'm not sure we can do anything here since elfutils
> can't know whether a symbol is rightfully marked as address-significant.

I tried to lookup some more information about SHT_LLVM_ADDRSIG, but
couldn't really find much. There is this comment about it from the
binutils maintainers:
https://www.sourceware.org/bugzilla/show_bug.cgi?id=23817#c1

I would suggest following that and wait till the design is fixed before
trying to handle SHT_LLVM_ADDRSIG.

Cheers,

Mark

Re: [PATCH] define SHT_LLVM_ADDRSIG section rather than error out

[Timm Bäder]

On 04/03/2021 14:59, Mark Wielaard wrote:
> Hi Timm,
> 
> On Thu, 2021-03-04 at 14:44 +0100, Timm Bäder wrote:
>> any update on this? I see that SHT_LLVM_ADDRSIG is still not in upstream
>> glibc. Are you working on that, Navin?
>>
>> As for the checks, I'm not sure we can do anything here since elfutils
>> can't know whether a symbol is rightfully marked as address-significant.
> 
> I tried to lookup some more information about SHT_LLVM_ADDRSIG, but
> couldn't really find much. There is this comment about it from the
> binutils maintainers:
> https://www.sourceware.org/bugzilla/show_bug.cgi?id=23817#c1

Hm, that is unfortunate of course. I can't find a bug report about this
in llvm's bugzilla. I could try to get a discussion going there.

Are you opposed to work around this in elflint in the meantime?

E.g. ignore all sections starting with 'llvm_'?

diff --git a/src/elflint.c b/src/elflint.c
index 85cc7833..9a40045c 100644
--- a/src/elflint.c
+++ b/src/elflint.c
@@ -3766,6 +3766,9 @@ cannot get section header for section [%2zu] '%s': 
%s\n"),

        const char *scnname = elf_strptr (ebl->elf, shstrndx, 
shdr->sh_name);

+      if (strncmp (scnname, ".llvm_", 6) == 0)
+        continue;
+
        if (scnname == NULL)
         ERROR (_("section [%2zu]: invalid name\n"), cnt);
        else

(But I guess that belong after the NULL check)

Or does elflint use the any llvm_ section?
Asking because they only exist when compiling with clang of course and
elflint not handling (or ignoring) them causes the testsuite to fail.


- Timm

-- 
Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric 
Shander

Re: [PATCH] define SHT_LLVM_ADDRSIG section rather than error out

[Navin P]

On Thu, Mar 4, 2021 at 7:14 PM Timm Bäder <tbaeder@redhat.com> wrote:
>
> On 18/11/2020 06:34, Navin P via Elfutils-devel wrote:
> >>> diff --git a/src/elflint.c b/src/elflint.c
> >>> index ef3e3732..62663800 100644
> >>> --- a/src/elflint.c
> >>> +++ b/src/elflint.c
> >>> @@ -3905,6 +3905,7 @@ section [%2zu] '%s': size not multiple of entry
> >>> size\n"),
> >>>      && shdr->sh_type != SHT_GNU_ATTRIBUTES
> >>>      && shdr->sh_type != SHT_GNU_LIBLIST
> >>>      && shdr->sh_type != SHT_CHECKSUM
> >>> +   && shdr->sh_type != SHT_LLVM_ADDRSIG
> >>>      && shdr->sh_type != SHT_GNU_verdef
> >>>      && shdr->sh_type != SHT_GNU_verneed
> >>>      && shdr->sh_type != SHT_GNU_versym
> >>
> >> Note that for various of these SHT_GNU extensions we actually do have
> >> some extra checks. Do we need to check anything for a section marked
> >> SHT_LLVM_ADDRSIG?
> >>
> > We can do two things here
> > a) Recognize the section exists but ignore its contents which is what i
> > do. This needn't be the correct approach.
> > You may need to check the contents to sht_llvm_addrsig but that is
> > lot of work after the format has been frozen.
> > https://www.mail-archive.com/netdev@vger.kernel.org/msg348254.html
> > readelf output
> > [22] .llvm_addrsig     LOOS+0xfff4c03  0000000000000000 ...
> >
> > b) If we don't want to recognize SHT_LLVM_ADDRSIG
> > you can strip section from object file by objcopy -R .llvm_addrsig size.o
> > conditionally  based on clang compiler.
> >
>
> Hey Navin and Mark,
>
> any update on this? I see that SHT_LLVM_ADDRSIG is still not in upstream
> glibc. Are you working on that, Navin?
>
> As for the checks, I'm not sure we can do anything here since elfutils
> can't know whether a symbol is rightfully marked as address-significant.
>

clang has a flag  -fno-addrsig that doesn't generate the addrsig section.
https://releases.llvm.org/7.0.0/tools/clang/docs/ClangCommandLineReference.html#cmdoption-clang-faddrsig
So adding that for option for only clang should work.  I remember I
ran the tests and it worked. But you should check again.
As far as i remember this should complete the changes

Here is a small example
navin@mint-Aspire:~/c$ cat tmp.c
int main() { return 0 ; }
navin@mint-Aspire:~/c$ clang tmp.c -c
navin@mint-Aspire:~/c$ readelf -a tmp.o | grep -i addrsig
  [ 7] .llvm_addrsig     LOOS+0xfff4c03   0000000000000000  00000120
navin@mint-Aspire:~/c$ clang tmp.c -c -fno-addrsig
navin@mint-Aspire:~/c$ readelf -a tmp.o | grep -i addrsig
navin@mint-Aspire:~/c$