=?utf-8?q?Donát?= Nagy <donat.n...@ericsson.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/72...@github.com>
DonatNagyE wrote: > I think the original `alloca(0)` warning message might be clearer/easier to > understand. The difficulty is that we switch from the `SymbolicRegion` that represents the heap allocation to an `AllocaRegion` that doesn't have an associated symbol, while the code that produces the warnings about zero-allocated memory areas (essentially) maintains a set of _symbols_ that correspond to zero-allocated regions (I'd guess that this was implemented before the introduction of dynamic extent). It would be possible to reintroduce the original `alloca(0)` message (while keeping the improvement that `alloca` returns an `AllocaRegion` and not a `SymbolicRegion` on the heap), but that would require significant refactoring (and/or ugly code duplication) and frankly I feel that `alloca` is rare and this would be a waste of time. > While it might have platform or compiler dependent meaning, those behaviors > are non-portable, so I think it is undesirable in most cases and people > probably want to be notified about it. I'd say that "non-portable" and "undesirable in most cases" is true for _practically all_ applications of `alloca`, not just the `alloca(0)` corner case. Experienced programmers who use `alloca` check that their code works with _their own particular_ `alloca` and wouldn't want to see generic messages that may or may not be relevant for them; while novices ( / projects that don't want to deal with `alloca` issues) would be better served by a generic "don't use `alloca`, it's platform-specific" warning. > Regarding binding the return value outside of `evalCall`, I think that could > be addressed in a separate PR. This one does not make the current situation > any worse. But the very least we should add a FIXME now (and potentially open > a ticket), to reduce the chances of this getting forgotten. > > Edit: we already have a TODO, but I think that one does not really do justice > to the urgency of the problem, it does not mention that doing bindings in > checkers outside of `evalCall` is not a good idea. I'll spice up that TODO note; and after merging this commit I could start working on fixing it. https://github.com/llvm/llvm-project/pull/72402 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
