[PATCH] avutil/mem: remove av_realloc / av_malloc incompatibility warning
memalign() is not guranteed to be compatible with free() or realloc() and for platforms in this category we have --enable-memalign-hack (which should be enabled automatically if such system is detected) Trying to somehow half support systems that can free() memalign memory but not reallocate it seems not worth the amount of work needed to keep 2 then incompatible allocation systems and ensure their seperation. That is unless this would affect a major platform on which we want to avoid the memalign hack code Signed-off-by: Michael Niedermayer <michaelni@gmx.at> --- libavutil/mem.h | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/libavutil/mem.h b/libavutil/mem.h index 2a1e36d..761d3c6 100644 --- a/libavutil/mem.h +++ b/libavutil/mem.h @@ -107,12 +107,6 @@ av_alloc_size(1, 2) static inline void *av_malloc_array(size_t nmemb, size_t siz * reallocated. * @return Pointer to a newly-reallocated block or NULL if the block * cannot be reallocated or the function is used to free the memory block. - * @warning Pointers originating from the av_malloc() family of functions must - * not be passed to av_realloc(). The former can be implemented using - * memalign() (or other functions), and there is no guarantee that - * pointers from such functions can be passed to realloc() at all. - * The situation is undefined according to POSIX and may crash with - * some libc implementations. * @see av_fast_realloc() */ void *av_realloc(void *ptr, size_t size) av_alloc_size(2); @@ -137,12 +131,6 @@ void *av_realloc_f(void *ptr, size_t nelem, size_t elsize); * @param size Size in bytes for the memory block to be allocated or * reallocated * @return Zero on success, an AVERROR error code on failure. - * @warning Pointers originating from the av_malloc() family of functions must - * not be passed to av_reallocp(). The former can be implemented using - * memalign() (or other functions), and there is no guarantee that - * pointers from such functions can be passed to realloc() at all. - * The situation is undefined according to POSIX and may crash with - * some libc implementations. */ int av_reallocp(void *ptr, size_t size); @@ -156,12 +144,6 @@ int av_reallocp(void *ptr, size_t size); * @param size Size of the single element * @return Pointer to a newly-reallocated block or NULL if the block * cannot be reallocated or the function is used to free the memory block. - * @warning Pointers originating from the av_malloc() family of functions must - * not be passed to av_realloc(). The former can be implemented using - * memalign() (or other functions), and there is no guarantee that - * pointers from such functions can be passed to realloc() at all. - * The situation is undefined according to POSIX and may crash with - * some libc implementations. */ av_alloc_size(2, 3) void *av_realloc_array(void *ptr, size_t nmemb, size_t size); @@ -175,12 +157,6 @@ av_alloc_size(2, 3) void *av_realloc_array(void *ptr, size_t nmemb, size_t size) * @param nmemb Number of elements * @param size Size of the single element * @return Zero on success, an AVERROR error code on failure. - * @warning Pointers originating from the av_malloc() family of functions must - * not be passed to av_realloc(). The former can be implemented using - * memalign() (or other functions), and there is no guarantee that - * pointers from such functions can be passed to realloc() at all. - * The situation is undefined according to POSIX and may crash with - * some libc implementations. */ av_alloc_size(2, 3) int av_reallocp_array(void *ptr, size_t nmemb, size_t size); -- 1.7.9.5
On Tue, 21 Apr 2015 15:12:36 +0200 Michael Niedermayer <michaelni@gmx.at> wrote:
memalign() is not guranteed to be compatible with free() or realloc() and for platforms in this category we have --enable-memalign-hack (which should be enabled automatically if such system is detected) Trying to somehow half support systems that can free() memalign memory but not reallocate it seems not worth the amount of work needed to keep 2 then incompatible allocation systems and ensure their seperation. That is unless this would affect a major platform on which we want to avoid the memalign hack code
Isn't the problem that realloc() doesn't keep the alignment?
Le duodi 2 floréal, an CCXXIII, wm4 a écrit :
Isn't the problem that realloc() doesn't keep the alignment?
No. That problem exists too, but is irrelevant for the current code base: there has been one bug of that kind about three years ago, AFAIK that is all. The problem that was addressed here was that, officially according to POSIX, using realloc() on a buffer returned by posix_memalign() is an undefined behaviour. In practice, we had quite a few of these cases but that never caused any problem. Regards, -- Nicolas George
On Tue, Apr 21, 2015 at 03:43:59PM +0200, Nicolas George wrote:
Le duodi 2 floréal, an CCXXIII, wm4 a écrit :
Isn't the problem that realloc() doesn't keep the alignment?
No. That problem exists too, but is irrelevant for the current code base: there has been one bug of that kind about three years ago, AFAIK that is all.
the only case iam aware of was some memory debugger which crashed when realloc & malloc where mixed (thats clearly a bug in the debugger as it should never crash, warn / fail maybe but not crash) [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB What does censorship reveal? It reveals fear. -- Julian Assange
Le duodi 2 floréal, an CCXXIII, Michael Niedermayer a écrit :
the only case iam aware of was some memory debugger which crashed when realloc & malloc where mixed (thats clearly a bug in the debugger as it should never crash, warn / fail maybe but not crash)
I was referring to this one: commit a5704659e3e41b7698812b89f67d9a7467a74d20 Author: Nicolas George <nicolas.george@normalesup.org> Date: 2012-08-28 18:15:49 +0200 lavfi/af_atempo: use av_malloc for rDFT buffers. Memory obtained from av_realloc is not aligned enough for AVX. The other similar allocations are changed too because they use the same macro. The buffers were cleared afterwards anyway. Fix trac ticket #1692. Regards, -- Nicolas George
Hi, On Tue, Apr 21, 2015 at 9:43 AM, Nicolas George <george@nsup.org> wrote:
Le duodi 2 floréal, an CCXXIII, wm4 a écrit :
Isn't the problem that realloc() doesn't keep the alignment?
No. That problem exists too
That seems counter-intuitive. The problem does not exist, but it does exist too? If you're saying that "the problem exists, but a user of av_realloc should be aware that the alignment will be lost", that's fine. Ronald
Le septidi 7 fructidor, an CCXXIV, Ronald S. Bultje a écrit :
On Tue, Apr 21, 2015 at 9:43 AM, Nicolas George <george@nsup.org> wrote:
Le duodi 2 floréal, an CCXXIII, wm4 a écrit :
Isn't the problem that realloc() doesn't keep the alignment? No. That problem exists too
That seems counter-intuitive. The problem does not exist, but it does exist too?
If you're saying that "the problem exists, but a user of av_realloc should be aware that the alignment will be lost", that's fine.
I do not remember the exact terms of the discussion. There are two issues to be aware of: 1. Memory obtained from av_malloc() should not be used with av_realloc(). 2. Memory obtained from av_realloc() is not aligned enough. I do not remember which issue was "the" "problem" in the discussion. Issue #1 is treacherous because it will almost always work. It comes from POSIX, that says about realloc "If ptr does not match a pointer returned earlier by calloc(), malloc(), or realloc() [...] the behavior is undefined.": av_malloc() uses posix_memalign() when available, and posix_memalign() is neither calloc(), malloc() nor realloc(), and POSIX neglected to include a posix_realloc_memalign(). Still, with most implementations, it will work, because "it works" is an acceptable undefined behavior; valgrind does not even report it. I do not remember if someone found an actual libc where it did not work, or if we had trouble with MEMALIGN_HACK between av_malloc() and av_realloc(), or if it is purely theoretical. Issue #2 can be relevant when ignoring issue #1; it was the cause of this: https://trac.ffmpeg.org/ticket/1692 HtH. Regards, -- Nicolas George
On Tue, Apr 21, 2015 at 03:12:36PM +0200, Michael Niedermayer wrote:
memalign() is not guranteed to be compatible with free() or realloc() and for platforms in this category we have --enable-memalign-hack (which should be enabled automatically if such system is detected) Trying to somehow half support systems that can free() memalign memory but not reallocate it seems not worth the amount of work needed to keep 2 then incompatible allocation systems and ensure their seperation. That is unless this would affect a major platform on which we want to avoid the memalign hack code
Signed-off-by: Michael Niedermayer <michaelni@gmx.at> --- libavutil/mem.h | 24 ------------------------ 1 file changed, 24 deletions(-)
is anyone against this patch ? people replace av_realloc by av_malloc, memcpy, av_free due to this text (example: [FFmpeg-devel] [PATCH v5] lavf/matroskadec: Normalize noncompliant A_QUICKTIME/V_QUICKTIME private data) [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is dangerous to be right in matters on which the established authorities are wrong. -- Voltaire
On Sun, Jan 10, 2016 at 04:29:47AM +0100, Michael Niedermayer wrote:
On Tue, Apr 21, 2015 at 03:12:36PM +0200, Michael Niedermayer wrote:
memalign() is not guranteed to be compatible with free() or realloc() and for platforms in this category we have --enable-memalign-hack (which should be enabled automatically if such system is detected) Trying to somehow half support systems that can free() memalign memory but not reallocate it seems not worth the amount of work needed to keep 2 then incompatible allocation systems and ensure their seperation. That is unless this would affect a major platform on which we want to avoid the memalign hack code
Signed-off-by: Michael Niedermayer <michaelni@gmx.at> --- libavutil/mem.h | 24 ------------------------ 1 file changed, 24 deletions(-)
is anyone against this patch ?
people replace av_realloc by av_malloc, memcpy, av_free due to this text (example: [FFmpeg-devel] [PATCH v5] lavf/matroskadec: Normalize noncompliant A_QUICKTIME/V_QUICKTIME private data)
applied this warning is causing alot of problems, there just was another developer that added complex code to seperate the allocations [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If a bugfix only changes things apparently unrelated to the bug with no further explanation, that is a good sign that the bugfix is wrong.
participants (5)
-
Michael Niedermayer -
Michael Niedermayer -
Nicolas George -
Ronald S. Bultje -
wm4