Re: CVS commit: src/external/cddl/osnet/lib/libdtrace
On Mon, Mar 16, 2020 at 8:18 PM Santhosh Raju wrote: > > On Mon, Mar 16, 2020 at 8:02 PM Christos Zoulas wrote: > > > > In article <20200317005012.daf4af...@cvs.netbsd.org>, > > Santhosh Raju wrote: > > >-=-=-=-=-=- > > > > > >Module Name: src > > >Committed By: fox > > >Date: Tue Mar 17 00:50:12 UTC 2020 > > > > > >Modified Files: > > > src/external/cddl/osnet/lib/libdtrace: Makefile > > > > > >Log Message: > > >external/cddl/osnet: Supress -Werror=maybe-uninitialized error in > > >libdtrace. > > > > > >It looks like this is a false positive, since the section of code > > >triggering the error > > > > > >external/cddl/osnet/dist/lib/libdtrace/common/dt_proc.c:400:42: > > > > > >is only accessed after "err" is initialized. > > > > > >Error was reported when build.sh was run with MKLIBCSANITIZER=yes flag. > > > > You did not just suppress the error; you suppressed the warning too... > > There is a difference between -Wno-error=maybe-uninitialized and > > -Wno-maybe-uninitialized. I think we want the first flavor, otherwise > > this is a large axe that will hide other warnings in the long run. > > > > Agreed, I shall make the change to be -Wno-error=maybe-uninitialized. > https://mail-index.netbsd.org/source-changes/2020/03/17/msg115173.html Should be fixed in this commit. -- Santhosh
Re: CVS commit: src/external/cddl/osnet/lib/libdtrace
On Mon, Mar 16, 2020 at 8:02 PM Christos Zoulas wrote: > > In article <20200317005012.daf4af...@cvs.netbsd.org>, > Santhosh Raju wrote: > >-=-=-=-=-=- > > > >Module Name: src > >Committed By: fox > >Date: Tue Mar 17 00:50:12 UTC 2020 > > > >Modified Files: > > src/external/cddl/osnet/lib/libdtrace: Makefile > > > >Log Message: > >external/cddl/osnet: Supress -Werror=maybe-uninitialized error in libdtrace. > > > >It looks like this is a false positive, since the section of code > >triggering the error > > > >external/cddl/osnet/dist/lib/libdtrace/common/dt_proc.c:400:42: > > > >is only accessed after "err" is initialized. > > > >Error was reported when build.sh was run with MKLIBCSANITIZER=yes flag. > > You did not just suppress the error; you suppressed the warning too... > There is a difference between -Wno-error=maybe-uninitialized and > -Wno-maybe-uninitialized. I think we want the first flavor, otherwise > this is a large axe that will hide other warnings in the long run. > Agreed, I shall make the change to be -Wno-error=maybe-uninitialized. > Now perhaps it is better to do what we've been doing traditionallly: > over-initialize the variable with 'foo = 0; // XXX: gcc', but that's > more book-keeping (but at least it is localized as opposed to suppress > for the entire compilation unit). Please note that we don't have a good > way to go around and test those error-avoidance overrides each time we > upgrade the compiler so they tend to stick forever. > I did think of over-initialize here, but I did not know if it would change the desired behavior of the code. > christos1 > -- Santhosh
Re: CVS commit: src/external/cddl/osnet/dist/uts/common/fs/zfs
On Wed, Mar 11, 2020 at 7:22 AM Greg Troxel wrote: > > J. Hannken-Illjes writes: > > >> Forgot to add in the commit log, the changes have been pulled in from > >> upstream openzfs. > >> > >> https://github.com/openzfs/zfs/commit/928e8ad47d3478a3d5d01f0dd6ae74a9371af65e#diff-9fd6b453f8153161abe0728c449e6505R4386 > > > > This is NOT our upstream -- > For the sake of completeness, the upstream issue was discussed in a private thread and kamil@ pointed out that the commit exists in the ZFS on FreeBSD repository too. https://github.com/zfsonfreebsd/ZoF/commit/928e8ad47d3478a3d5d01f0dd6ae74a9371af65e Which was pointed out as the upstream from which current implementation of NetBSD's ZFS was taken. > This seems to be hard to figure out. Is there someplace in the tree > that says what our upsream is, and what theirs is, and how all of the > various trees out there relate? And how we should be sending things, > and getting things? > > I tried to figure this out, and ended up with > > http://wiki.netbsd.org/zfs/ > > which has lots of \todo statements. -- Santhosh
Re: CVS commit: src/external/cddl/osnet/dist/uts/common/fs/zfs
On Tue, Mar 10, 2020 at 7:25 AM Santhosh Raju wrote: > > Module Name:src > Committed By: fox > Date: Mon Mar 9 15:40:50 UTC 2020 > > Modified Files: > src/external/cddl/osnet/dist/uts/common/fs/zfs: metaslab.c > > Log Message: > external/cddl/osnet: Fix possible null pointer access. > > Detected by UBSan and fixed upstream, pick only the fix from the commit. > > Cherry-pick: > From 928e8ad47d3478a3d5d01f0dd6ae74a9371af65e Mon Sep 17 00:00:00 2001 > From: Serapheim Dimitropoulos > Date: Wed, 20 Feb 2019 09:59:57 -0800 > Subject: [PATCH] Introduce auxiliary metaslab histograms > > Reviewed by: Paul Dagnelie > Reviewed-by: Brian Behlendorf > Reviewed by: Matt Ahrens > Signed-off-by: Serapheim Dimitropoulos > Closes #8358 > > Reviewed by: kamil@ > > > To generate a diff of this commit: > cvs rdiff -u -r1.1.1.3 -r1.2 \ > src/external/cddl/osnet/dist/uts/common/fs/zfs/metaslab.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > Forgot to add in the commit log, the changes have been pulled in from upstream openzfs. https://github.com/openzfs/zfs/commit/928e8ad47d3478a3d5d01f0dd6ae74a9371af65e#diff-9fd6b453f8153161abe0728c449e6505R4386 -- Santhosh
Re: CVS commit: src/external/bsd/dhcpcd/dist/src
On Thu, Feb 13, 2020 at 4:44 PM Santhosh Raju wrote:
>
> On Thu, Feb 13, 2020 at 4:32 PM Kamil Rytarowski wrote:
> >
> > On 13.02.2020 22:20, Valery Ushakov wrote:
> > > I did not propose to disable the warning. I proposed to downgrade
> > > -Werror to -Wno-error (i.e. a warning) and only for the buggy
> > > sanitizer build. That file will still be compiled in normal builds
> > > with all the warnings=errors enabled, so real problems won't be
> > > overlooked.
> >
> > OK, we can try this path.
> >
> > Santosh, could you please revert and try -Wno-error + upstream it?
> >
> > Thank you in advance!
> >
>
> Sure, let me prepare the patch.
>
The patch has been prepared.
The builds were run both with and without MKLIBCSANITIZER=yes and it
was completed successfully.
Let us know if it alright to commit this.
> --
> Santhosh
--
Santhosh
Index: dist/src/dhcp.c
===
RCS file: /cvsroot/src/external/bsd/dhcpcd/dist/src/dhcp.c,v
retrieving revision 1.33
diff -u -p -u -r1.33 dhcp.c
--- dist/src/dhcp.c 8 Feb 2020 12:17:16 - 1.33
+++ dist/src/dhcp.c 14 Feb 2020 20:32:20 -
@@ -3307,7 +3307,7 @@ is_packet_udp_bootp(void *packet, size_t
memcpy(, (char *)ip + ip_hlen, sizeof(udp));
if (ntohs(udp.uh_ulen) < sizeof(udp))
return false;
- if (ip_hlen + (size_t)ntohs(udp.uh_ulen) > plen)
+ if (ip_hlen + ntohs(udp.uh_ulen) > plen)
return false;
/* Check it's to and from the right ports. */
Index: sbin/dhcpcd/Makefile
===
RCS file: /cvsroot/src/external/bsd/dhcpcd/sbin/dhcpcd/Makefile,v
retrieving revision 1.50
diff -u -p -u -r1.50 Makefile
--- sbin/dhcpcd/Makefile 29 Jan 2020 23:42:57 - 1.50
+++ sbin/dhcpcd/Makefile 14 Feb 2020 20:32:20 -
@@ -27,6 +27,11 @@ SRCS+= auth.c
.if (${USE_INET} != "no")
CPPFLAGS+= -DINET
SRCS+= bpf.c dhcp.c ipv4.c
+.if (${MKLIBCSANITIZER:Uno} == "yes")
+.if (${ACTIVE_CC} == "gcc" && ${HAVE_GCC:U0} == 8)
+COPTS.dhcp.c+= -Wno-error=sign-conversion
+.endif
+.endif
.if !defined(SMALLPROG)
CPPFLAGS+= -DARP
SRCS+= arp.c
Re: CVS commit: src/external/bsd/dhcpcd/dist/src
On Thu, Feb 13, 2020 at 4:32 PM Kamil Rytarowski wrote: > > On 13.02.2020 22:20, Valery Ushakov wrote: > > I did not propose to disable the warning. I proposed to downgrade > > -Werror to -Wno-error (i.e. a warning) and only for the buggy > > sanitizer build. That file will still be compiled in normal builds > > with all the warnings=errors enabled, so real problems won't be > > overlooked. > > OK, we can try this path. > > Santosh, could you please revert and try -Wno-error + upstream it? > > Thank you in advance! > Sure, let me prepare the patch. -- Santhosh
