Hello!
On Wed, Jan 03, 2024 at 11:57:57PM +0000, 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".
>
> The point I'm trying to make is that ensuring correctness with
> function-like macros is difficult, both because of operator precedence
> and argument reevaluation. Expecting contributors to read the
> definitions of every macro they use becomes more and more cumbersome
> as the codebase expands, especially when some symbols are variably
> macros or functions depending on the state of (even infrequently-used)
> compile-time constants.
Sure, and hence the style.
> All that said, upon further reflection, I think the UB issue is best
> solved outside of ngx_strcpy, where the overhead of an extra check may
> have a performance impact. The following patch is sufficient to
> silence UBSan in my configuration:
I've already pointed you to the Vladimir's patch which contains at
least a dozen of places where the same "UB issue" is reported when
running nginx tests with UBSan. 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.
If we want to follow this approach, we need some way to trace
places where zero length memcpy() can occur. My best guess is
that the only way is to look through all ngx_cpymem() /
ngx_memcpy() / ngx_memmove() / ngx_movemem() calls, as nginx
routinely uses { 0, NULL } as an empty string. Given that there
are 600+ of such calls in the codebase, and at least some need
serious audit to find out if { 0, NULL } can appear in the call,
this is going to be a huge work. And, given that the only
expected effect is theoretical correctness of the code, I doubt it
worth the effort, especially given that the end result will likely
reduce readability of the code.
--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
