[PATCH] debuginfod-client: Don't compare a double to a long
From: Timm Bäder clang warns about this: ../../debuginfod/debuginfod-client.c:899:28: error: implicit conversion from 'long' to 'double' changes value from 9223372036854775807 to 9223372036854775808 [-Werror,-Wimplicit-int-float-conversion] pa = (dl > LONG_MAX ? LONG_MAX : (long)dl); ~ ^~~~ /usr/lib64/clang/10.0.1/include/limits.h:47:19: note: expanded from macro 'LONG_MAX' ^~~~ :38:22: note: expanded from here ^~~~ ../../debuginfod/debuginfod-client.c:917:28: error: implicit conversion from 'long' to 'double' changes value from 9223372036854775807 to 9223372036854775808 [-Werror,-Wimplicit-int-float-conversion] pb = (cl > LONG_MAX ? LONG_MAX : (long)cl); ~ ^~~~ /usr/lib64/clang/10.0.1/include/limits.h:47:19: note: expanded from macro 'LONG_MAX' ^~~~ :38:22: note: expanded from here ^~~~ --- debuginfod/debuginfod-client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index de26af5b..df238e07 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -896,7 +896,7 @@ debuginfod_query_server (debuginfod_client *c, CURLINFO_SIZE_DOWNLOAD, &dl); if (curl_res == 0) -pa = (dl > LONG_MAX ? LONG_MAX : (long)dl); +pa = (dl > (double)LONG_MAX ? LONG_MAX : (long)dl); #endif /* NB: If going through deflate-compressing proxies, this @@ -914,7 +914,7 @@ debuginfod_query_server (debuginfod_client *c, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &cl); if (curl_res == 0) -pb = (cl > LONG_MAX ? LONG_MAX : (long)cl); +pb = (cl > (double)LONG_MAX ? LONG_MAX : (long)cl); #endif } -- 2.26.2
debuginfod-client: Fix typo in curl feature detection
I just looked at this for too long and became confused by the naming. It seems like there's one CURLINFO_ too much in there. Googling seems to confirm. - Timm
[PATCH] debginfod-client: Fix typo in curl feature detection
From: Timm Bäder CURLINFO_CURLINFO_CONTENT_LENGTH_DOWNLOAD_T is one CURLINFO_ too much. Signed-off-by: Timm Bäder --- debuginfod/debuginfod-client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index df238e07..a1394737 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -901,7 +901,7 @@ debuginfod_query_server (debuginfod_client *c, /* NB: If going through deflate-compressing proxies, this number is likely to be unavailable, so -1 may show. */ -#ifdef CURLINFO_CURLINFO_CONTENT_LENGTH_DOWNLOAD_T +#ifdef CURLINFO_CONTENT_LENGTH_DOWNLOAD_T curl_off_t cl; curl_res = curl_easy_getinfo(target_handle, CURLINFO_CONTENT_LENGTH_DOWNLOAD_T, -- 2.26.2
Re: [PATCH] define SHT_LLVM_ADDRSIG section rather than error out
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 ... 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
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
On Thu, Mar 4, 2021 at 7:14 PM Timm Bäder 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 ...
> >
> > 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 0120
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$
