Re: [PATCH v2 2/2] Fix handling of null stap semaphores

[Simon Marchi <simark at simark dot ca>]

Thanks, I put a couple of comments.  The patch LGTM with those addressed.

On 2020-01-02 6:12 p.m., George Barrett wrote:
> @@ -47,6 +53,18 @@ proc stap_test {exec_name {arg ""}} {
>  	fail "run to -pstap test:user"
>      }
>  
> +    if {[string first "-DUSE_SEMAPHORES" $args] == -1} {
> +	set relocation_base \
> +	   [expr [get_hexadecimal_valueof "&relocation_marker" "0"] - $semaphore_addr_var]
> +	if {$relocation_base == 0} {
> +	    xfail "skipping null semaphore test: no relocation performed"

Hmm, I probably wouldn't use "xfail" here, since it's not like something is
failing, it's just that this test doesn't apply to this configuration.  What
about just not mentioning at all when it does not apply?

  if {$relocation_base != 0} {
    gdb_test ...
  }

> +	} else {
> +	   gdb_test "p (*(char*) $relocation_base)@4" \
> +		" = \"\\\\177ELF\"" \
> +		"null semaphore relocation"

This test is a bit out of the ordinary, and I can imagine readers being
confused as to why we are testing this.  Could you please add a comment
(just a short sentence) above this to hint to why we are doing this test?
Something like (feel free to improve the formulation):

# Check that GDB doesn't wrongfully try setting null semaphore, and doing so
# overwriting the ELF magic number.

Simon