On 1/24/22 19:40, Adrian Moreno wrote: > > > On 1/24/22 18:49, Jeffrey Walton wrote: >> On Mon, Jan 24, 2022 at 9:17 AM Dumitru Ceara <[email protected]> wrote: >>> >>> As privately reported by Aaron Conole, and by Jeffrey Walton [0] >>> there's currently a number of undefined behavior instances in >>> the OVS code base. Running the OVS (and OVN) tests with UBSan [1] >>> enabled uncovers these. >>> ... >> >>> Note: depending on the order of reviews, if Adrian's "Fix undefined >>> behavior in loop macros" series [2] (or a follow up) is accepted first, >>> then patch 12/14 ("util: Avoid false positive UB when iterating >>> collections.") can be skipped. Adrian's series seems to be the more >>> correct way of fixing the issue. >> >> One small nit... UBsan (and Asan) do not produce false positives. They >> operate on real data, and when they produce a finding it is valid. >> That's also why a complete set of self tests are important. The >> complete set of tests are important because UBsan and Asan need real >> data. >> > > I agree, it's not a false positive. Furthermore, the patch that Dumitru > is referring to ("util: Avoid false positive UB when iterating > collections") reduces UBsan's sensitivity by changing some pointer > arithmetics to integer arithmetics. This silences the UBsan but > according to the discussion with the compiler folks [1], this can still > yield UB. > > Therefore, I think it would be safer to keep the pointer arithmetics > (and UBsan's high-sensitivity), fix the actual callers (i.e: the > iterator macros), and run UBsan in the CI to spot all future errors > (which Dumitru's series does). > > [1]https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103964 >
Yes, I probably should've rephrased the commit title of patch 12/14. It's not a false positive. I just kept it for now until Adrian's series [2] is merged. Otherwise jobs would've failed in CI, and it's technically not worse than before. But I completely agree, once Adrian's changes get accepted, the safest way is to keep the pointer arithmetic and rely on UBSan to complain if undefined behavior is detected. [2] https://patchwork.ozlabs.org/project/openvswitch/list/?series=282481&state=* _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
