Thanks for the patch. Anyway your code reads much more easily than the
original. [Jenkins is screwed up at the moment, Dave Wallace is working on it.]
We could do something like this to track down min_log2(0) calls:
#if defined (count_leading_zeros)
always_inline uword
min_log2 (uword x)
{
uword n;
#if CLIB_DEBUG > 0
if (x == 0) abort();
#endif
n = count_leading_zeros (x);
return BITS (uword) - n - 1;
}
#else
... obvious variation ...
#undef _
From: vpp-dev@lists.fd.io <vpp-dev@lists.fd.io> On Behalf Of Andreas Schultz
Sent: Tuesday, May 12, 2020 8:51 AM
To: Dave Barach (dbarach) <dbar...@cisco.com>
Cc: vpp-dev <vpp-dev@lists.fd.io>
Subject: Re: [vpp-dev] min_log2 abuse
Am Di., 12. Mai 2020 um 13:17 Uhr schrieb Dave Barach (dbarach)
<dbar...@cisco.com<mailto:dbar...@cisco.com>>:
Dear Andreas,
Do you have a handy list of places which convert netmasks to lengths?
Regardless of what one might do with min_log2, we ought to clean up those
places in time for the 20.05 release (if possible).
Unfortunately not.
This is the code that caused problems for us:
https://gerrit.fd.io/r/c/vpp/+/27016
Grepping through the code shows additional places that appear to have the
potential to pass a 0 to one of the log2 functions. But we haven't investigated
any of them closer.
I was trying to add an ASSERT (x != 0) to the log2 functions. But those
functions are defined in clib.h, and that header is required by
error_bootstrap.h that defines ASSERTs. The resulting include dependency loop
is only solvable by moving the log2 functions out of clib.h
Andreas
Dave
From: vpp-dev@lists.fd.io<mailto:vpp-dev@lists.fd.io>
<vpp-dev@lists.fd.io<mailto:vpp-dev@lists.fd.io>> On Behalf Of Andreas Schultz
Sent: Tuesday, May 12, 2020 5:42 AM
To: vpp-dev <vpp-dev@lists.fd.io<mailto:vpp-dev@lists.fd.io>>
Subject: [vpp-dev] min_log2 abuse
Hi,
There are few places in VPP (most notable when trying to convert a netmask into
a length) that can pass 0 (zero) into min_log2 and expect to get a meaningful
result.
Obviously log2(0) is undefined. It turns out that this also applies to the
return of min_log2. Under the hood the function uses __builtin_clzl, and the
return of that function is also undefined for input 0.
__builtin_clzl could be replaced with __builtin_ia32_lzcnt_u64 on supported
CPUs to avoid the undefined behaviour. That would still not fix the problem
that passing 0 into a log function is broken by design.
Any comments?
Andreas
--
Andreas Schultz
--
Andreas Schultz
--
Principal Engineer
t: +49 391 819099-224
------------------------------- enabling your networks
-----------------------------
Travelping GmbH
Roentgenstraße 13
39108 Magdeburg
Germany
t: +49 391 819099-0
f: +49 391 819099-299
e: i...@travelping.com<mailto:i...@travelping.com>
w: https://www.travelping.com/
Company registration: Amtsgericht Stendal
Geschaeftsfuehrer: Holger Winkelmann
Reg. No.: HRB 10578
VAT ID: DE236673780
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#16341): https://lists.fd.io/g/vpp-dev/message/16341
Mute This Topic: https://lists.fd.io/mt/74155196/21656
Group Owner: vpp-dev+ow...@lists.fd.io
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
