espressif/binutils-gdb
1
0
Fork 0
mirror of https://github.com/espressif/binutils-gdb.git synced 2025-06-23 11:39:26 +08:00
Code Issues Packages Projects Releases Wiki Activity

Stale breakpoint instructions, spurious SIGTRAPS.

Browse Source 
Without the code portion of the patch, we get these failures:

 FAIL: gdb.base/break-unload-file.exp: always-inserted on: break: continue
 FAIL: gdb.base/break-unload-file.exp: always-inserted on: hbreak: continue
 FAIL: gdb.base/sym-file.exp: stale bkpts: continue to breakpoint: end here

They all looks like random SIGTRAPs:

 continue
 Continuing.

 Program received signal SIGTRAP, Trace/breakpoint trap.
 0x0000000000400541 in foo () at ../../../src/gdb/testsuite/gdb.base/break-unload-file.c:21
 21      }
 (gdb) FAIL: gdb.base/break-unload-file.exp: always-inserted on: break: continue

(This is a regression caused by the remove-symbol-file command
series.)

break-unload-file.exp is about having breakpoints inserted, and then
doing "file".  I caught this while writing a test that does "file
PROGRAM", while PROGRAM was already loaded, which internally does
"file" first, because I wanted to force a breakpoint_re_set, but the
test is more explicit in case GDB ever optimizes out that re-set.

The problem is that unloading the file with "file" ends up in
disable_breakpoints_in_freed_objfile, which marks all breakpoint
locations of the objfile as both shlib_disabled, _and_ clears the
inserted flag, without actually removing the breakpoints from the
inferior.  Now, usually, in all-stop, breakpoints will already be
removed from the inferior before the user can issue the "file"
command, but, with non-stop, or breakpoints always-inserted on mode,
breakpoints stay inserted even while the user has the prompt.  In the
latter case, then, if we let the program continue, and it executes the
address where we had previously set the breakpoint, it'll actually
execute the breakpoint instruction that we left behind...

Now, one issue is that the intent of
disable_breakpoints_in_freed_objfile is really to handle the unloading
of OBJF_USERLOADED objfiles.  These are objfiles that were added with
add-symbol-file and that are removed with remove-symbol-file.

"add-symbol-file"'s docs in the manual clearly say these commands are
used to let GDB know about dynamically loaded code:

 You would use this command when @var{filename} has been dynamically
 loaded (by some other means) into the program that is running.

Similarly, the online help says:

 (gdb) help add-symbol-file
 Load symbols from FILE, assuming FILE has been dynamically loaded.

So it makes sense to, like when shared libraries are unloaded through
the generic solib machinery, mark the breakpoint locations as
shlib_disabled.  But, the "file" command is not about dynamically
loaded code, it's about the main program.  So the patch makes
disable_breakpoints_in_freed_objfile skip all objfiles but
OBJF_USERLOADED ones, thus skipping the main objfile.

Then, the reason that disable_breakpoints_in_freed_objfile was
clearing the inserted flag isn't clear, but likely to avoid breakpoint
removal errors, assuming remove-symbol-file was called after the
dynamic object was already unmapped from the inferior.  In that case,
it'd okay to simply clear the inserted flag, but not so if the user
for example does remove-symbol-file to remove the library because he
made a mistake in the library's address, and wants to re-do
add-symbol-file with the correct address.

To address all that, I propose an alternative implementation, that
handles both cases.  The patch includes changes to sym-file.exp to
cover them.

This implementation leaves the inserted flag alone, and handles
breakpoint insertion/removal failure gracefully when the locations are
in OBJF_USERLOADED objfiles, just like we handle insertion/removal
failure gracefully for locations in shared libraries.

To try to make sure we aren't patching back stale shadow memory
contents into the inferior, in case the program mapped a different
library at the same address where we had the breakpoint, without the
user having had a chance of remove-symbol-file'ing before, this adds a
new memory_validate_breakpoint function that checks if the breakpoint
instruction is still in memory.  ppc_linux_memory_remove_breakpoint
does this unconditionally for all memory breakpoints, and questions
whether memory_remove_breakpoint should be changed to do this for all
breakpoints.  Possibly yes, though I'm not certain, hence this
baby-steps patch.

Tested on x86_64 Fedora 17, native and gdbserver.

gdb/
2014-04-23  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (insert_bp_location): Tolerate errors if the
	breakpoint is set in a user-loaded objfile.
	(remove_breakpoint_1): Likewise.  Also tolerate errors if the
	location is marked shlib_disabled.  If the breakpoint is set in a
	user-loaded objfile is a GDB-side memory breakpoint, validate it
	before uninsertion.  (disable_breakpoints_in_freed_objfile): Skip
	non-OBJF_USERLOADED objfiles.  Don't clear the location's inserted
	flag.
	* mem-break.c (memory_validate_breakpoint): New function.
	* objfiles.c (userloaded_objfile_contains_address_p): New
	function.
	* objfiles.h (userloaded_objfile_contains_address_p): Declare.
	* target.h (memory_validate_breakpoint): New declaration.

gdb/testsuite/
2014-04-23  Pedro Alves  <palves@redhat.com>

	* gdb.base/break-unload-file.c: New file.
	* gdb.base/break-unload-file.exp: New file.
	* gdb.base/sym-file-lib.c (baz): New function.
	* gdb.base/sym-file-loader.c (struct segment) <mapped_size>: New
	field.
	(load): Store the segment's mapped size.
	(unload): New function.
	(unload_shlib): New function.
	* gdb.base/sym-file-loader.h (unload_shlib): New declaration.
	* gdb.base/sym-file-main.c (main): Unload, and reload the library,
	set a breakpoint at baz, and call it.
	* gdb.base/sym-file.exp: New tests for stale breakpoint
	instructions.
This commit is contained in:
Pedro Alves
2014-04-22 23:19:19 +01:00
parent 076855f9e3
commit 08351840ea
14 changed files with 425 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

71
gdb/breakpoint.c
View File

@ -2648,7 +2648,9 @@ insert_bp_location (struct bp_location *bl,
errors as memory errors. */
if ((bp_err == GENERIC_ERROR || bp_err == MEMORY_ERROR)
&& bl->loc_type == bp_loc_software_breakpoint
&& solib_name_from_address (bl->pspace, bl->address))
&& (solib_name_from_address (bl->pspace, bl->address)
|| userloaded_objfile_contains_address_p (bl->pspace,
bl->address)))
{
/* See also: disable_breakpoints_in_shlibs. */
bl->shlib_disabled = 1;
@ -3778,7 +3780,31 @@ remove_breakpoint_1 (struct bp_location *bl, insertion_state_t is)
|| !(section_is_overlay (bl->section)))
{
/* No overlay handling: just remove the breakpoint. */
val = bl->owner->ops->remove_location (bl);
/* If we're trying to uninsert a memory breakpoint that we
know is set in a dynamic object that is marked
shlib_disabled, then either the dynamic object was
removed with "remove-symbol-file" or with
"nosharedlibrary". In the former case, we don't know
whether another dynamic object might have loaded over the
breakpoint's address -- the user might well let us know
about it next with add-symbol-file (the whole point of
OBJF_USERLOADED is letting the user manually maintain a
list of dynamically loaded objects). If we have the
breakpoint's shadow memory, that is, this is a software
breakpoint managed by GDB, check whether the breakpoint
is still inserted in memory, to avoid overwriting wrong
code with stale saved shadow contents. Note that HW
breakpoints don't have shadow memory, as they're
implemented using a mechanism that is not dependent on
being able to modify the target's memory, and as such
they should always be removed. */
if (bl->shlib_disabled
&& bl->target_info.shadow_len != 0
&& !memory_validate_breakpoint (bl->gdbarch, &bl->target_info))
val = 0;
else
val = bl->owner->ops->remove_location (bl);
}
else
{
@ -3823,12 +3849,21 @@ remove_breakpoint_1 (struct bp_location *bl, insertion_state_t is)
}
}
/* In some cases, we might not be able to remove a breakpoint
in a shared library that has already been removed, but we
have not yet processed the shlib unload event. */
/* In some cases, we might not be able to remove a breakpoint in
a shared library that has already been removed, but we have
not yet processed the shlib unload event. Similarly for an
unloaded add-symbol-file object - the user might not yet have
had the chance to remove-symbol-file it. shlib_disabled will
be set if the library/object has already been removed, but
the breakpoint hasn't been uninserted yet, e.g., after
"nosharedlibrary" or "remove-symbol-file" with breakpoints
always-inserted mode. */
if (val
&& bl->loc_type == bp_loc_software_breakpoint
&& solib_name_from_address (bl->pspace, bl->address))
&& (bl->loc_type == bp_loc_software_breakpoint
&& (bl->shlib_disabled
|| solib_name_from_address (bl->pspace, bl->address)
|| userloaded_objfile_contains_address_p (bl->pspace,
bl->address))))
val = 0;
if (val)
@ -7665,10 +7700,18 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile)
if (objfile == NULL)
return;
/* If the file is a shared library not loaded by the user then
solib_unloaded was notified and disable_breakpoints_in_unloaded_shlib
was called. In that case there is no need to take action again. */
if ((objfile->flags & OBJF_SHARED) && !(objfile->flags & OBJF_USERLOADED))
/* OBJF_USERLOADED are dynamic modules manually managed by the user
with add-symbol-file/remove-symbol-file. Similarly to how
breakpoints in shared libraries are handled in response to
"nosharedlibrary", mark breakpoints in OBJF_USERLOADED modules
shlib_disabled so they end up uninserted on the next global
location list update. Shared libraries not loaded by the user
aren't handled here -- they're already handled in
disable_breakpoints_in_unloaded_shlib, called by solib.c's
solib_unloaded observer. We skip objfiles that are not
OBJF_USERLOADED (nor OBJF_SHARED) as those aren't considered
dynamic objects (e.g. the main objfile). */
if ((objfile->flags & OBJF_USERLOADED) == 0)
return;
ALL_BREAKPOINTS (b)
@ -7700,7 +7743,11 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile)
if (is_addr_in_objfile (loc_addr, objfile))
{
loc->shlib_disabled = 1;
loc->inserted = 0;
/* At this point, we don't know whether the object was
unmapped from the inferior or not, so leave the
inserted flag alone. We'll handle failure to
uninsert quietly, in case the object was indeed
unmapped. */
mark_breakpoint_location_modified (loc);