george.burgess.iv added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2776 +def warn_alloca : Warning< + "use of builtin function %0">, + InGroup<DiagGroup<"alloca">>, DefaultIgnore; ---------------- aaron.ballman wrote: > george.burgess.iv wrote: > > nit: I'd just say "use of function '%0'" here; "builtin" doesn't really add > > much. > > > > I also wonder if we should be saying anything more than "we found a use of > > this function." Looks like GCC doesn't (https://godbolt.org/z/sYs_8G), but > > since this warning is sort of opinionated in itself, might it be better to > > expand this to "use of '%0' is discouraged"? > > > > WDYT, Aaron? > What is the purpose to this diagnostic, aside from GCC compatibility? What > does it protect against? > > If there's a reason users should not use alloc(), it would be better for the > diagnostic to spell it out. > > Btw, I'm okay with this being default-off because the GCC warning is as well. > I'm mostly hoping we can do better with our diagnostic wording. > I'm mostly hoping we can do better with our diagnostic wording +1 > What is the purpose to this diagnostic? I think the intent boils down to that `alloca` is easily misused, and leads to e.g., https://www.qualys.com/2017/06/19/stack-clash/stack-clash.txt . Since its use often boils down to nothing but a tiny micro-optimization, some parties would like to discourage its use. Both glibc and bionic recommend against the use of `alloca` in their documentation, though glibc's docs are less assertive than bionic's, and explicitly call out "[alloca's] use can improve efficiency compared to the use of malloc plus free." Greping a codebase and investigating the first 15 results: - all of them look like microoptimizations; many of them also sit close to other `malloc`/`new` ops, so allocating on these paths presumably isn't prohibitively expensive - all but two of the uses of `alloca` have no logic to fall back to the heap `malloc` if the array they want to allocate passes a certain threshold. Some of the uses make it look *really* easy for the array to grow very large. - one of the uses compares the result of `alloca` to `NULL`. Since `alloca`'s behavior is undefined if it fails, ... I'm having trouble putting this into a concise and actionable diagnostic message, though. The best I can come up with at the moment is something along the lines of "use of function %0 is subtle; consider using heap allocation instead." Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64883/new/ https://reviews.llvm.org/D64883 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
