It turns out that on my actual target, declaring to_free as volatile does
NOT solve the leak problem.
Here is the diff of the hush.c source between my previous solution (putting
to_free in the globals, which does solve the leak) and the volatile attempt
at the same (I left it declared in the definition of the global struct in
both cases):
--- Vhush.c 2025-08-07 13:13:42.863447663 -0400
+++ Ghush.c 2025-08-07 13:53:53.171455825 -0400
@@ -7731,8 +7732,8 @@
pid_t pid;
int channel[2];
# if !BB_MMU
- volatile char ** to_free = NULL;
-// G.to_free = NULL;
+// volatile char ** to_free = NULL;
+ G.to_free = NULL;
# endif
xpipe(channel);
@@ -7810,7 +7810,7 @@
* huge=`cat BIG` # was blocking here forever
* echo OK
*/
- re_execute_shell(&to_free,
+ re_execute_shell(&G.to_free,
s,
G.global_argv[0],
G.global_argv + 1,
@@ -7828,9 +7828,9 @@
# endif
enable_restore_tty_pgrp_on_exit();
# if !BB_MMU
- free(to_free);
-// free(G.to_free);
-// G.to_free = NULL;
+// free(to_free);
+ free(G.to_free);
+ G.to_free = NULL;
# endif
close(channel[1]);
return channel[0];
And here is the diff from running "objdump -d -j
.text.generate_stream_from_string hush.o" from the two different builds.
Note that I removed the first 5 character from each line (the address
offset from the start of the function) because otherwise the different
address offsets pollutes the diff too much to be useful:
--- Vhush.s 2025-08-07 13:54:11.735455888 -0400
+++ Ghush.s 2025-08-07 13:54:11.735455888 -0400
@@ -6,79 +6,84 @@
000 <generate_stream_from_string>:
b530 push {r4, r5, lr}
- b08b sub sp, #44 ; 0x2c
+ b089 sub sp, #36 ; 0x24
9004 str r0, [sp, #16]
- 2300 movs r3, #0
- a808 add r0, sp, #32
+ a806 add r0, sp, #24
f8cd 900c str.w r9, [sp, #12]
9105 str r1, [sp, #20]
- 9307 str r3, [sp, #28]
f7ff fffe bl 0 <xpipe>
f7ff fffe bl 0 <vfork>
2800 cmp r0, #0
f8dd 900c ldr.w r9, [sp, #12]
- da03 bge.n 2a <generate_stream_from_string+0x2a>
- 4827 ldr r0, [pc, #156] ; (c0
<generate_stream_from_string+0xc0>)
+ da03 bge.n 26 <generate_stream_from_string+0x26>
+ 482d ldr r0, [pc, #180] ; (d4
<generate_stream_from_string+0xd4>)
4478 add r0, pc
f7ff fffe bl 0 <bb_simple_perror_msg_and_die>
- d138 bne.n 9e <generate_stream_from_string+0x9e>
- 4b25 ldr r3, [pc, #148] ; (c4
<generate_stream_from_string+0xc4>)
+ d138 bne.n 9a <generate_stream_from_string+0x9a>
+ 4b2b ldr r3, [pc, #172] ; (d8
<generate_stream_from_string+0xd8>)
2101 movs r1, #1
f859 2003 ldr.w r2, [r9, r3]
- 4b24 ldr r3, [pc, #144] ; (c8
<generate_stream_from_string+0xc8>)
+ 4b2a ldr r3, [pc, #168] ; (dc
<generate_stream_from_string+0xdc>)
f44f 00e0 mov.w r0, #7340032 ; 0x700000
444b add r3, r9
6013 str r3, [r2, #0]
f7ff fffe bl 0 <bb_signals>
- 9808 ldr r0, [sp, #32]
+ 9806 ldr r0, [sp, #24]
f7ff fffe bl 0 <close>
2101 movs r1, #1
- 9809 ldr r0, [sp, #36] ; 0x24
+ 9807 ldr r0, [sp, #28]
f8dd 900c ldr.w r9, [sp, #12]
f7ff fffe bl 0 <xmove_fd>
9804 ldr r0, [sp, #16]
f7ff fffe bl 0 <skip_whitespace>
- 491c ldr r1, [pc, #112] ; (cc
<generate_stream_from_string+0xcc>)
+ 4922 ldr r1, [pc, #136] ; (e0
<generate_stream_from_string+0xe0>)
4604 mov r4, r0
4479 add r1, pc
f7ff fffe bl 0 <is_prefixed_with>
- b170 cbz r0, 84 <generate_stream_from_string+0x84>
+ b170 cbz r0, 80 <generate_stream_from_string+0x80>
1d20 adds r0, r4, #4
f7ff fffe bl 0 <skip_whitespace>
7805 ldrb r5, [r0, #0]
- b94d cbnz r5, 84 <generate_stream_from_string+0x84>
- 4b17 ldr r3, [pc, #92] ; (d0
<generate_stream_from_string+0xd0>)
+ b94d cbnz r5, 80 <generate_stream_from_string+0x80>
+ 4b1d ldr r3, [pc, #116] ; (e4
<generate_stream_from_string+0xe4>)
f859 0003 ldr.w r0, [r9, r3]
f7ff fffe bl 0 <generate_stream_from_string>
f7ff fffe bl 0 <fflush_all>
4628 mov r0, r5
f7ff fffe bl 0 <_exit>
- 4b13 ldr r3, [pc, #76] ; (d4
<generate_stream_from_string+0xd4>)
+ 4b19 ldr r3, [pc, #100] ; (e8
<generate_stream_from_string+0xe8>)
2200 movs r2, #0
f859 3003 ldr.w r3, [r9, r3]
- a807 add r0, sp, #28
- 681b ldr r3, [r3, #0]
4621 mov r1, r4
- 6edb ldr r3, [r3, #108] ; 0x6c
+ 6818 ldr r0, [r3, #0]
+ 6ec3 ldr r3, [r0, #108] ; 0x6c
9200 str r2, [sp, #0]
681a ldr r2, [r3, #0]
+ 3080 adds r0, #128 ; 0x80
3304 adds r3, #4
f7ff fffe bl 0 <generate_stream_from_string>
9b05 ldr r3, [sp, #20]
6018 str r0, [r3, #0]
- 4b08 ldr r3, [pc, #32] ; (c4
<generate_stream_from_string+0xc4>)
- 9809 ldr r0, [sp, #36] ; 0x24
+ 4b0e ldr r3, [pc, #56] ; (d8
<generate_stream_from_string+0xd8>)
f859 2003 ldr.w r2, [r9, r3]
- 4b0b ldr r3, [pc, #44] ; (d8
<generate_stream_from_string+0xd8>)
+ 4b11 ldr r3, [pc, #68] ; (ec
<generate_stream_from_string+0xec>)
444b add r3, r9
6013 str r3, [r2, #0]
+ 4b0f ldr r3, [pc, #60] ; (e8
<generate_stream_from_string+0xe8>)
+ f859 3003 ldr.w r3, [r9, r3]
+ 681c ldr r4, [r3, #0]
+ f8d4 0080 ldr.w r0, [r4, #128] ; 0x80
+ f7ff fffe bl 0 <free>
+ 2300 movs r3, #0
+ f8c4 3080 str.w r3, [r4, #128] ; 0x80
+ 9807 ldr r0, [sp, #28]
+ f8dd 900c ldr.w r9, [sp, #12]
f7ff fffe bl 0 <close>
- 9808 ldr r0, [sp, #32]
+ 9806 ldr r0, [sp, #24]
f8dd 900c ldr.w r9, [sp, #12]
- b00b add sp, #44 ; 0x2c
+ b009 add sp, #36 ; 0x24
bd30 pop {r4, r5, pc}
- bf00 nop
- 00000098 .word 0x00000098
+ 000000b0 .word 0x000000b0
- 0000006a .word 0x0000006a
+ 00000082 .word 0x00000082
As you can see from the assembly code, with the volatile char **to_free on
the stack, it doesn't even bother to call free()!
That explains why I thought the stack was being overwritten when I was
using hardware watchpoints on the stack variable. It does allocate space
for the variable, it does initialize it to 0, but since it doesn't bother
to call free(), it certainly never reads it back. So my watchpoint caught
the entry into generate_stream_from_string, and the saving of the pointer
from the xmalloc(), but the next access to that stack location was not
inside the generate_stream_from_string() parent call, which I assumed it
would be, so I mistakenly thought somehow some errant thing took over the
stack.
If I turn on my leak hunt code, it does make a call to free() because it's
a different function that also gets the line number etc. But that assembly
code also does not read the variable value from the stack memory, it just
inserts a 0.
On Mon, Aug 4, 2025 at 10:38 PM Harry Eaton <[email protected]> wrote:
>
> >Because the parent attempted to free that memory
>> >by calling free(to_free), but the child (as I explained)
>> >did not store the pointer in need of freeing into to_free.
>> >to_free remained NULL, and parent didn't free anything.
>>
>> For the record, the above is not quite correct. The child absolutely does
> store the pointer into the stack variable of the parent.
> What GCC has optimized out is fetching that pointer from "to_free" in
> order to pass it to the free() call.
> Instead the compiler simply inserts NULL instead of actually loading the
> variable from memory.
>
> Attached is a gdb session log that conclusively proves that that is what
> happens.
>
> But it is still an optimization problem and declaring to_free as volatile
> should solve it.
>
>
_______________________________________________
busybox mailing list
[email protected]
https://lists.busybox.net/mailman/listinfo/busybox
