lib: OMGWTFBBQ × 2 #4740

eqvinox · 2019-07-28T07:32:26Z

prefix_cmp() isn't like strcmp()/memcmp() 💣💥
a misplaced ) broke some of the typesafe lists 💣💥

At least I'm only responsible for one of these 😞

OMGWTFBBQ Signed-off-by: David Lamparter <equinox@diac24.net>

... so we can pass prefix_ipv4/prefix_ipv6 in. Signed-off-by: David Lamparter <equinox@diac24.net>

OMGWTFBBQ no. 2... Signed-off-by: David Lamparter <equinox@diac24.net>

eqvinox · 2019-07-28T07:35:43Z

Sidenote: the typesafe.h one didn't break anything; the SORTLIST structure isn't used anywhere yet except in unit tests.

the prefix_cmp one however cascades into our route_table hash and may be actively causing breakage

LabN-CI · 2019-07-28T08:26:59Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/4740 `4d5cf6b`
Date	07/28/2019
Start	04:05:22
Finish	04:26:58
Run-Time	21:36
Total	1815
Pass	1815
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-07-28-04:05:22.txt
Log	autoscript-2019-07-28-04:06:08.log.bz2
Memory	432 432 360

For details, please contact louberger

NetDEF-CI · 2019-07-28T09:32:58Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8456/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotest tests on Ubuntu 16.04 i386: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOI386-8456/test

Topology Tests failed for Topotest tests on Ubuntu 16.04 i386:

RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
2019-07-28 01:08:00,331 WARNING: BGP is not yet Converged for router r4
2019-07-28 01:08:19,117 WARNING: BGP is not yet Converged for router r1 after bgp clear
2019-07-28 01:15:26,265 WARNING: BGP is not yet Converged for router r1 after bgp clear
2019-07-28 01:21:52,521 ERROR: '_verify_rib' failed after 323.54 seconds
*** defaultIntf: warning: r1 has no interfaces

r3: zebra crashed. Core file found - Backtrace follows:
[New LWP 18228]
[New LWP 18229]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/i386-linux-gnu/libthread_db.so.1".
Core was generated by `/usr/lib/frr/zebra'.
Program terminated with signal SIGABRT, Aborted.
#0  0xb7efcd05 in ?? ()
[Current thread is 1 (Thread 0xb78517c0 (LWP 18228))]
#0  0xb7efcd05 in ?? ()
#1  <signal handler called>
#2  0xb7efcd05 in ?? ()
#3  0xb7e60f2e in frr_run () from /usr/lib/i386-linux-gnu/frr/libfrr.so.0
#4  0x00437734 in main ()
2019-07-28 01:31:02,678 ERROR: assert failed at "bgp_l3vpn_to_bgp_direct.test_bgp_l3vpn_to_bgp_direct/test_memory_leak": 
r3: zebra crashed. Core file found - Backtrace follows:
[New LWP 18228]
[New LWP 18229]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/i386-linux-gnu/libthread_db.so.1".
Core was generated by `/usr/lib/frr/zebra'.
Program terminated with signal SIGABRT, Aborted.
#0  0xb7efcd05 in ?? ()
[Current thread is 1 (Thread 0xb78517c0 (LWP 18228))]
#0  0xb7efcd05 in ?? ()
#1  <signal handler called>
#2  0xb7efcd05 in ?? ()
#3  0xb7e60f2e in frr_run () from /usr/lib/i386-linux-gnu/frr/libfrr.so.0
#4  0x00437734 in main ()

RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument

see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8456/artifact/TOPOI386/ErrorLog/log_topotests.txt

Successful on other platforms

Ubuntu 14.04 deb pkg check
IPv4 ldp protocol on Ubuntu 16.04
Debian 8 deb pkg check
Addresssanitizer topotest
IPv4 protocols on Ubuntu 14.04
Topology tests on Ubuntu 16.04 amd64
Debian 9 deb pkg check
CentOS 7 rpm pkg check
Debian 10 deb pkg check
Static analyzer (clang)
Ubuntu 16.04 deb pkg check
IPv6 protocols on Ubuntu 14.04
Topology tests on Ubuntu 18.04 amd64
Fedora 29 rpm pkg check
Ubuntu 12.04 deb pkg check

Topology Tests memory analysis: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8456/artifact/TOPOI386/MemoryLeaks/

Warnings Generated during build:

Checkout code: Successful with additional warnings

Topotest tests on Ubuntu 16.04 i386: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOI386-8456/test

Topology Tests failed for Topotest tests on Ubuntu 16.04 i386:

RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
2019-07-28 01:08:00,331 WARNING: BGP is not yet Converged for router r4
2019-07-28 01:08:19,117 WARNING: BGP is not yet Converged for router r1 after bgp clear
2019-07-28 01:15:26,265 WARNING: BGP is not yet Converged for router r1 after bgp clear
2019-07-28 01:21:52,521 ERROR: '_verify_rib' failed after 323.54 seconds
*** defaultIntf: warning: r1 has no interfaces

r3: zebra crashed. Core file found - Backtrace follows:
[New LWP 18228]
[New LWP 18229]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/i386-linux-gnu/libthread_db.so.1".
Core was generated by `/usr/lib/frr/zebra'.
Program terminated with signal SIGABRT, Aborted.
#0  0xb7efcd05 in ?? ()
[Current thread is 1 (Thread 0xb78517c0 (LWP 18228))]
#0  0xb7efcd05 in ?? ()
#1  <signal handler called>
#2  0xb7efcd05 in ?? ()
#3  0xb7e60f2e in frr_run () from /usr/lib/i386-linux-gnu/frr/libfrr.so.0
#4  0x00437734 in main ()
2019-07-28 01:31:02,678 ERROR: assert failed at "bgp_l3vpn_to_bgp_direct.test_bgp_l3vpn_to_bgp_direct/test_memory_leak": 
r3: zebra crashed. Core file found - Backtrace follows:
[New LWP 18228]
[New LWP 18229]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/i386-linux-gnu/libthread_db.so.1".
Core was generated by `/usr/lib/frr/zebra'.
Program terminated with signal SIGABRT, Aborted.
#0  0xb7efcd05 in ?? ()
[Current thread is 1 (Thread 0xb78517c0 (LWP 18228))]
#0  0xb7efcd05 in ?? ()
#1  <signal handler called>
#2  0xb7efcd05 in ?? ()
#3  0xb7e60f2e in frr_run () from /usr/lib/i386-linux-gnu/frr/libfrr.so.0
#4  0x00437734 in main ()

RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument
RTNETLINK answers: Invalid argument

see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8456/artifact/TOPOI386/ErrorLog/log_topotests.txt

Report for prefix.h | 12 issues
===============================================
< WARNING: function definition argument 'union prefixconstptr' should also have an identifier name
< #413: FILE: /tmp/f1-20426/prefix.h:413:
< WARNING: function definition argument 'union prefixconstptr' should also have an identifier name
< #413: FILE: /tmp/f1-20426/prefix.h:413:
< WARNING: function definition argument 'union prefixconstptr' should also have an identifier name
< #414: FILE: /tmp/f1-20426/prefix.h:414:
< WARNING: function definition argument 'union prefixconstptr' should also have an identifier name
< #414: FILE: /tmp/f1-20426/prefix.h:414:
< WARNING: function definition argument 'union prefixptr' should also have an identifier name
< #416: FILE: /tmp/f1-20426/prefix.h:416:
< WARNING: function definition argument 'union prefixconstptr' should also have an identifier name
< #416: FILE: /tmp/f1-20426/prefix.h:416:

Warnings Generated during build:

Debian 10 amd64 build: Successful with additional warnings

Debian Package lintian failed for Debian 10 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8456/artifact/DEB10BUILD/ErrorLog/log_lintian.txt)

W: frr source: changelog-should-mention-nmu

CLANG Static Analyzer Summary

Github Pull Request 4740, comparing to Git base SHA 51e75ed

Fixed warnings:

Logic error: Dereference of null pointer in nhrpd.h, function notifier_call, line 79

New warnings:

Logic error: Assigned value is garbage or undefined in bgpd/bgp_attr_evpn.c, function bgp_build_evpn_prefix, line 266
- See https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8456/artifact/shared/static_analysis/report-f2df8f.html#EndPath
Logic error: Dereference of null pointer in nhrpd/nhrpd.h, function notifier_call, line 79
- See https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8456/artifact/shared/static_analysis/report-cd2c4f.html#EndPath
Logic error: Assigned value is garbage or undefined in pimd/pim_nht.c, function pim_nexthop_cache_find, line 78
- See https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8456/artifact/shared/static_analysis/report-55223c.html#EndPath

Static Analysis warning summary compared to base:

Fixed warnings: 1
New warnings: 3

3 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8456/artifact/shared/static_analysis/index.html

eqvinox · 2019-07-28T18:34:36Z

CI:rerun

NetDEF-CI · 2019-07-28T20:29:48Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8458/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Checkout code: Successful with additional warnings

Report for prefix.h | 12 issues
===============================================
< WARNING: function definition argument 'union prefixconstptr' should also have an identifier name
< #413: FILE: /tmp/f1-10197/prefix.h:413:
< WARNING: function definition argument 'union prefixconstptr' should also have an identifier name
< #413: FILE: /tmp/f1-10197/prefix.h:413:
< WARNING: function definition argument 'union prefixconstptr' should also have an identifier name
< #414: FILE: /tmp/f1-10197/prefix.h:414:
< WARNING: function definition argument 'union prefixconstptr' should also have an identifier name
< #414: FILE: /tmp/f1-10197/prefix.h:414:
< WARNING: function definition argument 'union prefixptr' should also have an identifier name
< #416: FILE: /tmp/f1-10197/prefix.h:416:
< WARNING: function definition argument 'union prefixconstptr' should also have an identifier name
< #416: FILE: /tmp/f1-10197/prefix.h:416:

Warnings Generated during build:

Debian 10 amd64 build: Successful with additional warnings

Debian Package lintian failed for Debian 10 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8458/artifact/DEB10BUILD/ErrorLog/log_lintian.txt)

W: frr source: changelog-should-mention-nmu

CLANG Static Analyzer Summary

Github Pull Request 4740, comparing to Git base SHA 51e75ed

Fixed warnings:

Logic error: Dereference of null pointer in nhrpd.h, function notifier_call, line 79

New warnings:

Logic error: Assigned value is garbage or undefined in bgpd/bgp_attr_evpn.c, function bgp_build_evpn_prefix, line 266
- See https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8458/artifact/shared/static_analysis/report-716d3f.html#EndPath
Logic error: Dereference of null pointer in nhrpd/nhrpd.h, function notifier_call, line 79
- See https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8458/artifact/shared/static_analysis/report-8ac016.html#EndPath
Logic error: Assigned value is garbage or undefined in pimd/pim_nht.c, function pim_nexthop_cache_find, line 78
- See https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8458/artifact/shared/static_analysis/report-fa98bd.html#EndPath

Static Analysis warning summary compared to base:

Fixed warnings: 1
New warnings: 3

3 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8458/artifact/shared/static_analysis/index.html

donaldsharp

So I ran scan-build against master and this PR and am seeing the same new warnings. If I was a guessing man I would guess that the changes for prefix_copy and prefix_same in the second commit is causing the changes in the scan. I'd like to understand/cleanup these new SA issues before this goes in.

https://bugs.llvm.org/show_bug.cgi?id=42811 Signed-off-by: David Lamparter <equinox@diac24.net>

LabN-CI · 2019-07-29T15:15:50Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/4740 `4937287`
Date	07/29/2019
Start	10:54:07
Finish	11:15:49
Run-Time	21:42
Total	1815
Pass	1815
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-07-29-10:54:07.txt
Log	autoscript-2019-07-29-10:54:59.log.bz2
Memory	436 426 361

For details, please contact louberger

NetDEF-CI · 2019-07-29T16:12:06Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8464/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Checkout code: Successful with additional warnings

Report for prefix.h | 12 issues
===============================================
< WARNING: function definition argument 'union prefixconstptr' should also have an identifier name
< #413: FILE: /tmp/f1-13013/prefix.h:413:
< WARNING: function definition argument 'union prefixconstptr' should also have an identifier name
< #413: FILE: /tmp/f1-13013/prefix.h:413:
< WARNING: function definition argument 'union prefixconstptr' should also have an identifier name
< #414: FILE: /tmp/f1-13013/prefix.h:414:
< WARNING: function definition argument 'union prefixconstptr' should also have an identifier name
< #414: FILE: /tmp/f1-13013/prefix.h:414:
< WARNING: function definition argument 'union prefixptr' should also have an identifier name
< #416: FILE: /tmp/f1-13013/prefix.h:416:
< WARNING: function definition argument 'union prefixconstptr' should also have an identifier name
< #416: FILE: /tmp/f1-13013/prefix.h:416:

Warnings Generated during build:

Debian 10 amd64 build: Successful with additional warnings

Debian Package lintian failed for Debian 10 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8464/artifact/DEB10BUILD/ErrorLog/log_lintian.txt)

W: frr source: changelog-should-mention-nmu

CLANG Static Analyzer Summary

Github Pull Request 4740, comparing to Git base SHA 51e75ed

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8464/artifact/shared/static_analysis/index.html

eqvinox added 3 commits July 28, 2019 09:25

lib: fix prefix_cmp() return values

1315d74

OMGWTFBBQ Signed-off-by: David Lamparter <equinox@diac24.net>

lib: use some more transparent unions for prefixes

9c3a217

... so we can pass prefix_ipv4/prefix_ipv6 in. Signed-off-by: David Lamparter <equinox@diac24.net>

lib: fix misplaced brace in typesafe lists

4d5cf6b

OMGWTFBBQ no. 2... Signed-off-by: David Lamparter <equinox@diac24.net>

polychaeta added the libfrr label Jul 28, 2019

donaldsharp requested changes Jul 29, 2019

View reviewed changes

lib: fix prefix_copy() for clang-SA

4937287

https://bugs.llvm.org/show_bug.cgi?id=42811 Signed-off-by: David Lamparter <equinox@diac24.net>

eqvinox requested a review from donaldsharp July 29, 2019 13:51

donaldsharp merged commit a6ffcbd into FRRouting:master Jul 29, 2019

eqvinox deleted the omgwtfbbq branch July 31, 2019 00:57

donaldsharp mentioned this pull request Sep 24, 2019

BGP VNC is crashing in our Address Sanitizer tests #5025

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib: OMGWTFBBQ × 2 #4740

lib: OMGWTFBBQ × 2 #4740

eqvinox commented Jul 28, 2019

eqvinox commented Jul 28, 2019

LabN-CI commented Jul 28, 2019

NetDEF-CI commented Jul 28, 2019

eqvinox commented Jul 28, 2019

NetDEF-CI commented Jul 28, 2019

donaldsharp left a comment

LabN-CI commented Jul 29, 2019

NetDEF-CI commented Jul 29, 2019

lib: OMGWTFBBQ × 2 #4740

lib: OMGWTFBBQ × 2 #4740

Conversation

eqvinox commented Jul 28, 2019

eqvinox commented Jul 28, 2019

LabN-CI commented Jul 28, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Jul 28, 2019

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Warnings Generated during build:

Warnings Generated during build:

CLANG Static Analyzer Summary

Fixed warnings:

New warnings:

Static Analysis warning summary compared to base:

3 Static Analyzer issues remaining.

eqvinox commented Jul 28, 2019

NetDEF-CI commented Jul 28, 2019

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

Warnings Generated during build:

CLANG Static Analyzer Summary

Fixed warnings:

New warnings:

Static Analysis warning summary compared to base:

3 Static Analyzer issues remaining.

donaldsharp left a comment

Choose a reason for hiding this comment

LabN-CI commented Jul 29, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Jul 29, 2019

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

Warnings Generated during build:

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.