[Bug 297758] usr.sbin/pmc: builds failed with gcc14

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=297758

            Bug ID: 297758
           Summary: usr.sbin/pmc: builds failed with gcc14
           Product: Base System
           Version: 16.0-CURRENT
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Only Me
          Priority: ---
         Component: bin
          Assignee: [email protected]
          Reporter: [email protected]

Created attachment 273995
  --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=273995&action=edit
pmc-gcc14.patch

usr.sbin/pmc does not build with the amd64-gcc14 cross toolchain.  The
FreeBSD-main-amd64-gcc14_build CI job fails at a209aac86882:

  https://ci.freebsd.org/job/FreeBSD-main-amd64-gcc14_build/4531/console

This is the only compilation error in that log.  I get the same error
locally at the same revision.

  usr.sbin/pmc/headers.hh:79:25: error: flexible array member
  'pmchdr_cpuidinfo::cpuid' in an otherwise empty 'struct pmchdr_cpuidinfo'
     79 |         uint32_t        cpuid[];


Cause
-----

C++ has no flexible array members.  Each C++ compiler accepts them as an
extension.  GCC 14 and older versions refuse one that is the only member
of a struct.  GCC 15 removes this limit.  The adjacent struct
pmchdr_pmcinfo has a uint64_t rate before its char pmc[], thus it builds.

  uint32_t cpuid[]   clang++ 21 & g++ 15: OK, g++ 14: ERR
  uint32_t cpuid[0]  all three OK

A Makefile change cannot correct this.  The g++ frontend gives an
unconditional error, not a diagnostic behind a -W option.  These options
have no effect: -fpermissive, -fms-extensions, -w, -Wno-error,
-std=gnu++14, -std=gnu++20.


Remove the GCC 14 CI job?
-------------------------

Does the tree still need to support the GCC 14 cross toolchain?  I can
change the CI GCC jobs to GCC 15 only.  If that is acceptable, please
ignore the options below.


Options
-------

The attached patch does option 1.  The choice is for the author and the
reviewers.

1. Keep the flexible array member, and limit the workaround (attached).

Use cpuid[0] in an #if block for GCC versions before 15.  cpuid[] stays
the primary form, thus clang and GCC 15 get it.  The layout is the same in
both branches.  Remove the block at either of these events:

  - GCC 14 is not a supported cross toolchain.
  - The struct gets another member.

Note that a zero-length array and a flexible array member differ under
hardening options.  With -fstrict-flex-arrays=3, clang gives
__builtin_object_size(p->a, 1) as 0 for a[0], but the correct size for
a[].  FreeBSD does not use -fstrict-flex-arrays now.  This is the reason
to keep cpuid[0] conditional.

I am not very satisfied with this option.  It puts a preprocessor
conditional in a header that gives the pmc log file format.  I chose it
because pmchdr_cpuidinfo is possibly there for a future extension.  If
that is not correct, option 2 or option 3 is better.

2. Use a typedef.

  typedef uint32_t pmchdr_cpuidinfo;

process_cpuidinfo() keeps its pmchdr_cpuidinfo *cpuidinfo declaration and
its delete[].  The cast goes away, and cpuidinfo->cpuid[i] becomes
cpuidinfo[i].  There is no extension, no conditional, and no -Wpedantic
diagnostic on the three compilers.

3. Remove the struct.

Use uint32_t * in process_cpuidinfo(), and delete the struct.
write_cpuinfo() in cmd_pmc_record.cc already builds the payload as a
uint32_t * buffer and does not use the struct.  Thus the struct holds no
data that uint32_t * does not hold.

If the struct is for future fields, note that another member also
corrects the GCC 14 error.  The limit applies only while cpuid[] is the
only member.  You can add the struct again at that time, with a plain
cpuid[].

A fourth option changes the pmc log file format: give the struct a leading
count field, keep cpuid[], and write the field in write_cpuinfo().  This
is a design decision.


Related defect
--------------

process_cpuidinfo() releases the buffer through the wrong type:

  cpuidinfo = (pmchdr_cpuidinfo *)new uint32_t[len];
  ...
  delete[] cpuidinfo;

new uint32_t[] gives the buffer, but delete[] gets a pmchdr_cpuidinfo *.
This is undefined behavior.  The patch keeps a uint32_t *buf for the
allocation and the release, and uses the struct pointer only to read the
data.  Options 2 and 3 correct this on their own.

-- 
You are receiving this mail because:
You are the assignee for the bug.