Bug #2972
warning: attempt to free a non-heap object 'dvr_disk_space_tasklet'
100%
Description
trying to compile latest version on Openelec. Got this error, compile finished but service fails to start.
In function 'tasklet_disarm.constprop',
inlined from 'dvr_disk_space_done' at src/dvr/dvr_rec.c:1187:3,
inlined from 'dvr_done' at src/dvr/dvr_config.c:1070:3,
inlined from 'main' at src/main.c:1108:145:
src/main.c:380:7: warning: attempt to free a non-heap object 'dvr_disk_space_tasklet' [-Wfree-nonheap-object]
free(tsk);
^
History
Updated by Jaroslav Kysela over 9 years ago
It's false positive. The tsk->tsk_allocated is false (zero) in this case. Provide TRACE or gdb backtrace to check why tvh process fails.
Updated by Jaroslav Kysela over 9 years ago
- Status changed from New to Fixed
- % Done changed from 0 to 100
Applied in changeset commit:tvheadend|e1072086c8b296b38136c603259903ad93bc6e52.
Updated by Tom Mang over 8 years ago
The Bug ist still present. I tried to compile the current master on an Ubuntu System with latest gcc. It was a cross compile with target WeTek_Play on ARM architecture for OpenElec.
I had a look in the code, and the patch provided in the change set has definitely nothing to do with the error.
Still, I was able to fix the issue!
The situation is the following:
- In src/dvr/dvr_vfsmgr.c
- in line 47 there is defined: static tasklet_t dvr_disk_space_tasklet
- in line 467 tasklet_disarm(&dvr_disk_space_tasklet) is called => this function in main.c calls a free() on this pointer
the compiler complains because the variable is defined as a non-pointer variable. Thus it is alloced at compile time. The compiler therefore thinks you are destroying a variable that was not generated by malloc (free destroys memory allocated by malloc on the heap). The code itself might work, as in line 427 tasklet_arm(&dvr_disk_space_tasklet, dvr_get_disk_space_tcb, path); is called. This function in main.c takes the memory address and allocated heap space via malloc.
To patch this, i did the following:
change line 47 to: static tasklet_t* dvr_disk_space_tasklet;
change line 467 to tasklet_disarm(dvr_disk_space_tasklet);
change line 427 to tasklet_arm(dvr_disk_space_tasklet, dvr_get_disk_space_tcb, path);
This way a pointer is allocated by malloc and destroyed by free, just as it should be :-)
Can somebody change this in the repository?
Best regards,
Thomas
Updated by Jaroslav Kysela over 8 years ago
You completely ommited tsk_allocated handling - it's member of tsk_callback_t . The code is correct - for statically declared structure, tsk_allocated is zero, thus the free is not called at all. But compiler cannot detect this at the compilation time - it's false positive as I wrote before.
Updated by Tom Mang over 8 years ago
No, i did not ommitted the allocation handling. I know that this is kind of a false positive. Read my post carefully.
However, if I am not mistaken (c++ years are some time back, but I read it up) the compiler is quite right to complain about the code.
The "static" allocates memory at program allocation. Once, program wide. Afterwards you take the address of this memory and test if it's zero in the deallocate function. If it is, you would destroy it. You are right that the compiler does allocate the static memory with zeros. However, you could change the memory afterwards in the code and you would then try to free those memory.
Long story short. It's extremely inconvenient for people new to tvheadend that try to compile the code that they run into such errors. Not everybody is able to fix it.
Even I had to recheck my "fix", as it does not work as intended. The tasklet_arm function does not allocate any memory, so without a malloc there is no memory present to work on. Another point why the code is not really "save". If somebody uses tasklet_arm + tasklet_disarm in the future on other locations (at the moment tasklet_disarm it is only called for the dvr_disk_space_tasklet variable as far as i could see), the person would have to allocate memory before tasklet_arm, but tasklet_disarm would free it. In fairness, you allocate inside of tasklet_arm_alloc, that calls tasklet_arm afterwards. Problem is, that means arm/disarm are not the counterpart they seem to be on the first look. Certainly not a good situation.
I see two options: Either you remove the "free" from the disarm function and introduce a tasklet_arm_disalloc that will free the memory. Or you change the variable dvr_disk_space_tasklet to a pointer with default NULL and call malloc/memset to alloc the memory in case the variable is NULL. You would also need to set the *tsk pointer to NULL after the call to free in main.c
Updated by Joe User almost 8 years ago
I saw this today when cross compiling Tvheadend 4.1-2474 for Libreelec 8.0. But it only threw a warning instead of an error and did finish to build. Unfortunately I am building it for someone else who has the required hardware (I don't) so I do not know yet if it will run. I will update if/when I find out...
In function 'tasklet_disarm.constprop', inlined from 'dvr_disk_space_done' at src/dvr/dvr_vfsmgr.c:493:3, inlined from 'dvr_done' at src/dvr/dvr_config.c:1370:3, inlined from 'main' at src/main.c:1293:3: src/main.c:414:7: warning: attempt to free a non-heap object 'dvr_disk_space_tasklet' [-Wfree-nonheap-object] free(tsk);
Updated by Jaroslav Kysela almost 8 years ago
Could you try v4.1-2446-gb71fbc2? My gcc 6.3.1 does not show this error at all, but I tried to resolve this compiler false-positive warning in http://tvheadend.org/projects/tvheadend/repository/revisions/b71fbc27d809a0cfb898d71ee7df7b2b89107684/diff/ .
Updated by Joe User almost 8 years ago
It compiles without any warnings now. But unfortunately I do not have any hardware to test on. I will try to find some tester...