Page MenuHomeFreeBSD

Fix acpica macros that subtract null pointers
ClosedPublic

Authored by dim on Aug 29 2021, 11:16 AM.
Tags
None
Referenced Files
F80144670: D31710.id94328.diff
Thu, Mar 28, 1:08 PM
Unknown Object (File)
Feb 17 2024, 6:13 PM
Unknown Object (File)
Dec 8 2023, 4:54 PM
Unknown Object (File)
Nov 1 2023, 6:28 PM
Unknown Object (File)
Sep 12 2023, 12:16 PM
Unknown Object (File)
Aug 26 2023, 10:31 PM
Unknown Object (File)
Aug 19 2023, 4:58 PM
Unknown Object (File)
Aug 16 2023, 8:28 AM
Subscribers

Details

Summary

Clang 13.0.0 produces a new -Werror warning about the ACPI_TO_INTEGER(p)
and ACPI_OFFSET(d, f) macros in acpica's actypes.h:

sys/contrib/dev/acpica/components/dispatcher/dsopcode.c:708:31: error: performing pointer subtraction with a null pointer has undefined behavior [-Werror,-Wnull-pointer-subtraction]
    ObjDesc->Region.Address = ACPI_PTR_TO_PHYSADDR (Table);
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
sys/contrib/dev/acpica/include/actypes.h:664:41: note: expanded from macro 'ACPI_PTR_TO_PHYSADDR'
#define ACPI_PTR_TO_PHYSADDR(i)         ACPI_TO_INTEGER(i)
                                        ^~~~~~~~~~~~~~~~~~
sys/contrib/dev/acpica/include/actypes.h:661:41: note: expanded from macro 'ACPI_TO_INTEGER'
#define ACPI_TO_INTEGER(p)              ACPI_PTR_DIFF (p, (void *) 0)
                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sys/contrib/dev/acpica/include/actypes.h:656:82: note: expanded from macro 'ACPI_PTR_DIFF'
#define ACPI_PTR_DIFF(a, b)             ((ACPI_SIZE) (ACPI_CAST_PTR (UINT8, (a)) - ACPI_CAST_PTR (UINT8, (b))))
                                                                                 ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

This problem of undefined behavior was also reported to acpica by @cem
in 2018: https://github.com/acpica/acpica/issues/407, but it seems there
was never any fix committed for it upstream.

Instead fix these locally, for ACPI_TO_INTEGER by simply casting the
incoming pointer to ACPI_SIZE (which corresponds roughly to uintptr_t
and size_t), and for ACPI_OFFSET by reusing our offsetof definition from
sys/cdefs.h.

I didn't want to use our offsetof() or builtin_offsetof(), as this
might not be acceptable for upstream. But if upstream is not going to
accept this patch anyway, we might as well just use or own macros.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 41249
Build 38138: arc lint + arc unit

Event Timeline

dim requested review of this revision.Aug 29 2021, 11:16 AM

Until something is fixed upstream, it might be better to use #ifdef __FreeBSD block, and then using our sys/cdefs.h facilities for FreeBSD would be both safer and better explain the intent.

Just my PoV, the last call is the ACPI maintainer' word of course.

I'm good with this or kib's suggestion

This revision is now accepted and ready to land.Aug 29 2021, 5:07 PM

Use conditional FreeBSD block, and our own __offsetof().

This revision now requires review to proceed.Aug 29 2021, 5:50 PM
This revision is now accepted and ready to land.Aug 29 2021, 6:30 PM

LGTM, and it's not the end of the world for us to revert if upstream addresses it differently. But let's give @jkim a couple of days.

This revision was automatically updated to reflect the committed changes.