Re: Core: Avoid memcpy from NULL

2024-01-24 Thread 洪志道
Hi, Here's a similar ticket in another OSS. https://github.com/bellard/quickjs/issues/225#issuecomment-1908279228 > QuickJS may pass NULL pointers to memcpy with zero size. The C spec tells it is an undefined behavior but most C code do it, so the spec should be fixed instead. On Wed, Jan 24,

Re: Core: Avoid memcpy from NULL

2024-01-24 Thread Maxim Dounin
Hello! On Wed, Jan 24, 2024 at 12:09:02AM +, Ben Kallus wrote: > > As already pointed out previously, there are no known cases > > when memcpy(p, NULL, 0) can result in miscompilation of nginx > > code, ... If you think there are cases when the code can be > > miscompiled in practice, and

Re: Core: Avoid memcpy from NULL

2024-01-23 Thread Ben Kallus
Hi Maxim, > As already pointed out previously, there are no known cases > when memcpy(p, NULL, 0) can result in miscompilation of nginx > code, ... If you think there are cases when the code can be > miscompiled in practice, and not theoretically, please share. There is no such thing as

Re: Core: Avoid memcpy from NULL

2024-01-09 Thread Maxim Dounin
Hello! On Tue, Jan 09, 2024 at 04:18:06PM +, Ben Kallus wrote: > > This demonstrates that your patch > > is clearly insufficient. Further, Vladimir's patch is clearly > > insufficient too, as shown for the another patch in the same > > patch series. > > "Insufficient" only when compared to

Re: Core: Avoid memcpy from NULL

2024-01-09 Thread Ben Kallus
> This demonstrates that your patch > is clearly insufficient. Further, Vladimir's patch is clearly > insufficient too, as shown for the another patch in the same > patch series. "Insufficient" only when compared to a hypothetical perfectly exhaustive patch that requires "huge work," as you put

Re: Core: Avoid memcpy from NULL

2024-01-04 Thread Maxim Dounin
Hello! On Wed, Jan 03, 2024 at 11:57:57PM +, Ben Kallus wrote: > > Still, general style guidelines suggests that the code shouldn't > > be written this way, and the only reason for j++ in the line in > > question is that it mimics corresponding IPv4 code. > > > It's not "just happens". > >

Re: Core: Avoid memcpy from NULL

2024-01-03 Thread Ben Kallus
> Still, general style guidelines suggests that the code shouldn't > be written this way, and the only reason for j++ in the line in > question is that it mimics corresponding IPv4 code. > It's not "just happens". The point I'm trying to make is that ensuring correctness with function-like

Re: Core: Avoid memcpy from NULL

2023-12-31 Thread Maxim Dounin
Hello! On Fri, Dec 29, 2023 at 04:50:36PM +, Ben Kallus wrote: > > Still, -O0 is often used at least during development, and it might > > be unreasonable to introduce extra function calls in basic > > primitives. > > I don't think this is a major cause for concern. It is perfectly >

Re: Core: Avoid memcpy from NULL

2023-12-29 Thread Ben Kallus
> Still, -O0 is often used at least during development, and it might > be unreasonable to introduce extra function calls in basic > primitives. I don't think this is a major cause for concern. It is perfectly reasonable for ngx_memcpy be a wrapper function around memcpy; I think most people would

Re: Core: Avoid memcpy from NULL

2023-12-20 Thread Maxim Dounin
Hello! On Sat, Dec 16, 2023 at 04:26:37PM -0500, Ben Kallus wrote: > > In general macro definitions in nginx are used everywhere for > > efficiency reasons > > Clang inlines short functions with -O1, and GCC does so with -O2 or > -O1 -finline-small-functions. Are there any platforms that Nginx

Re: Core: Avoid memcpy from NULL

2023-12-16 Thread Ben Kallus
> In general macro definitions in nginx are used everywhere for > efficiency reasons Clang inlines short functions with -O1, and GCC does so with -O2 or -O1 -finline-small-functions. Are there any platforms that Nginx needs to support for which short function inlining isn't sufficient to solve

Re: Core: Avoid memcpy from NULL

2023-12-15 Thread Maxim Dounin
Hello! On Fri, Dec 15, 2023 at 03:46:19PM +0100, Dipl. Ing. Sergey Brester via nginx-devel wrote: > Enclosed few thoughts to the subject: > > - since it is very rare situation that one needs only a memcpy without > to know whether previous alloc may fail >(e. g. some of pointers were

Re: Core: Avoid memcpy from NULL

2023-12-15 Thread Ben Kallus
> - rewrite of `ngx_memcpy` define like here: > ``` > + #define ngx_memcpy(dst, src, n) (void) ((n) == 0 ? (dst) : memcpy(dst, > src, n)) > ``` > may introduce a regression or compat issues, e. g. fully functioning codes > like that may become broken hereafter: > ``` >

Re: Core: Avoid memcpy from NULL

2023-12-15 Thread Dipl. Ing. Sergey Brester via nginx-devel
Enclosed few thoughts to the subject: - since it is very rare situation that one needs only a memcpy without to know whether previous alloc may fail (e. g. some of pointers were NULL), me too thinks that the caller should be responsible for the check. So I would not extend ngx_memcpy or

Re: Core: Avoid memcpy from NULL

2023-12-14 Thread Maxim Dounin
Hello! On Wed, Dec 13, 2023 at 11:09:28AM -0500, Ben Kallus wrote: > Nginx executes numerous `memcpy`s from NULL during normal execution. > `memcpy`ing to or from NULL is undefined behavior. Accordingly, some > compilers (gcc -O2) make optimizations that assume `memcpy` arguments > are not NULL.

Core: Avoid memcpy from NULL

2023-12-13 Thread Ben Kallus
Nginx executes numerous `memcpy`s from NULL during normal execution. `memcpy`ing to or from NULL is undefined behavior. Accordingly, some compilers (gcc -O2) make optimizations that assume `memcpy` arguments are not NULL. Nginx with UBSan crashes during startup due to this issue. Consider the