Re: CVS commit: src/usr.bin/make

2022-01-09 Thread Joerg Sonnenberger
On Fri, Jan 07, 2022 at 08:04:49PM +, Roland Illig wrote:
> Module Name:  src
> Committed By: rillig
> Date: Fri Jan  7 20:04:49 UTC 2022
> 
> Modified Files:
>   src/usr.bin/make: for.c
> 
> Log Message:
> make: use simpler code for handling .for loops

This seems backwards to me. For simpler logic I would expect computing
the var length first and then skipping variables that don't have the
same length, falling back to memcmp otherwise. De facto inlining a
memcmp loop is not an improvement.

Joerg


Re: CVS commit: src/sbin/cgdconfig

2021-11-29 Thread Joerg Sonnenberger
On Sun, Nov 28, 2021 at 07:42:55AM -0800, Jason Thorpe wrote:
> 
> 
> > On Nov 27, 2021, at 6:01 PM, Christos Zoulas  wrote:
> > 
> > Module Name:src
> > Committed By:   christos
> > Date:   Sun Nov 28 02:01:30 UTC 2021
> > 
> > Modified Files:
> > src/sbin/cgdconfig: Makefile
> > 
> > Log Message:
> > -lpthread to LDADD (fixes lint build)
> 
> This change is wrong.  The -pthread option to the compiler does more than 
> just add -lpthread to the link phase.

Yeah, but the other changes are pretty much useless.

Joerg


Re: CVS commit: src

2021-11-05 Thread Joerg Sonnenberger
On Thu, Nov 04, 2021 at 06:17:20PM -0700, Alistair Crooks wrote:
> I think you're misreading the diff - it will only wrap if the minimum size
> is 0x, which is...ummm...highly unlikely (it's defined to be 0
> right now, the type is unsigned)

I'm not so much worried about the constant, but the right hand size.
Without looking at the types, I can't be sure that "context->pwdlen + 1"
doesn't overflow or that "context->m_cost - 1" can't underflow. Much
easier and safer to just use a type cast...

Joerg


Re: CVS commit: src

2021-11-04 Thread Joerg Sonnenberger
On Mon, Nov 01, 2021 at 03:09:59AM +, Alistair G. Crooks wrote:
> Module Name:  src
> Committed By: agc
> Date: Mon Nov  1 03:09:59 UTC 2021
> 
> Modified Files:
>   src/external/apache2/argon2/dist/phc-winner-argon2/src: argon2.c core.c
>   src/lib/libcrypt: Makefile
> 
> Log Message:
> Remove the
> 
>   COPTS.*+=   -Wno-error=.*
> 
> lines for building argon2 sources, by fixing the problems at source.
> 
> Addresses Rin Okuyama's concerns on tech-userlevel/tech-crypto in
> 
>   Message-ID: 

The +1 part seems questionable to me as it can actually introduce a wrap
around, can't it?

Joerg


Re: CVS commit: src/lib/libc/string

2021-10-29 Thread Joerg Sonnenberger
On Fri, Oct 29, 2021 at 10:11:57AM +, Nia Alarie wrote:
> Module Name:  src
> Committed By: nia
> Date: Fri Oct 29 10:11:57 UTC 2021
> 
> Modified Files:
>   src/lib/libc/string: wcsdup.c
> 
> Log Message:
> wcsdup(3): use reallocarr to catch integer overflow

Except that no such integer overflow can happen, since the input string
already has done the same computation.

Joerg


Re: CVS commit: src

2021-10-26 Thread Joerg Sonnenberger
On Tue, Oct 26, 2021 at 10:48:45PM +0700, Robert Elz wrote:
> Date:Tue, 26 Oct 2021 15:37:56 +0200
> From:    Joerg Sonnenberger 
> Message-ID:  
> 
>   | Personally, I would prefer to just kill -pg support completely, but
>   | that's a separate discussion.
> 
> Yes, it is.
> 
>   | I don't think per-file profiling is that useful
> 
> Nor do I in general.   However, doing routine call counting on mcount
> (or anything it calls) would simply be absurd.   If you want to
> know how mant times mcount has been called, you simply sum all the other
> counters.   (pc sampling, however it is done, detecting mcount is fine).

The existing attribute already does that.

Joerg


Re: CVS commit: src

2021-10-26 Thread Joerg Sonnenberger
On Tue, Oct 26, 2021 at 04:00:41AM +0900, Ryo Shimizu wrote:
> 
> >On Mon, Oct 25, 2021 at 07:54:45AM +, Ryo Shimizu wrote:
> >> Module Name:   src
> >> Committed By:  ryo
> >> Date:  Mon Oct 25 07:54:44 UTC 2021
> >> 
> >> Modified Files:
> >>src/share/mk: bsd.README bsd.lib.mk
> >>src/sys/conf: Makefile.kern.inc
> >>src/sys/lib/libkern: Makefile.libkern
> >> 
> >> Log Message:
> >> In some arch, _mcount() would be called recursively when built with 
> >> COPTS=-O0.
> >> 
> >> Normally, functions called from mcount.c are expected to be expanded 
> >> inline,
> >> so _mcount() will never be called recursively. But when build with 
> >> COPTS=-O0,
> >> `static inline' functions aren't inlined, and _mcount() will be called
> >> recursively.
> >
> >So why not fix that by actually using always_inline (i.e.
> >__always_inline)?
> >
> >Joerg
> 
> Yes, that is correct. That method is also valid and should be done separately.
> 
> However, it is more direct to not add -pg to mcount.c.
> Also, I think this commit is valid because it is useful to be able to choose
> not to do per-file profiling.

Personally, I would prefer to just kill -pg support completely, but
that's a separate discussion. I don't think per-file profiling is that
useful and just begging for very confusing results. So I would strongly
prefer if this was done properly.

Joerg


Re: CVS commit: src

2021-10-25 Thread Joerg Sonnenberger
On Mon, Oct 25, 2021 at 07:54:45AM +, Ryo Shimizu wrote:
> Module Name:  src
> Committed By: ryo
> Date: Mon Oct 25 07:54:44 UTC 2021
> 
> Modified Files:
>   src/share/mk: bsd.README bsd.lib.mk
>   src/sys/conf: Makefile.kern.inc
>   src/sys/lib/libkern: Makefile.libkern
> 
> Log Message:
> In some arch, _mcount() would be called recursively when built with COPTS=-O0.
> 
> Normally, functions called from mcount.c are expected to be expanded inline,
> so _mcount() will never be called recursively. But when build with COPTS=-O0,
> `static inline' functions aren't inlined, and _mcount() will be called
> recursively.

So why not fix that by actually using always_inline (i.e.
__always_inline)?

Joerg


CVS commit: src

2021-09-17 Thread Joerg Sonnenberger
Module Name:src
Committed By:   joerg
Date:   Fri Sep 17 21:22:38 UTC 2021

Modified Files:
src: build.sh

Log Message:
Fix timestamp extraction logic for Mercurial repos to use UTC.
Extend logic to also cover "hg archive".


To generate a diff of this commit:
cvs rdiff -u -r1.356 -r1.357 src/build.sh

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/build.sh
diff -u src/build.sh:1.356 src/build.sh:1.357
--- src/build.sh:1.356	Thu Sep  9 15:00:01 2021
+++ src/build.sh	Fri Sep 17 21:22:38 2021
@@ -1,5 +1,5 @@
 #! /usr/bin/env sh
-#	$NetBSD: build.sh,v 1.356 2021/09/09 15:00:01 martin Exp $
+#	$NetBSD: build.sh,v 1.357 2021/09/17 21:22:38 joerg Exp $
 #
 # Copyright (c) 2001-2011 The NetBSD Foundation, Inc.
 # All rights reserved.
@@ -1972,7 +1972,7 @@ createmakewrapper()
 	eval cat <

CVS commit: src

2021-09-17 Thread Joerg Sonnenberger
Module Name:src
Committed By:   joerg
Date:   Fri Sep 17 21:22:38 UTC 2021

Modified Files:
src: build.sh

Log Message:
Fix timestamp extraction logic for Mercurial repos to use UTC.
Extend logic to also cover "hg archive".


To generate a diff of this commit:
cvs rdiff -u -r1.356 -r1.357 src/build.sh

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



Re: CVS commit: src/lib/libc/arch/powerpc/string

2021-07-24 Thread Joerg Sonnenberger
On Sat, Jul 24, 2021 at 05:27:26AM +, Rin Okuyama wrote:
> Module Name:  src
> Committed By: rin
> Date: Sat Jul 24 05:27:26 UTC 2021
> 
> Modified Files:
>   src/lib/libc/arch/powerpc/string: Makefile.inc
> 
> Log Message:
> For evbppc, use C version of bcopy(3), memcpy(3), memcmp(3), and
> memmove(3) consistently for debug library (*.go) in order to avoid
> alignment faults for 403.

Why do we want to pessimize all evbppc targets just for issues with the
403 design?

Joerg


Re: CVS commit: src/common/lib/libc/arch/aarch64/atomic

2021-07-04 Thread Joerg Sonnenberger
On Sun, Jul 04, 2021 at 06:55:47AM +, Nick Hudson wrote:
> Module Name:  src
> Committed By: skrll
> Date: Sun Jul  4 06:55:47 UTC 2021
> 
> Modified Files:
>   src/common/lib/libc/arch/aarch64/atomic: atomic_nand_16.S
>   atomic_nand_32.S atomic_nand_64.S atomic_nand_8.S
> 
> Log Message:
> Fix the logic operation for atomic_nand_{8,16,32,64}
> 
> From the gcc docs the operations are as follows
> 
>  { tmp = *ptr; *ptr = ~(tmp & value); return tmp; }   // nand
>  { tmp = ~(*ptr & value); *ptr = tmp; return *ptr; }   // nand
> 
> yes, this is really rather strange.

This depends on which GCC version you are looking at. They changed it
sometime in the early GCC 4 days.

Joerg


Re: CVS commit: src/sys/external/bsd/compiler_rt/dist/lib/builtins/arm

2021-06-30 Thread Joerg Sonnenberger
On Tue, Jun 29, 2021 at 11:26:00PM +, Rin Okuyama wrote:
> Module Name:  src
> Committed By: rin
> Date: Tue Jun 29 23:26:00 UTC 2021
> 
> Modified Files:
>   src/sys/external/bsd/compiler_rt/dist/lib/builtins/arm: aeabi_cfcmp.S
>   divmodsi4.S divsi3.S modsi3.S
> 
> Log Message:
> Align sp to 8-byte boundary as required by EABI.

Why not just push/pop another register? That's how it is normally done
for generated code.

Joerg


Re: vfork() and posix_spawn() [was: Re: CVS commit: src/lib/libc/sys]

2021-06-13 Thread Joerg Sonnenberger
On Mon, Jun 14, 2021 at 01:33:51AM +0700, Robert Elz wrote:
> Date:Sat, 12 Jun 2021 23:13:54 +0200
> From:    Joerg Sonnenberger 
> Message-ID:  
> 
> Sorry, missed this message when I was cherry picking messages to read in
> a timely fashion.
> 
>   | On Wed, Jun 09, 2021 at 01:03:20AM +0700, Robert Elz wrote:
>   | > after a vfork() the child can do anything
> 
>   | This is not true. After vfork(), it is only supposed to call async
>   | signal safe functions.
> 
> What you said, and what I said are not contradictory.   Note I said "can
> do" and you said "is only supposed to".
> 
> What is supposed to work and what actually does work (or can be made to
> work) are two different things.

Sure, but the set of what works reliably has actually been shrinking
over time and more bugs in various programs using vfork specifically
have been discovered. This is even more true for multi-threaded
applications (where POSIX explicitly suggests that requirement). On the
specific topic, I'm somewhat puzzled by the claim that fork is
async-signal-safe since most implementations will require
synchronisation for pthread_atfork and that seems to place quite a
burden on the implementation...

> I would note though our man page doesn't make that requirement, or not
> that I can see anyway, and vfork() is not in POSIX of course - and while
> I don't have a copy to check, I'd suspect not in the C standards either),
> so while that sounds like a reasonable requirement to impose for safer
> operation, I'm not sure that anyone or anything actually does that.

It most likely should. The main reason is that much of the system can
and often do depend on things like mutexes to ensure correctness and all
that is essentially UB after vfork(). That's even ignoring the stronger
issue of mutating state. vfork use really should die...

> 
>   | That said, a flag for the double fork semantic might be useful.
> 
> If the "that said" relates to fork() or vfork() not being async signal safe,
> so a double fork() (when the first is vfork anyway) would not be condoned,
> that doesn't matter, there to be an async-signal-safe _Fork() call added to
> the next version of POSIX, so even with the "only async signal safe"
> requirement for vfork() a:

No, it relates to one common pattern for used by or for daemon.

> Please don't see posix_spawn() as being (or ever becoming) a panacea that
> can replace all fork() (or even just vfork()) calls - even just in the
> cases where an exec() follows soon after.   It works fine for most simple
> cases, but there are lots of not-so-simple situations that it cannot handle,
> and burdening the interface with the ability to deal with all of those
> would reduce the "efficiently implementable" goal for little real benefit.

There are non-trivial uses of fork, yes. There are much less non-trivial
uses of vfork as the latter already has quite a few problems with spooky
actions otherwise. Supporting something like a double fork flag has very
little impact on the complexity of the implementation and even less
impact on the efficiency. We certainly are at the point where we can
start analyzing the remaining blockers for (v)fork+exec users.

> Another example is made obvious by the bug Martin committed a fix for
> in the past few days ... the order of operations was incorrect in our
> implementation.  But that this problem can exist shows that there
> are ordering issues - a process that wants to open files using its
> current privs, then reset those before (perhaps) opening more files,
> and then doing the exec() part cannot do that with posix_spawn().
> Again, this is rare enough that adding the complexity required to allow
> it to work just isn't worth it.   [In this area also note that the
> POSIX_SPAWN_RESETIDS flag causes (effectively) setuid(getuid()), there's
> no similar operation to do setuid(geteuid()) ... again too rare to bother.]

I quite disagree here, actually. The design-level issue is that
POSIX_SPAWN_RESETIDS is a flag and not an action. This means it can't be
sequenced and that is the reason for the limitation. There is an obvious
parallel with the semantics of the chdir action here--that needs to be
that, an action and not just a flag. The separate concern is of course
that we need more testing for posix_spawn, but that is hopefully also
going to become better as side effect of the non-GSoC project.

Joerg


Re: CVS commit: src/lib/libc/sys

2021-06-12 Thread Joerg Sonnenberger
On Wed, Jun 09, 2021 at 01:03:20AM +0700, Robert Elz wrote:
> It should, when it is workable for the application, make things
> faster, while also avoiding the vfork() perils.   But only when
> it works -- after a vfork() the child can do anything (it should
> avoid touching its parent's state, but it can) posix_spawn() isn't
> nearly that general, it just handles the most common things apps
> are likely to want to do between the fork() and exec().

This is not true. After vfork(), it is only supposed to call async
signal safe functions. That said, a flag for the double fork semantic
might be useful.

Joerg


Re: CVS commit: src/tests/lib/csu

2021-06-05 Thread Joerg Sonnenberger
On Fri, Jun 04, 2021 at 10:20:24PM -, Christos Zoulas wrote:
> In article , Joerg Sonnenberger   
> wrote:
> >On Thu, Jun 03, 2021 at 04:17:37PM -0400, Christos Zoulas wrote:
> >> Module Name:   src
> >> Committed By:  christos
> >> Date:  Thu Jun  3 20:17:37 UTC 2021
> >> 
> >> Modified Files:
> >>src/tests/lib/csu: h_ifunc_static.c
> >> 
> >> Log Message:
> >> PR/56230: Jan-Benedict Glaw: arm oabi does not and will not have ifunc
> >support.
> >
> >As I mentioned elsewhere, I disagree with this change. OABI is dead and
> >only really support for legacy compatibility. All the OABI build
> >variants should be removed and this is strongly a step in the wrong
> >direction.
> 
> I think it is better to do it after the 10 branch. It is not just removing
> the build.sh targets, we need to remove the compat glue and fix the sets,
> and perhaps something else I am missing. I will revert the two changes
> once we remove all oabi support.

I don't see how that's related. I'm not talking about removing the
compat layer right now. Remove the build.sh targets and call it a day.

Joerg


Re: CVS commit: src/tests/lib/csu

2021-06-03 Thread Joerg Sonnenberger
On Thu, Jun 03, 2021 at 04:17:37PM -0400, Christos Zoulas wrote:
> Module Name:  src
> Committed By: christos
> Date: Thu Jun  3 20:17:37 UTC 2021
> 
> Modified Files:
>   src/tests/lib/csu: h_ifunc_static.c
> 
> Log Message:
> PR/56230: Jan-Benedict Glaw: arm oabi does not and will not have ifunc 
> support.

As I mentioned elsewhere, I disagree with this change. OABI is dead and
only really support for legacy compatibility. All the OABI build
variants should be removed and this is strongly a step in the wrong
direction.

Joerg


Re: CVS commit: src/sys/lib/libunwind

2021-05-31 Thread Joerg Sonnenberger
On Mon, May 31, 2021 at 12:12:24PM +, Rin Okuyama wrote:
> Bump LAST_REGISTER and LAST_RESTORE_REG to REGNO_ARM32_S31 for arm.

This is not desirable as it significantly increases the memory use.
The goal here is to actually alias the single and double register in the
space. That's what the existing was doing.

Joerg


Re: CVS commit: src/sys/lib/libunwind

2021-05-31 Thread Joerg Sonnenberger
On Mon, May 31, 2021 at 12:12:24PM +, Rin Okuyama wrote:
> There are two numbering schemes for VFPv2 registers: s0-s31 and d0-d15.
> The former is used by GCC, and the latter is by LLVM. Since libunwind was
> derived from LLVM, it has never supported the former. This results in
> crashes for GCC-compiled binaries in exception handler of C++, if it
> encounters VFPv2 registers when unwinding frames.

This is only half correct. GCC actually switched at some point.

Joerg


Re: CVS commit: src/sys/lib/libunwind

2021-05-31 Thread Joerg Sonnenberger
On Mon, May 31, 2021 at 11:41:22AM +, Rin Okuyama wrote:
> Module Name:  src
> Committed By: rin
> Date: Mon May 31 11:41:22 UTC 2021
> 
> Modified Files:
>   src/sys/lib/libunwind: Registers.hpp unwind_registers.S
> 
> Log Message:
> ...
> 
> - Introduce enum for flags.

Please undo this part. enums should *not* be used for flags.

Joerg


Re: CVS commit: src

2021-05-17 Thread Joerg Sonnenberger
On Mon, May 17, 2021 at 01:12:12PM -0400, Christos Zoulas wrote:
> Module Name:  src
> Committed By: christos
> Date: Mon May 17 17:12:12 UTC 2021
> 
> Modified Files:
>   src: build.sh
> 
> Log Message:
> for mercurial, use the latest revision instead of limiting the output to 1
> (requested by joerg)

Just for the record, this uses the currently checked out revision and
not the latest revision. "hg log" willout a revset starts with whatever
was committed/pulled last.

Joerg


Re: CVS commit: src/sys/arch/powerpc/include/booke

2021-04-17 Thread Joerg Sonnenberger
On Sat, Apr 17, 2021 at 01:25:57PM +, Rin Okuyama wrote:
> Module Name:  src
> Committed By: rin
> Date: Sat Apr 17 13:25:57 UTC 2021
> 
> Modified Files:
>   src/sys/arch/powerpc/include/booke: vmparam.h
> 
> Log Message:
> Sync MAXfoo params with oea:
> 
>   MAXTSIZ: 512MB -> 128MB
>   MAXDSIZ: 3.25GB -> 1GB
> 
> There should be no particular reasons for having different values.

Is there an architectural reason for MAXTSIZ to be 128MB? Because we
have seen binaries larger than that.

Joerg


Re: CVS commit: src/lib

2021-04-08 Thread Joerg Sonnenberger
On Thu, Apr 08, 2021 at 05:53:42PM +0100, Joseph Koshy wrote:
> On Thu, Apr 08, 2021 at 01:51:30PM +0200, Joerg Sonnenberger wrote:
> jk> Redo r1.288: traverse the complete imported Elftoolchain tree during
> jk> a build.
> 
> js> Just the libs should be enough and ideally, libelf and libdwarf
> js> are folded into the main .WAIT groups. There is a global includes
> js> run first.
> 
> It appears that we would need to add the 'elftoolchain' subdirectory to
> "src/external/bsd/Makefile" for the global includes run to work.
> 
> Let me try that out - once I get it to work I will revert
> 'src/lib/Makefile' to traverse just the imported libraries.

Thanks!

Joerg


Re: CVS commit: src/lib

2021-04-08 Thread Joerg Sonnenberger
On Thu, Apr 08, 2021 at 08:10:30AM +, Joseph Koshy wrote:
> Module Name:  src
> Committed By: jkoshy
> Date: Thu Apr  8 08:10:30 UTC 2021
> 
> Modified Files:
>   src/lib: Makefile
> 
> Log Message:
> Redo r1.288: traverse the complete imported Elftoolchain tree during a build.

Just the libs should be enough and ideally, libelf and libdwarf are
folded into the main .WAIT groups. There is a global includes run first.

Joerg


Re: CVS commit: src/share/misc

2021-04-01 Thread Joerg Sonnenberger
On Fri, Apr 02, 2021 at 12:45:28AM +0200, Roland Illig wrote:
> For simple cases I agree with Matthew that parentheses should not be
> required.  Looking through the src tree, I noticed though that the vast
> majority of the code uses the redundant parentheses, so I also agree
> with Christos that it's good to have a simple rule.

I would strongly prefer if no new code gets added using the old style,
but I also don't want to see anyone going around to just do whitespace^W
parenthesis cleanups.

Joerg


Re: CVS commit: src/lib/libc/locale

2021-02-15 Thread Joerg Sonnenberger
On Mon, Feb 15, 2021 at 04:37:15PM +0100, Thomas Klausner wrote:
> Hi!
> 
> Thanks for the man pages.
> 
> There is none for uselocale(3), which is heavily referenced from
> these, nor do I find a prototype in /usr/include...
> so how does one use these? :)

We don't support uselocale. You use this functions by passing the
locale_t to the *_l functions directly.

Joerg


Re: CVS commit: src/sys

2021-02-04 Thread Joerg Sonnenberger
On Thu, Feb 04, 2021 at 09:07:06PM +, Jaromir Dolecek wrote:
> Module Name:  src
> Committed By: jdolecek
> Date: Thu Feb  4 21:07:06 UTC 2021
> 
> Modified Files:
>   src/sys/kern: vfs_init.c vfs_subr.c
>   src/sys/sys: mount.h
> 
> Log Message:
> introduce vfs.generic.timestamp_precision sysctl to control precision
> of the timer used for vfs_timestamp(); default stays the same
> to use nanotime(9), but option is there to use the faster, albeit
> less precise methods

Most of this seems to be misguided at best. I mean it makes sense to
have a variant of getnanotime vs nanotime to avoid touching the
timecounter hardware. But the rest is just begging for hard to debug bug
reports for little to no gain. Especially limiting it to microseconds
doesn't buy anything on this level really.

Joerg


Re: CVS commit: src/sys

2021-02-03 Thread Joerg Sonnenberger
On Wed, Feb 03, 2021 at 11:03:25AM +0100, Kamil Rytarowski wrote:
> On 03.02.2021 06:51, Roy Marples wrote:
> > Module Name:src
> > Committed By:   roy
> > Date:   Wed Feb  3 05:51:40 UTC 2021
> > 
> > Modified Files:
> > src/sys/net: if_arp.h if_ether.h if_gre.h
> > src/sys/netinet: if_ether.h igmp.h in.h ip.h ip6.h ip_carp.h ip_icmp.h
> > ip_mroute.h ip_var.h tcp.h tcp_debug.h tcp_var.h udp.h udp_var.h
> > 
> > Log Message:
> > Remove __packed from various network structures
> > 
> > They are already network aligned and adding the __packed attribute
> > just causes needless compiler warnings about accssing members of packed
> > objects.
> > 
> 
> This changes the ABI for userland programs. With __packed, these
> structures, whenever contain at least 2-byte data, can be placed in a
> different location inside another structure now.

It doesn't break any of *our* ABI contracts. I also refuse us to be taken
hostage here for any (hypothetical) user program exposing internals of
the network stack on ABI boundaries. There is a very real price for the
__packed and *any* fix for the misuse has the same ABI impacts.

Joerg


Re: CVS commit: src/sys

2021-02-03 Thread Joerg Sonnenberger
On Wed, Feb 03, 2021 at 05:51:40AM +, Roy Marples wrote:
> Module Name:  src
> Committed By: roy
> Date: Wed Feb  3 05:51:40 UTC 2021
> 
> Modified Files:
>   src/sys/net: if_arp.h if_ether.h if_gre.h
>   src/sys/netinet: if_ether.h igmp.h in.h ip.h ip6.h ip_carp.h ip_icmp.h
>   ip_mroute.h ip_var.h tcp.h tcp_debug.h tcp_var.h udp.h udp_var.h
> 
> Log Message:
> Remove __packed from various network structures
> 
> They are already network aligned and adding the __packed attribute
> just causes needless compiler warnings about accssing members of packed
> objects.

Please add a compile-time assert at least for _KERNEL that the size
matches the expectation in that case.

Joerg


Re: CVS commit: src/lib/libc/stdio

2021-01-31 Thread Joerg Sonnenberger
On Sun, Jan 31, 2021 at 05:27:28PM +0100, Kamil Rytarowski wrote:
> On 31.01.2021 17:18, Jaromir Dolecek wrote:
> > Module Name:src
> > Committed By:   jdolecek
> > Date:   Sun Jan 31 16:18:22 UTC 2021
> > 
> > Modified Files:
> > src/lib/libc/stdio: fread.c
> > 
> > Log Message:
> > for unbuffered I/O arrange for the destination buffer to be filled in one
> > go, instead of triggering long series of 1 byte read(2)s; this speeds up
> > fread() several order of magnitudes for this case, directly proportional
> > to the size of the supplied buffer
> > 
> > change adapted from OpenBSD rev. 1.19
> > 
> > fixes PR lib/55808 by Roland Illig
> > 
> 
> While there it would be cool to apply FreeBSD and OpenBSD hardening for
> invalid size x nmemb, checking for overflow. I propose to return EINVAL
> in such case.

That makes no sense. Just turn them into a short read, which is
something users have to deal with anyway.

Joerg


Re: CVS commit: src/external/gpl2/diffutils/dist/src

2020-12-12 Thread Joerg Sonnenberger
On Sun, Dec 13, 2020 at 12:04:40AM +, Roy Marples wrote:
> Module Name:  src
> Committed By: roy
> Date: Sun Dec 13 00:04:40 UTC 2020
> 
> Modified Files:
>   src/external/gpl2/diffutils/dist/src: util.c
> 
> Log Message:
> diffutils: execl requires a NULL sentinel

Strictly speaking, this needs a cast to work properly. It really needs
to be a pointer and NULL isn't necessarily one.

Joerg


Re: CVS commit: src

2020-11-12 Thread Joerg Sonnenberger
On Sun, Nov 08, 2020 at 09:56:48PM +, Nia Alarie wrote:
> Module Name:  src
> Committed By: nia
> Date: Sun Nov  8 21:56:48 UTC 2020
> 
> Modified Files:
>   src/external/bsd/kyua-cli: Makefile.inc
>   src/external/ibm-public/postfix: Makefile.inc
>   src/external/public-domain/sqlite: Makefile.inc
>   src/external/public-domain/sqlite/bin: Makefile
>   src/external/public-domain/sqlite/lib: Makefile sqlite3.pc.in
>   src/usr.sbin/makemandb: Makefile
> 
> Log Message:
> sqlite: do not build without multithreading support

So the core issue here is that it actually does two separate things. It
does not only enable threadsafety (which is good), but also enables the
worker thread support (which is bad). There is little reason for wanting
the latter in a general build, at least the way it is implemented right
now. Just doing the former is a lot less intrusive...

Joerg
diff -r 2bb2635be785 external/public-domain/sqlite/Makefile.inc
--- a/external/public-domain/sqlite/Makefile.incThu Nov 12 23:24:18 
2020 +0100
+++ b/external/public-domain/sqlite/Makefile.incThu Nov 12 23:28:36 
2020 +0100
@@ -15,6 +15,7 @@
-DHAVE_STRERROR_R=1 \
-DHAVE_USLEEP=1 \
-DHAVE_SYS_ENDIAN_H=1 \
+   -DSQLITE_THREADSAFE \
-DSQLITE_MAX_WORKER_THREADS=0 \
-DSQLITE_ENABLE_COLUMN_METADATA \
-DSQLITE_ENABLE_FTS3_PARENTHESIS \


Re: CVS commit: src/external/gpl3/gcc/dist/gcc/config/aarch64

2020-10-20 Thread Joerg Sonnenberger
On Wed, Oct 21, 2020 at 08:58:36AM +0900, Rin Okuyama wrote:
> I'm also one who feels hesitate to import Linux'ism into our basic
> components. However, for this problem in particular, I still think
> it is not a good choice to keep NetBSD support in driver-aarch64.c:
> 
> (a) Our sysctl(3)-based interface is not compliant to any standards,
> just like Linux's /proc/cpuinfo. But the latter is, unfortunately
> for us, the de facto standard.

It works properly in a chroot etc without needing new files. I would
call that a big plus.

Joerg


Re: CVS commit: src/external/gpl3/gcc

2020-09-12 Thread Joerg Sonnenberger
On Sat, Sep 12, 2020 at 10:24:16PM +0200, Kamil Rytarowski wrote:
> On 12.09.2020 22:06, Joerg Sonnenberger wrote:
> > On Fri, Sep 11, 2020 at 11:45:42PM +0200, Kamil Rytarowski wrote:
> >> On 11.09.2020 23:38, Joerg Sonnenberger wrote:
> >>> On Fri, Sep 11, 2020 at 04:07:24PM +0200, Kamil Rytarowski wrote:
> >>>> The current code is confusing, as it attempts to use unimplemented
> >>>> _PTHREAD_GETTCB_EXT() and in one place uses _lwp_getprivate_fast() in
> >>>> other _lwp_getprivate(). This caused my confusion... as I assumed that
> >>>> _lwp_getprivate_fast() is internal and _lwp_getprivate() for public
> >>>> consumption.
> >>>
> >>> _PTHREAD_GETTCB_EXT is a rump hack. There is no _lwp_getprivate_fast.
> >>> There is __lwp_getprivate_fast, which originally wasn't implemented on
> >>> architectures without a fast path (like VAX). Nowadays, all functional
> >>> ports provide either __lwp_getprivate_fast (potentially with a fall-back
> >>> to the system call) or __lwp_gettcb_fast. The difference is whether the
> >>> TLS register is biased or not.
> >>>
> >>
> >> Do you agree with this patch:
> >>
> >> http://netbsd.org/~kamil/patch-00278-_rtld_tls_self.txt
> > 
> > No, I don't see the point.
> > 
> 
> What's the alternative to use in 3rd party code?

Why do you need an alternative?

Joerg


Re: CVS commit: src/external/gpl3/gcc

2020-09-12 Thread Joerg Sonnenberger
On Fri, Sep 11, 2020 at 11:45:42PM +0200, Kamil Rytarowski wrote:
> On 11.09.2020 23:38, Joerg Sonnenberger wrote:
> > On Fri, Sep 11, 2020 at 04:07:24PM +0200, Kamil Rytarowski wrote:
> >> The current code is confusing, as it attempts to use unimplemented
> >> _PTHREAD_GETTCB_EXT() and in one place uses _lwp_getprivate_fast() in
> >> other _lwp_getprivate(). This caused my confusion... as I assumed that
> >> _lwp_getprivate_fast() is internal and _lwp_getprivate() for public
> >> consumption.
> > 
> > _PTHREAD_GETTCB_EXT is a rump hack. There is no _lwp_getprivate_fast.
> > There is __lwp_getprivate_fast, which originally wasn't implemented on
> > architectures without a fast path (like VAX). Nowadays, all functional
> > ports provide either __lwp_getprivate_fast (potentially with a fall-back
> > to the system call) or __lwp_gettcb_fast. The difference is whether the
> > TLS register is biased or not.
> > 
> 
> Do you agree with this patch:
> 
> http://netbsd.org/~kamil/patch-00278-_rtld_tls_self.txt

No, I don't see the point.

Joerg


Re: CVS commit: src/external/gpl3/gcc

2020-09-11 Thread Joerg Sonnenberger
On Fri, Sep 11, 2020 at 04:07:24PM +0200, Kamil Rytarowski wrote:
> The current code is confusing, as it attempts to use unimplemented
> _PTHREAD_GETTCB_EXT() and in one place uses _lwp_getprivate_fast() in
> other _lwp_getprivate(). This caused my confusion... as I assumed that
> _lwp_getprivate_fast() is internal and _lwp_getprivate() for public
> consumption.

_PTHREAD_GETTCB_EXT is a rump hack. There is no _lwp_getprivate_fast.
There is __lwp_getprivate_fast, which originally wasn't implemented on
architectures without a fast path (like VAX). Nowadays, all functional
ports provide either __lwp_getprivate_fast (potentially with a fall-back
to the system call) or __lwp_gettcb_fast. The difference is whether the
TLS register is biased or not.

Joerg


Re: CVS commit: src/sys/kern

2020-08-02 Thread Joerg Sonnenberger
On Sun, Aug 02, 2020 at 01:57:11PM +, Taylor R Campbell wrote:
> But it sounds like the original motivation is that it triggered
> -Wvla...which frankly strikes me as a compiler bug since there's
> obviously no actual VLA created in sizeof; as far as I can tell
> there's no semantic difference between sizeof(device_t[n]) and
> sizeof(device_t) * n.

Yes, it seems strange to trigger a VLA warning in code that it is
required to be compile-time evaluated.

Joerg


Re: CVS commit: src/share/mk

2020-07-13 Thread Joerg Sonnenberger
On Mon, Jul 13, 2020 at 07:22:51AM +, matthew green wrote:
> Module Name:  src
> Committed By: mrg
> Date: Mon Jul 13 07:22:51 UTC 2020
> 
> Modified Files:
>   src/share/mk: bsd.README
> 
> Log Message:
> MKLLVMRT is automatically enabled on x86 and arm64, not mesa18+.

It is both.

Joerg


Re: CVS commit: src/usr.bin/make

2020-07-04 Thread Joerg Sonnenberger
On Sat, Jul 04, 2020 at 03:44:07PM +, Roland Illig wrote:
> Module Name:  src
> Committed By: rillig
> Date: Sat Jul  4 15:44:07 UTC 2020
> 
> Modified Files:
>   src/usr.bin/make: var.c
> 
> Log Message:
> make(1): fix :hash modifier on 16-bit platforms
> 
> On platforms where int has only 16 bits the shifts would have been in
> 16-bit arithmetic, which would invoke undefined behavior for "ustr[3] <<
> 24" as well as "ustr[2] << 16" (C99, 6.5.7p3).

WTF should we care? This is just making things more complicated without
adding any value.

Joerg


Re: CVS commit: src/sys/arch/x86/x86

2020-06-24 Thread Joerg Sonnenberger
On Wed, Jun 24, 2020 at 10:28:08PM +, Jaromir Dolecek wrote:
> Module Name:  src
> Committed By: jdolecek
> Date: Wed Jun 24 22:28:08 UTC 2020
> 
> Modified Files:
>   src/sys/arch/x86/x86: multiboot2.c
> 
> Log Message:
> don't try allocating 16KB of scratch space on stack
> 
> it's too early for kmem_alloc(), so use static variable in BSS; it's used
> post reloc, so don't need to use the RELOC() macros

Why? This is the initial early stack, not the normal LWP stack.

Joerg


Re: CVS commit: src/sys/arch/powerpc

2020-06-22 Thread Joerg Sonnenberger
On Mon, Jun 22, 2020 at 05:34:57AM +, Rin Okuyama wrote:
> Module Name:  src
> Committed By: rin
> Date: Mon Jun 22 05:34:57 UTC 2020
> 
> Modified Files:
>   src/sys/arch/powerpc/include: mcontext.h types.h
>   src/sys/arch/powerpc/powerpc: sig_machdep.c
> 
> Log Message:
> Fix previous; hide userland ABI details for kernel as suggested by joerg:

Thanks!

Joerg


Re: CVS commit: src/sys/arch/powerpc

2020-06-21 Thread Joerg Sonnenberger
On Sun, Jun 21, 2020 at 12:40:00AM +, Rin Okuyama wrote:
> - Obsolete __lwp_settcb() in order to let kernel know new TLS address via
>   _lwp_setprivate(2). Alternatively, we can call _lwp_setprivate(2) within
>   __lwp_settcb() like mips, but it is just double handling; we adjust %r2
>   appropriately in _lwp_setprivate(2) via cpu_lwp_setprivate().

If an architecture stores the TCB at an offset, it should provide
__lwp_settcb to do the necessary adjust and then invoke _lwp_setprivate
as appropiate. The MIPS variant is correct. This should *not* be done in
the kernel, as it is an ABI detail of the userland TLS API.

Joerg


Re: CVS commit: src/common/lib/libprop

2020-06-11 Thread Joerg Sonnenberger
On Fri, Jun 12, 2020 at 02:59:40AM +0200, Kamil Rytarowski wrote:
> On 12.06.2020 02:07, Joerg Sonnenberger wrote:
> > On Fri, Jun 12, 2020 at 01:28:15AM +0200, Kamil Rytarowski wrote:
> >> Please list legitimate false positives. There is practically nothing
> >> like that possible for using deprecated APIs (at least kept longer
> >> term). Besides that, the report shall be lowered to warning (like it
> >> used to be for Clang).
> > 
> > Build a random KDE package and see warnings about XHR symbols?
> > 
> > Joerg
> > 
> 
> XDR?
> 
> $ grep -ir xdr .|grep warn
> ./libc/yp/xdryp.c:__warn_references(xdr_domainname,
> ./libc/yp/xdryp.c:"warning: this program uses xdr_domainname(),
> which is deprecated and buggy.")
> ./libc/yp/xdryp.c:__warn_references(xdr_peername,
> ./libc/yp/xdryp.c:"warning: this program uses xdr_peername(), which
> is deprecated and buggy.")
> ./libc/yp/xdryp.c:__warn_references(xdr_mapname,
> ./libc/yp/xdryp.c:"warning: this program uses xdr_mapname(), which
> is deprecated and buggy.")
> 
> KDE4?

No, KDE5.

> 
> After grepping, I don't see relevant users of these APIs. There is
> something around Python that has a NIS/YP module.

Exactly, nothing in Qt uses it, but it *still* triggers the warning.

Joerg


Re: CVS commit: src/common/lib/libprop

2020-06-11 Thread Joerg Sonnenberger
On Fri, Jun 12, 2020 at 01:28:15AM +0200, Kamil Rytarowski wrote:
> Please list legitimate false positives. There is practically nothing
> like that possible for using deprecated APIs (at least kept longer
> term). Besides that, the report shall be lowered to warning (like it
> used to be for Clang).

Build a random KDE package and see warnings about XHR symbols?

Joerg


Re: CVS commit: src/common/lib/libprop

2020-06-11 Thread Joerg Sonnenberger
On Fri, Jun 12, 2020 at 12:58:45AM +0200, Kamil Rytarowski wrote:
> On 12.06.2020 00:55, Christos Zoulas wrote:
> > In article <20200611222544.6d3a6f...@cvs.netbsd.org>,
> > Joerg Sonnenberger  wrote:
> >> -=-=-=-=-=-
> >>
> >> Module Name:   src
> >> Committed By:  joerg
> >> Date:  Thu Jun 11 22:25:44 UTC 2020
> >>
> >> Modified Files:
> >>src/common/lib/libprop: prop_object_impl.h
> >>
> >> Log Message:
> >> Unbreak clang builds by removing questionable linker warning sections
> >> trggered all over the place.
> > 
> > Why don't you do this just for clang, so that we can use gcc to track the
> > remaining ones down and finish the job? Now we can't easily find them.
> > 
> > christos
> > 
> 
> Can we please revert this and fix clang?
> 
> I'm strongly for linker warnings as they catch real issues.

Repeating that statement doesn't make it true. The amount of legit
problems found by them is dwarfed by far by the number of false
positives seen. That's complete ignoring basic QoI issues like "where is
this actually triggered" and no "I know, shut up".

Joerg


Re: CVS commit: src/common/lib/libprop

2020-06-11 Thread Joerg Sonnenberger
On Thu, Jun 11, 2020 at 10:55:16PM -, Christos Zoulas wrote:
> In article <20200611222544.6d3a6f...@cvs.netbsd.org>,
> Joerg Sonnenberger  wrote:
> >-=-=-=-=-=-
> >
> >Module Name: src
> >Committed By:joerg
> >Date:Thu Jun 11 22:25:44 UTC 2020
> >
> >Modified Files:
> > src/common/lib/libprop: prop_object_impl.h
> >
> >Log Message:
> >Unbreak clang builds by removing questionable linker warning sections
> >trggered all over the place.
> 
> Why don't you do this just for clang, so that we can use gcc to track the
> remaining ones down and finish the job? Now we can't easily find them.

Make them use the deprecation attribution and fix the fallout. But don't
leave the build broken for days. I said it before, but the linker
warning flags are a terrible way for handling deprecation. Look at many
KDE builds for nice examples...

Joerg


Re: CVS commit: src

2020-06-07 Thread Joerg Sonnenberger
On Sun, Jun 07, 2020 at 02:43:06PM -0700, Jason Thorpe wrote:
> 
> > On Jun 7, 2020, at 1:57 PM, Joerg Sonnenberger  wrote:
> > 
> >> Example?
> > 
> > https://gcc.gnu.org/onlinedocs/gcc-4.7.0/gcc/Function-Attributes.html#index-g_t_0040code_007bdeprecated_007d-attribute_002e-2502
> 
> I meant an example in our tree.  We use __warn_reference() in several places 
> already.  I also don't want to introduce compiler warnings just yet.

Yes, we do use it in a number of places. Most of them are ancient, some
of the modern cases like in XHR are exactly what I am talking about in
triggering bugs. The compiler warnings are strictly superior.

Joerg


Re: CVS commit: src

2020-06-07 Thread Joerg Sonnenberger
On Sun, Jun 07, 2020 at 01:27:48PM -0700, Jason Thorpe wrote:
> 
> > On Jun 7, 2020, at 1:22 PM, Joerg Sonnenberger  wrote:
> > 
> > On Sat, Jun 06, 2020 at 09:26:00PM +, Jason R Thorpe wrote:
> >> ==> Deprecate mutable prop_data(3) and prop_string(3) objects.  The old
> >>APIs that support them still exist, but will now produce link-time
> >>warnings when used.
> > 
> > Please replace them with proper deprecration annotation in the headers.
> 
> Example?

https://gcc.gnu.org/onlinedocs/gcc-4.7.0/gcc/Function-Attributes.html#index-g_t_0040code_007bdeprecated_007d-attribute_002e-2502

Joerg


Re: CVS commit: src

2020-06-07 Thread Joerg Sonnenberger
On Sat, Jun 06, 2020 at 09:26:00PM +, Jason R Thorpe wrote:
> ==> Deprecate mutable prop_data(3) and prop_string(3) objects.  The old
> APIs that support them still exist, but will now produce link-time
> warnings when used.

Please replace them with proper deprecration annotation in the headers.
The link-time warnings are obnoxious due to both various ld bugs and any
precision in disabling them.

Joerg


Re: CVS commit: src/sys/kern

2020-06-01 Thread Joerg Sonnenberger
On Sun, May 31, 2020 at 11:24:20PM +, Rin Okuyama wrote:
> Module Name:  src
> Committed By: rin
> Date: Sun May 31 23:24:20 UTC 2020
> 
> Modified Files:
>   src/sys/kern: kern_timeout.c
> 
> Log Message:
> Stop allocating buffers dynamically in a DDB session, in order not to
> disturb on-going debugged state of kernel datastructures.

This breaks the build with clang as it uses a declared-but-not-defined
type callout_cpu.

Joerg


Re: CVS commit: src

2020-05-30 Thread Joerg Sonnenberger
On Sat, May 30, 2020 at 04:48:00PM -0400, Christos Zoulas wrote:
> Module Name:  src
> Committed By: christos
> Date: Sat May 30 20:47:59 UTC 2020
> 
> Modified Files:
>   src/distrib/sets/lists/base: mi shl.mi
>   src/distrib/sets/lists/comp: mi shl.mi
>   src/distrib/sets/lists/debug: mi shl.mi
>   src/etc/mtree: NetBSD.dist.base
>   src/external/mit/libuv/lib: Makefile
>   src/external/mpl/bind: Makefile.inc
>   src/external/mpl/bind/lib/libisc: Makefile
>   src/external/mpl/dhcp: Makefile.inc
>   src/share/mk: bsd.README bsd.prog.mk
> Added Files:
>   src/external/mpl/bind/lib/libisc: isc.map
> 
> Log Message:
> Make libuv private, requested by joerg@

Thanks!

Joerg


Re: CVS commit: src/external/bsd/ntp/bin/ntpd

2020-05-29 Thread Joerg Sonnenberger
On Fri, May 29, 2020 at 10:53:02AM +, Kamil Rytarowski wrote:
> Module Name:  src
> Committed By: kamil
> Date: Fri May 29 10:53:02 UTC 2020
> 
> Modified Files:
>   src/external/bsd/ntp/bin/ntpd: Makefile
> 
> Log Message:
> Fix the ntpd build with Clang/LLVM
> 
> Set -Wno-format-nonliteral for ntp_refclock.c

Please fix this properly by adding the appropiate format string
annotation.

Joerg


Re: CVS commit: src/sys/arch

2020-05-21 Thread Joerg Sonnenberger
On Thu, May 21, 2020 at 09:12:31PM +, Andrew Doran wrote:
> Module Name:  src
> Committed By: ad
> Date: Thu May 21 21:12:31 UTC 2020
> 
> Modified Files:
>   src/sys/arch/x86/acpi: acpi_wakeup.c
>   src/sys/arch/x86/include: i82489var.h
>   src/sys/arch/x86/x86: cpu.c lapic.c x86_machdep.c
>   src/sys/arch/xen/x86: cpu.c
>   src/sys/arch/xen/xen: hypervisor.c xen_clock.c
> 
> Log Message:
> - Recalibrate the APIC timer using the TSC, once the TSC has in turn been
>   recalibrated using the HPET.  This gets the clock interrupt firing more
>   closely to HZ.

Why using the TSC and not the HPET directly? For systems with HPET, same
question with the ACPI / PCI timer.

Joerg


Re: CVS commit: src/tests/lib/libc/sys

2020-05-14 Thread Joerg Sonnenberger
On Thu, May 14, 2020 at 11:36:28PM +0200, Kamil Rytarowski wrote:
> On 14.05.2020 23:02, Joerg Sonnenberger wrote:
> > On Thu, May 14, 2020 at 07:21:35PM +, Kamil Rytarowski wrote:
> >> Module Name:   src
> >> Committed By:  kamil
> >> Date:  Thu May 14 19:21:35 UTC 2020
> >>
> >> Modified Files:
> >>src/tests/lib/libc/sys: t_ptrace_fork_wait.h
> >>
> >> Log Message:
> >> Ignore interception of the SIGCHLD signals.
> >>
> >> SIGCHLD once blocked is discarded by the kernel as it has the
> >> SA_IGNORE property. During the fork(2) operation all signals can be
> >> shortly blocked and missed (unless there is a registered signal
> >> handler in the traced child). This leads to a race in this test if
> >> there would be an intention to catch SIGCHLD.
> > 
> > This doesn't sound right. sigprocmask and pthread_sigmask shouldn't
> > affect whether the signal handler is called, only when. Temporarily
> > masking a signal is different from setting it to SIG_IGN.
> > 
> > Joerg
> > 
> 
> I was looking into it and as there is no signal handler for SIGCHLD, the
> signal is discarded.

Sure, but that doesn't really have anything to do with signal blocking.
That's the part that is confusing.

Joerg


Re: CVS commit: src/tests/lib/libc/sys

2020-05-14 Thread Joerg Sonnenberger
On Thu, May 14, 2020 at 07:21:35PM +, Kamil Rytarowski wrote:
> Module Name:  src
> Committed By: kamil
> Date: Thu May 14 19:21:35 UTC 2020
> 
> Modified Files:
>   src/tests/lib/libc/sys: t_ptrace_fork_wait.h
> 
> Log Message:
> Ignore interception of the SIGCHLD signals.
> 
> SIGCHLD once blocked is discarded by the kernel as it has the
> SA_IGNORE property. During the fork(2) operation all signals can be
> shortly blocked and missed (unless there is a registered signal
> handler in the traced child). This leads to a race in this test if
> there would be an intention to catch SIGCHLD.

This doesn't sound right. sigprocmask and pthread_sigmask shouldn't
affect whether the signal handler is called, only when. Temporarily
masking a signal is different from setting it to SIG_IGN.

Joerg


Re: CVS commit: src

2020-05-12 Thread Joerg Sonnenberger
On Tue, May 12, 2020 at 10:51:51AM +0200, Kamil Rytarowski wrote:
> On 12.05.2020 02:59, Joerg Sonnenberger wrote:
> > On Mon, May 11, 2020 at 11:07:02PM +0200, Kamil Rytarowski wrote:
> >> On 19.04.2020 03:06, Joerg Sonnenberger wrote:
> >>> Module Name:  src
> >>> Committed By: joerg
> >>> Date: Sun Apr 19 01:06:16 UTC 2020
> >>>
> >>> Modified Files:
> >>>   src/lib/libc/gen: pthread_atfork.c
> >>>   src/libexec/ld.elf_so: rtld.c rtld.h symbols.map
> >>>
> >>> Log Message:
> >>> Rename __atomic_fork to __locked_fork and give it  as argument.
> >>> rtld and libc use different storage, so the initial version would
> >>> incorrectly report the failure reason for fork().
> >>>
> >>> There is still a small race condition inside ld.elf_so as it doesn't use
> >>> thread-safe errno internally, but that's a more contained internal
> >>> issue.
> >>>
> >>>
> >>
> >>
> >> Should we add the same logic for clone(2)?
> > 
> > clone only exists for Linux compat. I see no reason to support any fork
> > emulation for it.
> > 
> > Joerg
> > 
> 
> This Linux compat is on the source code level and inside the kernel
> clone() shares the same code with fork().
> 
> clone(2) is a regular syscall available in the NetBSD native ABI syscall
> layers. There are also users (I have got one).
> 
> All problems for fork() can be reproduced for clone(). But if we want to
> just mitigate some issues of fork() users and fix/ignore promptly
> clone() ones, it is fine.

clone(2) already has a huge set of race conditions it will hit in
various parts of libc, libpthread etc when it is used to emulate fork.
It is a non-standard interface too with no mechanisms for resolving any
of those races. I see no reason for pretending that it is anywhere
usable.

Joerg


Re: CVS commit: src

2020-05-11 Thread Joerg Sonnenberger
On Mon, May 11, 2020 at 11:07:02PM +0200, Kamil Rytarowski wrote:
> On 19.04.2020 03:06, Joerg Sonnenberger wrote:
> > Module Name:src
> > Committed By:   joerg
> > Date:   Sun Apr 19 01:06:16 UTC 2020
> > 
> > Modified Files:
> > src/lib/libc/gen: pthread_atfork.c
> > src/libexec/ld.elf_so: rtld.c rtld.h symbols.map
> > 
> > Log Message:
> > Rename __atomic_fork to __locked_fork and give it  as argument.
> > rtld and libc use different storage, so the initial version would
> > incorrectly report the failure reason for fork().
> > 
> > There is still a small race condition inside ld.elf_so as it doesn't use
> > thread-safe errno internally, but that's a more contained internal
> > issue.
> > 
> > 
> 
> 
> Should we add the same logic for clone(2)?

clone only exists for Linux compat. I see no reason to support any fork
emulation for it.

Joerg


Re: CVS commit: src/sys/uvm

2020-05-11 Thread Joerg Sonnenberger
On Sun, May 10, 2020 at 11:53:00PM +0100, Alexander Nasonov wrote:
> Taylor R Campbell wrote:
> > Log Message:
> > Implement swap encryption.
> > 
> > Enabled by sysctl -w vm.swap_encrypt=1.
> 
> If secmodel_securelevel(9) is still a thing, locking down this sysctl
> at high securelevel may improve our security. Prior to this change,
> swap devices were readable (even if enrypted with cgd).  With this
> sysctl set to 1, all new swap devices will be encrypted, the only
> thing to worry about is if it's set back to 0 on a compromised host.

Well, the ability to turn it off should be locked down. Enabling it for
securelevel>0 seems fine?

Joerg


Re: CVS commit: src/sys

2020-04-27 Thread Joerg Sonnenberger
On Mon, Apr 27, 2020 at 06:19:18PM +0900, Tetsuya Isaki wrote:
> As a result of some discussion here, adding 60+ one-line header
> files like  is troublesome or annoying.
> And I have no good idea to generate a suitable border line from
> existing parameters or something.
> 
> Then how about this?
> (thanks tsutsui@ for comment about choosing archs)
> This #ifdefs may not look elegance, but I think it's simple and
> realistic.

I'd just move it the whole block into audio.c, but otherwise this sounds
like a good step forward.

Joerg


Re: CVS commit: src/include

2020-04-17 Thread Joerg Sonnenberger
On Fri, Apr 17, 2020 at 03:22:35PM +, Kamil Rytarowski wrote:
> Module Name:  src
> Committed By: kamil
> Date: Fri Apr 17 15:22:35 UTC 2020
> 
> Modified Files:
>   src/include: assert.h
> 
> Log Message:
> Remove the static_assert() fallback for pre-C11 and pre-C++11
> 
> C++ without real static_assert() can be incompatible with the C fallback
> as presented in openjdk.
> 
> A pre-C11 compiler can be picky on the implementation.

So did you actually test that the tree compiles with this? Just asking,
since your own ptrace tests depend on static_assert...

Joerg


Re: CVS commit: src/sys

2020-04-16 Thread Joerg Sonnenberger
On Fri, Apr 17, 2020 at 07:31:51AM +0900, Rin Okuyama wrote:
> On 2020/04/17 6:56, Rin Okuyama wrote:
> > Module Name:src
> > Committed By:   rin
> > Date:   Thu Apr 16 21:56:43 UTC 2020
> > 
> > Modified Files:
> > src/sys/arch/arm/omap: omap3_sdmareg.h omap3_sdmavar.h omapfbreg.h
> > src/sys/arch/arm/ti: omap3_dssreg.h
> > src/sys/arch/macppc/dev: batteryvar.h cudavar.h deqvar.h obiovar.h
> > platinumfbreg.h pmuvar.h valkyriefbreg.h videopllvar.h
> > src/sys/arch/mips/adm5120/dev: if_admswvar.h
> > src/sys/arch/powerpc/include: intr.h
> > src/sys/arch/powerpc/pic: ipivar.h picvar.h
> > src/sys/dev/acpi/wmi: wmi_acpivar.h
> > src/sys/dev/adb: adbvar.h
> > src/sys/dev/i2c: adm1026reg.h dbcool_reg.h dbcool_var.h sgsmixvar.h
> > src/sys/dev/ic: i128reg.h i128var.h
> > src/sys/dev/pci: gffbreg.h ppbvar.h voyagervar.h
> > src/sys/dev/qbus: rlvar.h
> > src/sys/external/bsd/ena-com: ena_plat.h
> > src/sys/lib/libkern: crc16.h
> > src/sys/ufs/ext2fs: ext2fs_xattr.h
> > 
> > Log Message:
> > Stop using __KERNEL_RCSID() in header files; it confuses ident(1) by
> > overwriting RCSID in main source files.
> > 
> > XXX
> > The first argument of __KERNEL_RCSID() is neglected for ELF. If we wish
> > to have RCSID of header files in kernel binary, we need something like
> > __FBSDID() macro in FreeBSD.
> 
> Oops, this description is not correct. __KERNEL_RCSID() in header files
> does *not* overwrite RCSID in main source files. The problem is that it
> inserts its own RCSID in *every* object files.
> 
> As a result, ident(1) shows more than 1000 duplicated lines for GENERIC
> kernel of macppc, for example.

That can be fixed generically? .ident needs the SHF_MERGE|SHF_STRINGS as
section flags and then the linker should do the rest by itself.

Joerg


Re: CVS commit: src/sys

2020-04-09 Thread Joerg Sonnenberger
On Thu, Apr 09, 2020 at 09:52:37PM +0900, Tetsuya Isaki wrote:
> At Fri, 3 Apr 2020 17:51:21 +0200,
> Joerg Sonnenberger wrote:
> > It seems perfectly
> > sensible to me that the final output device can provide a lower limit as
> > well as having one derived from HZ and using whatever is higher.
> 
> Sorry, I could not translate well and I didn't understand.
> Could you write that in another way?

There are two possible reasons for a lower limit for the buffer size:
(1) The device requires a certain amount.
(2) The system wants to ensure the interrupt rate doesn't go over a
certain rate.

The real value shouldn't be a constant, but the maximum of the two?

Joerg


Re: CVS commit: src/sys

2020-04-03 Thread Joerg Sonnenberger
On Fri, Apr 03, 2020 at 09:45:20PM +0900, Tetsuya Isaki wrote:
>  [*] On my alpha (500MHz), wss(4)/ISA works even on blk_ms=1.
>  But I was not able to set 1msec on yds(4) PCI sound card on
>  the same machine.  Its lower limit was 5msec (due to driver's
>  or hardware's restriction, I don't know details though).

It sounds wrong to hold users hostage to the restrictions of some
devices. Why does this have to be a constant anyway? It seems perfectly
sensible to me that the final output device can provide a lower limit as
well as having one derived from HZ and using whatever is higher.

Joerg


Re: CVS commit: src/sys

2020-03-29 Thread Joerg Sonnenberger
On Sun, Mar 29, 2020 at 07:26:26AM -0700, Jason Thorpe wrote:
> 
> > On Mar 29, 2020, at 6:11 AM, Joerg Sonnenberger  wrote:
> > 
> > I would allow at least 1/HZ as baseline.
> 
> Be careful, some platforms have a relatively high HZ (e.g. Alpha HZ=1024).

Yes and I see no fundamental reason for not allowing buffering with 1ms
frames in that case. I expect Alpha to be fast enough to do that with
little overhead.

Joerg


Re: CVS commit: src/sys

2020-03-29 Thread Joerg Sonnenberger
On Sun, Mar 29, 2020 at 01:11:47PM +0900, Tetsuya Isaki wrote:
> > I would prefer if blk_ms were kept below 5ms on amd64 and aarch64.
> > We don't have to worry about the CPU load of playing audio on these 
> > platforms.
> 
> CPU load or performance is not subject.  I know that my
> implementation will work on the most modern real hardware.
> But I feel that at least 4msec is too rush to be default.
> A default should not be for one game application.

I would allow at least 1/HZ as baseline.

Joerg


Re: CVS commit: src/lib/libc/string

2020-03-26 Thread Joerg Sonnenberger
On Thu, Mar 26, 2020 at 10:54:21AM +0700, Robert Elz wrote:
> Date:Wed, 25 Mar 2020 20:51:25 +
> From:David Holland 
> Message-ID:  <20200325205125.ga11...@netbsd.org>
> 
>   | I don't agree -- because applications shouldn't attempt to modify the
>   | result, it should be const.
> 
> The only reason apps shouldn't modify the string is in case of porting
> the app to an (well, some) ancient implementation.  Because of the NLS
> requirements, the message these days (any modern implementation) must be
> read from some external file - which means the storage for it must be
> writable. 

Actually, the only reason why we really need writable space is the
unknown error case. NLS could in principle be using mmaped data as well,
modulo not being able to unmap it again.

Joerg


Re: CVS commit: src/lib/libc/compiler_rt

2020-03-08 Thread Joerg Sonnenberger
On Sun, Mar 08, 2020 at 06:30:06AM +, Rin Okuyama wrote:
> Module Name:  src
> Committed By: rin
> Date: Sun Mar  8 06:30:06 UTC 2020
> 
> Modified Files:
>   src/lib/libc/compiler_rt: Makefile.inc
> 
> Log Message:
> Fix broken printf(3) %d output for numbers more than two digits, e.g.,
> 
>   printf("%d\n", 42) ---> "::" instead of "42"
> 
> Our __{,u}modsi3 codes assume that __udivsi3 returns remainder to
> %d1 (volatile register). __udivsi3 in libgcc does not, and therefore
> mixing them up results in mess.

This looks like the wrong fix to me. This is not about libgcc at all.
The depency in the src/common code should be fixed IMO. If __{,u}modsi3
wants to make assumptions about the remainder being implicitly computed,
it really should be using the divmod primitive instead.

Joerg


Re: CVS commit: src/include

2020-03-01 Thread Joerg Sonnenberger
On Sun, Mar 01, 2020 at 10:26:08PM +0100, Kamil Rytarowski wrote:
> On 01.03.2020 19:31, Joerg Sonnenberger wrote:
> > On Sun, Mar 01, 2020 at 03:08:16PM +, Kamil Rytarowski wrote:
> >> Module Name:   src
> >> Committed By:  kamil
> >> Date:  Sun Mar  1 15:08:16 UTC 2020
> >>
> >> Modified Files:
> >>src/include: stddef.h
> >>
> >> Log Message:
> >> Expose max_align_t to C99/C++
> > 
> > Please revert this immediately. It is just wrong.
> > 
> 
> Rationale? libc++ does not work and I perceive insisting it as a losing
> battle and and zero gain.

As you do know, there is a patch for libc++ under review to fix this
properly. It is even generally agreed that the current behavior is a
bug. Your patch broke well defined C++03 programs.

> >> This problem does not exist on OSs like Linux as they get this namespace
> >> visibility defined inside LLVM or GNU toolchain headers. NetBSD ships with
> >> its own stddef.h, rather than relying on a toolchain and its internal
> >> extensions.
> > 
> > This is incorrect as well.
> > 
> 
> No stddef.h in GLIBC is a matter of fact.

$ printf '#include \nmax_align_t foo;' | clang++ -std=c++03 -x c++ -
:2:1: error: unknown type name 'max_align_t'
$ printf '#include \nmax_align_t foo;' | g++ -std=c++03 -x c++ -
:2:1: error: ‘max_align_t’ does not name a type

Joerg


Re: CVS commit: src/doc

2020-03-01 Thread Joerg Sonnenberger
On Sun, Mar 01, 2020 at 10:20:44PM +0100, Kamil Rytarowski wrote:
> If that will nor realize, I will request to enable GNU_HASH for NetBSD-11.

You have so far demonstrated no justification at all beyond handwaving
for why it should be included in first place. Seriously, stop rushing
code without doing your homework to demonstrate that it is (1) correct
(2) an actual improvement (3) understanding concerns raised in the past.
I have limited ressources and having to deal with crap like the
max_align_t commit just costs a lot of time for no good reason.

> GNU_HASH is industry standard for all modern Linux and any other
> mainstream BSD distribution already for around a decade.

If you want an industry standard, please use the GNU/Linux distribution
of your choice. Arguing based on industry standards is a non sequitur.

Joerg


Re: CVS commit: src/include

2020-03-01 Thread Joerg Sonnenberger
On Sun, Mar 01, 2020 at 03:08:16PM +, Kamil Rytarowski wrote:
> Module Name:  src
> Committed By: kamil
> Date: Sun Mar  1 15:08:16 UTC 2020
> 
> Modified Files:
>   src/include: stddef.h
> 
> Log Message:
> Expose max_align_t to C99/C++

Please revert this immediately. It is just wrong.

> This problem does not exist on OSs like Linux as they get this namespace
> visibility defined inside LLVM or GNU toolchain headers. NetBSD ships with
> its own stddef.h, rather than relying on a toolchain and its internal
> extensions.

This is incorrect as well.

Joerg


Re: CVS commit: src/doc

2020-02-29 Thread Joerg Sonnenberger
On Sat, Feb 29, 2020 at 10:29:02PM +0100, Kamil Rytarowski wrote:
> On 29.02.2020 21:58, Joerg Sonnenberger wrote:
> > On Sat, Feb 29, 2020 at 07:36:00PM +0100, Kamil Rytarowski wrote:
> >> On 29.02.2020 19:00, Taylor R Campbell wrote:
> >>>> Module Name:src
> >>>> Committed By:   kamil
> >>>> Date:   Sat Feb 29 04:27:01 UTC 2020
> >>>>
> >>>> Modified Files:
> >>>> src/doc: CHANGES
> >>>>
> >>>> Log Message:
> >>>> ld.elf_so(1): Implement DT_GNU_HASH
> >>>
> >>> Was this discussed anywhere?
> >>
> >> In the toolchain circles, for some time now (2+ years).
> >>
> >> It was one of the pending task to modernize our ELF loader on par with
> >> at least FreeBSD.
> > 
> > Can we please stop with this "we must need XXX to be on par with YYY"
> > nonsense without actually looking at the details first and discuss them?
> > Practically speaking, there was moderately little discussion about the
> > actual design choices of DT_GNU_HASH, especially some of its deficits.
> > Especially because we already have some important improvements in our
> > tree *without breaking compatibility*. Especially the size claims are
> > questionable at best as justification are weak at best. It also ignores
> > that by design DT_GNU_HASH conflicts with at least one platform ABI.
> > 
> > Joerg
> > 
> 
> Just keeping DT_GNU_HASH support around does not break compat. Full
> replacement of HASH algorithm would break compat but nobody wants to do
> it (at least in NetBSD).

Can you please stop speaking for the NetBSD community? Seriously, that
alone makes me want to just /dev/null all messages. "Keeping DT_GNU_HASH
around" is funny, given that it just got added without any discussion.

Joerg


Re: CVS commit: src/doc

2020-02-29 Thread Joerg Sonnenberger
On Sat, Feb 29, 2020 at 07:36:00PM +0100, Kamil Rytarowski wrote:
> On 29.02.2020 19:00, Taylor R Campbell wrote:
> >> Module Name:src
> >> Committed By:   kamil
> >> Date:   Sat Feb 29 04:27:01 UTC 2020
> >>
> >> Modified Files:
> >> src/doc: CHANGES
> >>
> >> Log Message:
> >> ld.elf_so(1): Implement DT_GNU_HASH
> > 
> > Was this discussed anywhere?
> 
> In the toolchain circles, for some time now (2+ years).
> 
> It was one of the pending task to modernize our ELF loader on par with
> at least FreeBSD.

Can we please stop with this "we must need XXX to be on par with YYY"
nonsense without actually looking at the details first and discuss them?
Practically speaking, there was moderately little discussion about the
actual design choices of DT_GNU_HASH, especially some of its deficits.
Especially because we already have some important improvements in our
tree *without breaking compatibility*. Especially the size claims are
questionable at best as justification are weak at best. It also ignores
that by design DT_GNU_HASH conflicts with at least one platform ABI.

Joerg


Re: CVS commit: src/external/apache2/llvm/config/llvm/Config

2020-02-21 Thread Joerg Sonnenberger
On Fri, Feb 21, 2020 at 11:13:24AM +0100, Kamil Rytarowski wrote:
> On 21.02.2020 07:20, Martin Husemann wrote:
> > On Fri, Feb 21, 2020 at 03:01:45AM +0100, Kamil Rytarowski wrote:
> >> I consider calling ncurses-dev essential as a bug.
> > 
> > My knee jerk reaction was: if a unix compiler needs terminfo (especially
> > when it only does that to create coloured unreadable messages) it needs
> > to be moved to pkgsrc. Same goes for a command line debugger.
> > 
> > But if I understood Joerg's (extremely terse) objection:
> > there should be logic in the tools build dealing with missing terminfo
> > libs and this case needs more analyzis to understand why it fails.
> > 
> > Martin
> > 
> 
> This patch changes cryptic failure in linking to a verbose message:
> 
> http://netbsd.org/~kamil/patch-00231-llvm-terminfo-tools.txt
> 
> Does it look to commit?

No, absolutely not. This is just making the problem a 100 times worse.

Joerg


Re: CVS commit: src/external/apache2/llvm/config/llvm/Config

2020-02-19 Thread Joerg Sonnenberger
On Wed, Feb 19, 2020 at 11:29:09PM +0100, Kamil Rytarowski wrote:
> On 19.02.2020 23:03, Joerg Sonnenberger wrote:
> > On Wed, Feb 19, 2020 at 04:10:17PM +0100, Kamil Rytarowski wrote:
> >> On 19.02.2020 14:32, Joerg Sonnenberger wrote:
> >>> Module Name:  src
> >>> Committed By: joerg
> >>> Date: Wed Feb 19 13:32:40 UTC 2020
> >>>
> >>> Modified Files:
> >>>   src/external/apache2/llvm/config/llvm/Config: config.h.in
> >>>
> >>> Log Message:
> >>> Revert last. It was objected to pre-commit, there is no actual error
> >>> analysis or report and there is existing logic supposed to handle this.
> >>>
> >>>
> >>> To generate a diff of this commit:
> >>> cvs rdiff -u -r1.2 -r1.3 \
> >>> src/external/apache2/llvm/config/llvm/Config/config.h.in
> >>>
> >>> Please note that diffs are not public domain; they are subject to the
> >>> copyright notices on the relevant files.
> >>>
> >>
> >> Objected pre-commit? Nothing reached me and I see nothing in logs.
> >>
> >> No report? It was discussed in public syzkaller ML and semipublic IRC
> >> channel with 5 people involved.
> >>
> >> https://syzkaller.appspot.com/text?tag=CrashLog=11aafc09e0
> >>
> >> No analysis? There is analysis in the commit message that we remove
> >> optional features.
> >>
> >> I request either to reintroduce it now or land another fix.
> > 
> > While I am on many mailing lists, I'm certainly not on all lists. I
> > answered in the only forum where I saw the message. I am strongly
> > objecting here because you make the tools compiler a lot less usable.
> > You haven't acutally shown *why* the existing logic doesn't work.
> > 
> 
> The existing logic does not work as it is breaking bootstrap on
> Linux/Ubuntu as presented here:
> 
> https://syzkaller.appspot.com/text?tag=CrashLog=11aafc09e0

Yes and that is completely useless for figuring out the why.

Joerg


Re: CVS commit: src/external/apache2/llvm/config/llvm/Config

2020-02-19 Thread Joerg Sonnenberger
On Wed, Feb 19, 2020 at 04:10:17PM +0100, Kamil Rytarowski wrote:
> On 19.02.2020 14:32, Joerg Sonnenberger wrote:
> > Module Name:src
> > Committed By:   joerg
> > Date:   Wed Feb 19 13:32:40 UTC 2020
> > 
> > Modified Files:
> > src/external/apache2/llvm/config/llvm/Config: config.h.in
> > 
> > Log Message:
> > Revert last. It was objected to pre-commit, there is no actual error
> > analysis or report and there is existing logic supposed to handle this.
> > 
> > 
> > To generate a diff of this commit:
> > cvs rdiff -u -r1.2 -r1.3 \
> > src/external/apache2/llvm/config/llvm/Config/config.h.in
> > 
> > Please note that diffs are not public domain; they are subject to the
> > copyright notices on the relevant files.
> > 
> 
> Objected pre-commit? Nothing reached me and I see nothing in logs.
> 
> No report? It was discussed in public syzkaller ML and semipublic IRC
> channel with 5 people involved.
> 
> https://syzkaller.appspot.com/text?tag=CrashLog=11aafc09e0
> 
> No analysis? There is analysis in the commit message that we remove
> optional features.
> 
> I request either to reintroduce it now or land another fix.

While I am on many mailing lists, I'm certainly not on all lists. I
answered in the only forum where I saw the message. I am strongly
objecting here because you make the tools compiler a lot less usable.
You haven't acutally shown *why* the existing logic doesn't work.

Joerg


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-13 Thread Joerg Sonnenberger
On Thu, Feb 13, 2020 at 02:05:12PM +0100, Kamil Rytarowski wrote:
> On 13.02.2020 00:58, Joerg Sonnenberger wrote:
> > On Mon, Feb 10, 2020 at 04:45:35PM +, Roy Marples wrote:
> >> On 09/02/2020 19:21, Joerg Sonnenberger wrote:
> >>> On Sat, Feb 08, 2020 at 12:17:16PM +, Santhosh Raju wrote:
> >>>> Module Name: src
> >>>> Committed By:fox
> >>>> Date:Sat Feb  8 12:17:16 UTC 2020
> >>>>
> >>>> Modified Files:
> >>>>  src/external/bsd/dhcpcd/dist/src: dhcp.c
> >>>>
> >>>> Log Message:
> >>>> external/bsd/dhcpcd: Fix a -Wconversion warning.
> >>>>
> >>>> Type cast uint16_t to size_t to prevent implicit type conversion.
> >>>
> >>> Seriously? That should not warn and no cast should be used either.
> >>
> >> What fix would you recommend then?
> > 
> > Disable the warning in GCC and fill an upstream PR against it. A
> > conversion from uint16_t to size_t is value preserving by definition of
> > the ISO C platform limits. It should never create a warning.
> > 
> > Joerg
> > 
> 
> If we disable warnings in our core software in non-trivial source file
> for over 4000 lines we will mute more serious issues in a pretty
> sensitive file. We fixed this warning in upstream dhcpcd.

The fix is IMO significantly worse as it actually creates technical
debt. Frankly, I would go so far and say that this should be reverted.
This warning seems to be pretty fundamentally broken by design if the
analysis so far is correct.

Joerg


Re: CVS commit: src/tests/lib/libc/sys

2020-02-13 Thread Joerg Sonnenberger
On Thu, Feb 13, 2020 at 10:50:19AM +0100, Joerg Sonnenberger wrote:
> On Wed, Feb 12, 2020 at 09:53:46PM -0500, Christos Zoulas wrote:
> > Module Name:src
> > Committed By:   christos
> > Date:   Thu Feb 13 02:53:46 UTC 2020
> > 
> > Modified Files:
> > src/tests/lib/libc/sys: t_ptrace_x86_wait.h
> > 
> > Log Message:
> > Turn off optimization on a function which contains constant labels.
> > The optimizer splits it and we end up with 2 copies and duplicate symbols.
> 
> Why not just turn them into local labels instead?

Alternatively, suffixing them with %= would create unique labels.

Joerg


Re: CVS commit: src/tests/lib/libc/sys

2020-02-13 Thread Joerg Sonnenberger
On Wed, Feb 12, 2020 at 09:53:46PM -0500, Christos Zoulas wrote:
> Module Name:  src
> Committed By: christos
> Date: Thu Feb 13 02:53:46 UTC 2020
> 
> Modified Files:
>   src/tests/lib/libc/sys: t_ptrace_x86_wait.h
> 
> Log Message:
> Turn off optimization on a function which contains constant labels.
> The optimizer splits it and we end up with 2 copies and duplicate symbols.

Why not just turn them into local labels instead?

Joerg


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-12 Thread Joerg Sonnenberger
On Thu, Feb 13, 2020 at 02:07:23AM +, Roy Marples wrote:
> On 12/02/2020 23:58, Joerg Sonnenberger wrote:
> > On Mon, Feb 10, 2020 at 04:45:35PM +, Roy Marples wrote:
> > > On 09/02/2020 19:21, Joerg Sonnenberger wrote:
> > > > On Sat, Feb 08, 2020 at 12:17:16PM +, Santhosh Raju wrote:
> > > > > Module Name:  src
> > > > > Committed By: fox
> > > > > Date: Sat Feb  8 12:17:16 UTC 2020
> > > > > 
> > > > > Modified Files:
> > > > >   src/external/bsd/dhcpcd/dist/src: dhcp.c
> > > > > 
> > > > > Log Message:
> > > > > external/bsd/dhcpcd: Fix a -Wconversion warning.
> > > > > 
> > > > > Type cast uint16_t to size_t to prevent implicit type conversion.
> > > > 
> > > > Seriously? That should not warn and no cast should be used either.
> > > 
> > > What fix would you recommend then?
> > 
> > Disable the warning in GCC and fill an upstream PR against it. A
> > conversion from uint16_t to size_t is value preserving by definition of
> > the ISO C platform limits. It should never create a warning.
> 
> I thought this fell under int promotion and thus became signed vs unsigned?

size_t is guaranteed to be at least 16bit. If INT_MAX == 32767, an
implicit cast of uint16_t would go to unsigned anyway and in all other
cases, any implicit cast must be value preserving.

Joerg


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-12 Thread Joerg Sonnenberger
On Mon, Feb 10, 2020 at 04:45:35PM +, Roy Marples wrote:
> On 09/02/2020 19:21, Joerg Sonnenberger wrote:
> > On Sat, Feb 08, 2020 at 12:17:16PM +, Santhosh Raju wrote:
> > > Module Name:  src
> > > Committed By: fox
> > > Date: Sat Feb  8 12:17:16 UTC 2020
> > > 
> > > Modified Files:
> > >   src/external/bsd/dhcpcd/dist/src: dhcp.c
> > > 
> > > Log Message:
> > > external/bsd/dhcpcd: Fix a -Wconversion warning.
> > > 
> > > Type cast uint16_t to size_t to prevent implicit type conversion.
> > 
> > Seriously? That should not warn and no cast should be used either.
> 
> What fix would you recommend then?

Disable the warning in GCC and fill an upstream PR against it. A
conversion from uint16_t to size_t is value preserving by definition of
the ISO C platform limits. It should never create a warning.

Joerg


Re: CVS commit: src/external/bsd/dhcpcd/dist/src

2020-02-09 Thread Joerg Sonnenberger
On Sat, Feb 08, 2020 at 12:17:16PM +, Santhosh Raju wrote:
> Module Name:  src
> Committed By: fox
> Date: Sat Feb  8 12:17:16 UTC 2020
> 
> Modified Files:
>   src/external/bsd/dhcpcd/dist/src: dhcp.c
> 
> Log Message:
> external/bsd/dhcpcd: Fix a -Wconversion warning.
> 
> Type cast uint16_t to size_t to prevent implicit type conversion.

Seriously? That should not warn and no cast should be used either.

Joerg


Re: CVS commit: src/games/battlestar

2020-02-05 Thread Joerg Sonnenberger
On Wed, Feb 05, 2020 at 08:11:54PM +, Santhosh Raju wrote:
> Module Name:  src
> Committed By: fox
> Date: Wed Feb  5 20:11:54 UTC 2020
> 
> Modified Files:
>   src/games/battlestar: parse.c
> 
> Log Message:
> games/battlestar: Fix the -Werror=restrict warning.
> 
> Replace strcpy(1) with the safer snprintf(3) which guarantees NULL
> termination of strings.
> 
> Error was reported when build.sh was run with MKLIBCSANITIZER=yes flag.

This can all use plain memcpy?

Joerg


Re: CVS commit: src/lib/libc/gen

2020-02-03 Thread Joerg Sonnenberger
On Sat, Feb 01, 2020 at 03:38:46PM +, Kamil Rytarowski wrote:
> Module Name:  src
> Committed By: kamil
> Date: Sat Feb  1 15:38:46 UTC 2020
> 
> Modified Files:
>   src/lib/libc/gen: pthread_atfork.c
> 
> Log Message:
> Switch atform allocations from malloc()+free() to mmap()+munmap()
> 
> This avoid bootstrapping malloc too early when libc+libpthread are not
> ready. It is called through pthread__init() -> _pthread_atfork().
> 
> This also helps LLVM Leak Sanitizer to pacify false positive reports.

Can we please stop adding more and more hacks for the still questionable
"new" jemalloc and sit down for a sane init model first please? I'm
still at the point to ask to just revert to the known-to-be-working
version and therefore would also strongly prefer this change to be
reverted.

Joerg


Re: CVS commit: src/external/mit/xorg/lib/gallium

2020-02-03 Thread Joerg Sonnenberger
On Fri, Jan 31, 2020 at 09:13:40PM +, Jared D. McNeill wrote:
> Module Name:  src
> Committed By: jmcneill
> Date: Fri Jan 31 21:13:40 UTC 2020
> 
> Modified Files:
>   src/external/mit/xorg/lib/gallium: Makefile
> 
> Log Message:
> Bump MESA_LLVM_VERSION_STRING

Directly use ${LLVM_VERSION}?

Joerg


Re: CVS commit: src/external/mit/xorg/lib

2020-02-03 Thread Joerg Sonnenberger
On Fri, Jan 31, 2020 at 08:58:18PM +, Jared D. McNeill wrote:
> Module Name:  src
> Committed By: jmcneill
> Date: Fri Jan 31 20:58:18 UTC 2020
> 
> Modified Files:
>   src/external/mit/xorg/lib: libmesa.mk
>   src/external/mit/xorg/lib/libglapi: Makefile
> 
> Log Message:
> Change HAVE_LLVM from 0x0700 to 0x0900.

Can we change this to something like:

NUMERIC_LLVM_VERSION!=  ${LLVM_VERSION:R:R} * 256
-DHAVE_LLVM=${NUMERIC_LLVM_VERSION}

(not bothering with the minor, since it will be 0 anyway).

Joerg


Re: CVS commit: src/sys/arch/x86

2020-01-12 Thread Joerg Sonnenberger
On Sun, Jan 12, 2020 at 01:01:12PM +, Andrew Doran wrote:
> Module Name:  src
> Committed By: ad
> Date: Sun Jan 12 13:01:12 UTC 2020
> 
> Modified Files:
>   src/sys/arch/x86/include: pmap.h pmap_pv.h
>   src/sys/arch/x86/x86: pmap.c vm_machdep.c x86_tlb.c
> 
> Log Message:
> x86 pmap:
> 
> - It turns out that every page the pmap frees is necessarily zeroed.  Tell
>   the VM system about this and use the pmap as a source of pre-zeroed pages.

Would it make sense to actually verify this under DEBUG? I fear the
debugging we have to do if a chance ever violates this assumption.

Joerg


Re: CVS commit: src/usr.bin/crunch/crunchgen

2019-12-23 Thread Joerg Sonnenberger
On Mon, Dec 23, 2019 at 06:54:47PM -, Christos Zoulas wrote:
> Here is a patch that:
> 
> 1. removes all the special handling of variables (-d -p -P -s -S) that
>were dealing with DBG (-d) LDSTATIC/NOPIE (-p), and the rest with
>disabling/enabling sanitizers.
> 2. uses emalloc/estrdup for all the allocators instead of only some cases.
> 3. adds -V varspec which passes variables on the command line (as DBG
>and LDSTATIC used to be passed before) instead of appending them
>to the on-the-fly Makefile using -v varspec.
> 
> The motivation of this is to make variable handling consistent, less magical,
> and remove the need for changing crunchgen each time we want to add disabling
> an option by default.

I like this in principle, but I haven't double checked everything.

Joerg


Re: CVS commit: src/sys

2019-12-21 Thread Joerg Sonnenberger
On Fri, Dec 20, 2019 at 09:05:34PM +, Andrew Doran wrote:
> Module Name:  src
> Committed By: ad
> Date: Fri Dec 20 21:05:34 UTC 2019
> 
> Modified Files:
>   src/sys/arch/aarch64/aarch64: cpu.c cpufunc.c
>   src/sys/arch/arm/arm32: arm32_boot.c cpu.c
>   src/sys/arch/macppc/macppc: cpu.c
>   src/sys/arch/mips/mips: cpu_subr.c
>   src/sys/arch/x86/x86: cpu.c cpu_topology.c identcpu.c
>   src/sys/kern: kern_cpu.c
>   src/sys/sys: cpu.h cpu_data.h sched.h
> 
> Log Message:
> Some more CPU topology stuff:
> 
> - Use cegger@'s ACPI SRAT parsing code to figure out NUMA node ID for each
>   CPU as it is attached.

My build system panics on boot with this due to zero sized allocations
in acpisrat_refresh.

Joerg


Re: CVS commit: src/share/mk

2019-12-19 Thread Joerg Sonnenberger
On Mon, Jan 21, 2019 at 04:11:55PM -0500, Christos Zoulas wrote:
> Module Name:  src
> Committed By: christos
> Date: Mon Jan 21 21:11:55 UTC 2019
> 
> Modified Files:
>   src/share/mk: bsd.dep.mk bsd.hostprog.mk bsd.info.mk bsd.kmodule.mk
>   bsd.lib.mk bsd.man.mk bsd.prog.mk bsd.sys.mk bsd.test.mk bsd.x11.mk
>   sys.mk
> 
> Log Message:
> Most of the mv operations are to move temporary files to their final place.
> Some use -f, others don't. This can lead to spurious build failures when
> the user performing the build changes. Centralize, and always use -f.

At least the sys.mk part of this breaks standalone Makefiles, because MV
is not defined.

Joerg


Re: CVS commit: src/usr.bin/crunch/crunchgen

2019-12-19 Thread Joerg Sonnenberger
On Thu, Dec 19, 2019 at 12:27:23PM -0500, Christos Zoulas wrote:
> On Dec 19,  4:19pm, jo...@bec.de (Joerg Sonnenberger) wrote:
> -- Subject: Re: CVS commit: src/usr.bin/crunch/crunchgen
> 
> | > I think that there are two uses: install media and rescue. I can reinstate
> | > it for rescue. Is that ok? Note that by default both ssp and fortify are
> | > off for most things, so my change was mostly a no-op for the default 
> settings.
> | 
> | We seem to default to off for more things, PIE too for example.
> 
> Indeed, I also dislike the alphabet soup of flags for disabling
> things.  I propose that instead we provide a way to add a preamble
> to the generated Makefile via -f  file, default to a
> either a built-in configuration file, or one installed in the tree.
> It is inappropriate for crunchgen to have knowledge of variables
> specific to our build system that change over time.
> 
> So I propose:
> 
> 1. to make that change [configuration file]
> 2. leave the defaults as they are for the installation binaries
> 3. change the defaults to turn nothing off for the rescue binaries

With the exception of the default, I agree. IMO crunchgen is a general
purpose tool. It is used in different circumstances and shouldn't
dictate whether "normal" use is space constrained or not.

Joerg


Re: CVS commit: src/usr.bin/crunch/crunchgen

2019-12-19 Thread Joerg Sonnenberger
On Thu, Dec 19, 2019 at 01:46:43PM -, Christos Zoulas wrote:
> In article <20191218222723.ga17...@bec.de>,
> Joerg Sonnenberger   wrote:
> >On Wed, Dec 18, 2019 at 03:51:21PM -, Christos Zoulas wrote:
> >> In article <20191218152113.ga7...@bec.de>,
> >> Joerg Sonnenberger   wrote:
> >> >On Tue, Dec 17, 2019 at 09:16:04PM -0500, Christos Zoulas wrote:
> >> >> Module Name:src
> >> >> Committed By:   christos
> >> >> Date:   Wed Dec 18 02:16:04 UTC 2019
> >> >> 
> >> >> Modified Files:
> >> >> src/usr.bin/crunch/crunchgen: crunchgen.1 crunchgen.c
> >> >> 
> >> >> Log Message:
> >> >> Also disable ssp and fortify by default.
> >> >
> >> >Why?
> >> 
> >> Size reduction on install media. There are flags to turn it on.
> >
> >Not all users of crunchgen are space constrained, so this seems like a
> >bad default to me.
> 
> I think that there are two uses: install media and rescue. I can reinstate
> it for rescue. Is that ok? Note that by default both ssp and fortify are
> off for most things, so my change was mostly a no-op for the default settings.

We seem to default to off for more things, PIE too for example.

Joerg


Re: CVS commit: src/usr.bin/crunch/crunchgen

2019-12-18 Thread Joerg Sonnenberger
On Wed, Dec 18, 2019 at 03:51:21PM -, Christos Zoulas wrote:
> In article <20191218152113.ga7...@bec.de>,
> Joerg Sonnenberger   wrote:
> >On Tue, Dec 17, 2019 at 09:16:04PM -0500, Christos Zoulas wrote:
> >> Module Name:   src
> >> Committed By:  christos
> >> Date:  Wed Dec 18 02:16:04 UTC 2019
> >> 
> >> Modified Files:
> >>src/usr.bin/crunch/crunchgen: crunchgen.1 crunchgen.c
> >> 
> >> Log Message:
> >> Also disable ssp and fortify by default.
> >
> >Why?
> 
> Size reduction on install media. There are flags to turn it on.

Not all users of crunchgen are space constrained, so this seems like a
bad default to me.

Joerg


Re: CVS commit: src/usr.bin/crunch/crunchgen

2019-12-18 Thread Joerg Sonnenberger
On Tue, Dec 17, 2019 at 09:16:04PM -0500, Christos Zoulas wrote:
> Module Name:  src
> Committed By: christos
> Date: Wed Dec 18 02:16:04 UTC 2019
> 
> Modified Files:
>   src/usr.bin/crunch/crunchgen: crunchgen.1 crunchgen.c
> 
> Log Message:
> Also disable ssp and fortify by default.

Why?

Joerg


Re: CVS commit: src/external/apache2/llvm

2019-11-29 Thread Joerg Sonnenberger
On Fri, Nov 29, 2019 at 11:28:31AM +, m...@netbsd.org wrote:
> On Thu, Nov 28, 2019 at 11:01:22PM +0000, Joerg Sonnenberger wrote:
> > Log Message:
> > Use -fno-strict-aliasing unconditionally for the cross compiler.
> 
> What's the context for that?

This was meant to be part of the earlier change to build clang and
associated things with it when using GCC. The same problem applies for
the cross-compiler when the host compiler is a new enough GCC.

Joerg


CVS commit: src/external/apache2/llvm

2019-11-28 Thread Joerg Sonnenberger
Module Name:src
Committed By:   joerg
Date:   Thu Nov 28 23:01:22 UTC 2019

Modified Files:
src/external/apache2/llvm: Makefile.inc

Log Message:
Use -fno-strict-aliasing unconditionally for the cross compiler.


To generate a diff of this commit:
cvs rdiff -u -r1.3 -r1.4 src/external/apache2/llvm/Makefile.inc

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/apache2/llvm

2019-11-28 Thread Joerg Sonnenberger
Module Name:src
Committed By:   joerg
Date:   Thu Nov 28 23:01:22 UTC 2019

Modified Files:
src/external/apache2/llvm: Makefile.inc

Log Message:
Use -fno-strict-aliasing unconditionally for the cross compiler.


To generate a diff of this commit:
cvs rdiff -u -r1.3 -r1.4 src/external/apache2/llvm/Makefile.inc

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/apache2/llvm/Makefile.inc
diff -u src/external/apache2/llvm/Makefile.inc:1.3 src/external/apache2/llvm/Makefile.inc:1.4
--- src/external/apache2/llvm/Makefile.inc:1.3	Mon Nov 18 19:54:23 2019
+++ src/external/apache2/llvm/Makefile.inc	Thu Nov 28 23:01:22 2019
@@ -1,4 +1,4 @@
-#	$NetBSD: Makefile.inc,v 1.3 2019/11/18 19:54:23 joerg Exp $
+#	$NetBSD: Makefile.inc,v 1.4 2019/11/28 23:01:22 joerg Exp $
 
 .if !defined(LLVM_TOPLEVEL_MK)
 LLVM_TOPLEVEL_MK=
@@ -28,7 +28,7 @@ CLANG_INCLUDE_CONFIG=	${LLVM_TOOLCONF_OB
 LLVM_INCLUDE_OBJDIR!=	cd ${NETBSDSRCDIR}/tools/llvm-include && ${PRINTOBJDIR}
 LLVM_TOOLCONF_OBJDIR!=	cd ${NETBSDSRCDIR}/tools/llvm && ${PRINTOBJDIR}
 HOST_CPPFLAGS+=	${CPPFLAGS}
-HOST_CXXFLAGS+=	-O2 -fno-rtti -fno-exceptions 
+HOST_CXXFLAGS+=	-O2 -fno-rtti -fno-exceptions -fno-strict-aliasing
 HOST_CPPFLAGS+=	-std=c++14
 LLVM_TARGETS=	x86,powerpc,sparc,aarch64,arm,mips
 .else



CVS commit: src/sys/arch/powerpc/oea

2019-11-27 Thread Joerg Sonnenberger
Module Name:src
Committed By:   joerg
Date:   Wed Nov 27 21:07:32 UTC 2019

Modified Files:
src/sys/arch/powerpc/oea: ofw_autoconf.c

Log Message:
Add a hack for qemu/macppc. OF_finddevice calls will crash depending on
the boot loader and kernel being used. This patch allows using
-prom-env qemu_boot_hack=y to disable the lookup.


To generate a diff of this commit:
cvs rdiff -u -r1.23 -r1.24 src/sys/arch/powerpc/oea/ofw_autoconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/sys/arch/powerpc/oea

2019-11-27 Thread Joerg Sonnenberger
Module Name:src
Committed By:   joerg
Date:   Wed Nov 27 21:07:32 UTC 2019

Modified Files:
src/sys/arch/powerpc/oea: ofw_autoconf.c

Log Message:
Add a hack for qemu/macppc. OF_finddevice calls will crash depending on
the boot loader and kernel being used. This patch allows using
-prom-env qemu_boot_hack=y to disable the lookup.


To generate a diff of this commit:
cvs rdiff -u -r1.23 -r1.24 src/sys/arch/powerpc/oea/ofw_autoconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/arch/powerpc/oea/ofw_autoconf.c
diff -u src/sys/arch/powerpc/oea/ofw_autoconf.c:1.23 src/sys/arch/powerpc/oea/ofw_autoconf.c:1.24
--- src/sys/arch/powerpc/oea/ofw_autoconf.c:1.23	Wed Nov 21 17:54:42 2018
+++ src/sys/arch/powerpc/oea/ofw_autoconf.c	Wed Nov 27 21:07:32 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: ofw_autoconf.c,v 1.23 2018/11/21 17:54:42 scole Exp $ */
+/* $NetBSD: ofw_autoconf.c,v 1.24 2019/11/27 21:07:32 joerg Exp $ */
 /*
  * Copyright (C) 1995, 1996 Wolfgang Solfrank.
  * Copyright (C) 1995, 1996 TooLs GmbH.
@@ -31,7 +31,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: ofw_autoconf.c,v 1.23 2018/11/21 17:54:42 scole Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ofw_autoconf.c,v 1.24 2019/11/27 21:07:32 joerg Exp $");
 
 #ifdef ofppc
 #include "gtpci.h"
@@ -137,10 +137,17 @@ canonicalize_bootpath(void)
 	 *   /pci/mac-io/ata-3@2000/disk@0:0/netbsd.new		(OF-3.x)
 	 */
 	strcpy(cbootpath, bootpath);
-	while ((node = OF_finddevice(cbootpath)) == -1) {
-		if ((p = strrchr(cbootpath, '/')) == NULL)
-			break;
-		*p = '\0';
+
+	if ((node = OF_finddevice("/options")) == -1 ||
+	OF_getprop(node, "qemu_boot_hack", type, sizeof(type) - 1) == -1 ||
+	type[0] != 'y') {
+		while ((node = OF_finddevice(cbootpath)) == -1) {
+			if ((p = strrchr(cbootpath, '/')) == NULL)
+break;
+			*p = '\0';
+		}
+	} else {
+		node = -1;
 	}
 
 	printf("bootpath: %s\n", bootpath);



Re: CVS commit: src/lib/libc/tls

2019-11-22 Thread Joerg Sonnenberger
On Fri, Nov 22, 2019 at 08:53:25AM +0900, Takeshi Nakayama wrote:
> >>> Takeshi Nakayama  wrote
> 
> > >>> Joerg Sonnenberger  wrote
> > 
> > > On Thu, Nov 21, 2019 at 11:06:16PM +, Takeshi Nakayama wrote:
> > > > Module Name:src
> > > > Committed By:   nakayama
> > > > Date:   Thu Nov 21 23:06:16 UTC 2019
> > > > 
> > > > Modified Files:
> > > > src/lib/libc/tls: tls.c
> > > > 
> > > > Log Message:
> > > > Fix PR/54074 and PR/54093 completely.
> > > > 
> > > > More similar to the ld.elf_so logic, it is necessary to align with
> > > > p_align first.  Also, invert the #ifdef condition for consistency.
> > > 
> > > This commit just wastes space without reason.
> > 
> > No, without this commit the TLS offset calculation is mismatch if
> > alignof(max_align_t)) != p_align.
> 
> Ah, this is wrong.
> Correctly, the TLS offset calculation is mismatch with what ld(1)
> expected if p_memsz is not aligned with p_align.

For TLS variant I, it literally just adds padding at the end of the
allocation. For TLS variant II, it is redundant, because the rounding is
already done in with max_align_t. We do not support p_align > malloc
alignment and the patch is certainly nowhere near enough to change that.
It is actively harmful in that it can make people believe that it could
ever work.

Joerg


Re: CVS commit: src/lib/libc/tls

2019-11-21 Thread Joerg Sonnenberger
On Thu, Nov 21, 2019 at 11:06:16PM +, Takeshi Nakayama wrote:
> Module Name:  src
> Committed By: nakayama
> Date: Thu Nov 21 23:06:16 UTC 2019
> 
> Modified Files:
>   src/lib/libc/tls: tls.c
> 
> Log Message:
> Fix PR/54074 and PR/54093 completely.
> 
> More similar to the ld.elf_so logic, it is necessary to align with
> p_align first.  Also, invert the #ifdef condition for consistency.

This commit just wastes space without reason.

Joerg


  1   2   3   4   5   6   7   8   >