[PATCH] Skip gdb.base/break.exp catchpoint tests without catchpoints [PR34535]

[PATCH] Skip gdb.base/break.exp catchpoint tests without catchpoints [PR34535]

[Rainer Orth]

[-- Attachment #1: Type: text/plain, Size: 910 bytes --]

As detailed in PR PR testsuite/34535, the gdb.base/break.exp test FAILs
on Solaris:

FAIL: gdb.base/break.exp: test_no_break_on_catchpoint: continue until exit (the program exited)

warning: Error inserting catchpoint 2: Your system does not support this type of catchpoint.
warning: Error inserting catchpoint 3: Your system does not support this type of catchpoint.
warning: Error inserting catchpoint 4: Your system does not support this type of catchpoint.

This patch checks for that condition and skips the test if necessary.

Tested on x86_64-pc-solaris2.11, sparcv9-sun-solaris2.11, and
x86_64-pc-linux-gnu.

Ok for trunk?


While this patch avoids the failure, I wonder if there's some document
on what it takes to actually implement catchpoints.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sol2-break-no-catchpoint.patch --]
[-- Type: text/x-patch, Size: 1249 bytes --]

# HG changeset patch
# Parent  8ed1f8219b70efa33fd9525699d984918e07bc4c
Skip gdb.base/break.exp catchpoint tests without catchpoints [PR34535]

diff --git a/gdb/testsuite/gdb.base/break.exp b/gdb/testsuite/gdb.base/break.exp
--- a/gdb/testsuite/gdb.base/break.exp
+++ b/gdb/testsuite/gdb.base/break.exp
@@ -455,6 +455,33 @@ test_tbreak
 # inappropriately.  (There are no calls to those system functions
 # in this test program.)
 
+set fork_catchpoints_supported 0
+
+proc probe_fork_catchpoint {} {
+    clean_restart break
+
+    if {![runto_main]} {
+	return
+    }
+
+    gdb_test "catch fork" "Catchpoint.*fork.*"
+
+    gdb_run_cmd
+
+    set test "probe fork catchpoint"
+    gdb_test_multiple "" $test {
+	-re "Your system does not support this type.*" {
+	    unsupported $test
+	}
+	-re "$::gdb_prompt $" {
+	    set ::fork_catchpoints_supported 1
+	    pass $test
+	}
+    }
+}
+
+probe_fork_catchpoint
+
 proc_with_prefix test_no_break_on_catchpoint {} {
     clean_restart break
 
@@ -462,6 +489,10 @@ proc_with_prefix test_no_break_on_catchp
 	return
     }
 
+    if {!$::fork_catchpoints_supported} {
+	return
+    }
+
     gdb_test "catch fork" "Catchpoint ${::decimal} \\(fork\\)" \
 	"set catch fork, never expected to trigger"
 

Re: [PATCH] Skip gdb.base/break.exp catchpoint tests without catchpoints [PR34535]

[Tom Tromey]

>>>>> "Rainer" == Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

Rainer> As detailed in PR PR testsuite/34535, the gdb.base/break.exp test FAILs
Rainer> on Solaris:

Thanks for the patch.

Rainer> While this patch avoids the failure, I wonder if there's some document
Rainer> on what it takes to actually implement catchpoints.

There's not really docs on anything.  However, most catchpoints are
implemented in break-catch-*.c.  Looking at break-catch-fork.c:

int
fork_catchpoint::insert_location (struct bp_location *bl)
{
  if (is_vfork)
    return target_insert_vfork_catchpoint (inferior_ptid.pid ());
  else
    return target_insert_fork_catchpoint (inferior_ptid.pid ());
}

So basically the target has to implement the insert_fork_catchpoint
target method; then arrange to tell gdb about TARGET_WAITKIND_FORKED
when appropriate.

Unfortunately the target methods don't have comments; while we try to
add descriptions nowadays, in the past gdb was lax about this :(
But there's a "paragraph" of them in target.h that you'd probably need
to handle:

    virtual int insert_fork_catchpoint (int)
      TARGET_DEFAULT_RETURN (1);
    virtual int remove_fork_catchpoint (int)
      TARGET_DEFAULT_RETURN (1);
    virtual int insert_vfork_catchpoint (int)
      TARGET_DEFAULT_RETURN (1);
    virtual int remove_vfork_catchpoint (int)
      TARGET_DEFAULT_RETURN (1);
    virtual void follow_fork (inferior *, ptid_t, target_waitkind, bool, bool)
      TARGET_DEFAULT_FUNC (default_follow_fork);

Rainer> +    set test "probe fork catchpoint"
Rainer> +    gdb_test_multiple "" $test {
Rainer> +	-re "Your system does not support this type.*" {
Rainer> +	    unsupported $test
Rainer> +	}
Rainer> +	-re "$::gdb_prompt $" {
Rainer> +	    set ::fork_catchpoints_supported 1
Rainer> +	    pass $test
Rainer> +	}
Rainer> +    }
Rainer> +}
 
Rainer> +    if {!$::fork_catchpoints_supported} {
Rainer> +	return
Rainer> +    }
Rainer> +
Rainer>      gdb_test "catch fork" "Catchpoint ${::decimal} \\(fork\\)" \
Rainer>  	"set catch fork, never expected to trigger"
 
I think it would be better to just use gdb_test_multiple here, and if
the "not supported" case is found, just return here.  That would avoid
starting another gdb.

thanks,
Tom

[PATCH v2] Handle missing catchpoint support in gdb.base/break.exp [PR34535]

[Rainer Orth]

[-- Attachment #1: Type: text/plain, Size: 2366 bytes --]

Hi Tom,

> Rainer> While this patch avoids the failure, I wonder if there's some document
> Rainer> on what it takes to actually implement catchpoints.
>
> There's not really docs on anything.  However, most catchpoints are
> implemented in break-catch-*.c.  Looking at break-catch-fork.c:

thanks for the explanation.  I may give it a try, but probably will go
for lower-hanging fruit first.  In the past, I've repeatedly failed
miserably with patches requiring to dive more deeply into gdb...

> Rainer> +    set test "probe fork catchpoint"
> Rainer> +    gdb_test_multiple "" $test {
> Rainer> +	-re "Your system does not support this type.*" {
> Rainer> +	    unsupported $test
> Rainer> +	}
> Rainer> +	-re "$::gdb_prompt $" {
> Rainer> +	    set ::fork_catchpoints_supported 1
> Rainer> +	    pass $test
> Rainer> +	}
> Rainer> +    }
> Rainer> +}
>  
> Rainer> +    if {!$::fork_catchpoints_supported} {
> Rainer> +	return
> Rainer> +    }
> Rainer> +
> Rainer>      gdb_test "catch fork" "Catchpoint ${::decimal} \\(fork\\)" \
> Rainer>  	"set catch fork, never expected to trigger"
>  
> I think it would be better to just use gdb_test_multiple here, and if
> the "not supported" case is found, just return here.  That would avoid
> starting another gdb.

Good idea.  I did it with the separate test before because the errors
only occur on the "continue" after setting the catchpoints.

So here's the revision: it certainly makes things clearer and simpler.
I think it's better to go for UNSUPPORTED here instead of just skipping
the rest of the test.

---------------------------------------------------------------------------

As detailed in PR PR testsuite/34535, the gdb.base/break.exp test FAILs
on Solaris:

FAIL: gdb.base/break.exp: test_no_break_on_catchpoint: continue until exit (the program exited)

warning: Error inserting catchpoint 2: Your system does not support this type of catchpoint.
warning: Error inserting catchpoint 3: Your system does not support this type of catchpoint.
warning: Error inserting catchpoint 4: Your system does not support this type of catchpoint.

This patch handles that condition.

Tested on x86_64-pc-solaris2.11, sparcv9-sun-solaris2.11, and
x86_64-pc-linux-gnu.

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sol2-break-no-catchpoint.patch --]
[-- Type: text/x-patch, Size: 766 bytes --]

# HG changeset patch
# Parent  d1c17963050f4ebaae63ff7cfb1eb8e5c5d7fc77
Skip gdb.base/break.exp catchpoint tests without catchpoints [PR34535]

diff --git a/gdb/testsuite/gdb.base/break.exp b/gdb/testsuite/gdb.base/break.exp
--- a/gdb/testsuite/gdb.base/break.exp
+++ b/gdb/testsuite/gdb.base/break.exp
@@ -471,7 +471,14 @@ proc_with_prefix test_no_break_on_catchp
     gdb_test "catch exec" "Catchpoint ${::decimal} \\(exec\\)" \
 	"set catch exec, never expected to trigger"
 
-    gdb_continue_to_end
+    gdb_test_multiple "continue" "" {
+	-re "Your system does not support this type.*" {
+	    unsupported "target does not support this type of catchpoint"
+	}
+	-re "Continuing." {
+	    pass "continue until exit"
+	}
+    }
 }
 
 test_no_break_on_catchpoint

Re: [PATCH v2] Handle missing catchpoint support in gdb.base/break.exp [PR34535]

[Tom Tromey]

>>>>> "Rainer" == Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

Rainer> So here's the revision: it certainly makes things clearer and simpler.
Rainer> I think it's better to go for UNSUPPORTED here instead of just skipping
Rainer> the rest of the test.

Thanks.  This seems reasonable to me.
Approved-By: Tom Tromey <tom@tromey.com>

Tom