Re: CVS commit: src/sys

2020-09-11 Thread Kamil Rytarowski
On 11.09.2020 17:16, Roy Marples wrote:
> Module Name:  src
> Committed By: roy
> Date: Fri Sep 11 15:16:00 UTC 2020
> 
> Modified Files:
>   src/sys/net: if_llatbl.c
>   src/sys/netinet: if_arp.c if_inarp.h tcp_input.c
> 
> Log Message:
> ARP: Use ND rather than our own.
> 
> This brings the benefit of Neighbour Unreachability Detection which is
> something ARP sorely lacks.
> 
> The new timings mirror those of IPv6 and are adjustable via sysctl(8).
> Unlike IPv6 ND, these are global and not per interface.
> 
> 
This change broke rump:

/public/netbsd-9/tooldir.NetBSD-9.99.72-amd64/lib/gcc/x86_64--netbsd/9.3.0/../../../../x86_64--netbsd/bin/ld:
/public/netbsd-9/destdir.amd64/usr/lib/librumpnet_net.so: undefined
reference to `rumpns_nd_set_timer'
/public/netbsd-9/tooldir.NetBSD-9.99.72-amd64/lib/gcc/x86_64--netbsd/9.3.0/../../../../x86_64--netbsd/bin/ld:
/public/netbsd-9/destdir.amd64/usr/lib/librumpnet_net.so: undefined
reference to `rumpns_nd_resolve'
/public/netbsd-9/tooldir.NetBSD-9.99.72-amd64/lib/gcc/x86_64--netbsd/9.3.0/../../../../x86_64--netbsd/bin/ld:
/public/netbsd-9/destdir.amd64/usr/lib/librumpnet_net.so: undefined
reference to `rumpns_nd_nud_hint'
/public/netbsd-9/tooldir.NetBSD-9.99.72-amd64/lib/gcc/x86_64--netbsd/9.3.0/../../../../x86_64--netbsd/bin/ld:
/public/netbsd-9/destdir.amd64/usr/lib/librumpnet_net.so: undefined
reference to `rumpns_nd_attach_domain'
collect2: error: ld returned 1 exit status



signature.asc
Description: OpenPGP digital signature


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

2020-09-11 Thread Kamil Rytarowski
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

And then, using _rtld_tls_self() in sanitizers (and wherever someone
finds it useful)?

As an alternative we will use __lwp_gettcb_fast() or
__lwp_getprivate_fast() manually in 3rd party code, which seems fragile.

> Joerg
> 




signature.asc
Description: OpenPGP digital signature


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/external/gpl3/gcc

2020-09-11 Thread Kamil Rytarowski
On 11.09.2020 07:13, Rin Okuyama wrote:
> Hi again,
> 
> On 2020/09/10 21:53, Kamil Rytarowski wrote:
>> Module Name:    src
>> Committed By:    kamil
>> Date:    Thu Sep 10 12:53:06 UTC 2020
>>
>> Modified Files:
>> src/external/gpl3/gcc/dist/libsanitizer/sanitizer_common:
>>     sanitizer_linux_libcdep.cc
>> src/external/gpl3/gcc/lib: Makefile.sanitizer
>>
>> Log Message:
>> Avoid using internal RTLD/libpthread/libc symbol in sanitizers
>>
> ...
>> Index:
>> src/external/gpl3/gcc/dist/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc
>>
>> diff -u
>> src/external/gpl3/gcc/dist/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc:1.15
>> src/external/gpl3/gcc/dist/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc:1.16
>>
>> ---
>> src/external/gpl3/gcc/dist/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc:1.15
>>    
>> Mon Sep  7 07:10:43 2020
>> +++
>> src/external/gpl3/gcc/dist/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc
>>    
>> Thu Sep 10 12:53:05 2020
>> @@ -47,6 +47,7 @@
>>   #if SANITIZER_NETBSD
>>   #include 
>>   #include 
>> +#include 
>>   #endif
>>    #if SANITIZER_SOLARIS
>> @@ -417,13 +418,7 @@ uptr ThreadSelf() {
>>    #if SANITIZER_NETBSD
>>   static struct tls_tcb * ThreadSelfTlsTcb() {
>> -  struct tls_tcb * tcb = NULL;
>> -# ifdef __HAVE___LWP_GETTCB_FAST
>> -  tcb = (struct tls_tcb *)__lwp_gettcb_fast();
>> -# elif defined(__HAVE___LWP_GETPRIVATE_FAST)
>> -  tcb = (struct tls_tcb *)__lwp_getprivate_fast();
>> -# endif
>> -  return tcb;
>> +  return (struct tls_tcb *)_lwp_getprivate();
>>   }
>>    uptr ThreadSelf() {
>>
> 
> This change breaks at least mips and powerpc, in which the return value of
> __lwp_getprivate(2), i.e., curlwp->l_private is not tcb address itself, but
> biased one. On the other hand, the return value of __lwp_gettcb_fast() is
> unbiased address; see sys/arch/{mips,powerpc}/include/mcontext.h.
> 
> For powerpc, I recently attempted to change l_private to store tcb address
> itself:
> 
> http://www.nerv.org/netbsd/?q=id:20200621T004000Z.95c1a18070b53713ce2c763df7f40743bf74172c
> 
> 
> But I reverted it soon as requested by joerg:
> 
> http://www.nerv.org/netbsd/?q=id:20200622T053457Z.05db3be87b5ad499f5d1adba755bc573fd241c87
> 
> 
> His reasoning was that kernel must not know the ABI details in userland.
> I fully agree with this. See above links for more details.
> 
> Thanks,
> rin

Thank you for noting it!

This is strange as I assumed that _lwp_getprivate() returns always the
correct private pointer and it is abstraction over fast ABI specific
calls . Also the usage of _lwp_getprivate() was suggested by Joerg back
then in sanitizers.

So we want exported to userland functionality to get the tls_tcb
pointer, something without using the internal RTLS/LIBPTHREAD/LIBC
namespaces.

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.

https://nxr.netbsd.org/xref/src/lib/libpthread/pthread_int.h#266

263 static inline pthread_t __constfunc
264 pthread__self(void)
265 {
266 #if defined(_PTHREAD_GETTCB_EXT)
267 struct tls_tcb * const tcb = _PTHREAD_GETTCB_EXT();
268 #elif defined(__HAVE___LWP_GETTCB_FAST)
269 struct tls_tcb * const tcb = __lwp_gettcb_fast();
270 #else
271 struct tls_tcb * const tcb = __lwp_getprivate_fast();
272 #endif
273 return (pthread_t)tcb->tcb_pthread;
274 }

https://nxr.netbsd.org/xref/src/lib/libpthread/pthread.c#1268

   1268 #if defined(_PTHREAD_GETTCB_EXT)
   1269 pthread__main->pt_tls = _PTHREAD_GETTCB_EXT();
   1270 #elif defined(__HAVE___LWP_GETTCB_FAST)
   1271 pthread__main->pt_tls = __lwp_gettcb_fast();
   1272 #else
   1273 pthread__main->pt_tls = _lwp_getprivate();
   1274 #endif
   1275 pthread__main->pt_tls->tcb_pthread = pthread__main;

https://nxr.netbsd.org/xref/src/libexec/ld.elf_so/tls.c#294

293 #ifdef __HAVE___LWP_GETTCB_FAST
294 struct tls_tcb * const tcb = __lwp_gettcb_fast();
295 #else
296 struct tls_tcb * const tcb = __lwp_getprivate_fast();
297 #endif


1. Could we please synchronize above three code chunks, avoiding the
situation of having each of them implemented differently?

2. Could we please export _rtld_tls_self() or something similar and
register in  ?

Does this patch look good?

https://www.netbsd.org/~kamil/patch-00278-_rtld_tls_self.txt


In the worst case I will need to reexpose internal APIs in sanitizers
and pick one of the above tls_tcb retrieval implementations and use in
LLVM/GCC sanitizers.

PS. There is an ongoing GCC and Linux kernel discussion on a related
topic in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96200 "Implement
__builtin_thread_pointer() and