Re: unlock mmap(2) for anonymous mappings
> Date: Tue, 11 Jan 2022 23:13:20 +
> From: Klemens Nanni
>
> On Tue, Jan 11, 2022 at 09:54:44AM -0700, Theo de Raadt wrote:
> > > Now this is clearly a "slow" path. I don't think there is any reason
> > > not to put all the code in that if (uvw_wxabort) block under the
> > > kernel lock. For now I think making access to ps_wxcounter atomic is
> > > just too fine grained.
> >
> > Right. Lock the whole block.
>
> Thanks everyone, here's the combined diff for that.
I think mpi@ should be involved in the actual unlocking of mmap(2),
munmap(2) and mprotect(2). But the changes to uvm_mmap.c are ok
kettenis@ and can be committed now.
> Index: kern/syscalls.master
> ===
> RCS file: /cvs/src/sys/kern/syscalls.master,v
> retrieving revision 1.222
> diff -u -p -r1.222 syscalls.master
> --- kern/syscalls.master 11 Jan 2022 08:09:14 - 1.222
> +++ kern/syscalls.master 11 Jan 2022 23:10:50 -
> @@ -126,7 +126,7 @@
> struct sigaction *osa); }
> 47 STD NOLOCK { gid_t sys_getgid(void); }
> 48 STD NOLOCK { int sys_sigprocmask(int how, sigset_t mask); }
> -49 STD { void *sys_mmap(void *addr, size_t len, int prot, \
> +49 STD NOLOCK { void *sys_mmap(void *addr, size_t len, int prot, \
> int flags, int fd, off_t pos); }
> 50 STD { int sys_setlogin(const char *namebuf); }
> #ifdef ACCOUNTING
> Index: uvm/uvm_mmap.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_mmap.c,v
> retrieving revision 1.168
> diff -u -p -r1.168 uvm_mmap.c
> --- uvm/uvm_mmap.c5 Jan 2022 17:53:44 - 1.168
> +++ uvm/uvm_mmap.c11 Jan 2022 23:02:13 -
> @@ -183,12 +183,14 @@ uvm_wxcheck(struct proc *p, char *call)
> return 0;
>
> if (uvm_wxabort) {
> + KERNEL_LOCK();
> /* Report W^X failures */
> if (pr->ps_wxcounter++ == 0)
> log(LOG_NOTICE, "%s(%d): %s W^X violation\n",
> pr->ps_comm, pr->ps_pid, call);
> /* Send uncatchable SIGABRT for coredump */
> sigexit(p, SIGABRT);
> + KERNEL_UNLOCK();
> }
>
> return ENOTSUP;
>
Driver for Cadence SD/SDIO/eMMC host controller
This adds a basic driver (glue) for the Cadence SD/SDIO/eMMC host controller. The controller is mostly SDHC compatible. However, the SDHC register set is at an offset from the start of the register region. In addition, the controller handles card detect through SDHC registers, and will probably need extra code for tuning for higher speed modes. Therefore I have added a separate driver module, instead of tweaking sdhc_fdt.c. OK? Index: share/man/man4/Makefile === RCS file: src/share/man/man4/Makefile,v retrieving revision 1.816 diff -u -p -r1.816 Makefile --- share/man/man4/Makefile 10 Jan 2022 04:59:19 - 1.816 +++ share/man/man4/Makefile 14 Jan 2022 17:12:53 - @@ -24,7 +24,7 @@ MAN= aac.4 abcrtc.4 abl.4 ac97.4 acphy.4 berkwdt.4 bge.4 bgw.4 bio.4 bpe.4 bktr.4 bmtphy.4 bnx.4 bnxt.4 \ boca.4 bpf.4 brgphy.4 bridge.4 brswphy.4 bse.4 bwfm.4 bwi.4 bytgpio.4 \ cac.4 cad.4 cas.4 cardbus.4 carp.4 ccp.4 ccpmic.4 cd.4 cdce.4 \ - cduart.4 cfxga.4 \ + cdsdhc.4 cduart.4 cfxga.4 \ ch.4 chvgpio.4 ciphy.4 ciss.4 clcs.4 clct.4 cmpci.4 \ com.4 cue.4 cwfg.4 cy.4 cz.4 \ dapmic.4 \ Index: share/man/man4/cdsdhc.4 === RCS file: share/man/man4/cdsdhc.4 diff -N share/man/man4/cdsdhc.4 --- /dev/null 1 Jan 1970 00:00:00 - +++ share/man/man4/cdsdhc.4 14 Jan 2022 17:12:53 - @@ -0,0 +1,40 @@ +.\"$OpenBSD$ +.\" +.\" Copyright (c) 2022 Visa Hankala +.\" +.\" Permission to use, copy, modify, and distribute this software for any +.\" purpose with or without fee is hereby granted, provided that the above +.\" copyright notice and this permission notice appear in all copies. +.\" +.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES +.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR +.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES +.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN +.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF +.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +.\" +.Dd $Mdocdate$ +.Dt CDSDHC 4 +.Os +.Sh NAME +.Nm cdsdhc +.Nd Cadence SD/SDIO/eMMC host controller +.Sh SYNOPSIS +.Cd "cdsdhc* at fdt?" +.Cd "sdmmc* at cdsdhc?" +.Sh DESCRIPTION +The +.Nm +driver provides support for the Cadence SD/SDIO/eMMC host controller, +which provides an interface to the +.Xr sdmmc 4 +bus. +.Sh SEE ALSO +.Xr intro 4 , +.Xr sdmmc 4 +.Sh HISTORY +The +.Nm +driver first appeared in +.Ox 7.1 . Index: sys/arch/riscv64/conf/GENERIC === RCS file: src/sys/arch/riscv64/conf/GENERIC,v retrieving revision 1.32 diff -u -p -r1.32 GENERIC --- sys/arch/riscv64/conf/GENERIC 5 Jan 2022 03:32:44 - 1.32 +++ sys/arch/riscv64/conf/GENERIC 14 Jan 2022 17:12:54 - @@ -45,6 +45,8 @@ intc0 at cpu0 com* at fdt? # PolarFire SoCs +cdsdhc*at fdt? +sdmmc* at cdsdhc? mpfclock* at fdt? early 1 # SiFive SoCs Index: sys/arch/riscv64/conf/RAMDISK === RCS file: src/sys/arch/riscv64/conf/RAMDISK,v retrieving revision 1.28 diff -u -p -r1.28 RAMDISK --- sys/arch/riscv64/conf/RAMDISK 5 Jan 2022 03:32:44 - 1.28 +++ sys/arch/riscv64/conf/RAMDISK 14 Jan 2022 17:12:54 - @@ -36,6 +36,8 @@ intc0 at cpu0 com* at fdt? # PolarFire SoCs +cdsdhc*at fdt? +sdmmc* at cdsdhc? mpfclock* at fdt? early 1 # SiFive SoCs Index: sys/dev/fdt/cdsdhc.c === RCS file: sys/dev/fdt/cdsdhc.c diff -N sys/dev/fdt/cdsdhc.c --- /dev/null 1 Jan 1970 00:00:00 - +++ sys/dev/fdt/cdsdhc.c14 Jan 2022 17:12:54 - @@ -0,0 +1,168 @@ +/* $OpenBSD$ */ + +/* + * Copyright (c) 2022 Visa Hankala + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +/* + * Driver glue for Cadence SD/SDIO/eMMC host controller. + */ + +#include
Re: clang: compile static analyzer
On Fri, Jan 14, 2022 at 04:44:49PM +, Stuart Henderson wrote: > On 2022/01/14 16:52, Rafael Sadowski wrote: > > On Fri Jan 14, 2022 at 03:17:21PM +0100, Tobias Heider wrote: > > > Hi, > > > > > > clang ships with a pretty useful static analyzer to find all kinds of bugs > > > in C and C++ code: > > > > > > https://clang-analyzer.llvm.org/ > > > > > > I use it regularly to check my own diffs and found plenty of bugs I could > > > have missed otherwise. While we have the code in base we don't actually > > > build it into our libclang currently, so the only ways to use it are > > > manually modifying the Makefiles or installing llvm from ports. > > > > > > I was wondering if anyone else uses this and if there was any interest to > > > have this in our base clang? > > > > Please checkout devel/clang-tools-extra, if you missed something let me > > know. > > CLANG_ENABLE_STATIC_ANALYZER=ON is enabled by default. > > No need for clang-tools-extra, it is in the llvm package > > Like others I am happy just having the static analyzer / scan-build in > ports. It is useful, but I think not really useful enough to add to the > time taken by llvm in every base build when it's available in ports. The build spends a lot of time in lldb and its usefulness is questionable. Why do not tell people to install lldb from ports as well? Honestly the build time of clang is already so bad that adding an extra few 100 objects to it would not make it much worse. -- :wq Claudio
Re: clang: compile static analyzer
On 2022/01/14 16:52, Rafael Sadowski wrote: > On Fri Jan 14, 2022 at 03:17:21PM +0100, Tobias Heider wrote: > > Hi, > > > > clang ships with a pretty useful static analyzer to find all kinds of bugs > > in C and C++ code: > > > > https://clang-analyzer.llvm.org/ > > > > I use it regularly to check my own diffs and found plenty of bugs I could > > have missed otherwise. While we have the code in base we don't actually > > build it into our libclang currently, so the only ways to use it are > > manually modifying the Makefiles or installing llvm from ports. > > > > I was wondering if anyone else uses this and if there was any interest to > > have this in our base clang? > > Please checkout devel/clang-tools-extra, if you missed something let me know. > CLANG_ENABLE_STATIC_ANALYZER=ON is enabled by default. No need for clang-tools-extra, it is in the llvm package Like others I am happy just having the static analyzer / scan-build in ports. It is useful, but I think not really useful enough to add to the time taken by llvm in every base build when it's available in ports.
Re: clang: compile static analyzer
On Fri Jan 14, 2022 at 03:17:21PM +0100, Tobias Heider wrote:
> Hi,
>
> clang ships with a pretty useful static analyzer to find all kinds of bugs
> in C and C++ code:
>
> https://clang-analyzer.llvm.org/
>
> I use it regularly to check my own diffs and found plenty of bugs I could
> have missed otherwise. While we have the code in base we don't actually
> build it into our libclang currently, so the only ways to use it are
> manually modifying the Makefiles or installing llvm from ports.
>
> I was wondering if anyone else uses this and if there was any interest to
> have this in our base clang?
Please checkout devel/clang-tools-extra, if you missed something let me know.
CLANG_ENABLE_STATIC_ANALYZER=ON is enabled by default.
Rafael
>
> diff --git gnu/usr.bin/clang/Makefile gnu/usr.bin/clang/Makefile
> index 6cf71d36cf8..b47abc02474 100644
> --- gnu/usr.bin/clang/Makefile
> +++ gnu/usr.bin/clang/Makefile
> @@ -39,6 +39,11 @@ SUBDIR+=libclangSerialization
> SUBDIR+=libclangFrontend
> SUBDIR+=libclangRewriteFrontend
> SUBDIR+=libclangFrontendTool
> +SUBDIR+=libclangASTMatchers
> +SUBDIR+=libclangCrossTU
> +SUBDIR+=libclangStaticAnalyzer
> +SUBDIR+=libclangIndex
> +SUBDIR+=libclangTooling
>
> SUBDIR+=clang
>
> diff --git gnu/usr.bin/clang/clang/Makefile gnu/usr.bin/clang/clang/Makefile
> index 76535ee8842..7d3847f883d 100644
> --- gnu/usr.bin/clang/clang/Makefile
> +++ gnu/usr.bin/clang/clang/Makefile
> @@ -43,6 +43,11 @@ LLVM_LIBDEPS= LLVM \
> clangEdit \
> clangAST \
> clangLex \
> - clangBasic
> + clangBasic \
> + clangStaticAnalyzer \
> + clangCrossTU \
> + clangASTMatchers \
> + clangIndex \
> + clangTooling \
>
> .include
> diff --git gnu/usr.bin/clang/include/clang/Config/config.h
> gnu/usr.bin/clang/include/clang/Config/config.h
> index 02a049bf2be..93494d6a0b1 100644
> --- gnu/usr.bin/clang/include/clang/Config/config.h
> +++ gnu/usr.bin/clang/include/clang/Config/config.h
> @@ -78,7 +78,7 @@
> /* Enable each functionality of modules */
> #define CLANG_ENABLE_ARCMT 0
> #define CLANG_ENABLE_OBJC_REWRITER 0
> -#define CLANG_ENABLE_STATIC_ANALYZER 0
> +#define CLANG_ENABLE_STATIC_ANALYZER 1
>
> /* Spawn a new process clang.exe for the CC1 tool invocation, when necessary
> */
> #define CLANG_SPAWN_CC1 0
> diff --git gnu/usr.bin/clang/libclangASTMatchers/Makefile
> gnu/usr.bin/clang/libclangASTMatchers/Makefile
> new file mode 100644
> index 000..76c79c738cb
> --- /dev/null
> +++ gnu/usr.bin/clang/libclangASTMatchers/Makefile
> @@ -0,0 +1,25 @@
> +LIB= clangASTMatchers
> +NOPIC=
> +NOPROFILE=
> +
> +CPPFLAGS+=${CLANG_INCLUDES}
> +
> +.include
> +
> +# Core
> +SRCS+= ASTMatchFinder.cpp \
> + ASTMatchersInternal.cpp \
> + GtestMatchers.cpp \
> + Diagnostics.cpp \
> + Marshallers.cpp \
> + Parser.cpp \
> + Registry.cpp \
> + VariantValue.cpp \
> +
> +.PATH: ${.CURDIR}/../../../llvm/clang/lib/ASTMatchers
> +.PATH: ${.CURDIR}/../../../llvm/clang/lib/ASTMatchers/Dynamic
> +
> +install:
> + @# Nothing here so far ...
> +
> +.include
> diff --git gnu/usr.bin/clang/libclangAnalysis/Makefile
> gnu/usr.bin/clang/libclangAnalysis/Makefile
> index 25669a34cc0..7be18f24685 100644
> --- gnu/usr.bin/clang/libclangAnalysis/Makefile
> +++ gnu/usr.bin/clang/libclangAnalysis/Makefile
> @@ -23,6 +23,7 @@ SRCS= AnalysisDeclContext.cpp \
> ExprMutationAnalyzer.cpp \
> IssueHash.cpp \
> LiveVariables.cpp \
> + MacroExpansionContext.cpp \
> ObjCNoReturn.cpp \
> PathDiagnostic.cpp \
> PostOrderCFGView.cpp \
> diff --git gnu/usr.bin/clang/libclangCrossTU/Makefile
> gnu/usr.bin/clang/libclangCrossTU/Makefile
> new file mode 100644
> index 000..59541bce012
> --- /dev/null
> +++ gnu/usr.bin/clang/libclangCrossTU/Makefile
> @@ -0,0 +1,17 @@
> +LIB= clangCrossTU
> +NOPIC=
> +NOPROFILE=
> +
> +CPPFLAGS+= -I ${.CURDIR}/../../../llvm/clang/lib/CrossTU
> +CPPFLAGS+=${CLANG_INCLUDES}
> +
> +.include
> +
> +SRCS+= CrossTranslationUnit.cpp \
> +
> +.PATH: ${.CURDIR}/../../../llvm/clang/lib/CrossTU/
> +
> +install:
> + @# Nothing here so far ...
> +
> +.include
> diff --git gnu/usr.bin/clang/libclangIndex/Makefile
> gnu/usr.bin/clang/libclangIndex/Makefile
> new file mode 100644
> index 000..b31d906a5ee
> --- /dev/null
> +++ gnu/usr.bin/clang/libclangIndex/Makefile
> @@ -0,0 +1,18 @@
> +LIB= clangIndex
> +NOPIC=
> +NOPROFILE=
> +
> +CPPFLAGS+= -I ${.CURDIR}/../../../llvm/clang/lib/Index
> +CPPFLAGS+=${CLANG_INCLUDES}
> +
> +.include
> +
> +SRCS+= CommentToXML.cpp \
> + USRGeneration.cpp \
> +
> +.PATH: ${.CURDIR}/../../../llvm/clang/lib/Index/
> +
> +install:
> + @# Nothing here so far ...
> +
> +.include
> diff --git gnu/usr.bin/clang/libclangStaticAnalyzer/Makefile
>
Re: clang: compile static analyzer
On Fri, Jan 14, 2022 at 03:17:21PM +0100, Tobias Heider wrote:
> Hi,
>
> clang ships with a pretty useful static analyzer to find all kinds of bugs
> in C and C++ code:
>
> https://clang-analyzer.llvm.org/
>
> I use it regularly to check my own diffs and found plenty of bugs I could
> have missed otherwise. While we have the code in base we don't actually
> build it into our libclang currently, so the only ways to use it are
> manually modifying the Makefiles or installing llvm from ports.
>
> I was wondering if anyone else uses this and if there was any interest to
> have this in our base clang?
>
> diff --git gnu/usr.bin/clang/Makefile gnu/usr.bin/clang/Makefile
> index 6cf71d36cf8..b47abc02474 100644
> --- gnu/usr.bin/clang/Makefile
> +++ gnu/usr.bin/clang/Makefile
> @@ -39,6 +39,11 @@ SUBDIR+=libclangSerialization
> SUBDIR+=libclangFrontend
> SUBDIR+=libclangRewriteFrontend
> SUBDIR+=libclangFrontendTool
> +SUBDIR+=libclangASTMatchers
> +SUBDIR+=libclangCrossTU
> +SUBDIR+=libclangStaticAnalyzer
> +SUBDIR+=libclangIndex
> +SUBDIR+=libclangTooling
>
> SUBDIR+=clang
>
> diff --git gnu/usr.bin/clang/clang/Makefile gnu/usr.bin/clang/clang/Makefile
> index 76535ee8842..7d3847f883d 100644
> --- gnu/usr.bin/clang/clang/Makefile
> +++ gnu/usr.bin/clang/clang/Makefile
> @@ -43,6 +43,11 @@ LLVM_LIBDEPS= LLVM \
> clangEdit \
> clangAST \
> clangLex \
> - clangBasic
> + clangBasic \
> + clangStaticAnalyzer \
> + clangCrossTU \
> + clangASTMatchers \
> + clangIndex \
> + clangTooling \
>
> .include
> diff --git gnu/usr.bin/clang/include/clang/Config/config.h
> gnu/usr.bin/clang/include/clang/Config/config.h
> index 02a049bf2be..93494d6a0b1 100644
> --- gnu/usr.bin/clang/include/clang/Config/config.h
> +++ gnu/usr.bin/clang/include/clang/Config/config.h
> @@ -78,7 +78,7 @@
> /* Enable each functionality of modules */
> #define CLANG_ENABLE_ARCMT 0
> #define CLANG_ENABLE_OBJC_REWRITER 0
> -#define CLANG_ENABLE_STATIC_ANALYZER 0
> +#define CLANG_ENABLE_STATIC_ANALYZER 1
>
> /* Spawn a new process clang.exe for the CC1 tool invocation, when necessary
> */
> #define CLANG_SPAWN_CC1 0
> diff --git gnu/usr.bin/clang/libclangASTMatchers/Makefile
> gnu/usr.bin/clang/libclangASTMatchers/Makefile
> new file mode 100644
> index 000..76c79c738cb
> --- /dev/null
> +++ gnu/usr.bin/clang/libclangASTMatchers/Makefile
> @@ -0,0 +1,25 @@
> +LIB= clangASTMatchers
> +NOPIC=
> +NOPROFILE=
> +
> +CPPFLAGS+=${CLANG_INCLUDES}
> +
> +.include
> +
> +# Core
> +SRCS+= ASTMatchFinder.cpp \
> + ASTMatchersInternal.cpp \
> + GtestMatchers.cpp \
> + Diagnostics.cpp \
> + Marshallers.cpp \
> + Parser.cpp \
> + Registry.cpp \
> + VariantValue.cpp \
> +
> +.PATH: ${.CURDIR}/../../../llvm/clang/lib/ASTMatchers
> +.PATH: ${.CURDIR}/../../../llvm/clang/lib/ASTMatchers/Dynamic
> +
> +install:
> + @# Nothing here so far ...
> +
> +.include
> diff --git gnu/usr.bin/clang/libclangAnalysis/Makefile
> gnu/usr.bin/clang/libclangAnalysis/Makefile
> index 25669a34cc0..7be18f24685 100644
> --- gnu/usr.bin/clang/libclangAnalysis/Makefile
> +++ gnu/usr.bin/clang/libclangAnalysis/Makefile
> @@ -23,6 +23,7 @@ SRCS= AnalysisDeclContext.cpp \
> ExprMutationAnalyzer.cpp \
> IssueHash.cpp \
> LiveVariables.cpp \
> + MacroExpansionContext.cpp \
> ObjCNoReturn.cpp \
> PathDiagnostic.cpp \
> PostOrderCFGView.cpp \
> diff --git gnu/usr.bin/clang/libclangCrossTU/Makefile
> gnu/usr.bin/clang/libclangCrossTU/Makefile
> new file mode 100644
> index 000..59541bce012
> --- /dev/null
> +++ gnu/usr.bin/clang/libclangCrossTU/Makefile
> @@ -0,0 +1,17 @@
> +LIB= clangCrossTU
> +NOPIC=
> +NOPROFILE=
> +
> +CPPFLAGS+= -I ${.CURDIR}/../../../llvm/clang/lib/CrossTU
> +CPPFLAGS+=${CLANG_INCLUDES}
> +
> +.include
> +
> +SRCS+= CrossTranslationUnit.cpp \
> +
> +.PATH: ${.CURDIR}/../../../llvm/clang/lib/CrossTU/
> +
> +install:
> + @# Nothing here so far ...
> +
> +.include
> diff --git gnu/usr.bin/clang/libclangIndex/Makefile
> gnu/usr.bin/clang/libclangIndex/Makefile
> new file mode 100644
> index 000..b31d906a5ee
> --- /dev/null
> +++ gnu/usr.bin/clang/libclangIndex/Makefile
> @@ -0,0 +1,18 @@
> +LIB= clangIndex
> +NOPIC=
> +NOPROFILE=
> +
> +CPPFLAGS+= -I ${.CURDIR}/../../../llvm/clang/lib/Index
> +CPPFLAGS+=${CLANG_INCLUDES}
> +
> +.include
> +
> +SRCS+= CommentToXML.cpp \
> + USRGeneration.cpp \
> +
> +.PATH: ${.CURDIR}/../../../llvm/clang/lib/Index/
> +
> +install:
> + @# Nothing here so far ...
> +
> +.include
> diff --git gnu/usr.bin/clang/libclangStaticAnalyzer/Makefile
> gnu/usr.bin/clang/libclangStaticAnalyzer/Makefile
> new file mode 100644
> index 000..6d152afe283
> --- /dev/null
> +++ gnu/usr.bin/clang/libclangStaticAnalyzer/Makefile
>
Re: clang: compile static analyzer
On Fri, 14 Jan 2022 14:53:21 +, Visa Hankala wrote: > I think the tool is useful. However, installing it from ports is good > enough for me. Same here. - todd
Re: clang: compile static analyzer
On Fri, Jan 14, 2022 at 03:17:21PM +0100, Tobias Heider wrote: > Hi, > > clang ships with a pretty useful static analyzer to find all kinds of bugs > in C and C++ code: > > https://clang-analyzer.llvm.org/ > > I use it regularly to check my own diffs and found plenty of bugs I could > have missed otherwise. While we have the code in base we don't actually > build it into our libclang currently, so the only ways to use it are > manually modifying the Makefiles or installing llvm from ports. > > I was wondering if anyone else uses this and if there was any interest to > have this in our base clang? I think the tool is useful. However, installing it from ports is good enough for me. How much would this addition increase the build time and space usage of the finished sets?
Re: possible memory leak in ipmi get_sdr
On Fri, 14 Jan 2022 09:25:53 +0100
Moritz Buhl wrote:
> I thought I'd walk through my understanding of the code
> to make it easier to review.
>
> from sys/dev/ipmi.c:
>
> 1019 int
> 1020 get_sdr(struct ipmi_softc *sc, u_int16_t recid, u_int16_t *nxtrec)
> 1021 {
> ...
> 1046 psdr = malloc(sdrlen, M_DEVBUF, M_NOWAIT);
> 1047 if (psdr == NULL)
> 1048 return (1);
> Now psdr is allocated. In the following loop it is freed correctly on
> failure:
> 1062 free(psdr, M_DEVBUF, sdrlen);
> 1063 return (1);
> If everything works, we attach it to a list:
> 1067 /* Add SDR to sensor list, if not wanted, free buffer */
> 1068 if (add_sdr_sensor(sc, psdr, sdrlen) == 0)
> 1069 free(psdr, M_DEVBUF, sdrlen);
> 1070
> 1071 return (0);
> So if add_sdr_sensor doesn't return 0 it has to keep a reference
> to psdr somewhere.
>
> 1335 int
> 1336 add_sdr_sensor(struct ipmi_softc *sc, u_int8_t *psdr, int sdrlen)
> 1337 {
> There are two switch cases that both call add_child_sensors and set
> rc with it, if no previous failures occour:
> 1349 rc = add_child_sensors(sc, psdr, 1, s1->sensor_num,
> 1350 s1->sensor_type, s1->event_code, 0, s1->entity_id,
> name);
> 1351 break;
> and
> 1358 rc = add_child_sensors(sc, psdr, s2->share1 & 0xF,
> 1359 s2->sensor_num, s2->sensor_type, s2->event_code,
> 1360 s2->share2 & 0x7F, s2->entity_id, name);
> 1361 break;
> These two calls are the only locations that can make add_sdr_sensor
> return something else than 0.
>
> 1370 int
> 1371 add_child_sensors(struct ipmi_softc *sc, u_int8_t *psdr, int count,
> 1372 int sensor_num, int sensor_type, int ext_type, int sensor_base,
> 1373 int entity, const char *name)
> 1374 {
> So as mentioned before, this function needs to return 0 or keep a
> reference to psdr.
> This is not the case in two situations, but to keep it simple I
> will only show one:
> 1388 psensor = malloc(sizeof(*psensor), M_DEVBUF, M_NOWAIT |
> M_ZERO);
> 1389 if (psensor == NULL)
> 1390 break;
> ...
> 1420 return (1);
>
> I hope this helps
> mbuhl
>
Hi, llvm/scan-build agrees.
http://www.netzbasis.de/openbsd/scan-build/kernel/2022-01-12-131800-47421-1/report-c7382e.html#EndPath
Patch seems correct, but I'm not a dev and I don't have the hardware eighter.
Greetings Ben
clang: compile static analyzer
Hi,
clang ships with a pretty useful static analyzer to find all kinds of bugs
in C and C++ code:
https://clang-analyzer.llvm.org/
I use it regularly to check my own diffs and found plenty of bugs I could
have missed otherwise. While we have the code in base we don't actually
build it into our libclang currently, so the only ways to use it are
manually modifying the Makefiles or installing llvm from ports.
I was wondering if anyone else uses this and if there was any interest to
have this in our base clang?
diff --git gnu/usr.bin/clang/Makefile gnu/usr.bin/clang/Makefile
index 6cf71d36cf8..b47abc02474 100644
--- gnu/usr.bin/clang/Makefile
+++ gnu/usr.bin/clang/Makefile
@@ -39,6 +39,11 @@ SUBDIR+=libclangSerialization
SUBDIR+=libclangFrontend
SUBDIR+=libclangRewriteFrontend
SUBDIR+=libclangFrontendTool
+SUBDIR+=libclangASTMatchers
+SUBDIR+=libclangCrossTU
+SUBDIR+=libclangStaticAnalyzer
+SUBDIR+=libclangIndex
+SUBDIR+=libclangTooling
SUBDIR+=clang
diff --git gnu/usr.bin/clang/clang/Makefile gnu/usr.bin/clang/clang/Makefile
index 76535ee8842..7d3847f883d 100644
--- gnu/usr.bin/clang/clang/Makefile
+++ gnu/usr.bin/clang/clang/Makefile
@@ -43,6 +43,11 @@ LLVM_LIBDEPS=LLVM \
clangEdit \
clangAST \
clangLex \
- clangBasic
+ clangBasic \
+ clangStaticAnalyzer \
+ clangCrossTU \
+ clangASTMatchers \
+ clangIndex \
+ clangTooling \
.include
diff --git gnu/usr.bin/clang/include/clang/Config/config.h
gnu/usr.bin/clang/include/clang/Config/config.h
index 02a049bf2be..93494d6a0b1 100644
--- gnu/usr.bin/clang/include/clang/Config/config.h
+++ gnu/usr.bin/clang/include/clang/Config/config.h
@@ -78,7 +78,7 @@
/* Enable each functionality of modules */
#define CLANG_ENABLE_ARCMT 0
#define CLANG_ENABLE_OBJC_REWRITER 0
-#define CLANG_ENABLE_STATIC_ANALYZER 0
+#define CLANG_ENABLE_STATIC_ANALYZER 1
/* Spawn a new process clang.exe for the CC1 tool invocation, when necessary */
#define CLANG_SPAWN_CC1 0
diff --git gnu/usr.bin/clang/libclangASTMatchers/Makefile
gnu/usr.bin/clang/libclangASTMatchers/Makefile
new file mode 100644
index 000..76c79c738cb
--- /dev/null
+++ gnu/usr.bin/clang/libclangASTMatchers/Makefile
@@ -0,0 +1,25 @@
+LIB= clangASTMatchers
+NOPIC=
+NOPROFILE=
+
+CPPFLAGS+= ${CLANG_INCLUDES}
+
+.include
+
+# Core
+SRCS+= ASTMatchFinder.cpp \
+ ASTMatchersInternal.cpp \
+ GtestMatchers.cpp \
+ Diagnostics.cpp \
+ Marshallers.cpp \
+ Parser.cpp \
+ Registry.cpp \
+ VariantValue.cpp \
+
+.PATH: ${.CURDIR}/../../../llvm/clang/lib/ASTMatchers
+.PATH: ${.CURDIR}/../../../llvm/clang/lib/ASTMatchers/Dynamic
+
+install:
+ @# Nothing here so far ...
+
+.include
diff --git gnu/usr.bin/clang/libclangAnalysis/Makefile
gnu/usr.bin/clang/libclangAnalysis/Makefile
index 25669a34cc0..7be18f24685 100644
--- gnu/usr.bin/clang/libclangAnalysis/Makefile
+++ gnu/usr.bin/clang/libclangAnalysis/Makefile
@@ -23,6 +23,7 @@ SRCS= AnalysisDeclContext.cpp \
ExprMutationAnalyzer.cpp \
IssueHash.cpp \
LiveVariables.cpp \
+ MacroExpansionContext.cpp \
ObjCNoReturn.cpp \
PathDiagnostic.cpp \
PostOrderCFGView.cpp \
diff --git gnu/usr.bin/clang/libclangCrossTU/Makefile
gnu/usr.bin/clang/libclangCrossTU/Makefile
new file mode 100644
index 000..59541bce012
--- /dev/null
+++ gnu/usr.bin/clang/libclangCrossTU/Makefile
@@ -0,0 +1,17 @@
+LIB= clangCrossTU
+NOPIC=
+NOPROFILE=
+
+CPPFLAGS+= -I ${.CURDIR}/../../../llvm/clang/lib/CrossTU
+CPPFLAGS+= ${CLANG_INCLUDES}
+
+.include
+
+SRCS+= CrossTranslationUnit.cpp \
+
+.PATH: ${.CURDIR}/../../../llvm/clang/lib/CrossTU/
+
+install:
+ @# Nothing here so far ...
+
+.include
diff --git gnu/usr.bin/clang/libclangIndex/Makefile
gnu/usr.bin/clang/libclangIndex/Makefile
new file mode 100644
index 000..b31d906a5ee
--- /dev/null
+++ gnu/usr.bin/clang/libclangIndex/Makefile
@@ -0,0 +1,18 @@
+LIB= clangIndex
+NOPIC=
+NOPROFILE=
+
+CPPFLAGS+= -I ${.CURDIR}/../../../llvm/clang/lib/Index
+CPPFLAGS+= ${CLANG_INCLUDES}
+
+.include
+
+SRCS+= CommentToXML.cpp \
+ USRGeneration.cpp \
+
+.PATH: ${.CURDIR}/../../../llvm/clang/lib/Index/
+
+install:
+ @# Nothing here so far ...
+
+.include
diff --git gnu/usr.bin/clang/libclangStaticAnalyzer/Makefile
gnu/usr.bin/clang/libclangStaticAnalyzer/Makefile
new file mode 100644
index 000..6d152afe283
--- /dev/null
+++ gnu/usr.bin/clang/libclangStaticAnalyzer/Makefile
@@ -0,0 +1,206 @@
+LIB= clangStaticAnalyzer
+NOPIC=
+NOPROFILE=
+
+CPPFLAGS+= -I ${.CURDIR}/../../../llvm/clang/lib/StaticAnalyzer/Checkers
+CPPFLAGS+= ${CLANG_INCLUDES}
+
+.include
+
+# Core
+SRCS+= APSIntType.cpp \
+ AnalysisManager.cpp \
+ AnalyzerOptions.cpp \
+ BasicValueFactory.cpp \
+ BlockCounter.cpp \
+
Re: rpki-client introduce validated cache
On Fri, Jan 14, 2022 at 01:45:19PM +, Job Snijders wrote:
> Thanks Claudio,
>
> A question about stats below
>
> On Fri, Jan 14, 2022 at 10:29:20AM +0100, Claudio Jeker wrote:
> > @@ -1246,8 +1249,8 @@ main(int argc, char *argv[])
> > logx("Certificate revocation lists: %zu", stats.crls);
> > logx("Ghostbuster records: %zu", stats.gbrs);
> > logx("Repositories: %zu", stats.repos);
> > - logx("Cleanup: removed %zu files, %zu directories",
> > - stats.del_files, stats.del_dirs);
> > + logx("Cleanup: removed %zu files, %zu directories, %zu superfluous",
> > + stats.del_files, stats.del_dirs, stats.extra_files);
>
> The above stats message is not entirely clear to me, is something along
> these lines more precise?
>
> logx("Cleanup: removed %zu previously valid objects, %zu directories, %zu"
> " superfluous files", stats.del_files, stats.del_dirs,
> stats.extra_files);
I still need to think about this more. The way the numbers are calculated
is not what you suggested. Right now the delete numbers are including
files that where never valid, etc. It may make sense to spend some time
and fix the reporting so that the stats returned make sense.
I think we now have files in ta and valid that should be accounted
differently than file in rrdp and rsync. The latter are now mostly
temporary files (e.g. rsync will always be empty after a run).
I'll get the validated cache in and then we can bikeshed over this :)
--
:wq Claudio
Re: rpki-client introduce validated cache
Thanks Claudio,
A question about stats below
On Fri, Jan 14, 2022 at 10:29:20AM +0100, Claudio Jeker wrote:
> @@ -1246,8 +1249,8 @@ main(int argc, char *argv[])
> logx("Certificate revocation lists: %zu", stats.crls);
> logx("Ghostbuster records: %zu", stats.gbrs);
> logx("Repositories: %zu", stats.repos);
> - logx("Cleanup: removed %zu files, %zu directories",
> - stats.del_files, stats.del_dirs);
> + logx("Cleanup: removed %zu files, %zu directories, %zu superfluous",
> + stats.del_files, stats.del_dirs, stats.extra_files);
The above stats message is not entirely clear to me, is something along
these lines more precise?
logx("Cleanup: removed %zu previously valid objects, %zu directories, %zu"
" superfluous files", stats.del_files, stats.del_dirs,
stats.extra_files);
Kind regards,
Job
Re: rpki-client introduce validated cache
On Fri, Jan 14, 2022 at 10:29:20AM +0100, Claudio Jeker wrote: > On Thu, Jan 13, 2022 at 10:51:33PM +0100, Theo Buehler wrote: > > On Thu, Jan 13, 2022 at 05:05:57PM +0100, Claudio Jeker wrote: > > > This diff adds a new cache subdir called "valid". This is the place where > > > all verified and good files are stored after a run. It makes -n work a lot > > > better since -n will now only look at what's inside "valid" and ignore > > > "rsync" and "rrdp". > > > > > > The trust anchors are still stored in "ta" even if valid. > > > The rsync repo will only hold temporary files and be empty otherwise. > > > The rrdp repo still may contain some superfluous files that people > > > included in their snapshots. Not keeping them could result in extra > > > snapshot downloads since delta updates could refer to these files. > > > Since there is a valid repo, rrdp no longer needs an additional temp > > > directory for its sync. > > > > > > With this rsync will use the --compare-dest feature and use the valid > > > cache as a base. > > > > > > The file cleanup at the end got more complex. There is now > > > repo_move_valid() and repo_cleanup_rrdp() to do two initial steps of > > > cleanup before the tree traversal starts. The fts function now uses > > > some fts features to find a) empty directories and b) superfluous files. > > > > > > I think the code changes outside of repo.c should be straight forward. > > > > > > To the rpki connoisseur, this currently only implements a simple newest > > > file wins startegy. The handling of .mft and their serial number will > > > follow once this is in. > > > > I have made a first pass over this. I will need to sleep on this and > > think through the repo.c changes in more detail with a fresh head. > > > > Couple leaks and a few stylistic remarks noted inline. > > Adjusted diff below. Thanks for the review of this and also for all the > previous diffs. This looks good to me now. Couldn't find anything of substance to complain about. Let's get it in so we can make further progress in tree. ok tb
Re: rpki-client introduce validated cache
On Thu, Jan 13, 2022 at 10:51:33PM +0100, Theo Buehler wrote:
> On Thu, Jan 13, 2022 at 05:05:57PM +0100, Claudio Jeker wrote:
> > This diff adds a new cache subdir called "valid". This is the place where
> > all verified and good files are stored after a run. It makes -n work a lot
> > better since -n will now only look at what's inside "valid" and ignore
> > "rsync" and "rrdp".
> >
> > The trust anchors are still stored in "ta" even if valid.
> > The rsync repo will only hold temporary files and be empty otherwise.
> > The rrdp repo still may contain some superfluous files that people
> > included in their snapshots. Not keeping them could result in extra
> > snapshot downloads since delta updates could refer to these files.
> > Since there is a valid repo, rrdp no longer needs an additional temp
> > directory for its sync.
> >
> > With this rsync will use the --compare-dest feature and use the valid
> > cache as a base.
> >
> > The file cleanup at the end got more complex. There is now
> > repo_move_valid() and repo_cleanup_rrdp() to do two initial steps of
> > cleanup before the tree traversal starts. The fts function now uses
> > some fts features to find a) empty directories and b) superfluous files.
> >
> > I think the code changes outside of repo.c should be straight forward.
> >
> > To the rpki connoisseur, this currently only implements a simple newest
> > file wins startegy. The handling of .mft and their serial number will
> > follow once this is in.
>
> I have made a first pass over this. I will need to sleep on this and
> think through the repo.c changes in more detail with a fresh head.
>
> Couple leaks and a few stylistic remarks noted inline.
Adjusted diff below. Thanks for the review of this and also for all the
previous diffs.
--
:wq Claudio
Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.103
diff -u -p -r1.103 extern.h
--- extern.h13 Jan 2022 13:46:03 - 1.103
+++ extern.h13 Jan 2022 13:52:42 -
@@ -341,7 +341,7 @@ enum publish_type {
struct entity {
TAILQ_ENTRY(entity) entries;
char*path; /* path relative to repository */
- char*file; /* filename */
+ char*file; /* filename or valid repo path */
unsigned char *data; /* optional data blob */
size_t datasz;/* length of optional data blob */
unsigned int repoid;/* repository identifier */
@@ -380,6 +380,7 @@ struct stats {
size_t vrps; /* total number of vrps */
size_t uniqs; /* number of unique vrps */
size_t del_files; /* number of files removed in cleanup */
+ size_t extra_files; /* number of superfluous files */
size_t del_dirs; /* number of directories removed in cleanup */
size_t brks; /* number of BGPsec Router Key (BRK) certificates */
struct timeval elapsed_time;
@@ -506,7 +507,7 @@ void rrdp_clear(unsigned int);
voidrrdp_save_state(unsigned int, struct rrdp_session *);
int rrdp_handle_file(unsigned int, enum publish_type, char *,
char *, size_t, char *, size_t);
-char *repo_basedir(const struct repo *);
+char *repo_basedir(const struct repo *, int);
unsigned intrepo_id(const struct repo *);
const char *repo_uri(const struct repo *);
struct repo*ta_lookup(int, struct tal *);
@@ -520,7 +521,8 @@ void rsync_finish(unsigned int, int);
voidhttp_finish(unsigned int, enum http_result, const char *);
voidrrdp_finish(unsigned int, int);
-voidrsync_fetch(unsigned int, const char *, const char *);
+voidrsync_fetch(unsigned int, const char *, const char *,
+ const char *);
voidhttp_fetch(unsigned int, const char *, const char *, int);
voidrrdp_fetch(unsigned int, const char *, const char *,
struct rrdp_session *);
Index: main.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.175
diff -u -p -r1.175 main.c
--- main.c 13 Jan 2022 13:18:41 - 1.175
+++ main.c 14 Jan 2022 08:57:30 -
@@ -151,20 +151,22 @@ entity_write_repo(struct repo *rp)
struct ibuf *b;
enum rtype type = RTYPE_REPO;
unsigned int repoid;
- char *path;
+ char *path, *altpath;
int talid = 0;
repoid = repo_id(rp);
- path = repo_basedir(rp);
+ path = repo_basedir(rp, 0);
+ altpath = repo_basedir(rp, 1);
b = io_new_buffer();
io_simple_buffer(b, , sizeof(type));
io_simple_buffer(b, , sizeof(repoid));
io_simple_buffer(b, , sizeof(talid));
io_str_buffer(b, path);
-
Re: possible memory leak in ipmi get_sdr
I thought I'd walk through my understanding of the code
to make it easier to review.
from sys/dev/ipmi.c:
1019 int
1020 get_sdr(struct ipmi_softc *sc, u_int16_t recid, u_int16_t *nxtrec)
1021 {
...
1046 psdr = malloc(sdrlen, M_DEVBUF, M_NOWAIT);
1047 if (psdr == NULL)
1048 return (1);
Now psdr is allocated. In the following loop it is freed correctly on
failure:
1062 free(psdr, M_DEVBUF, sdrlen);
1063 return (1);
If everything works, we attach it to a list:
1067 /* Add SDR to sensor list, if not wanted, free buffer */
1068 if (add_sdr_sensor(sc, psdr, sdrlen) == 0)
1069 free(psdr, M_DEVBUF, sdrlen);
1070
1071 return (0);
So if add_sdr_sensor doesn't return 0 it has to keep a reference
to psdr somewhere.
1335 int
1336 add_sdr_sensor(struct ipmi_softc *sc, u_int8_t *psdr, int sdrlen)
1337 {
There are two switch cases that both call add_child_sensors and set
rc with it, if no previous failures occour:
1349 rc = add_child_sensors(sc, psdr, 1, s1->sensor_num,
1350 s1->sensor_type, s1->event_code, 0, s1->entity_id,
name);
1351 break;
and
1358 rc = add_child_sensors(sc, psdr, s2->share1 & 0xF,
1359 s2->sensor_num, s2->sensor_type, s2->event_code,
1360 s2->share2 & 0x7F, s2->entity_id, name);
1361 break;
These two calls are the only locations that can make add_sdr_sensor
return something else than 0.
1370 int
1371 add_child_sensors(struct ipmi_softc *sc, u_int8_t *psdr, int count,
1372 int sensor_num, int sensor_type, int ext_type, int sensor_base,
1373 int entity, const char *name)
1374 {
So as mentioned before, this function needs to return 0 or keep a
reference to psdr.
This is not the case in two situations, but to keep it simple I
will only show one:
1388 psensor = malloc(sizeof(*psensor), M_DEVBUF, M_NOWAIT |
M_ZERO);
1389 if (psensor == NULL)
1390 break;
...
1420 return (1);
I hope this helps
mbuhl
