[v3,0/2] Fix handling of null stap semaphores

Message ID vtdp7dvkr_t0ijbgqvjhnb9-407cwj/l1_.qb_h7/8yfx8&/5blw@mail.bob131.so
Headers show
Series
  • Fix handling of null stap semaphores
Related show

Message

George Barrett Jan. 10, 2020, 7:32 p.m.
GDB's handling of userspace SystemTap probes is currently inconsistent
with the specification of the format[0]: the absence of a semaphore for
a particular probe is recorded within its ELF note by setting the
address value to zero, but the current implementation doesn't check the
value against zero until after a relocation has been applied.

The primary patch in this set is #2 ('Fix handling of null stap
semaphores'). Writing the test case revealed some issues with other
stap-related tests, fixes for which have been split into patch #1 as
suggested in a previous review.

v3 of this patchset carries changes to the test case suggested in a
previous review, namely the removal of an xfail directive and an
additional explanatory comment.

[0]: https://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation

George Barrett (2):
  gdb/testsuite/gdb.base/stap-probe: Minor clean-up
  Fix handling of null stap semaphores

 gdb/stap-probe.c                      |  7 ++--
 gdb/testsuite/gdb.base/stap-probe.c   |  4 ++-
 gdb/testsuite/gdb.base/stap-probe.exp | 47 ++++++++++++++++++++++-----
 3 files changed, 45 insertions(+), 13 deletions(-)

-- 
2.24.1

Comments

Simon Marchi Jan. 10, 2020, 7:51 p.m. | #1
On 2020-01-10 2:32 p.m., George Barrett wrote:
> GDB's handling of userspace SystemTap probes is currently inconsistent

> with the specification of the format[0]: the absence of a semaphore for

> a particular probe is recorded within its ELF note by setting the

> address value to zero, but the current implementation doesn't check the

> value against zero until after a relocation has been applied.

> 

> The primary patch in this set is #2 ('Fix handling of null stap

> semaphores'). Writing the test case revealed some issues with other

> stap-related tests, fixes for which have been split into patch #1 as

> suggested in a previous review.

> 

> v3 of this patchset carries changes to the test case suggested in a

> previous review, namely the removal of an xfail directive and an

> additional explanatory comment.

> 

> [0]: https://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation

> 

> George Barrett (2):

>   gdb/testsuite/gdb.base/stap-probe: Minor clean-up

>   Fix handling of null stap semaphores

> 

>  gdb/stap-probe.c                      |  7 ++--

>  gdb/testsuite/gdb.base/stap-probe.c   |  4 ++-

>  gdb/testsuite/gdb.base/stap-probe.exp | 47 ++++++++++++++++++++++-----

>  3 files changed, 45 insertions(+), 13 deletions(-)

> 

> -- 

> 2.24.1

> 


Thanks, this LGTM.  I can't recall, do you have write access on the repo?

Simon
George Barrett Jan. 10, 2020, 7:52 p.m. | #2
On Fri, Jan 10, 2020 at 02:51:03PM -0500, Simon Marchi wrote:
> Thanks, this LGTM.  I can't recall, do you have write access on the repo?


I don't, no. I'd very much appreciate if you could push it on my behalf.

Thanks
Simon Marchi Jan. 10, 2020, 8 p.m. | #3
On 2020-01-10 2:52 p.m., George Barrett wrote:
> On Fri, Jan 10, 2020 at 02:51:03PM -0500, Simon Marchi wrote:

>> Thanks, this LGTM.  I can't recall, do you have write access on the repo?

> 

> I don't, no. I'd very much appreciate if you could push it on my behalf.

> 

> Thanks

> 


Done, and thanks again for updating the test, it's very appreciated!

Simon