bhyve: Avoid triggering false -Wfree-nonheap-object warnings.
ClosedPublic
Actions

Authored by jhb on Sep 29 2022, 10:40 PM.

Details

Reviewers

markj
corvink

Group Reviewers

bhyve

Commits

rG411ea77fed69: bhyve: Avoid triggering false -Wfree-nonheap-object warnings.
rGb36b14beda4f: bhyve: Avoid triggering false -Wfree-nonheap-object warnings.

Summary

XHCI port and slot numbers are 1-based rather than 0-based. To handle
this, bhyve was subtracting one item from the pointers saved in the
softc so that index 1 accessed index 0 of the allocated array.

However, this is UB and confused GCC 12. The compiler noticed that
the calls to free() were using an offset and emitted a warning.
Rather than storing UB pointers in the softc, push the decrement
operation into the existing macros that wrap accesses to the relevant
arrays.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 48882
Build 45771: arc lint + arc unit

Event Timeline

jhb created this revision.Sep 29 2022, 10:40 PM

Herald added a reviewer: bhyve. · View Herald TranscriptSep 29 2022, 10:40 PM

Herald added subscribers: bcran, rgrimes, imp. · View Herald Transcript

jhb requested review of this revision.Sep 29 2022, 10:40 PM

Harbormaster completed remote builds in B47643: Diff 111249.Sep 29 2022, 10:40 PM

jhb added a parent revision: D36828: bhyve: Don't free an invalid pointer..Sep 29 2022, 10:40 PM

Can we avoid the UB by simply allocating an extra slot in each array? This trick is saving 16 bytes of memory per xhci instance...

This revision is now accepted and ready to land.Sep 30 2022, 2:41 PM

In D36829#836024, @markj wrote:

Can we avoid the UB by simply allocating an extra slot in each array? This trick is saving 16 bytes of memory per xhci instance...

Well, this is even safe on CHERI (too much C uses tricks like this). But we could allocate an extra one perhaps and just leave index 0 unused. We could also perhaps just adjust the references, especially if they are all wrapped via macros like XHCI_DEVINST_PTR() which could deal with subtracting one from the index.

jhb added a parent revision: D36888: bhyve: Use XHCI_PORTREG_PTR in one place that open-coded it..Oct 5 2022, 5:30 PM

Adjust indices on use

This revision now requires review to proceed.Dec 8 2022, 11:52 PM

Harbormaster completed remote builds in B48657: Diff 113957.Dec 8 2022, 11:52 PM

jhb edited the summary of this revision. (Show Details)Dec 8 2022, 11:53 PM

corvink accepted this revision.Dec 9 2022, 6:51 AM

This revision is now accepted and ready to land.Dec 9 2022, 6:51 AM

markj added inline comments.Dec 9 2022, 4:16 PM

usr.sbin/bhyve/pci_xhci.c
294	These should technically have parens around the expansion.
2833–2834	We're still using the old trick for portregs - probably it's better to be consistent one way or the other?