* [PATCH][gdb/breakpoints] Fix longjmp master breakpoint with separate debug info @ 2021-01-27 14:31 Tom de Vries 2021-01-27 21:37 ` Tom Tromey 0 siblings, 1 reply; 4+ messages in thread From: Tom de Vries @ 2021-01-27 14:31 UTC (permalink / raw) To: gdb-patches Hi, When running test-case gdb.base/longjmp.exp with target board unix/-m32, we run into: ... (gdb) next^M Warning:^M Cannot insert breakpoint 0.^M Cannot access memory at address 0x7dbf7353^M ^M __libc_siglongjmp (env=0x804a040 <env>, val=1) at longjmp.c:28^M 28 longjmps++;^M (gdb) FAIL: gdb.base/longjmp.exp: next over longjmp(1) ... The failure to access memory happens in i386_get_longjmp_target and is due to glibc having pointer encryption (aka "pointer mangling" or "pointer guard") of the long jump buffer. This is a known problem. In create_longjmp_master_breakpoint (which attempts to install a master longjmp breakpoint) a preference scheme is present, which installs a probe breakpoint if a libc:longjmp probe is present, and otherwise falls back to setting breakpoints at the names in the longjmp_names array. But in fact, both the probe breakpoint and the longjmp_names breakpoints are set. The latter ones are set when processing libc.so.debug, and the former one when processing libc.so. In other words, this is the longjmp variant of PR26881, which describes the same problem for master exception breakpoints. This problem only triggers when the glibc debug info package is installed, which is not due to the debug info itself in libc.so.debug, but due to the minimal symbols (because create_longjmp_master_breakpoint uses minimal symbols to translate the longjmp_names to addresses). The problem doesn't trigger for -m64, because there tdep->jb_pc_offset is not set. Fix this similar to commit 1940319c0ef (the fix for PR26881): only install longjmp_names breakpoints in libc.so/libc.so.debug if installing the libc:longjmp probe in libc.so failed. Tested on x86_64-linux. Any comments? Thanks, - Tom [gdb/breakpoints] Fix longjmp master breakpoint with separate debug info gdb/ChangeLog: 2021-01-27 Tom de Vries <tdevries@suse.de> PR breakpoints/27205 * breakpoint.c (create_longjmp_master_breakpoint_probe) (create_longjmp_master_breakpoint_names): New function, factored out of ... (create_longjmp_master_breakpoint): ... here. Only try to install longjmp_names breakpoints in libc.so/libc.so.debug if installing probe breakpoint in libc.so failed. --- gdb/breakpoint.c | 187 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 113 insertions(+), 74 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 70a4d0e1690..cdf647b44b2 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -3327,100 +3327,139 @@ create_overlay_event_breakpoint (void) } } -static void -create_longjmp_master_breakpoint (void) +/* Install a master longjmp breakpoint for OBJFILE using a probe. Return + true if a breakpoint was installed. */ + +static bool +create_longjmp_master_breakpoint_probe (objfile *objfile) { - scoped_restore_current_program_space restore_pspace; + struct gdbarch *gdbarch = objfile->arch (); + struct breakpoint_objfile_data *bp_objfile_data + = get_breakpoint_objfile_data (objfile); - for (struct program_space *pspace : program_spaces) + if (!bp_objfile_data->longjmp_searched) { - set_current_program_space (pspace); + std::vector<probe *> ret + = find_probes_in_objfile (objfile, "libc", "longjmp"); - for (objfile *objfile : current_program_space->objfiles ()) + if (!ret.empty ()) { - int i; - struct gdbarch *gdbarch; - struct breakpoint_objfile_data *bp_objfile_data; + /* We are only interested in checking one element. */ + probe *p = ret[0]; - gdbarch = objfile->arch (); + if (!p->can_evaluate_arguments ()) + { + /* We cannot use the probe interface here, + because it does not know how to evaluate + arguments. */ + ret.clear (); + } + } + bp_objfile_data->longjmp_probes = ret; + bp_objfile_data->longjmp_searched = 1; + } - bp_objfile_data = get_breakpoint_objfile_data (objfile); + if (bp_objfile_data->longjmp_probes.empty ()) + return false; - if (!bp_objfile_data->longjmp_searched) - { - std::vector<probe *> ret - = find_probes_in_objfile (objfile, "libc", "longjmp"); + for (probe *p : bp_objfile_data->longjmp_probes) + { + struct breakpoint *b; - if (!ret.empty ()) - { - /* We are only interested in checking one element. */ - probe *p = ret[0]; + b = create_internal_breakpoint (gdbarch, + p->get_relocated_address (objfile), + bp_longjmp_master, + &internal_breakpoint_ops); + b->location = new_probe_location ("-probe-stap libc:longjmp"); + b->enable_state = bp_disabled; + } - if (!p->can_evaluate_arguments ()) - { - /* We cannot use the probe interface here, - because it does not know how to evaluate - arguments. */ - ret.clear (); - } - } - bp_objfile_data->longjmp_probes = ret; - bp_objfile_data->longjmp_searched = 1; - } + return true; +} - if (!bp_objfile_data->longjmp_probes.empty ()) - { - for (probe *p : bp_objfile_data->longjmp_probes) - { - struct breakpoint *b; - - b = create_internal_breakpoint (gdbarch, - p->get_relocated_address (objfile), - bp_longjmp_master, - &internal_breakpoint_ops); - b->location = new_probe_location ("-probe-stap libc:longjmp"); - b->enable_state = bp_disabled; - } +/* Install master longjmp breakpoints for OBJFILE using longjmp_names. + Return true if at least one breakpoint was installed. */ + +static bool +create_longjmp_master_breakpoint_names (objfile *objfile) +{ + struct gdbarch *gdbarch = objfile->arch (); + if (!gdbarch_get_longjmp_target_p (gdbarch)) + return false; + + struct breakpoint_objfile_data *bp_objfile_data + = get_breakpoint_objfile_data (objfile); + unsigned int installed_bp = 0; + for (int i = 0; i < NUM_LONGJMP_NAMES; i++) + { + struct breakpoint *b; + const char *func_name; + CORE_ADDR addr; + struct explicit_location explicit_loc; + + if (msym_not_found_p (bp_objfile_data->longjmp_msym[i].minsym)) + continue; + + func_name = longjmp_names[i]; + if (bp_objfile_data->longjmp_msym[i].minsym == NULL) + { + struct bound_minimal_symbol m; + + m = lookup_minimal_symbol_text (func_name, objfile); + if (m.minsym == NULL) + { + /* Prevent future lookups in this objfile. */ + bp_objfile_data->longjmp_msym[i].minsym = &msym_not_found; continue; } + bp_objfile_data->longjmp_msym[i] = m; + } - if (!gdbarch_get_longjmp_target_p (gdbarch)) - continue; + addr = BMSYMBOL_VALUE_ADDRESS (bp_objfile_data->longjmp_msym[i]); + b = create_internal_breakpoint (gdbarch, addr, bp_longjmp_master, + &internal_breakpoint_ops); + initialize_explicit_location (&explicit_loc); + explicit_loc.function_name = ASTRDUP (func_name); + b->location = new_explicit_location (&explicit_loc); + b->enable_state = bp_disabled; + installed_bp++; + } - for (i = 0; i < NUM_LONGJMP_NAMES; i++) - { - struct breakpoint *b; - const char *func_name; - CORE_ADDR addr; - struct explicit_location explicit_loc; + return installed_bp > 0; +} - if (msym_not_found_p (bp_objfile_data->longjmp_msym[i].minsym)) - continue; +/* Create a master longjmp breakpoint. */ - func_name = longjmp_names[i]; - if (bp_objfile_data->longjmp_msym[i].minsym == NULL) - { - struct bound_minimal_symbol m; +static void +create_longjmp_master_breakpoint (void) +{ + scoped_restore_current_program_space restore_pspace; - m = lookup_minimal_symbol_text (func_name, objfile); - if (m.minsym == NULL) - { - /* Prevent future lookups in this objfile. */ - bp_objfile_data->longjmp_msym[i].minsym = &msym_not_found; - continue; - } - bp_objfile_data->longjmp_msym[i] = m; - } + for (struct program_space *pspace : program_spaces) + { + set_current_program_space (pspace); - addr = BMSYMBOL_VALUE_ADDRESS (bp_objfile_data->longjmp_msym[i]); - b = create_internal_breakpoint (gdbarch, addr, bp_longjmp_master, - &internal_breakpoint_ops); - initialize_explicit_location (&explicit_loc); - explicit_loc.function_name = ASTRDUP (func_name); - b->location = new_explicit_location (&explicit_loc); - b->enable_state = bp_disabled; - } + for (objfile *obj : current_program_space->objfiles ()) + { + /* Skip separate debug object. */ + if (obj->separate_debug_objfile_backlink) + continue; + + /* Try a probe kind breakpoint. */ + if (create_longjmp_master_breakpoint_probe (obj)) + continue; + + /* Try longjmp_names kind breakpoints. */ + if (create_longjmp_master_breakpoint_names (obj)) + continue; + + /* Iterate over separate debug objects and try longjmp_names + kind breakpoints. */ + for (objfile *sepdebug = obj->separate_debug_objfile; + sepdebug != nullptr; sepdebug = sepdebug->separate_debug_objfile) + if (create_longjmp_master_breakpoint_names (sepdebug)) + break; } } } ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][gdb/breakpoints] Fix longjmp master breakpoint with separate debug info 2021-01-27 14:31 [PATCH][gdb/breakpoints] Fix longjmp master breakpoint with separate debug info Tom de Vries @ 2021-01-27 21:37 ` Tom Tromey 2021-01-28 9:55 ` Tom de Vries 0 siblings, 1 reply; 4+ messages in thread From: Tom Tromey @ 2021-01-27 21:37 UTC (permalink / raw) To: Tom de Vries; +Cc: gdb-patches >>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: Tom> 2021-01-27 Tom de Vries <tdevries@suse.de> Tom> PR breakpoints/27205 Tom> * breakpoint.c (create_longjmp_master_breakpoint_probe) Tom> (create_longjmp_master_breakpoint_names): New function, factored out Tom> of ... Tom> (create_longjmp_master_breakpoint): ... here. Only try to install Tom> longjmp_names breakpoints in libc.so/libc.so.debug if installing probe Tom> breakpoint in libc.so failed. This looks reasonable to me. Thanks for doing it. Tom> + /* Skip separate debug object. */ Tom> + if (obj->separate_debug_objfile_backlink) Normally this would be written != nullptr. Tom> + /* Iterate over separate debug objects and try longjmp_names Tom> + kind breakpoints. */ Tom> + for (objfile *sepdebug = obj->separate_debug_objfile; Tom> + sepdebug != nullptr; sepdebug = sepdebug->separate_debug_objfile) Tom> + if (create_longjmp_master_breakpoint_names (sepdebug)) Tom> + break; I guess this can't use separate_debug_iterator / separate_debug_range since that returns the parent objfile first. Tom ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][gdb/breakpoints] Fix longjmp master breakpoint with separate debug info 2021-01-27 21:37 ` Tom Tromey @ 2021-01-28 9:55 ` Tom de Vries 2021-01-28 20:07 ` Tom Tromey 0 siblings, 1 reply; 4+ messages in thread From: Tom de Vries @ 2021-01-28 9:55 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 1988 bytes --] On 1/27/21 10:37 PM, Tom Tromey wrote: >>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes: > > Tom> 2021-01-27 Tom de Vries <tdevries@suse.de> > > Tom> PR breakpoints/27205 > Tom> * breakpoint.c (create_longjmp_master_breakpoint_probe) > Tom> (create_longjmp_master_breakpoint_names): New function, factored out > Tom> of ... > Tom> (create_longjmp_master_breakpoint): ... here. Only try to install > Tom> longjmp_names breakpoints in libc.so/libc.so.debug if installing probe > Tom> breakpoint in libc.so failed. > > This looks reasonable to me. Thanks for doing it. > > Tom> + /* Skip separate debug object. */ > Tom> + if (obj->separate_debug_objfile_backlink) > > Normally this would be written != nullptr. > Ack, fixed. > Tom> + /* Iterate over separate debug objects and try longjmp_names > Tom> + kind breakpoints. */ > Tom> + for (objfile *sepdebug = obj->separate_debug_objfile; > Tom> + sepdebug != nullptr; sepdebug = sepdebug->separate_debug_objfile) > Tom> + if (create_longjmp_master_breakpoint_names (sepdebug)) > Tom> + break; > > I guess this can't use separate_debug_iterator / separate_debug_range > since that returns the parent objfile first. Ah, I didn't know about this iterator. I think this is precisely what we need (there is some extra code before this bit that calls create_longjmp_master_breakpoint_names (obj), so we could eliminate that). So, thanks a lot for pointing that out :) [ BTW, I think separate_debug_iterator / separate_debug_range is a bit of a misnomer, if it iterates over the parent obj first. It iterates over all the objects that could contain debug information, also if there's no separate debug info to begin with, f.i. if the parent object is unstripped and contains all the debug info. So perhaps debug_objfile_iterator / debug_objfile_range is more appropriate ? ] Anyway, updated the patch to use separate_debug_range, attached below. Will commit shortly. Thanks, - Tom [-- Attachment #2: 0001-gdb-breakpoints-Fix-longjmp-master-breakpoint-with-separate-debug-info.patch --] [-- Type: text/x-patch, Size: 9232 bytes --] [gdb/breakpoints] Fix longjmp master breakpoint with separate debug info When running test-case gdb.base/longjmp.exp with target board unix/-m32, we run into: ... (gdb) next^M Warning:^M Cannot insert breakpoint 0.^M Cannot access memory at address 0x7dbf7353^M ^M __libc_siglongjmp (env=0x804a040 <env>, val=1) at longjmp.c:28^M 28 longjmps++;^M (gdb) FAIL: gdb.base/longjmp.exp: next over longjmp(1) ... The failure to access memory happens in i386_get_longjmp_target and is due to glibc having pointer encryption (aka "pointer mangling" or "pointer guard") of the long jump buffer. This is a known problem. In create_longjmp_master_breakpoint (which attempts to install a master longjmp breakpoint) a preference scheme is present, which installs a probe breakpoint if a libc:longjmp probe is present, and otherwise falls back to setting breakpoints at the names in the longjmp_names array. But in fact, both the probe breakpoint and the longjmp_names breakpoints are set. The latter ones are set when processing libc.so.debug, and the former one when processing libc.so. In other words, this is the longjmp variant of PR26881, which describes the same problem for master exception breakpoints. This problem only triggers when the glibc debug info package is installed, which is not due to the debug info itself in libc.so.debug, but due to the minimal symbols (because create_longjmp_master_breakpoint uses minimal symbols to translate the longjmp_names to addresses). The problem doesn't trigger for -m64, because there tdep->jb_pc_offset is not set. Fix this similar to commit 1940319c0ef (the fix for PR26881): only install longjmp_names breakpoints in libc.so/libc.so.debug if installing the libc:longjmp probe in libc.so failed. Tested on x86_64-linux. gdb/ChangeLog: 2021-01-27 Tom de Vries <tdevries@suse.de> PR breakpoints/27205 * breakpoint.c (create_longjmp_master_breakpoint_probe) (create_longjmp_master_breakpoint_names): New function, factored out of ... (create_longjmp_master_breakpoint): ... here. Only try to install longjmp_names breakpoints in libc.so/libc.so.debug if installing probe breakpoint in libc.so failed. --- gdb/breakpoint.c | 182 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 108 insertions(+), 74 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 70a4d0e1690..70b0d88cb45 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -3327,100 +3327,134 @@ create_overlay_event_breakpoint (void) } } -static void -create_longjmp_master_breakpoint (void) +/* Install a master longjmp breakpoint for OBJFILE using a probe. Return + true if a breakpoint was installed. */ + +static bool +create_longjmp_master_breakpoint_probe (objfile *objfile) { - scoped_restore_current_program_space restore_pspace; + struct gdbarch *gdbarch = objfile->arch (); + struct breakpoint_objfile_data *bp_objfile_data + = get_breakpoint_objfile_data (objfile); - for (struct program_space *pspace : program_spaces) + if (!bp_objfile_data->longjmp_searched) { - set_current_program_space (pspace); + std::vector<probe *> ret + = find_probes_in_objfile (objfile, "libc", "longjmp"); - for (objfile *objfile : current_program_space->objfiles ()) + if (!ret.empty ()) { - int i; - struct gdbarch *gdbarch; - struct breakpoint_objfile_data *bp_objfile_data; + /* We are only interested in checking one element. */ + probe *p = ret[0]; - gdbarch = objfile->arch (); + if (!p->can_evaluate_arguments ()) + { + /* We cannot use the probe interface here, + because it does not know how to evaluate + arguments. */ + ret.clear (); + } + } + bp_objfile_data->longjmp_probes = ret; + bp_objfile_data->longjmp_searched = 1; + } - bp_objfile_data = get_breakpoint_objfile_data (objfile); + if (bp_objfile_data->longjmp_probes.empty ()) + return false; - if (!bp_objfile_data->longjmp_searched) - { - std::vector<probe *> ret - = find_probes_in_objfile (objfile, "libc", "longjmp"); + for (probe *p : bp_objfile_data->longjmp_probes) + { + struct breakpoint *b; - if (!ret.empty ()) - { - /* We are only interested in checking one element. */ - probe *p = ret[0]; + b = create_internal_breakpoint (gdbarch, + p->get_relocated_address (objfile), + bp_longjmp_master, + &internal_breakpoint_ops); + b->location = new_probe_location ("-probe-stap libc:longjmp"); + b->enable_state = bp_disabled; + } - if (!p->can_evaluate_arguments ()) - { - /* We cannot use the probe interface here, - because it does not know how to evaluate - arguments. */ - ret.clear (); - } - } - bp_objfile_data->longjmp_probes = ret; - bp_objfile_data->longjmp_searched = 1; - } + return true; +} - if (!bp_objfile_data->longjmp_probes.empty ()) - { - for (probe *p : bp_objfile_data->longjmp_probes) - { - struct breakpoint *b; - - b = create_internal_breakpoint (gdbarch, - p->get_relocated_address (objfile), - bp_longjmp_master, - &internal_breakpoint_ops); - b->location = new_probe_location ("-probe-stap libc:longjmp"); - b->enable_state = bp_disabled; - } +/* Install master longjmp breakpoints for OBJFILE using longjmp_names. + Return true if at least one breakpoint was installed. */ + +static bool +create_longjmp_master_breakpoint_names (objfile *objfile) +{ + struct gdbarch *gdbarch = objfile->arch (); + if (!gdbarch_get_longjmp_target_p (gdbarch)) + return false; + struct breakpoint_objfile_data *bp_objfile_data + = get_breakpoint_objfile_data (objfile); + unsigned int installed_bp = 0; + + for (int i = 0; i < NUM_LONGJMP_NAMES; i++) + { + struct breakpoint *b; + const char *func_name; + CORE_ADDR addr; + struct explicit_location explicit_loc; + + if (msym_not_found_p (bp_objfile_data->longjmp_msym[i].minsym)) + continue; + + func_name = longjmp_names[i]; + if (bp_objfile_data->longjmp_msym[i].minsym == NULL) + { + struct bound_minimal_symbol m; + + m = lookup_minimal_symbol_text (func_name, objfile); + if (m.minsym == NULL) + { + /* Prevent future lookups in this objfile. */ + bp_objfile_data->longjmp_msym[i].minsym = &msym_not_found; continue; } + bp_objfile_data->longjmp_msym[i] = m; + } - if (!gdbarch_get_longjmp_target_p (gdbarch)) - continue; + addr = BMSYMBOL_VALUE_ADDRESS (bp_objfile_data->longjmp_msym[i]); + b = create_internal_breakpoint (gdbarch, addr, bp_longjmp_master, + &internal_breakpoint_ops); + initialize_explicit_location (&explicit_loc); + explicit_loc.function_name = ASTRDUP (func_name); + b->location = new_explicit_location (&explicit_loc); + b->enable_state = bp_disabled; + installed_bp++; + } - for (i = 0; i < NUM_LONGJMP_NAMES; i++) - { - struct breakpoint *b; - const char *func_name; - CORE_ADDR addr; - struct explicit_location explicit_loc; + return installed_bp > 0; +} - if (msym_not_found_p (bp_objfile_data->longjmp_msym[i].minsym)) - continue; +/* Create a master longjmp breakpoint. */ - func_name = longjmp_names[i]; - if (bp_objfile_data->longjmp_msym[i].minsym == NULL) - { - struct bound_minimal_symbol m; +static void +create_longjmp_master_breakpoint (void) +{ + scoped_restore_current_program_space restore_pspace; - m = lookup_minimal_symbol_text (func_name, objfile); - if (m.minsym == NULL) - { - /* Prevent future lookups in this objfile. */ - bp_objfile_data->longjmp_msym[i].minsym = &msym_not_found; - continue; - } - bp_objfile_data->longjmp_msym[i] = m; - } + for (struct program_space *pspace : program_spaces) + { + set_current_program_space (pspace); - addr = BMSYMBOL_VALUE_ADDRESS (bp_objfile_data->longjmp_msym[i]); - b = create_internal_breakpoint (gdbarch, addr, bp_longjmp_master, - &internal_breakpoint_ops); - initialize_explicit_location (&explicit_loc); - explicit_loc.function_name = ASTRDUP (func_name); - b->location = new_explicit_location (&explicit_loc); - b->enable_state = bp_disabled; - } + for (objfile *obj : current_program_space->objfiles ()) + { + /* Skip separate debug object, it's handled in the loop below. */ + if (obj->separate_debug_objfile_backlink != nullptr) + continue; + + /* Try a probe kind breakpoint on main objfile. */ + if (create_longjmp_master_breakpoint_probe (obj)) + continue; + + /* Try longjmp_names kind breakpoints on main and separate_debug + objfiles. */ + for (objfile *debug_objfile : obj->separate_debug_objfiles ()) + if (create_longjmp_master_breakpoint_names (debug_objfile)) + break; } } } ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][gdb/breakpoints] Fix longjmp master breakpoint with separate debug info 2021-01-28 9:55 ` Tom de Vries @ 2021-01-28 20:07 ` Tom Tromey 0 siblings, 0 replies; 4+ messages in thread From: Tom Tromey @ 2021-01-28 20:07 UTC (permalink / raw) To: Tom de Vries; +Cc: Tom Tromey, gdb-patches Tom> [ BTW, I think separate_debug_iterator / separate_debug_range is a bit Tom> of a misnomer, if it iterates over the parent obj first. It iterates Tom> over all the objects that could contain debug information, also if Tom> there's no separate debug info to begin with, f.i. if the parent object Tom> is unstripped and contains all the debug info. Tom> So perhaps debug_objfile_iterator / debug_objfile_range is more Tom> appropriate ? ] Yeah, now that you point it out, I agree that the current name seems a bit off. debug_objfile_* seems a little generic though. What about maybe 'all_related_objfiles', the idea being that you pass in an objfile and it iterates over all the "related" ones -- both it and the separate debug files? Tom ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-28 20:07 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-27 14:31 [PATCH][gdb/breakpoints] Fix longjmp master breakpoint with separate debug info Tom de Vries 2021-01-27 21:37 ` Tom Tromey 2021-01-28 9:55 ` Tom de Vries 2021-01-28 20:07 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox