Page MenuHomeFreeBSD

dtrace: quiet gcc9 Walloca-larger-than
ClosedPublic

Authored by rlibby on Dec 19 2019, 8:37 PM.

Details

Summary

gcc9 grew a new warning for unbounded allocas. Checking against some
bound is a way to quiet the warning. The bound I have inserted here is
copy and pasted from the kernel dtrace_dof_maxsize.

However both this check and the alloca remain a little gross. I am open
to input as to the right way or place to fix this. A few questions:

  • Does any patch belong here, or do we go through an upstream?
  • Would it be better just to disable the warning?
  • Would it be better to convert the allocas in the file to dt_alloc? I think this is the right thing to do, I'm just wary of writing a bigger patch given that I don't know how dtrace is being maintained.
Test Plan

make CROSS_TOOLCHAIN=amd64-gcc9 buildworld

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

illumos is the nominal upstream, but there is not much active development there. I stopped trying to upstream changes a long time ago and will typically strip out code under #ifdef illumos when I modify something nearby. If my example is enough to convince you, you can just modify dtrace components like they're native FreeBSD code.

I do think adding a limit is reasonable and simply getting rid of the alloca() for DOF files would be even better. There are quite a few alloca()s in libdtrace and I'm not really sure it's worth the effort to get rid of them all, but I haven't looked to see how hard it would be. Note that there are some uses of setjmp()/longjmp() in the D parser so a naive conversion from alloca() to malloc() may introduce (probably inconsequential) memory leaks.

cddl/contrib/opensolaris/lib/libdtrace/common/dt_options.c
906 ↗(On Diff #65820)

If you keep this as-is, could you add a comment explaining where this comment comes from? You could go a bit further and fetch the limit from kern.dtrace.dof_maxsize, but a hard-coded limit is probably fine. 8MB is a lot for a DOF file.

This revision is now accepted and ready to land.Dec 19 2019, 9:49 PM
This revision now requires review to proceed.Dec 20 2019, 7:18 AM
This revision is now accepted and ready to land.Dec 20 2019, 4:13 PM
This revision was automatically updated to reflect the committed changes.