diff options
author | Sergio Durigan Junior <sergiodj@redhat.com> | 2017-11-25 01:13:03 -0500 |
---|---|---|
committer | Sergio Durigan Junior <sergiodj@redhat.com> | 2017-11-25 01:13:03 -0500 |
commit | 685de8c2990a04fba5e2954b7ff089a2c641071f (patch) | |
tree | dc260651ce3c8122f6db87d059cae5d3cb22356c /gdb/stap-probe.c | |
parent | deeeba559bd0c18e06dba19f44571ee8a218fcdb (diff) | |
download | gdb-685de8c2990a04fba5e2954b7ff089a2c641071f.zip gdb-685de8c2990a04fba5e2954b7ff089a2c641071f.tar.gz gdb-685de8c2990a04fba5e2954b7ff089a2c641071f.tar.bz2 |
Fix PR gdb/22491: Regression when setting SystemTap probe semaphores
Pedro has kindly pointed out that
gdb.arch/amd64-stap-optional-prefix.exp was failing after my
C++-ification patches touching the probe interface. The failure is
kind of cryptic:
77 break -pstap bar
78 Breakpoint 3 at 0x40048d
79 (gdb) PASS: gdb.arch/amd64-stap-optional-prefix.exp: bar: break -pstap bar
80 continue
81 Continuing.
82
83 Program received signal SIGILL, Illegal instruction.
84 main () at amd64-stap-optional-prefix.S:26
85 26 STAP_PROBE1(probe, foo, (%rsp))
It took me a while to figure out where this SIGILL is coming from.
Initially I thought it was something related to writing registers to
the inferior when dealing with probe arguments, but I discarded this
since the arguments were not touching any registers.
In the end, this was a mistake that was introduced during the review
process of the patch. When setting/clearing a SystemTap probe's
semaphore, the code was using 'm_address' (which refers the probe's
address) instead of 'm_sem_addr' (which refers to the semaphore's
address). This caused GDB to write a bogus value in the wrong memory
position, which in turn caused the SIGILL.
I am pushing this patch to correct the mistake.
On a side note: I told Pedro that the BuildBot hadn't caught the
failure during my try build, and for a moment there was a suspicion
that the BuildBot might be at fault here. However, I investigate this
and noticed that I only did one try build, with a patch that was
correctly using 'm_sem_addr' where applicable, and therefore no
failure should have happened indeed. I probably should have requested
another try build after addressing the review's comments, but they
were mostly basic and I didn't think it was needed. Oh, well.
2017-11-25 Sergio Durigan Junior <sergiodj@redhat.com>
PR gdb/22491
* stap-probe.c (relocate_address): New function.
(stap_probe::get_relocated_address): Use 'relocate_address'.
(stap_probe::set_semaphore): Use 'relocate_address' and pass
'm_sem_addr'.
(stap_probe::clear_semaphore): Likewise.
Diffstat (limited to 'gdb/stap-probe.c')
-rw-r--r-- | gdb/stap-probe.c | 16 |
1 files changed, 12 insertions, 4 deletions
diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c index 63100c9..9f4e008 100644 --- a/gdb/stap-probe.c +++ b/gdb/stap-probe.c @@ -1278,13 +1278,21 @@ stap_probe::parse_arguments (struct gdbarch *gdbarch) } } +/* Helper function to relocate an address. */ + +static CORE_ADDR +relocate_address (CORE_ADDR address, struct objfile *objfile) +{ + return address + ANOFFSET (objfile->section_offsets, + SECT_OFF_DATA (objfile)); +} + /* Implementation of the get_relocated_address method. */ CORE_ADDR stap_probe::get_relocated_address (struct objfile *objfile) { - return this->get_address () + ANOFFSET (objfile->section_offsets, - SECT_OFF_DATA (objfile)); + return relocate_address (this->get_address (), objfile); } /* Given PROBE, returns the number of arguments present in that probe's @@ -1455,7 +1463,7 @@ stap_modify_semaphore (CORE_ADDR address, int set, struct gdbarch *gdbarch) void stap_probe::set_semaphore (struct objfile *objfile, struct gdbarch *gdbarch) { - stap_modify_semaphore (this->get_relocated_address (objfile), 1, gdbarch); + stap_modify_semaphore (relocate_address (m_sem_addr, objfile), 1, gdbarch); } /* Implementation of the 'clear_semaphore' method. */ @@ -1463,7 +1471,7 @@ stap_probe::set_semaphore (struct objfile *objfile, struct gdbarch *gdbarch) void stap_probe::clear_semaphore (struct objfile *objfile, struct gdbarch *gdbarch) { - stap_modify_semaphore (this->get_relocated_address (objfile), 0, gdbarch); + stap_modify_semaphore (relocate_address (m_sem_addr, objfile), 0, gdbarch); } /* Implementation of the 'get_static_ops' method. */ |