[RFA] Fix leak in displaced step.

[Philippe Waroquiers]

Valgrind reports a definite leak of displaced->step_saved_copy
(full leak stack trace below).

This patch fixes the leak by calling xfree (displaced->step_saved_copy)
in displaced_step_clear.

==4736== VALGRIND_GDB_ERROR_BEGIN
==4736== 128 bytes in 8 blocks are definitely lost in loss record 980 of 3,108
==4736== at 0x4C2BE2D: malloc (vg_replace_malloc.c:299)
==4736== by 0x41B627: xmalloc (common-utils.c:44)
==4736== by 0x50D4E3: displaced_step_prepare_throw (infrun.c:1837)
==4736== by 0x50D4E3: displaced_step_prepare (infrun.c:1898)
==4736== by 0x50D4E3: resume_1 (infrun.c:2545)
==4736== by 0x50D4E3: resume(gdb_signal) (infrun.c:2741)
==4736== by 0x50DCD5: keep_going_pass_signal(execution_control_state*) (infrun.c:7793)
==4736== by 0x50E903: process_event_stop_test(execution_control_state*) (infrun.c:6843)
==4736== by 0x510925: handle_signal_stop(execution_control_state*) (infrun.c:6176)
==4736== by 0x513F79: handle_inferior_event_1 (infrun.c:5354)
==4736== by 0x513F79: handle_inferior_event(execution_control_state*) (infrun.c:5389)
==4736== by 0x51541B: fetch_inferior_event(void*) (infrun.c:3916)
==4736== by 0x4B3EEC: gdb_wait_for_event(int) (event-loop.c:859)
==4736== by 0x4B3FF6: gdb_do_one_event() [clone .part.4] (event-loop.c:322)
==4736== by 0x4B41B4: gdb_do_one_event (common-exceptions.h:219)
==4736== by 0x4B41B4: start_event_loop() (event-loop.c:371)
==4736== by 0x551237: captured_command_loop() (main.c:330)
==4736== by 0x55222C: captured_main (main.c:1177)
==4736== by 0x55222C: gdb_main(captured_main_args*) (main.c:1193)
==4736== by 0x29B4F7: main (gdb.c:32)
==4736==
==4736== VALGRIND_GDB_ERROR_END

gdb/ChangeLog

2018-11-04 Philippe Waroquiers <***@skynet.be>

* infrun.c (displaced_step_clear): Fix leak by xfree-ing
displaced->step_saved_copy.
---
gdb/infrun.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 9473d1f20f..526ad2acb1 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1709,6 +1709,12 @@ displaced_step_clear (struct displaced_step_inferior_state *displaced)

delete displaced->step_closure;
displaced->step_closure = NULL;
+
+ if (displaced->step_saved_copy != NULL)
+ {
+ xfree (displaced->step_saved_copy);
+ displaced->step_saved_copy = NULL;
+ }
}

static void

[Simon Marchi]

Post by Philippe Waroquiers
Valgrind reports a definite leak of displaced->step_saved_copy
(full leak stack trace below).
This patch fixes the leak by calling xfree (displaced->step_saved_copy)
in displaced_step_clear.
==4736== VALGRIND_GDB_ERROR_BEGIN
==4736== 128 bytes in 8 blocks are definitely lost in loss record 980 of 3,108
==4736== at 0x4C2BE2D: malloc (vg_replace_malloc.c:299)
==4736== by 0x41B627: xmalloc (common-utils.c:44)
==4736== by 0x50D4E3: displaced_step_prepare_throw (infrun.c:1837)
==4736== by 0x50D4E3: displaced_step_prepare (infrun.c:1898)
==4736== by 0x50D4E3: resume_1 (infrun.c:2545)
==4736== by 0x50D4E3: resume(gdb_signal) (infrun.c:2741)
==4736== by 0x50DCD5: keep_going_pass_signal(execution_control_state*) (infrun.c:7793)
==4736== by 0x50E903: process_event_stop_test(execution_control_state*) (infrun.c:6843)
==4736== by 0x510925: handle_signal_stop(execution_control_state*) (infrun.c:6176)
==4736== by 0x513F79: handle_inferior_event_1 (infrun.c:5354)
==4736== by 0x513F79: handle_inferior_event(execution_control_state*) (infrun.c:5389)
==4736== by 0x51541B: fetch_inferior_event(void*) (infrun.c:3916)
==4736== by 0x4B3EEC: gdb_wait_for_event(int) (event-loop.c:859)
==4736== by 0x4B3FF6: gdb_do_one_event() [clone .part.4] (event-loop.c:322)
==4736== by 0x4B41B4: gdb_do_one_event (common-exceptions.h:219)
==4736== by 0x4B41B4: start_event_loop() (event-loop.c:371)
==4736== by 0x551237: captured_command_loop() (main.c:330)
==4736== by 0x55222C: captured_main (main.c:1177)
==4736== by 0x55222C: gdb_main(captured_main_args*) (main.c:1193)
==4736== by 0x29B4F7: main (gdb.c:32)
==4736==
==4736== VALGRIND_GDB_ERROR_END
gdb/ChangeLog
* infrun.c (displaced_step_clear): Fix leak by xfree-ing
displaced->step_saved_copy.

I assume you have regtested this? If s

[Philippe Waroquiers]

Post by Philippe Waroquiers
* infrun.c (displaced_step_clear): Fix leak by xfree-ing
displaced->step_saved_copy.

I assume you have regtested this? If so, LGTM.

Yes, before submitting a patch, I run the gdb tests (including gdb.ada)
and (try to) detect regressions by comparing with an untouched master build.
I also re-run all tests once more before pushing,
with which I now see one more 'unresolved testcase' in
step-over-syscall.exp, which is clearly a regression I did not
detect initially :(.

So, I will further investigate, and submit another patch once I
understand how to properly fix this leak.

Thanks for looking at the patch, sorry for this wrong fix ...

Philippe

FYI, I am intending to run the full gdb testsuite under valgrind,
and analyse the reported problems.
Currently, there are only 2 remaining definite leaks seen when
running a few small tests:
* this leak (which is triggered for each step/next I think)
* a 'one shot initialization' leak of 2 small blocks 
'ui::ui(_IO_FILE*, _IO_FILE*, _IO_FILE*)'

There are many 'possibly lost' leaks in Python, which I guess are normal.
There is also a bunch of possibly lost blocks created by the arch init code
(seems to be 'one shot initialization' leaks also, unclear to me
if these are normal).
In summary, GDB seems reasonably 'leak free'.

Then there were a bunch of uninitialized memory usage (conditional
jumps, or used in syscalls) in various tests, but the whole testsuite
came to a halt due to the 'lingering processes' problem, see other patch.

So, no risk of having nothing to do during the week-end :)

[Simon Marchi]

Post by Philippe Waroquiers

Post by Philippe Waroquiers
* infrun.c (displaced_step_clear): Fix leak by xfree-ing
displaced->step_saved_copy.

I assume you have regtested this? If so, LGTM.

Yes, before submitting a patch, I run the gdb tests (including gdb.ada)
and (try to) detect regressions by comparing with an untouched master build.
I also re-run all tests once more before pushing,
with which I now see one more 'unresolved testcase' in
step-over-syscall.exp, which is clearly a regression I did not
detect initially :(.
So, I will further investigate, and submit another patch once I
understand how to properly fix this leak.
Thanks for looking at the patch, sorry for this wrong fix ...
Philippe
FYI, I am intending to run the full gdb testsuite under valgrind,
and analyse the reported problems.
Currently, there are only 2 remaining definite leaks seen when
* this leak (which is triggered for each step/next I think)
* a 'one shot initialization' leak of 2 small blocks 
'ui::ui(_IO_FILE*, _IO_FILE*, _IO_FILE*)'
There are many 'possibly lost' leaks in Python, which I guess are normal.
There is also a bunch of possibly lost blocks created by the arch init code
(seems to be 'one shot initialization' leaks also, unclear to me
if these are normal).
In summary, GDB seems reasonably 'leak free'.
Then there were a bunch of uninitialized memory usage (conditional
jumps, or used in syscalls) in various tests, but the whole testsuite
came to a halt due to the 'lingering processes' problem, see other patch.
So, no risk of having nothing to do during the week-end :)

Thank you very much for doing this!

Simon

[Philippe Waroquiers]

Valgrind reports a definite leak of displaced->step_saved_copy
(full leak stack trace below).

This patch fixes the leak by only allocating a new step_saved_copy
if the process displaced_step_inferior_state does not yet have one,
and by freeing it when the displaced_step_inferior_state of a process
is removed, when the inferior exits.

Regtested on debian/amd64 + step-over-syscall.exp rerun under valgrind.

==4736== VALGRIND_GDB_ERROR_BEGIN
==4736== 128 bytes in 8 blocks are definitely lost in loss record 980 of 3,108
==4736== at 0x4C2BE2D: malloc (vg_replace_malloc.c:299)
==4736== by 0x41B627: xmalloc (common-utils.c:44)
==4736== by 0x50D4E3: displaced_step_prepare_throw (infrun.c:1837)
==4736== by 0x50D4E3: displaced_step_prepare (infrun.c:1898)
==4736== by 0x50D4E3: resume_1 (infrun.c:2545)
==4736== by 0x50D4E3: resume(gdb_signal) (infrun.c:2741)
==4736== by 0x50DCD5: keep_going_pass_signal(execution_control_state*) (infrun.c:7793)
==4736== by 0x50E903: process_event_stop_test(execution_control_state*) (infrun.c:6843)
==4736== by 0x510925: handle_signal_stop(execution_control_state*) (infrun.c:6176)
==4736== by 0x513F79: handle_inferior_event_1 (infrun.c:5354)
==4736== by 0x513F79: handle_inferior_event(execution_control_state*) (infrun.c:5389)
==4736== by 0x51541B: fetch_inferior_event(void*) (infrun.c:3916)
==4736== by 0x4B3EEC: gdb_wait_for_event(int) (event-loop.c:859)
==4736== by 0x4B3FF6: gdb_do_one_event() [clone .part.4] (event-loop.c:322)
==4736== by 0x4B41B4: gdb_do_one_event (common-exceptions.h:219)
==4736== by 0x4B41B4: start_event_loop() (event-loop.c:371)
==4736== by 0x551237: captured_command_loop() (main.c:330)
==4736== by 0x55222C: captured_main (main.c:1177)
==4736== by 0x55222C: gdb_main(captured_main_args*) (main.c:1193)
==4736== by 0x29B4F7: main (gdb.c:32)
==4736==
==4736== VALGRIND_GDB_ERROR_END

gdb/ChangeLog

2018-11-11 Philippe Waroquiers <***@skynet.be>

* infrun.c (displaced_step_inferior_state): Explain why step_saved_copy
is sometimes needed after the step-over is finished.
(remove_displaced_stepping_state): xfree step_saved_copy.
(displaced_step_clear): Add note that explains why we do not xfree
step_saved_copy here.
---
gdb/infrun.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 9473d1f20f..1c40cb2e0f 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1510,7 +1510,13 @@ struct displaced_step_inferior_state
made. */
CORE_ADDR step_original, step_copy;

- /* Saved contents of copy area. */
+ /* Saved contents of copy area. In most cases, we could get rid
+ of this copy when the displaced single-step is finished, after
+ having restored the content, when setting step_thread to nullptr.
+ However, we need to keep this content in case the step-over is
+ over a fork syscall: in such a case, the step-over was done in
+ the parent, but we also have to restore the copy area content
+ in the child, after the parent has finished the step-over. */
gdb_byte *step_saved_copy;
};

@@ -1638,6 +1644,11 @@ remove_displaced_stepping_state (inferior *inf)
if (it->inf == inf)
{
*prev_next_p = it->next;
+ if (it->step_saved_copy != NULL)
+ {
+ xfree (it->step_saved_copy);
+ it->step_saved_copy = NULL;
+ }
xfree (it);
return;
}
@@ -1709,6 +1720,10 @@ displaced_step_clear (struct displaced_step_inferior_state *displaced)

delete displaced->step_closure;
displaced->step_closure = NULL;
+
+ /* Note: we cannot xfree (displaced->step_saved_copy), as this
+ is needed to restore the content in the child, if
+ the step-over was over a fork syscall. */
}

static void
@@ -1834,7 +1849,11 @@ displaced_step_prepare_throw (thread_info *tp)
}

/* Save the original contents of the copy area. */
- displaced->step_saved_copy = (gdb_byte *) xmalloc (len);
+ if (displaced->step_saved_copy == NULL)
+ displaced->step_saved_copy = (gdb_byte *) xmalloc (len);
+ /* Even if we have not allocated step_saved_copy now, make a
+ (temporary) cleanup for it, in case the setup below fails to
+ complete the copy. */
ignore_cleanups = make_cleanup (free_current_contents,
&displaced->step_saved_copy);
status = target_read_memory (copy, displaced->step_saved_copy, len);

[David Blaikie]

Any chance this could use unique_xmalloc_ptr to reduce the risk of leaking
a bit? (I guess that'd probably mean handling the ownership of
displaced_step_inferorior_state, since it'd then have a non-trivial dtor,
so it couldn't just be xmalloc/xfree'd and would need similar
unique_xmalloc_ptr handling, etc?)

On Sun, Nov 11, 2018 at 4:11 AM Philippe Waroquiers <

Post by Philippe Waroquiers
Valgrind reports a definite leak of displaced->step_saved_copy
(full leak stack trace below).
This patch fixes the leak by only allocating a new step_saved_copy
if the process displaced_step_inferior_state does not yet have one,
and by freeing it when the displaced_step_inferior_state of a process
is removed, when the inferior exits.
Regtested on debian/amd64 + step-over-syscall.exp rerun under valgrind.
==4736== VALGRIND_GDB_ERROR_BEGIN
==4736== 128 bytes in 8 blocks are definitely lost in loss record 980 of 3,108
==4736== at 0x4C2BE2D: malloc (vg_replace_malloc.c:299)
==4736== by 0x41B627: xmalloc (common-utils.c:44)
==4736== by 0x50D4E3: displaced_step_prepare_throw (infrun.c:1837)
==4736== by 0x50D4E3: displaced_step_prepare (infrun.c:1898)
==4736== by 0x50D4E3: resume_1 (infrun.c:2545)
==4736== by 0x50D4E3: resume(gdb_signal) (infrun.c:2741)
==4736== by 0x50DCD5: keep_going_pass_signal(execution_control_state*) (infrun.c:7793)
==4736== by 0x50E903: process_event_stop_test(execution_control_state*) (infrun.c:6843)
==4736== by 0x510925: handle_signal_stop(execution_control_state*) (infrun.c:6176)
==4736== by 0x513F79: handle_inferior_event_1 (infrun.c:5354)
==4736== by 0x513F79: handle_inferior_event(execution_control_state*) (infrun.c:5389)
==4736== by 0x51541B: fetch_inferior_event(void*) (infrun.c:3916)
==4736== by 0x4B3EEC: gdb_wait_for_event(int) (event-loop.c:859)
==4736== by 0x4B3FF6: gdb_do_one_event() [clone .part.4]
(event-loop.c:322)
==4736== by 0x4B41B4: gdb_do_one_event (common-exceptions.h:219)
==4736== by 0x4B41B4: start_event_loop() (event-loop.c:371)
==4736== by 0x551237: captured_command_loop() (main.c:330)
==4736== by 0x55222C: captured_main (main.c:1177)
==4736== by 0x55222C: gdb_main(captured_main_args*) (main.c:1193)
==4736== by 0x29B4F7: main (gdb.c:32)
==4736==
==4736== VALGRIND_GDB_ERROR_END
gdb/ChangeLog
* infrun.c (displaced_step_inferior_state): Explain why step_saved_copy
is sometimes needed after the step-over is finished.
(remove_displaced_stepping_state): xfree step_saved_copy.
(displaced_step_clear): Add note that explains why we do not xfree
step_saved_copy here.
---
gdb/infrun.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 9473d1f20f..1c40cb2e0f 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1510,7 +1510,13 @@ struct displaced_step_inferior_state
made. */
CORE_ADDR step_original, step_copy;
- /* Saved contents of copy area. */
+ /* Saved contents of copy area. In most cases, we could get rid
+ of this copy when the displaced single-step is finished, after
+ having restored the content, when setting step_thread to nullptr.
+ However, we need to keep this content in case the step-over is
+ over a fork syscall: in such a case, the step-over was done in
+ the parent, but we also have to restore the copy area content
+ in the child, after the parent has finished the step-over. */
gdb_byte *step_saved_copy;
};
@@ -1638,6 +1644,11 @@ remove_displaced_stepping_state (inferior *inf)
if (it->inf == inf)
{
*prev_next_p = it->next;
+ if (it->step_saved_copy != NULL)
+ {
+ xfree (it->step_saved_copy);
+ it->step_saved_copy = NULL;
+ }
xfree (it);
return;
}
@@ -1709,6 +1720,10 @@ displaced_step_clear (struct
displaced_step_inferior_state *displaced)
delete displaced->step_closure;
displaced->step_closure = NULL;
+
+ /* Note: we cannot xfree (displaced->step_saved_copy), as this
+ is needed to restore the content in the child, if
+ the step-over was over a fork syscall. */
}
static void
@@ -1834,7 +1849,11 @@ displaced_step_prepare_throw (thread_info *tp)
}
/* Save the original contents of the copy area. */
- displaced->step_saved_copy = (gdb_byte *) xmalloc (len);
+ if (displaced->step_saved_copy == NULL)
+ displaced->step_saved_copy = (gdb_byte *) xmalloc (len);
+ /* Even if we have not allocated step_saved_copy now, make a
+ (temporary) cleanup for it, in case the setup below fails to
+ complete the copy. */
ignore_cleanups = make_cleanup (free_current_contents,
&displaced->step_saved_copy);
status = target_read_memory (copy, displaced->step_saved_copy, len);
--
2.19.1

[Simon Marchi]

Post by David Blaikie
Any chance this could use unique_xmalloc_ptr to reduce the risk of leaking
a bit? (I guess that'd probably mean handling the ownership of
displaced_step_inferorior_state, since it'd then have a non-trivial dtor,
so it couldn't just be xmalloc/xfree'd and would need similar
unique_xmalloc_ptr handling, etc?)

I think it would be the right way, yes. If we need the buffer to be resizable,
we could use a vector (gdb::byte_vector), but in any case
displaced_step_inferior_st

[Simon Marchi]

Post by Philippe Waroquiers
Valgrind reports a definite leak of displaced->step_saved_copy
(full leak stack trace below).
This patch fixes the leak by only allocating a new step_saved_copy
if the process displaced_step_inferior_state does not yet have one,
and by freeing it when the displaced_step_inferior_state of a process
is removed, when the inferior exits.
Regtested on debian/amd64 + step-over-syscall.exp rerun under valgrind.
==4736== VALGRIND_GDB_ERROR_BEGIN
==4736== 128 bytes in 8 blocks are definitely lost in loss record 980 of 3,108
==4736== at 0x4C2BE2D: malloc (vg_replace_malloc.c:299)
==4736== by 0x41B627: xmalloc (common-utils.c:44)
==4736== by 0x50D4E3: displaced_step_prepare_throw (infrun.c:1837)
==4736== by 0x50D4E3: displaced_step_prepare (infrun.c:1898)
==4736== by 0x50D4E3: resume_1 (infrun.c:2545)
==4736== by 0x50D4E3: resume(gdb_signal) (infrun.c:2741)
==4736== by 0x50DCD5: keep_going_pass_signal(execution_control_state*) (infrun.c:7793)
==4736== by 0x50E903: process_event_stop_test(execution_control_state*) (infrun.c:6843)
==4736== by 0x510925: handle_signal_stop(execution_control_state*) (infrun.c:6176)
==4736== by 0x513F79: handle_inferior_event_1 (infrun.c:5354)
==4736== by 0x513F79: handle_inferior_event(execution_control_state*) (infrun.c:5389)
==4736== by 0x51541B: fetch_inferior_event(void*) (infrun.c:3916)
==4736== by 0x4B3EEC: gdb_wait_for_event(int) (event-loop.c:859)
==4736== by 0x4B3FF6: gdb_do_one_event() [clone .part.4] (event-loop.c:322)
==4736== by 0x4B41B4: gdb_do_one_event (common-exceptions.h:219)
==4736== by 0x4B41B4: start_event_loop() (event-loop.c:371)
==4736== by 0x551237: captured_command_loop() (main.c:330)
==4736== by 0x55222C: captured_main (main.c:1177)
==4736== by 0x55222C: gdb_main(captured_main_args*) (main.c:1193)
==4736== by 0x29B4F7: main (gdb.c:32)
==4736==
==4736== VALGRIND_GDB_ERROR_END
gdb/ChangeLog
* infrun.c (displaced_step_inferior_state): Explain why step_saved_copy
is sometimes needed after the step-over is finished.
(remove_displaced_stepping_state): xfree step_saved_copy.
(displaced_step_clear): Add note that explains why we do not xfree
step_saved_copy here.
---
gdb/infrun.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 9473d1f20f..1c40cb2e0f 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1510,7 +1510,13 @@ struct displaced_step_inferior_state
made. */
CORE_ADDR step_original, step_copy;
- /* Saved contents of copy area. */
+ /* Saved contents of copy area. In most cases, we could get rid
+ of this copy when the displaced single-step is finished, after
+ having restored the content, when setting step_thread to nullptr.
+ However, we need to keep this content in case the step-over is
+ over a fork syscall: in such a case, the step-over was done in
+ the parent, but we also have to restore the copy area content
+ in the child, after the parent has finished the step-over. */
gdb_byte *step_saved_copy;
};
@@ -1638,6 +1644,11 @@ remove_displaced_stepping_state (inferior *inf)
if (it->inf == inf)
{
*prev_next_p = it->next;
+ if (it->step_saved_copy != NULL)
+ {
+ xfree (it->step_saved_copy);
+ it->step_saved_copy = NULL;
+ }
xfree (it);
return;
}
@@ -1709,6 +1720,10 @@ displaced_step_clear (struct displaced_step_inferior_state *displaced)
delete displaced->step_closure;
displaced->step_closure = NULL;
+
+ /* Note: we cannot xfree (displaced->step_saved_copy), as this
+ is needed to restore the content in the child, if
+ the step-over was over a fork syscall. */
}
static void
@@ -1834,7 +1849,11 @@ displaced_step_prepare_throw (thread_info *tp)
}
/* Save the original contents of the copy area. */
- displaced->step_saved_copy = (gdb_byte *) xmalloc (len);
+ if (displaced->step_saved_copy == NULL)
+ displaced->step_saved_copy = (gdb_byte *) xmalloc (len);
+ /* Even if we have not allocated step_saved_copy now, make a
+ (temporary) cleanup for it, in case the setup below fails to
+ complete the copy. */
ignore_cleanups = make_cleanup (free_current_contents,
&displaced->step_saved_copy);
status = target_read_memory (copy, displaced->step_saved_copy, len);

From what I understand, the allocation model you propose in this patch is to
allocate a buffer the first time we do a displaced step for an inferior and
free it when the inferior exits. The allocated size is

len = gdbarch_max_insn_length (gdbarch);

Given that there can be multiple architectures inside a single inferior, can
the required buffer size change between multiple displaced step?

Also, if freeing the buffer on inferior exit is indeed what we want to do, why do
we need the above cleanup? Even if the setup fails, shouldn't be fine to keep the buffer
allocated?

Simo

[Philippe Waroquiers]

Post by Simon Marchi
From what I understand, the allocation model you propose in this patch is to
allocate a buffer the first time we do a displaced step for an inferior and
free it when the inferior exits.

Yes, that is the plan.

Post by Simon Marchi
The allocated size is
len = gdbarch_max_insn_length (gdbarch);
Given that there can be multiple architectures inside a single inferior, can
the required buffer size change between multiple displaced step?

Good remark : I did not know of this multiple architecture for a
single inferior, and then yes possibly the buffer might have to change size.
So, we should rather do (unconditionally) :
displaced->step_saved_copy = (gdb_byte *) xrealloc (len);

Post by Simon Marchi
Also, if freeing the buffer on inferior exit is indeed what we want to do, why do
we need the above cleanup? Even if the setup fails, shouldn't be fine to keep the buffer
allocated?

The idea was to avoid having a piece of memory containing not properly 
initialized data. But probably this is not a problem, as this data will
only be used if the displaced setup is finished, at the end of the function.
So, effectively no real need for the cleanup anymore.

Philippe