Re: some warnings in prep for LLVM 13
On Mon, Oct 25 2021, Mark Kettenis wrote: >> From: Jeremie Courreges-Anglas >> Date: Mon, 25 Oct 2021 14:55:16 +0100 >> >> On Mon, Oct 25 2021, Mark Kettenis wrote: >> >> Date: Mon, 25 Oct 2021 12:37:41 +0200 (CEST) >> >> From: Mark Kettenis >> >> >> >> > Date: Mon, 25 Oct 2021 12:01:11 +0200 >> >> > From: Patrick Wildt >> >> > >> >> > Hi, >> >> > >> >> > mortimer@ has this diff in his tree for LLVM 13. I actually haven't >> >> > tried to see if it works fine with LLVM 11, but I feel it needs to be >> >> > sent out and not just be blindly committed. >> >> > >> >> > If someone wants to take care of this, it would be nice, so I can take >> >> > care of the remaining parts of sending out the LLVM 13 diff. >> >> >> >> > diff --git a/lib/libc/arch/amd64/sys/brk.S >> >> > b/lib/libc/arch/amd64/sys/brk.S >> >> > index ce69679e389..ee1c11f7643 100644 >> >> > --- a/lib/libc/arch/amd64/sys/brk.S >> >> > +++ b/lib/libc/arch/amd64/sys/brk.S >> >> > @@ -48,7 +48,6 @@ __minbrk: >> >> > END(__minbrk) >> >> > .type __minbrk,@object >> >> > >> >> > - .weak brk >> >> > ENTRY(brk) >> >> > cmpq%rdi,__minbrk(%rip) >> >> > jb 1f >> >> > diff --git a/lib/libc/arch/amd64/sys/sbrk.S >> >> > b/lib/libc/arch/amd64/sys/sbrk.S >> >> > index 8d7d68909b2..db53a6bb643 100644 >> >> > --- a/lib/libc/arch/amd64/sys/sbrk.S >> >> > +++ b/lib/libc/arch/amd64/sys/sbrk.S >> >> > @@ -53,7 +53,6 @@ __curbrk: >> >> > END(__curbrk) >> >> > .type __curbrk,@object >> >> > >> >> > - .weak sbrk >> >> > ENTRY(sbrk) >> >> > movq__curbrk(%rip),%rax >> >> > movslq %edi,%rsi >> >> >> >> These functions are supposed to be weak, like they are on >> >> architectures that use a C implementation. I suppose the .weak >> >> directive needs to come *after* the .global emitted by ENTRY(). >> >> Putting it after the END() works. >> > >> > So I think the diff below is what we want here. >> >> There are others left: >> >> russell /usr/src/lib/libc$ grep -B 1 -R ^ENTRY | grep -A 1 weak >> arch/i386/sys/brk.S-.weak brk >> arch/i386/sys/brk.S:ENTRY(brk) >> arch/i386/sys/sbrk.S- .weak sbrk >> arch/i386/sys/sbrk.S:ENTRY(sbrk) >> arch/riscv64/sys/sbrk.S-.weak sbrk >> arch/riscv64/sys/sbrk.S:ENTRY(sbrk) >> arch/riscv64/sys/brk.S- .weak brk >> arch/riscv64/sys/brk.S:ENTRY(brk) >> >> (and a few others where grep -B 1 isn't enough) >> >> > ok? >> >> I checked that the resulting .*o files are the same on amd64, sparc64 >> and riscv64. Actually I have messed up my tests on amd64 and riscv64, where these changes *do* fix a problem and re-add the weak attribute to the affected symbol in the object files. >> ok jca@ >> >> Here's the diff for the other architectures. I didn't touch m88k, which >> won't use clang anytime soon. Also ok? > > ok kettenis@ Committed, thanks. -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: some warnings in prep for LLVM 13
> From: Jeremie Courreges-Anglas > Date: Mon, 25 Oct 2021 14:55:16 +0100 > > On Mon, Oct 25 2021, Mark Kettenis wrote: > >> Date: Mon, 25 Oct 2021 12:37:41 +0200 (CEST) > >> From: Mark Kettenis > >> > >> > Date: Mon, 25 Oct 2021 12:01:11 +0200 > >> > From: Patrick Wildt > >> > > >> > Hi, > >> > > >> > mortimer@ has this diff in his tree for LLVM 13. I actually haven't > >> > tried to see if it works fine with LLVM 11, but I feel it needs to be > >> > sent out and not just be blindly committed. > >> > > >> > If someone wants to take care of this, it would be nice, so I can take > >> > care of the remaining parts of sending out the LLVM 13 diff. > >> > >> > diff --git a/lib/libc/arch/amd64/sys/brk.S > >> > b/lib/libc/arch/amd64/sys/brk.S > >> > index ce69679e389..ee1c11f7643 100644 > >> > --- a/lib/libc/arch/amd64/sys/brk.S > >> > +++ b/lib/libc/arch/amd64/sys/brk.S > >> > @@ -48,7 +48,6 @@ __minbrk: > >> > END(__minbrk) > >> > .type __minbrk,@object > >> > > >> > -.weak brk > >> > ENTRY(brk) > >> > cmpq%rdi,__minbrk(%rip) > >> > jb 1f > >> > diff --git a/lib/libc/arch/amd64/sys/sbrk.S > >> > b/lib/libc/arch/amd64/sys/sbrk.S > >> > index 8d7d68909b2..db53a6bb643 100644 > >> > --- a/lib/libc/arch/amd64/sys/sbrk.S > >> > +++ b/lib/libc/arch/amd64/sys/sbrk.S > >> > @@ -53,7 +53,6 @@ __curbrk: > >> > END(__curbrk) > >> > .type __curbrk,@object > >> > > >> > -.weak sbrk > >> > ENTRY(sbrk) > >> > movq__curbrk(%rip),%rax > >> > movslq %edi,%rsi > >> > >> These functions are supposed to be weak, like they are on > >> architectures that use a C implementation. I suppose the .weak > >> directive needs to come *after* the .global emitted by ENTRY(). > >> Putting it after the END() works. > > > > So I think the diff below is what we want here. > > There are others left: > > russell /usr/src/lib/libc$ grep -B 1 -R ^ENTRY | grep -A 1 weak > arch/i386/sys/brk.S-.weak brk > arch/i386/sys/brk.S:ENTRY(brk) > arch/i386/sys/sbrk.S- .weak sbrk > arch/i386/sys/sbrk.S:ENTRY(sbrk) > arch/riscv64/sys/sbrk.S-.weak sbrk > arch/riscv64/sys/sbrk.S:ENTRY(sbrk) > arch/riscv64/sys/brk.S- .weak brk > arch/riscv64/sys/brk.S:ENTRY(brk) > > (and a few others where grep -B 1 isn't enough) > > > ok? > > I checked that the resulting .*o files are the same on amd64, sparc64 > and riscv64. ok jca@ > > Here's the diff for the other architectures. I didn't touch m88k, which > won't use clang anytime soon. Also ok? ok kettenis@ > Index: arch/arm/sys/brk.S > === > RCS file: /cvs/src/lib/libc/arch/arm/sys/brk.S,v > retrieving revision 1.10 > diff -u -p -r1.10 brk.S > --- arch/arm/sys/brk.S21 Nov 2017 19:08:36 - 1.10 > +++ arch/arm/sys/brk.S25 Oct 2021 13:48:05 - > @@ -44,8 +44,6 @@ __minbrk: > .word _C_LABEL(_end) > END(__minbrk) > > - .weak brk > - > /* > * Change the data segment size > */ > @@ -96,3 +94,4 @@ ENTRY(brk) > .Lcurbrk: > .word PIC_SYM(__curbrk, GOT) > END(brk) > + .weak brk > Index: arch/arm/sys/sbrk.S > === > RCS file: /cvs/src/lib/libc/arch/arm/sys/sbrk.S,v > retrieving revision 1.10 > diff -u -p -r1.10 sbrk.S > --- arch/arm/sys/sbrk.S 21 Nov 2017 19:08:36 - 1.10 > +++ arch/arm/sys/sbrk.S 25 Oct 2021 13:48:05 - > @@ -45,7 +45,6 @@ __curbrk: > .word _C_LABEL(_end) > END(__curbrk) > > - .weak sbrk > /* > * Change the data segment size > */ > @@ -85,3 +84,4 @@ ENTRY(sbrk) > .Lcurbrk: > .word PIC_SYM(__curbrk, GOT) > END(sbrk) > + .weak sbrk > Index: arch/i386/sys/brk.S > === > RCS file: /cvs/src/lib/libc/arch/i386/sys/brk.S,v > retrieving revision 1.13 > diff -u -p -r1.13 brk.S > --- arch/i386/sys/brk.S 19 Aug 2017 18:24:06 - 1.13 > +++ arch/i386/sys/brk.S 25 Oct 2021 13:48:05 - > @@ -42,7 +42,6 @@ __minbrk: > END(__minbrk) > .type __minbrk,@object > > - .weak brk > ENTRY(brk) > #ifdef __PIC__ > movl4(%esp),%ecx > @@ -82,3 +81,4 @@ ENTRY(brk) > SET_ERRNO() > ret > END(brk) > + .weak brk > Index: arch/i386/sys/sbrk.S > === > RCS file: /cvs/src/lib/libc/arch/i386/sys/sbrk.S,v > retrieving revision 1.13 > diff -u -p -r1.13 sbrk.S > --- arch/i386/sys/sbrk.S 19 Aug 2017 18:24:06 - 1.13 > +++ arch/i386/sys/sbrk.S 25 Oct 2021 13:48:05 - > @@ -42,7 +42,6 @@ __curbrk: .long _end > END(__curbrk) > .type __curbrk,@object > > - .weak sbrk > ENTRY(sbrk) > #ifdef __PIC__ > movl4(%esp),%ecx > @@ -76,3 +75,4 @@ ENTRY(sbrk) > SET_ERRNO() > ret > END(sbrk) >
Re: some warnings in prep for LLVM 13
On Mon, Oct 25 2021, Mark Kettenis wrote: >> Date: Mon, 25 Oct 2021 12:37:41 +0200 (CEST) >> From: Mark Kettenis >> >> > Date: Mon, 25 Oct 2021 12:01:11 +0200 >> > From: Patrick Wildt >> > >> > Hi, >> > >> > mortimer@ has this diff in his tree for LLVM 13. I actually haven't >> > tried to see if it works fine with LLVM 11, but I feel it needs to be >> > sent out and not just be blindly committed. >> > >> > If someone wants to take care of this, it would be nice, so I can take >> > care of the remaining parts of sending out the LLVM 13 diff. >> >> > diff --git a/lib/libc/arch/amd64/sys/brk.S b/lib/libc/arch/amd64/sys/brk.S >> > index ce69679e389..ee1c11f7643 100644 >> > --- a/lib/libc/arch/amd64/sys/brk.S >> > +++ b/lib/libc/arch/amd64/sys/brk.S >> > @@ -48,7 +48,6 @@ __minbrk: >> >END(__minbrk) >> >.type __minbrk,@object >> > >> > - .weak brk >> > ENTRY(brk) >> >cmpq%rdi,__minbrk(%rip) >> >jb 1f >> > diff --git a/lib/libc/arch/amd64/sys/sbrk.S >> > b/lib/libc/arch/amd64/sys/sbrk.S >> > index 8d7d68909b2..db53a6bb643 100644 >> > --- a/lib/libc/arch/amd64/sys/sbrk.S >> > +++ b/lib/libc/arch/amd64/sys/sbrk.S >> > @@ -53,7 +53,6 @@ __curbrk: >> >END(__curbrk) >> >.type __curbrk,@object >> > >> > - .weak sbrk >> > ENTRY(sbrk) >> >movq__curbrk(%rip),%rax >> >movslq %edi,%rsi >> >> These functions are supposed to be weak, like they are on >> architectures that use a C implementation. I suppose the .weak >> directive needs to come *after* the .global emitted by ENTRY(). >> Putting it after the END() works. > > So I think the diff below is what we want here. There are others left: russell /usr/src/lib/libc$ grep -B 1 -R ^ENTRY | grep -A 1 weak arch/i386/sys/brk.S-.weak brk arch/i386/sys/brk.S:ENTRY(brk) arch/i386/sys/sbrk.S- .weak sbrk arch/i386/sys/sbrk.S:ENTRY(sbrk) arch/riscv64/sys/sbrk.S-.weak sbrk arch/riscv64/sys/sbrk.S:ENTRY(sbrk) arch/riscv64/sys/brk.S- .weak brk arch/riscv64/sys/brk.S:ENTRY(brk) (and a few others where grep -B 1 isn't enough) > ok? I checked that the resulting .*o files are the same on amd64, sparc64 and riscv64. ok jca@ Here's the diff for the other architectures. I didn't touch m88k, which won't use clang anytime soon. Also ok? Index: arch/arm/sys/brk.S === RCS file: /cvs/src/lib/libc/arch/arm/sys/brk.S,v retrieving revision 1.10 diff -u -p -r1.10 brk.S --- arch/arm/sys/brk.S 21 Nov 2017 19:08:36 - 1.10 +++ arch/arm/sys/brk.S 25 Oct 2021 13:48:05 - @@ -44,8 +44,6 @@ __minbrk: .word _C_LABEL(_end) END(__minbrk) - .weak brk - /* * Change the data segment size */ @@ -96,3 +94,4 @@ ENTRY(brk) .Lcurbrk: .word PIC_SYM(__curbrk, GOT) END(brk) + .weak brk Index: arch/arm/sys/sbrk.S === RCS file: /cvs/src/lib/libc/arch/arm/sys/sbrk.S,v retrieving revision 1.10 diff -u -p -r1.10 sbrk.S --- arch/arm/sys/sbrk.S 21 Nov 2017 19:08:36 - 1.10 +++ arch/arm/sys/sbrk.S 25 Oct 2021 13:48:05 - @@ -45,7 +45,6 @@ __curbrk: .word _C_LABEL(_end) END(__curbrk) - .weak sbrk /* * Change the data segment size */ @@ -85,3 +84,4 @@ ENTRY(sbrk) .Lcurbrk: .word PIC_SYM(__curbrk, GOT) END(sbrk) + .weak sbrk Index: arch/i386/sys/brk.S === RCS file: /cvs/src/lib/libc/arch/i386/sys/brk.S,v retrieving revision 1.13 diff -u -p -r1.13 brk.S --- arch/i386/sys/brk.S 19 Aug 2017 18:24:06 - 1.13 +++ arch/i386/sys/brk.S 25 Oct 2021 13:48:05 - @@ -42,7 +42,6 @@ __minbrk: END(__minbrk) .type __minbrk,@object - .weak brk ENTRY(brk) #ifdef __PIC__ movl4(%esp),%ecx @@ -82,3 +81,4 @@ ENTRY(brk) SET_ERRNO() ret END(brk) + .weak brk Index: arch/i386/sys/sbrk.S === RCS file: /cvs/src/lib/libc/arch/i386/sys/sbrk.S,v retrieving revision 1.13 diff -u -p -r1.13 sbrk.S --- arch/i386/sys/sbrk.S19 Aug 2017 18:24:06 - 1.13 +++ arch/i386/sys/sbrk.S25 Oct 2021 13:48:05 - @@ -42,7 +42,6 @@ __curbrk: .long _end END(__curbrk) .type __curbrk,@object - .weak sbrk ENTRY(sbrk) #ifdef __PIC__ movl4(%esp),%ecx @@ -76,3 +75,4 @@ ENTRY(sbrk) SET_ERRNO() ret END(sbrk) + .weak sbrk Index: arch/powerpc/sys/brk.S === RCS file: /cvs/src/lib/libc/arch/powerpc/sys/brk.S,v retrieving revision 1.15 diff -u -p -r1.15 brk.S --- arch/powerpc/sys/brk.S 26 Oct 2020 22:07:05 - 1.15 +++ arch/powerpc/sys/brk.S 25 Oct 2021 13:48:05 - @@ -30,8 +30,6 @@ .extern __curbrk .extern _C_LABEL(_end) - .weak brk
Re: some warnings in prep for LLVM 13
> Date: Mon, 25 Oct 2021 12:37:41 +0200 (CEST) > From: Mark Kettenis > > > Date: Mon, 25 Oct 2021 12:01:11 +0200 > > From: Patrick Wildt > > > > Hi, > > > > mortimer@ has this diff in his tree for LLVM 13. I actually haven't > > tried to see if it works fine with LLVM 11, but I feel it needs to be > > sent out and not just be blindly committed. > > > > If someone wants to take care of this, it would be nice, so I can take > > care of the remaining parts of sending out the LLVM 13 diff. > > > diff --git a/lib/libc/arch/amd64/sys/brk.S b/lib/libc/arch/amd64/sys/brk.S > > index ce69679e389..ee1c11f7643 100644 > > --- a/lib/libc/arch/amd64/sys/brk.S > > +++ b/lib/libc/arch/amd64/sys/brk.S > > @@ -48,7 +48,6 @@ __minbrk: > > END(__minbrk) > > .type __minbrk,@object > > > > - .weak brk > > ENTRY(brk) > > cmpq%rdi,__minbrk(%rip) > > jb 1f > > diff --git a/lib/libc/arch/amd64/sys/sbrk.S b/lib/libc/arch/amd64/sys/sbrk.S > > index 8d7d68909b2..db53a6bb643 100644 > > --- a/lib/libc/arch/amd64/sys/sbrk.S > > +++ b/lib/libc/arch/amd64/sys/sbrk.S > > @@ -53,7 +53,6 @@ __curbrk: > > END(__curbrk) > > .type __curbrk,@object > > > > - .weak sbrk > > ENTRY(sbrk) > > movq__curbrk(%rip),%rax > > movslq %edi,%rsi > > These functions are supposed to be weak, like they are on > architectures that use a C implementation. I suppose the .weak > directive needs to come *after* the .global emitted by ENTRY(). > Putting it after the END() works. So I think the diff below is what we want here. ok? Index: lib/libc/arch/amd64/sys/brk.S === RCS file: /cvs/src/lib/libc/arch/amd64/sys/brk.S,v retrieving revision 1.10 diff -u -p -r1.10 brk.S --- lib/libc/arch/amd64/sys/brk.S 19 Aug 2017 18:24:06 - 1.10 +++ lib/libc/arch/amd64/sys/brk.S 25 Oct 2021 12:13:57 - @@ -48,7 +48,6 @@ __minbrk: END(__minbrk) .type __minbrk,@object - .weak brk ENTRY(brk) cmpq%rdi,__minbrk(%rip) jb 1f @@ -63,3 +62,4 @@ ENTRY(brk) SET_ERRNO ret END(brk) + .weak brk Index: lib/libc/arch/amd64/sys/sbrk.S === RCS file: /cvs/src/lib/libc/arch/amd64/sys/sbrk.S,v retrieving revision 1.10 diff -u -p -r1.10 sbrk.S --- lib/libc/arch/amd64/sys/sbrk.S 19 Aug 2017 18:24:06 - 1.10 +++ lib/libc/arch/amd64/sys/sbrk.S 25 Oct 2021 12:13:57 - @@ -53,7 +53,6 @@ __curbrk: END(__curbrk) .type __curbrk,@object - .weak sbrk ENTRY(sbrk) movq__curbrk(%rip),%rax movslq %edi,%rsi @@ -68,3 +67,4 @@ ENTRY(sbrk) SET_ERRNO ret END(sbrk) + .weak sbrk
Re: some warnings in prep for LLVM 13
On Mon, Oct 25, 2021 at 01:46:51PM +0200, Sebastien Marie wrote: > I have a working llvm13 here for building zig 0.9.0-dev. > > /usr/src/usr.bin/openssl/s_client.c:897:16: error: variable 'pbuf_off' set > but not used [-Werror,-Wunused-but-set-variable] > int pbuf_len, pbuf_off; > ^ > /usr/src/usr.bin/openssl/s_client.c:897:6: error: variable 'pbuf_len' set but > not used [-Werror,-Wunused-but-set-variable] > int pbuf_len, pbuf_off; > ^ Thanks. Your tree is slightly out of date. I removed pbuf_off already but missed that I could remove pbuf_len for the same reason. > > Your call regarding the 2nd & 3rd. If that structure really can be used > > in multiple files then "static" is probably not appropriate. Or maybe > > the structure should just be moved to x509/x509_lib.c. I missed this on review of job's addition of the RFC 3779 support. These files should not include this. I already gave an ok for all three diffs.
Re: some warnings in prep for LLVM 13
On Mon, Oct 25, 2021 at 12:35:03PM +0100, Jeremie Courreges-Anglas wrote:
> On Mon, Oct 25 2021, Theo Buehler wrote:
> >> index 664a5200037..e33763e7420 100644
> >> --- a/usr.bin/openssl/Makefile
> >> +++ b/usr.bin/openssl/Makefile
> >> @@ -17,6 +17,7 @@ CFLAGS+= -Wuninitialized
> >> CFLAGS+= -Wunused
> >> .if ${COMPILER_VERSION:L} == "clang"
> >> CFLAGS+= -Werror
> >> +CFLAGS+= -Wno-unused-but-set-variable
> >
> > This will break the build with LLVM 11 because of -Werror:
> >
> > error: unknown warning option '-Wno-unused-but-set-variable'; did you mean
> > '-Wno-unused-const-variable'? [-Werror,-Wunknown-warning-option]
> > *** Error 1 in /usr/src/usr.bin/openssl (:87 'apps.o')
> >
> > Also, it would be nice to know what triggered this addition.
I have a working llvm13 here for building zig 0.9.0-dev.
/usr/src/usr.bin/openssl/s_client.c:897:16: error: variable 'pbuf_off' set but
not used [-Werror,-Wunused-but-set-variable]
int pbuf_len, pbuf_off;
^
/usr/src/usr.bin/openssl/s_client.c:897:6: error: variable 'pbuf_len' set but
not used [-Werror,-Wunused-but-set-variable]
int pbuf_len, pbuf_off;
^
> You can use egcc to spot why clang 13 errors out, but they may not warn
> exactly about the same problems. This works for easy stuff, not so much
> for the kernel. We really need the clang 13 errors.
>
> Here's an attempt to use egcc over openssl and libcrypto. The s_client
> diff probably fixes what clang 13 complains about.
>
> The two latter diffs for libcrypto remove unneeded includes. This looks
> unrelated to the addition of -Wno-unused-but-set-variable by mortimer@,
> but I thought maybe you would be interested.
> egcc -Wno-unused-but-set-variable doesn't seem to find anything else in
> libcrypto.
>
> In file included from /usr/src/lib/libcrypto/x509/x509_asid.c:28:
> /usr/src/lib/libcrypto/x509/ext_dat.h:81:33: error: 'standard_exts' defined
> but not used [-Werror=unused-variable]
> static const X509V3_EXT_METHOD *standard_exts[] = {
>
> ok for the s_client diff?
ok semarie@ for s_client diff
but you could also remove pbuf_off :)
> Your call regarding the 2nd & 3rd. If that structure really can be used
> in multiple files then "static" is probably not appropriate. Or maybe
> the structure should just be moved to x509/x509_lib.c.
>
>
> Index: usr.bin/openssl/s_client.c
> ===
> RCS file: /home/cvs/src/usr.bin/openssl/s_client.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 s_client.c
> --- usr.bin/openssl/s_client.c22 Oct 2021 09:44:58 - 1.55
> +++ usr.bin/openssl/s_client.c25 Oct 2021 10:53:06 -
> @@ -894,7 +894,6 @@ s_client_main(int argc, char **argv)
> char *cbuf = NULL, *sbuf = NULL, *mbuf = NULL, *pbuf = NULL;
> int cbuf_len, cbuf_off;
> int sbuf_len, sbuf_off;
> - int pbuf_len;
> int full_log = 1;
> char *pass = NULL;
> X509 *cert = NULL;
> @@ -1195,7 +1194,6 @@ s_client_main(int argc, char **argv)
> cbuf_off = 0;
> sbuf_len = 0;
> sbuf_off = 0;
> - pbuf_len = 0;
>
> /* This is an ugly hack that does a lot of assumptions */
> /*
> @@ -1502,7 +1500,6 @@ s_client_main(int argc, char **argv)
> if (SSL_get_error(con, p) == SSL_ERROR_NONE) {
> if (p <= 0)
> goto end;
> - pbuf_len = p;
>
> k = SSL_read(con, sbuf, p);
> }
--
Sebastien Marie
Re: some warnings in prep for LLVM 13
On Mon, Oct 25 2021, Theo Buehler wrote:
>> index 664a5200037..e33763e7420 100644
>> --- a/usr.bin/openssl/Makefile
>> +++ b/usr.bin/openssl/Makefile
>> @@ -17,6 +17,7 @@ CFLAGS+= -Wuninitialized
>> CFLAGS+= -Wunused
>> .if ${COMPILER_VERSION:L} == "clang"
>> CFLAGS+= -Werror
>> +CFLAGS+= -Wno-unused-but-set-variable
>
> This will break the build with LLVM 11 because of -Werror:
>
> error: unknown warning option '-Wno-unused-but-set-variable'; did you mean
> '-Wno-unused-const-variable'? [-Werror,-Wunknown-warning-option]
> *** Error 1 in /usr/src/usr.bin/openssl (:87 'apps.o')
>
> Also, it would be nice to know what triggered this addition.
You can use egcc to spot why clang 13 errors out, but they may not warn
exactly about the same problems. This works for easy stuff, not so much
for the kernel. We really need the clang 13 errors.
Here's an attempt to use egcc over openssl and libcrypto. The s_client
diff probably fixes what clang 13 complains about.
The two latter diffs for libcrypto remove unneeded includes. This looks
unrelated to the addition of -Wno-unused-but-set-variable by mortimer@,
but I thought maybe you would be interested.
egcc -Wno-unused-but-set-variable doesn't seem to find anything else in
libcrypto.
In file included from /usr/src/lib/libcrypto/x509/x509_asid.c:28:
/usr/src/lib/libcrypto/x509/ext_dat.h:81:33: error: 'standard_exts' defined but
not used [-Werror=unused-variable]
static const X509V3_EXT_METHOD *standard_exts[] = {
ok for the s_client diff?
Your call regarding the 2nd & 3rd. If that structure really can be used
in multiple files then "static" is probably not appropriate. Or maybe
the structure should just be moved to x509/x509_lib.c.
Index: usr.bin/openssl/s_client.c
===
RCS file: /home/cvs/src/usr.bin/openssl/s_client.c,v
retrieving revision 1.55
diff -u -p -r1.55 s_client.c
--- usr.bin/openssl/s_client.c 22 Oct 2021 09:44:58 - 1.55
+++ usr.bin/openssl/s_client.c 25 Oct 2021 10:53:06 -
@@ -894,7 +894,6 @@ s_client_main(int argc, char **argv)
char *cbuf = NULL, *sbuf = NULL, *mbuf = NULL, *pbuf = NULL;
int cbuf_len, cbuf_off;
int sbuf_len, sbuf_off;
- int pbuf_len;
int full_log = 1;
char *pass = NULL;
X509 *cert = NULL;
@@ -1195,7 +1194,6 @@ s_client_main(int argc, char **argv)
cbuf_off = 0;
sbuf_len = 0;
sbuf_off = 0;
- pbuf_len = 0;
/* This is an ugly hack that does a lot of assumptions */
/*
@@ -1502,7 +1500,6 @@ s_client_main(int argc, char **argv)
if (SSL_get_error(con, p) == SSL_ERROR_NONE) {
if (p <= 0)
goto end;
- pbuf_len = p;
k = SSL_read(con, sbuf, p);
}
Index: lib/libcrypto/x509/x509_addr.c
===
RCS file: /home/cvs/src/lib/libcrypto/x509/x509_addr.c,v
retrieving revision 1.16
diff -u -p -r1.16 x509_addr.c
--- lib/libcrypto/x509/x509_addr.c 8 Sep 2021 10:49:34 - 1.16
+++ lib/libcrypto/x509/x509_addr.c 25 Oct 2021 11:09:00 -
@@ -23,8 +23,6 @@
#include
#include
-#include "ext_dat.h"
-
#ifndef OPENSSL_NO_RFC3779
/*
Index: lib/libcrypto/x509/x509_asid.c
===
RCS file: /home/cvs/src/lib/libcrypto/x509/x509_asid.c,v
retrieving revision 1.16
diff -u -p -r1.16 x509_asid.c
--- lib/libcrypto/x509/x509_asid.c 8 Sep 2021 09:49:24 - 1.16
+++ lib/libcrypto/x509/x509_asid.c 25 Oct 2021 11:09:17 -
@@ -25,8 +25,6 @@
#include
#include
-#include "ext_dat.h"
-
#ifndef OPENSSL_NO_RFC3779
static const ASN1_TEMPLATE ASRange_seq_tt[] = {
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: some warnings in prep for LLVM 13
> Date: Mon, 25 Oct 2021 12:01:11 +0200
> From: Patrick Wildt
>
> Hi,
>
> mortimer@ has this diff in his tree for LLVM 13. I actually haven't
> tried to see if it works fine with LLVM 11, but I feel it needs to be
> sent out and not just be blindly committed.
>
> If someone wants to take care of this, it would be nice, so I can take
> care of the remaining parts of sending out the LLVM 13 diff.
Some explanation on -Wno-unused-but-set-variable and
-Wno-null-pointer-subtraction and -Wno-gnu-folding-constant would be
useful. I doubt base gcc supports these so we may need to add these
only when building with clang. Or maybe disable these by default? Or
should we fix the code instead?
I guess non-amd64 arhcitectures need some love too...
> diff --git a/gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper
> b/gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper
> index 611a2169862..f4c4e224dc5 100644
> --- a/gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper
> +++ b/gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper
> @@ -7,7 +7,7 @@ TARGET_ARCH?= ${MACHINE_ARCH}
> SUBDIRS= opcodes bfd
> CONF_SUBDIRS=opcodes bfd
>
> -CFLAGS+= ${PIE_DEFAULT}
> +CFLAGS+= ${PIE_DEFAULT} -Wno-unused-but-set-variable
> -Wno-null-pointer-subtraction
> XCFLAGS= CC="${CC}" CFLAGS="${CFLAGS} ${COPTS}" LDFLAGS="${LDSTATIC}"
> # This allows moving the whole binutils installation around for
> # testing purposes
> diff --git a/lib/csu/crtbeginS.c b/lib/csu/crtbeginS.c
> index a4a7cd19fce..41500fc1442 100644
> --- a/lib/csu/crtbeginS.c
> +++ b/lib/csu/crtbeginS.c
> @@ -85,6 +85,7 @@ int _thread_atfork(void (*)(void), void (*)(void), void
> (*)(void), void *)
> __attribute__((weak));
>
> int
> +__attribute__((weak))
> pthread_atfork(void (*prep)(void), void (*parent)(void), void (*child)(void))
> {
> return (_thread_atfork(prep, parent, child, &__dso_handle));
Hmm, so in -current this function isn't actually weak. Not sure I
fully understand the consequences of that.
> diff --git a/lib/libc/arch/amd64/sys/brk.S b/lib/libc/arch/amd64/sys/brk.S
> index ce69679e389..ee1c11f7643 100644
> --- a/lib/libc/arch/amd64/sys/brk.S
> +++ b/lib/libc/arch/amd64/sys/brk.S
> @@ -48,7 +48,6 @@ __minbrk:
> END(__minbrk)
> .type __minbrk,@object
>
> - .weak brk
> ENTRY(brk)
> cmpq%rdi,__minbrk(%rip)
> jb 1f
> diff --git a/lib/libc/arch/amd64/sys/sbrk.S b/lib/libc/arch/amd64/sys/sbrk.S
> index 8d7d68909b2..db53a6bb643 100644
> --- a/lib/libc/arch/amd64/sys/sbrk.S
> +++ b/lib/libc/arch/amd64/sys/sbrk.S
> @@ -53,7 +53,6 @@ __curbrk:
> END(__curbrk)
> .type __curbrk,@object
>
> - .weak sbrk
> ENTRY(sbrk)
> movq__curbrk(%rip),%rax
> movslq %edi,%rsi
These functions are supposed to be weak, like they are on
architectures that use a C implementation. I suppose the .weak
directive needs to come *after* the .global emitted by ENTRY().
Putting it after the END() works.
> diff --git a/lib/libcrypto/Makefile b/lib/libcrypto/Makefile
> index 1a3a3888352..f6063ffb194 100644
> --- a/lib/libcrypto/Makefile
> +++ b/lib/libcrypto/Makefile
> @@ -16,7 +16,7 @@ LCRYPTO_SRC=${.CURDIR}
>
> CFLAGS+= -Wall -Wundef
> .if ${COMPILER_VERSION:L} == "clang"
> -CFLAGS+= -Werror
> +CFLAGS+= -Werror -Wno-unused-but-set-variable
> .endif
> CFLAGS+= -DLIBRESSL_INTERNAL
>
> diff --git a/sys/arch/amd64/conf/Makefile.amd64
> b/sys/arch/amd64/conf/Makefile.amd64
> index d36bae30417..83bd9a9fed1 100644
> --- a/sys/arch/amd64/conf/Makefile.amd64
> +++ b/sys/arch/amd64/conf/Makefile.amd64
> @@ -70,7 +70,8 @@ CMACHFLAGS+=-mno-retpoline
> .endif
> .if ${COMPILER_VERSION:Mclang}
> NO_INTEGR_AS=-no-integrated-as
> -CWARNFLAGS+= -Wno-address-of-packed-member -Wno-constant-conversion
> +CWARNFLAGS+= -Wno-address-of-packed-member -Wno-constant-conversion \
> + -Wno-unused-but-set-variable -Wno-gnu-folding-constant
> .endif
>
> DEBUG?= -g
> diff --git a/sys/arch/amd64/stand/pxeboot/Makefile
> b/sys/arch/amd64/stand/pxeboot/Makefile
> index 9b028d8f4aa..cc92233463b 100644
> --- a/sys/arch/amd64/stand/pxeboot/Makefile
> +++ b/sys/arch/amd64/stand/pxeboot/Makefile
> @@ -71,7 +71,7 @@ CPPFLAGS+=-DBOOTMAGIC=$(BOOTMAGIC) ${DEBUGFLAGS}
> -DLINKADDR=${LINKADDR}
> CPPFLAGS+=-DSLOW -DSMALL -DNOBYFOUR -DNO_GZIP -DDYNAMIC_CRC_TABLE
> -DBUILDFIXED
> CPPFLAGS+=-DHEAP_LIMIT=${HEAP_LIMIT} -I${S}/stand/boot #-DCOMPAT_UFS
> CFLAGS+=-m32
> -CFLAGS+=$(SACFLAGS) -D__INTERNAL_LIBSA_CREAD -fno-pie
> +CFLAGS+=$(SACFLAGS) -D__INTERNAL_LIBSA_CREAD -fno-pie
> -Wno-unused-but-set-variable
> AFLAGS+=${NO_INTEGR_AS}
> # AFLAGS+=-Wa,-a
> AFLAGS+=-m32 # -Wa,-R
> diff --git a/usr.bin/openssl/Makefile b/usr.bin/openssl/Makefile
> index 664a5200037..e33763e7420 100644
> --- a/usr.bin/openssl/Makefile
> +++ b/usr.bin/openssl/Makefile
> @@ -17,6 +17,7 @@ CFLAGS+= -Wuninitialized
> CFLAGS+= -Wunused
> .if ${COMPILER_VERSION:L} == "clang"
> CFLA
Re: some warnings in prep for LLVM 13
> index 664a5200037..e33763e7420 100644
> --- a/usr.bin/openssl/Makefile
> +++ b/usr.bin/openssl/Makefile
> @@ -17,6 +17,7 @@ CFLAGS+= -Wuninitialized
> CFLAGS+= -Wunused
> .if ${COMPILER_VERSION:L} == "clang"
> CFLAGS+= -Werror
> +CFLAGS+= -Wno-unused-but-set-variable
This will break the build with LLVM 11 because of -Werror:
error: unknown warning option '-Wno-unused-but-set-variable'; did you mean
'-Wno-unused-const-variable'? [-Werror,-Wunknown-warning-option]
*** Error 1 in /usr/src/usr.bin/openssl (:87 'apps.o')
Also, it would be nice to know what triggered this addition.
some warnings in prep for LLVM 13
Hi,
mortimer@ has this diff in his tree for LLVM 13. I actually haven't
tried to see if it works fine with LLVM 11, but I feel it needs to be
sent out and not just be blindly committed.
If someone wants to take care of this, it would be nice, so I can take
care of the remaining parts of sending out the LLVM 13 diff.
Patrick
diff --git a/gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper
b/gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper
index 611a2169862..f4c4e224dc5 100644
--- a/gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper
+++ b/gnu/usr.bin/binutils-2.17/Makefile.bsd-wrapper
@@ -7,7 +7,7 @@ TARGET_ARCH?= ${MACHINE_ARCH}
SUBDIRS= opcodes bfd
CONF_SUBDIRS= opcodes bfd
-CFLAGS+= ${PIE_DEFAULT}
+CFLAGS+= ${PIE_DEFAULT} -Wno-unused-but-set-variable
-Wno-null-pointer-subtraction
XCFLAGS= CC="${CC}" CFLAGS="${CFLAGS} ${COPTS}" LDFLAGS="${LDSTATIC}"
# This allows moving the whole binutils installation around for
# testing purposes
diff --git a/lib/csu/crtbeginS.c b/lib/csu/crtbeginS.c
index a4a7cd19fce..41500fc1442 100644
--- a/lib/csu/crtbeginS.c
+++ b/lib/csu/crtbeginS.c
@@ -85,6 +85,7 @@ int _thread_atfork(void (*)(void), void (*)(void), void
(*)(void), void *)
__attribute__((weak));
int
+__attribute__((weak))
pthread_atfork(void (*prep)(void), void (*parent)(void), void (*child)(void))
{
return (_thread_atfork(prep, parent, child, &__dso_handle));
diff --git a/lib/libc/arch/amd64/sys/brk.S b/lib/libc/arch/amd64/sys/brk.S
index ce69679e389..ee1c11f7643 100644
--- a/lib/libc/arch/amd64/sys/brk.S
+++ b/lib/libc/arch/amd64/sys/brk.S
@@ -48,7 +48,6 @@ __minbrk:
END(__minbrk)
.type __minbrk,@object
- .weak brk
ENTRY(brk)
cmpq%rdi,__minbrk(%rip)
jb 1f
diff --git a/lib/libc/arch/amd64/sys/sbrk.S b/lib/libc/arch/amd64/sys/sbrk.S
index 8d7d68909b2..db53a6bb643 100644
--- a/lib/libc/arch/amd64/sys/sbrk.S
+++ b/lib/libc/arch/amd64/sys/sbrk.S
@@ -53,7 +53,6 @@ __curbrk:
END(__curbrk)
.type __curbrk,@object
- .weak sbrk
ENTRY(sbrk)
movq__curbrk(%rip),%rax
movslq %edi,%rsi
diff --git a/lib/libcrypto/Makefile b/lib/libcrypto/Makefile
index 1a3a3888352..f6063ffb194 100644
--- a/lib/libcrypto/Makefile
+++ b/lib/libcrypto/Makefile
@@ -16,7 +16,7 @@ LCRYPTO_SRC= ${.CURDIR}
CFLAGS+= -Wall -Wundef
.if ${COMPILER_VERSION:L} == "clang"
-CFLAGS+= -Werror
+CFLAGS+= -Werror -Wno-unused-but-set-variable
.endif
CFLAGS+= -DLIBRESSL_INTERNAL
diff --git a/sys/arch/amd64/conf/Makefile.amd64
b/sys/arch/amd64/conf/Makefile.amd64
index d36bae30417..83bd9a9fed1 100644
--- a/sys/arch/amd64/conf/Makefile.amd64
+++ b/sys/arch/amd64/conf/Makefile.amd64
@@ -70,7 +70,8 @@ CMACHFLAGS+= -mno-retpoline
.endif
.if ${COMPILER_VERSION:Mclang}
NO_INTEGR_AS= -no-integrated-as
-CWARNFLAGS+= -Wno-address-of-packed-member -Wno-constant-conversion
+CWARNFLAGS+= -Wno-address-of-packed-member -Wno-constant-conversion \
+ -Wno-unused-but-set-variable -Wno-gnu-folding-constant
.endif
DEBUG?=-g
diff --git a/sys/arch/amd64/stand/pxeboot/Makefile
b/sys/arch/amd64/stand/pxeboot/Makefile
index 9b028d8f4aa..cc92233463b 100644
--- a/sys/arch/amd64/stand/pxeboot/Makefile
+++ b/sys/arch/amd64/stand/pxeboot/Makefile
@@ -71,7 +71,7 @@ CPPFLAGS+=-DBOOTMAGIC=$(BOOTMAGIC) ${DEBUGFLAGS}
-DLINKADDR=${LINKADDR}
CPPFLAGS+=-DSLOW -DSMALL -DNOBYFOUR -DNO_GZIP -DDYNAMIC_CRC_TABLE -DBUILDFIXED
CPPFLAGS+=-DHEAP_LIMIT=${HEAP_LIMIT} -I${S}/stand/boot #-DCOMPAT_UFS
CFLAGS+=-m32
-CFLAGS+=$(SACFLAGS) -D__INTERNAL_LIBSA_CREAD -fno-pie
+CFLAGS+=$(SACFLAGS) -D__INTERNAL_LIBSA_CREAD -fno-pie
-Wno-unused-but-set-variable
AFLAGS+=${NO_INTEGR_AS}
# AFLAGS+=-Wa,-a
AFLAGS+=-m32 # -Wa,-R
diff --git a/usr.bin/openssl/Makefile b/usr.bin/openssl/Makefile
index 664a5200037..e33763e7420 100644
--- a/usr.bin/openssl/Makefile
+++ b/usr.bin/openssl/Makefile
@@ -17,6 +17,7 @@ CFLAGS+= -Wuninitialized
CFLAGS+= -Wunused
.if ${COMPILER_VERSION:L} == "clang"
CFLAGS+= -Werror
+CFLAGS+= -Wno-unused-but-set-variable
.endif
CFLAGS+= -DLIBRESSL_INTERNAL
diff --git a/sys/arch/amd64/conf/Makefile.amd64
b/sys/arch/amd64/conf/Makefile.amd64
index d36bae30417..83bd9a9fed1 100644
--- a/sys/arch/amd64/conf/Makefile.amd64
+++ b/sys/arch/amd64/conf/Makefile.amd64
@@ -70,7 +70,8 @@ CMACHFLAGS+= -mno-retpoline
.endif
.if ${COMPILER_VERSION:Mclang}
NO_INTEGR_AS= -no-integrated-as
-CWARNFLAGS+= -Wno-address-of-packed-member -Wno-constant-conversion
+CWARNFLAGS+= -Wno-address-of-packed-member -Wno-constant-conversion \
+ -Wno-unused-but-set-variable -Wno-gnu-folding-constant
.endif
DEBUG?=-g
diff --git a/sys/arch/amd64/stand/pxeboot/Makefile
b/sys/arch/amd64/stand/pxeboot/Makefile
index 9b028d8f4aa..cc92233463b 100644
--- a/sys/arch/amd64/stand/pxeboot/Makefile
+++ b/sys/arch/amd64/stand/pxeboot/Makefile
@@ -71,7 +71,7 @@ CPPFLAGS+=-DBOOTMAGIC=$(BOOTMAGIC) ${DEBUGFLAGS}
-
