public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix gdb.base/corefile2.exp regression when running Docker/AUFS
@ 2020-10-14 23:03 Luis Machado
  2020-10-18 22:29 ` Kevin Buettner
  0 siblings, 1 reply; 3+ messages in thread
From: Luis Machado @ 2020-10-14 23:03 UTC (permalink / raw)
  To: gdb-patches, Alan.Hayward, kevinb; +Cc: tom

The following failures started showing up after commit
bb2a67773c - "Use a std::vector in target_section_table":

FAIL: gdb.base/corefile2.exp: renamed binfile: print/x mbuf_ro[0]@4
FAIL: gdb.base/corefile2.exp: renamed binfile: print/x mbuf_ro[pagesize-4]@4
FAIL: gdb.base/corefile2.exp: renamed binfile: print/x mbuf_ro[-3]@6
FAIL: gdb.base/corefile2.exp: renamed binfile: print/x mbuf_rw[pagesize-3]@6
FAIL: gdb.base/corefile2.exp: renamed binfile: print/x mbuf_ro[pagesize-3]@6

I tracked it down to a problem in core_target::xfer_partial, at this point:

	if (!m_core_file_mappings.empty ())
	  xfer_status = xfer_memory_via_mappings (readbuf, writebuf, offset,
						  len, xfered_len);
	else
	  xfer_status = this->beneath ()->xfer_partial (object, annex, readbuf,
							writebuf, offset, len,
							xfered_len);

It seems commit bb2a67773c uncovered a latent bug when handling a particular
case where things are running within a Docker container using the AUFS storage
driver.

When building the file mappings for a core file, we call
gdbarch_read_core_file_mappings, which in turn passes a couple lambda
callbacks. One pre-loop and one in-loop.

The catch is that commit bb2a67773c reworked the pre-loop lambda and
made it do nothing. Before that commit, we always allocated
m_core_file_mappings in that lambda.

Now, when calling the in-loop lambda, we don't touch m_core_file_mappings
because the bfd is nullptr (given Docker leaks the host system path, and that
file doesn't exist within the container itself).

So, instead, we add an entry to the m_core_unavailable_mappings vector.

When we reach core_target::xfer_partial, we're only checking for an empty
m_core_file_mappings. Given it is now empty, we take the path of reading
the contents from the file, not the core file. This reads back unexpected
results.

The following patch fixes this by also checking for
m_core_unavailable_mappings, given core_target::xfer_memory_via_mappings
already handles the Docker/AUFS situation.

Validated on x86_64-linux and aarch64-linux on Ubuntu 18.04.

Kevin, does this look sane?

gdb/ChangeLog:

YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>

	* corelow.c (core_target::xfer_partial): Also check for an empty
	m_core_unavailable_mappings vector.
---
 gdb/corelow.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/gdb/corelow.c b/gdb/corelow.c
index d557475e06..a54d81571a 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -813,10 +813,16 @@ core_target::xfer_partial (enum target_object object, const char *annex,
 	   core file provided mappings (e.g. from .note.linuxcore.file
 	   or the like) as this should provide a more accurate
 	   result.  If not, check the stratum beneath us, which should
-	   be the file stratum.  */
-	if (!m_core_file_mappings.empty ())
-	  xfer_status = xfer_memory_via_mappings (readbuf, writebuf, offset,
-						  len, xfered_len);
+	   be the file stratum.
+
+	   We also check unavailable mappings due to Docker/AUFS driver
+	   issues.  */
+	if (!m_core_file_mappings.empty ()
+	    || !m_core_unavailable_mappings.empty ())
+	  {
+	    xfer_status = xfer_memory_via_mappings (readbuf, writebuf, offset,
+						    len, xfered_len);
+	  }
 	else
 	  xfer_status = this->beneath ()->xfer_partial (object, annex, readbuf,
 							writebuf, offset, len,
-- 
2.17.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Fix gdb.base/corefile2.exp regression when running Docker/AUFS
  2020-10-14 23:03 [PATCH] Fix gdb.base/corefile2.exp regression when running Docker/AUFS Luis Machado
@ 2020-10-18 22:29 ` Kevin Buettner
  2020-10-22 14:24   ` Luis Machado
  0 siblings, 1 reply; 3+ messages in thread
From: Kevin Buettner @ 2020-10-18 22:29 UTC (permalink / raw)
  To: Luis Machado, gdb-patches; +Cc: Alan.Hayward, tom

On Wed, 14 Oct 2020 20:03:13 -0300
Luis Machado <luis.machado@linaro.org> wrote:

> The following failures started showing up after commit
> bb2a67773c - "Use a std::vector in target_section_table":
> 
> FAIL: gdb.base/corefile2.exp: renamed binfile: print/x mbuf_ro[0]@4
> FAIL: gdb.base/corefile2.exp: renamed binfile: print/x mbuf_ro[pagesize-4]@4
> FAIL: gdb.base/corefile2.exp: renamed binfile: print/x mbuf_ro[-3]@6
> FAIL: gdb.base/corefile2.exp: renamed binfile: print/x mbuf_rw[pagesize-3]@6
> FAIL: gdb.base/corefile2.exp: renamed binfile: print/x mbuf_ro[pagesize-3]@6
> 
> I tracked it down to a problem in core_target::xfer_partial, at this point:
> 
> 	if (!m_core_file_mappings.empty ())
> 	  xfer_status = xfer_memory_via_mappings (readbuf, writebuf, offset,
> 						  len, xfered_len);
> 	else
> 	  xfer_status = this->beneath ()->xfer_partial (object, annex, readbuf,
> 							writebuf, offset, len,
> 							xfered_len);
> 
> It seems commit bb2a67773c uncovered a latent bug when handling a particular
> case where things are running within a Docker container using the AUFS storage
> driver.
> 
> When building the file mappings for a core file, we call
> gdbarch_read_core_file_mappings, which in turn passes a couple lambda
> callbacks. One pre-loop and one in-loop.
> 
> The catch is that commit bb2a67773c reworked the pre-loop lambda and
> made it do nothing. Before that commit, we always allocated
> m_core_file_mappings in that lambda.
> 
> Now, when calling the in-loop lambda, we don't touch m_core_file_mappings
> because the bfd is nullptr (given Docker leaks the host system path, and that
> file doesn't exist within the container itself).
> 
> So, instead, we add an entry to the m_core_unavailable_mappings vector.
> 
> When we reach core_target::xfer_partial, we're only checking for an empty
> m_core_file_mappings. Given it is now empty, we take the path of reading
> the contents from the file, not the core file. This reads back unexpected
> results.
> 
> The following patch fixes this by also checking for
> m_core_unavailable_mappings, given core_target::xfer_memory_via_mappings
> already handles the Docker/AUFS situation.
> 
> Validated on x86_64-linux and aarch64-linux on Ubuntu 18.04.
> 
> Kevin, does this look sane?
> 
> gdb/ChangeLog:
> 
> YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>
> 
> 	* corelow.c (core_target::xfer_partial): Also check for an empty
> 	m_core_unavailable_mappings vector.

Hi Luis,

Thanks for figuring this out!

Your patch looks good to me.

Kevin


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Fix gdb.base/corefile2.exp regression when running Docker/AUFS
  2020-10-18 22:29 ` Kevin Buettner
@ 2020-10-22 14:24   ` Luis Machado
  0 siblings, 0 replies; 3+ messages in thread
From: Luis Machado @ 2020-10-22 14:24 UTC (permalink / raw)
  To: Kevin Buettner, gdb-patches; +Cc: Alan.Hayward, tom

On 10/18/20 7:29 PM, Kevin Buettner wrote:
> On Wed, 14 Oct 2020 20:03:13 -0300
> Luis Machado <luis.machado@linaro.org> wrote:
> 
>> The following failures started showing up after commit
>> bb2a67773c - "Use a std::vector in target_section_table":
>>
>> FAIL: gdb.base/corefile2.exp: renamed binfile: print/x mbuf_ro[0]@4
>> FAIL: gdb.base/corefile2.exp: renamed binfile: print/x mbuf_ro[pagesize-4]@4
>> FAIL: gdb.base/corefile2.exp: renamed binfile: print/x mbuf_ro[-3]@6
>> FAIL: gdb.base/corefile2.exp: renamed binfile: print/x mbuf_rw[pagesize-3]@6
>> FAIL: gdb.base/corefile2.exp: renamed binfile: print/x mbuf_ro[pagesize-3]@6
>>
>> I tracked it down to a problem in core_target::xfer_partial, at this point:
>>
>> 	if (!m_core_file_mappings.empty ())
>> 	  xfer_status = xfer_memory_via_mappings (readbuf, writebuf, offset,
>> 						  len, xfered_len);
>> 	else
>> 	  xfer_status = this->beneath ()->xfer_partial (object, annex, readbuf,
>> 							writebuf, offset, len,
>> 							xfered_len);
>>
>> It seems commit bb2a67773c uncovered a latent bug when handling a particular
>> case where things are running within a Docker container using the AUFS storage
>> driver.
>>
>> When building the file mappings for a core file, we call
>> gdbarch_read_core_file_mappings, which in turn passes a couple lambda
>> callbacks. One pre-loop and one in-loop.
>>
>> The catch is that commit bb2a67773c reworked the pre-loop lambda and
>> made it do nothing. Before that commit, we always allocated
>> m_core_file_mappings in that lambda.
>>
>> Now, when calling the in-loop lambda, we don't touch m_core_file_mappings
>> because the bfd is nullptr (given Docker leaks the host system path, and that
>> file doesn't exist within the container itself).
>>
>> So, instead, we add an entry to the m_core_unavailable_mappings vector.
>>
>> When we reach core_target::xfer_partial, we're only checking for an empty
>> m_core_file_mappings. Given it is now empty, we take the path of reading
>> the contents from the file, not the core file. This reads back unexpected
>> results.
>>
>> The following patch fixes this by also checking for
>> m_core_unavailable_mappings, given core_target::xfer_memory_via_mappings
>> already handles the Docker/AUFS situation.
>>
>> Validated on x86_64-linux and aarch64-linux on Ubuntu 18.04.
>>
>> Kevin, does this look sane?
>>
>> gdb/ChangeLog:
>>
>> YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>
>>
>> 	* corelow.c (core_target::xfer_partial): Also check for an empty
>> 	m_core_unavailable_mappings vector.
> 
> Hi Luis,
> 
> Thanks for figuring this out!
> 
> Your patch looks good to me.
> 
> Kevin
> 

Thanks. Pushed now.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-10-22 14:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-14 23:03 [PATCH] Fix gdb.base/corefile2.exp regression when running Docker/AUFS Luis Machado
2020-10-18 22:29 ` Kevin Buettner
2020-10-22 14:24   ` Luis Machado

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).