[1/2] Catch exceptions thrown from gdbarch_skip_prologue
Commit Message
Simon Marchi <simon.marchi@polymtl.ca> writes:
>> This patch doesn't re-indent the code, to keep the patch simple.
>> Regression tested on x86_64-linux (ubuntu xenial), and verified that
>> it fixes some fails on Debian/stretch.
>
> I wouldn't mind if you indented the code in this patch. Many diff
> viewers are good at highlighting what changed in a line, so it's
> obvious when it's only whitespaces. Otherwise, here's always git
> diff/show -w
> for those who like looking at raw diffs.
>
It was more about reading patch in email client. Anyway, I put these
two into one patch.
>>
>> gdb:
>>
>> 2017-07-18 Yao Qi <yao.qi@linaro.org>
>>
>> PR gdb/21555
>> * arch-utils.c (gdbarch_skip_prologue_noexcept): New function.
>> * arch-utils.h (gdbarch_skip_prologue_noexcept): Declare.
>> * infrun.c: Include arch-utils.h
>> (handle_step_into_function): Call gdbarch_skip_prologue_noexcept.
>> (handle_step_into_function_backward): Likewise.
>> * symtab.c (skip_prologue_sal): Likewise.
>>
>> gdb/testsuite:
>>
>> 2017-07-18 Yao Qi <yao.qi@linaro.org>
>>
>> PR gdb/21555
>> * gdb.base/reread.exp: Wrap the whole test with two kinds of
>> compilation flags, with -fPIE and without -fPIE.
>> ---
>> gdb/arch-utils.c | 19 +++++++++++++++++++
>> gdb/arch-utils.h | 3 +++
>> gdb/infrun.c | 9 +++++----
>> gdb/symtab.c | 3 ++-
>> gdb/testsuite/gdb.base/reread.exp | 12 +++++++++---
>> 5 files changed, 38 insertions(+), 8 deletions(-)
>>
>> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
>> index 2ae3413..2f63b80 100644
>> --- a/gdb/arch-utils.c
>> +++ b/gdb/arch-utils.c
>> @@ -985,6 +985,25 @@ default_print_insn (bfd_vma memaddr,
>> disassemble_info *info)
>> return (*disassemble_fn) (memaddr, info);
>> }
>>
>> +/* Wrapper to gdbarch_skip_prologue, but doesn't throw exception.
>> Catch
>> + exception thrown from gdbarch_skip_prologue, and return PC. */
>
> Could you explain in the comment what is returned if an exception is
> caught by the wrapper?
I think I did :), "Catch exception throw from gdbarch_skip_prologue, and
return PC.".
>
> The comment should probably go in the .h, with a "See arch-utils.h"
> comment here.
>
OK, done.
>> +CORE_ADDR
>> +gdbarch_skip_prologue_noexcept (gdbarch *gdbarch, CORE_ADDR pc)
>> noexcept
>> +{
>> + CORE_ADDR new_pc = pc;
>> +
>> + TRY
>> + {
>> + new_pc = gdbarch_skip_prologue (gdbarch, new_pc);
>
> I know it does the same thing, but I think it would be clearer to pass
> "pc" as a parameter to the function.
>
OK, fixed.
>>
>> -if { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}"
>> executable {debug nowarnings}] != ""
>> - && [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}"
>> executable {debug nowarnings additional_flags=-DNO_SECTIONS}] != ""} {
>> +if { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}"
>> executable [list debug nowarnings additional_flags=[lindex $opts 0]
>> [lindex $opts 1]]] != ""
>> + && [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}"
>> executable [list debug nowarnings "additional_flags=-DNO_SECTIONS
>> [lindex $opts 0]" [lindex $opts 1]]] != ""} {
>
> Could you split those really long lines? It would perhaps help to
> define an intermediary variable for compilation opts.
Fixed. Patch below is pushed in.
Comments
On Tue, Jul 25, 2017 at 11:38 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
>
> Fixed. Patch below is pushed in.
8.0 branch is affected by this bug too, so push it
to 8.0 branch as well. Wikipage
https://sourceware.org/gdb/wiki/GDB_8.0_Release
is updated too.
@@ -985,6 +985,24 @@ default_print_insn (bfd_vma memaddr, disassemble_info *info)
return (*disassemble_fn) (memaddr, info);
}
+/* See arch-utils.h. */
+
+CORE_ADDR
+gdbarch_skip_prologue_noexcept (gdbarch *gdbarch, CORE_ADDR pc) noexcept
+{
+ CORE_ADDR new_pc = pc;
+
+ TRY
+ {
+ new_pc = gdbarch_skip_prologue (gdbarch, pc);
+ }
+ CATCH (ex, RETURN_MASK_ALL)
+ {}
+ END_CATCH
+
+ return new_pc;
+}
+
/* -Wmissing-prototypes */
extern initialize_file_ftype _initialize_gdbarch_utils;
@@ -268,4 +268,10 @@ extern void default_guess_tracepoint_registers (struct gdbarch *gdbarch,
extern int default_print_insn (bfd_vma memaddr, disassemble_info *info);
+/* Wrapper to gdbarch_skip_prologue, but doesn't throw exception. Catch
+ exception thrown from gdbarch_skip_prologue, and return PC. */
+
+extern CORE_ADDR gdbarch_skip_prologue_noexcept (gdbarch *gdbarch,
+ CORE_ADDR pc) noexcept;
+
#endif
@@ -66,6 +66,7 @@
#include "common/enum-flags.h"
#include "progspace-and-thread.h"
#include "common/gdb_optional.h"
+#include "arch-utils.h"
/* Prototypes for local functions */
@@ -7317,8 +7318,8 @@ handle_step_into_function (struct gdbarch *gdbarch,
cust = find_pc_compunit_symtab (stop_pc);
if (cust != NULL && compunit_language (cust) != language_asm)
- ecs->stop_func_start = gdbarch_skip_prologue (gdbarch,
- ecs->stop_func_start);
+ ecs->stop_func_start
+ = gdbarch_skip_prologue_noexcept (gdbarch, ecs->stop_func_start);
stop_func_sal = find_pc_line (ecs->stop_func_start, 0);
/* Use the step_resume_break to step until the end of the prologue,
@@ -7396,8 +7397,8 @@ handle_step_into_function_backward (struct gdbarch *gdbarch,
cust = find_pc_compunit_symtab (stop_pc);
if (cust != NULL && compunit_language (cust) != language_asm)
- ecs->stop_func_start = gdbarch_skip_prologue (gdbarch,
- ecs->stop_func_start);
+ ecs->stop_func_start
+ = gdbarch_skip_prologue_noexcept (gdbarch, ecs->stop_func_start);
stop_func_sal = find_pc_line (stop_pc, 0);
@@ -64,6 +64,7 @@
#include "progspace-and-thread.h"
#include "common/gdb_optional.h"
#include "filename-seen-cache.h"
+#include "arch-utils.h"
/* Forward declarations for local functions. */
@@ -3627,7 +3628,7 @@ skip_prologue_sal (struct symtab_and_line *sal)
if (gdbarch_skip_entrypoint_p (gdbarch))
pc = gdbarch_skip_entrypoint (gdbarch, pc);
if (skip)
- pc = gdbarch_skip_prologue (gdbarch, pc);
+ pc = gdbarch_skip_prologue_noexcept (gdbarch, pc);
/* For overlays, map pc back into its mapped VMA range. */
pc = overlay_mapped_address (pc, section);
@@ -15,111 +15,131 @@
set prototypes 1
-# build the first test case
+# Build programs in PIE mode, to reproduce PR 21555.
+foreach_with_prefix opts {
+ { "" "" }
+ { "-fPIE" "ldflags=-pie" } } {
-set testfile1 "reread1"
-set srcfile1 ${testfile1}.c
-# Cygwin needs $EXEEXT.
-set binfile1 [standard_output_file ${testfile1}$EXEEXT]
-
-if { [gdb_compile "${srcdir}/${subdir}/${srcfile1}" "${binfile1}" executable {debug nowarnings}] != "" } {
- untested "failed to compile first testcase"
- return -1
-}
-
-# build the second test case
-
-set testfile2 "reread2"
-set srcfile2 ${testfile2}.c
-set binfile2 [standard_output_file ${testfile2}$EXEEXT]
-
-if { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" executable {debug nowarnings}] != ""
- && [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" executable {debug nowarnings additional_flags=-DNO_SECTIONS}] != ""} {
- untested "failed to compile second testcase"
- return -1
-}
-
-# Start with a fresh gdb.
-
-set testfile "reread"
-set binfile [standard_output_file ${testfile}$EXEEXT]
-
-gdb_start
-gdb_reinitialize_dir $srcdir/$subdir
-
-# Load the first executable.
-
-gdb_rename_execfile ${binfile1} ${binfile}
-gdb_load ${binfile}
-
-# Set a breakpoint at foo
-
-gdb_test "break foo" \
- "Breakpoint.*at.* file .*$srcfile1, line 14.*" \
- "breakpoint foo in first file"
-
-
-# Run, should see "Breakpoint 1, foo () at hello1.c:14"
-
-gdb_run_cmd
-gdb_test "" "Breakpoint.* foo .* at .*$srcfile1:14.*" "run to foo()"
-
-# Restore first executable to its original name, and move
-# second executable into its place. Ensure that the new
-# executable is at least a second newer than the old.
-
-gdb_rename_execfile ${binfile} ${binfile1}
-gdb_rename_execfile ${binfile2} ${binfile}
-gdb_test "shell sleep 1" ".*" ""
-gdb_touch_execfile ${binfile}
-
-# Run a second time; GDB should detect that the executable has changed
-# and reset the breakpoints correctly.
-# Should see "Breakpoint 1, foo () at reread2.c:9"
-
-set test "run to foo() second time"
-if [is_remote target] {
- unsupported $test
-} else {
- gdb_run_cmd
- gdb_test "" "Breakpoint.* foo .* at .*:9.*" $test
-}
-
-
-### Second pass: verify that GDB checks the executable file's
-### timestamp when the program is *restarted*, not just when it exits.
-
-if [is_remote target] {
- unsupported "second pass: GDB should check for changes before running"
-} else {
-
- # Put the older executable back in place.
- gdb_rename_execfile ${binfile} ${binfile2}
- gdb_rename_execfile ${binfile1} ${binfile}
-
- # Restart GDB entirely.
- clean_restart ${binfile}
-
- # Set a breakpoint on foo and run to it.
- gdb_test "break foo" \
- "Breakpoint.*at.* file .*$srcfile1, line 14.*" \
- "second pass: breakpoint foo in first file"
- gdb_run_cmd
- gdb_test "" "Breakpoint.* foo .* at .*$srcfile1:14.*" "second pass: run to foo()"
-
- # This time, let the program run to completion. If GDB checks the
- # executable file's timestamp now, it won't notice any change.
- gdb_continue_to_end "second pass"
-
- # Now move the newer executable into place, and re-run. GDB
- # should still notice that the executable file has changed,
- # and still re-set the breakpoint appropriately.
- gdb_rename_execfile ${binfile} ${binfile1}
- gdb_rename_execfile ${binfile2} ${binfile}
- gdb_run_cmd
- gdb_test "" "Breakpoint.* foo .* at .*:9.*" "second pass: run to foo() second time"
-}
+ # build the first test case
+ set testfile1 "reread1"
+ set srcfile1 ${testfile1}.c
+ # Cygwin needs $EXEEXT.
+ set binfile1 [standard_output_file ${testfile1}$EXEEXT]
+
+ set testfile1_opt [list debug nowarnings \
+ additional_flags=[lindex $opts 0] \
+ [lindex $opts 1] ]
+ if { [gdb_compile "${srcdir}/${subdir}/${srcfile1}" "${binfile1}" \
+ executable ${testfile1_opt}] != "" } {
+ untested "failed to compile first testcase"
+ return -1
+ }
+
+ # build the second test case
+
+ set testfile2 "reread2"
+ set srcfile2 ${testfile2}.c
+ set binfile2 [standard_output_file ${testfile2}$EXEEXT]
+
+ set testfile2_opt1 [list debug nowarnings \
+ additional_flags=[lindex $opts 0] \
+ [lindex $opts 1]]
+ set testfile2_op2 [list debug nowarnings \
+ "additional_flags=-DNO_SECTIONS [lindex $opts 0]" \
+ [lindex $opts 1]]
+ if { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" \
+ executable ${testfile2_opt1}] != ""
+ && [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" \
+ executable ${testfile2_opt2}] != ""} {
+ untested "failed to compile second testcase"
+ return -1
+ }
+
+ # Start with a fresh gdb.
+
+ set testfile "reread"
+ set binfile [standard_output_file ${testfile}$EXEEXT]
+
+ gdb_start
+ gdb_reinitialize_dir $srcdir/$subdir
+
+ # Load the first executable.
+
+ gdb_rename_execfile ${binfile1} ${binfile}
+ gdb_load ${binfile}
+
+ # Set a breakpoint at foo
+
+ gdb_test "break foo" \
+ "Breakpoint.*at.* file .*$srcfile1, line 14.*" \
+ "breakpoint foo in first file"
+
+
+ # Run, should see "Breakpoint 1, foo () at hello1.c:14"
+
+ gdb_run_cmd
+ gdb_test "" "Breakpoint.* foo .* at .*$srcfile1:14.*" "run to foo()"
+
+ # Restore first executable to its original name, and move
+ # second executable into its place. Ensure that the new
+ # executable is at least a second newer than the old.
+
+ gdb_rename_execfile ${binfile} ${binfile1}
+ gdb_rename_execfile ${binfile2} ${binfile}
+ gdb_test "shell sleep 1" ".*" ""
+ gdb_touch_execfile ${binfile}
+
+ # Run a second time; GDB should detect that the executable has changed
+ # and reset the breakpoints correctly.
+ # Should see "Breakpoint 1, foo () at reread2.c:9"
+
+ set test "run to foo() second time"
+ if [is_remote target] {
+ unsupported $test
+ } else {
+ gdb_run_cmd
+ gdb_test "" "Breakpoint.* foo .* at .*:9.*" $test
+ }
+
+
+ ### Second pass: verify that GDB checks the executable file's
+ ### timestamp when the program is *restarted*, not just when it exits.
+
+ if [is_remote target] {
+ unsupported "second pass: GDB should check for changes before running"
+ } else {
+
+ # Put the older executable back in place.
+ gdb_rename_execfile ${binfile} ${binfile2}
+ gdb_rename_execfile ${binfile1} ${binfile}
+
+ # Restart GDB entirely.
+ clean_restart ${binfile}
+
+ # Set a breakpoint on foo and run to it.
+ gdb_test "break foo" \
+ "Breakpoint.*at.* file .*$srcfile1, line 14.*" \
+ "second pass: breakpoint foo in first file"
+ gdb_run_cmd
+ gdb_test "" "Breakpoint.* foo .* at .*$srcfile1:14.*" \
+ "second pass: run to foo()"
+
+ # This time, let the program run to completion. If GDB checks the
+ # executable file's timestamp now, it won't notice any change.
+ gdb_continue_to_end "second pass"
+
+ # Now move the newer executable into place, and re-run. GDB
+ # should still notice that the executable file has changed,
+ # and still re-set the breakpoint appropriately.
+ gdb_rename_execfile ${binfile} ${binfile1}
+ gdb_rename_execfile ${binfile2} ${binfile}
+ gdb_run_cmd
+ gdb_test "" "Breakpoint.* foo .* at .*:9.*" \
+ "second pass: run to foo() second time"
+ }
+
+ }
# End of tests.
return 0