[pushed] Fix PR gdb/22491: Regression when setting SystemTap probe semaphores

[pushed] Fix PR gdb/22491: Regression when setting SystemTap probe semaphores

[Sergio Durigan Junior]

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.
---
 gdb/stap-probe.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
index 63100c9fb3..9f4e00845a 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.  */
-- 
2.13.3

[obvious] Adding ChangeLog entry for the last commit.

[Sergio Durigan Junior]

---
 gdb/ChangeLog | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 61db1b1fa8..613a5ae6bb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+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.
+
 2017-11-25  Pedro Alves  <palves@redhat.com>
 
 	* dictionary.c: Include "safe-ctype.h".
-- 
2.13.3