[PATCH] Fix -Wstring-compare testcase build failure

[PATCH] Fix -Wstring-compare testcase build failure

[Gary Benson]

Hi all,

Clang fails to compile the file gdb/testsuite/gdb.cp/try_catch.cc
with the following error:
  warning: result of comparison against a string literal is
  unspecified (use strncmp instead) [-Wstring-compare]

This patch replaces the string literal with a pointer, to avoid
the error.

Is it ok to commit?

Cheers,
Gary

--
gdb/testsuite/ChangeLog:

	* gdb.cp/try_catch.cc: Replace string literal with a
	pointer	to avoid build failure with -Wstring-compare.
---
 gdb/testsuite/ChangeLog           | 5 +++++
 gdb/testsuite/gdb.cp/try_catch.cc | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.cp/try_catch.cc b/gdb/testsuite/gdb.cp/try_catch.cc
index 4c4add2..b0c01ba 100644
--- a/gdb/testsuite/gdb.cp/try_catch.cc
+++ b/gdb/testsuite/gdb.cp/try_catch.cc
@@ -122,14 +122,15 @@ int main()
 
   // 3 use standard library
   using namespace std;
+  const char *throwme = "gdb.1";
   try
     {
       if (j < 100)
-	throw invalid_argument("gdb.1"); // marker 3-throw
+	throw invalid_argument(throwme); // marker 3-throw
     }
   catch (exception& obj)
     {
-      if (obj.what() != "gdb.1")	// marker 3-catch
+      if (obj.what() != throwme)	// marker 3-catch
 	test &= false;
     }
   return 0;
-- 
1.8.3.1

Re: [PATCH] Fix -Wstring-compare testcase build failure

[Pedro Alves]

On 6/23/20 3:57 PM, Gary Benson via Gdb-patches wrote:
> Hi all,
> 
> Clang fails to compile the file gdb/testsuite/gdb.cp/try_catch.cc
> with the following error:
>   warning: result of comparison against a string literal is
>   unspecified (use strncmp instead) [-Wstring-compare]
> 
> This patch replaces the string literal with a pointer, to avoid
> the error.
> 
> Is it ok to commit?

Hmm, no, I don't think so.

>  
>    // 3 use standard library
>    using namespace std;
> +  const char *throwme = "gdb.1";
>    try
>      {
>        if (j < 100)
> -	throw invalid_argument("gdb.1"); // marker 3-throw
> +	throw invalid_argument(throwme); // marker 3-throw
>      }
>    catch (exception& obj)
>      {
> -      if (obj.what() != "gdb.1")	// marker 3-catch
> +      if (obj.what() != throwme)	// marker 3-catch
>  	test &= false;
>      }
>    return 0;
invalid_argument stores a copy of the string, so with or
without your patch, that (obj.what() != "gdb.1") comparison
is returning false.  Thus the "test &= false;" statement is
executing.

IOW, that Clang warning found a bug in the test program...

The fix should be to use strcmp instead.

And also, please add a market at the "return 0;" line,
let the program run to it, and then add a gdb_test checking
that "test" is still true at that point.

Thanks,
Pedro Alves

[PATCH v2] Fix -Wstring-compare testcase build failure

[Gary Benson]

Pedro Alves wrote:
> On 6/23/20 3:57 PM, Gary Benson via Gdb-patches wrote:
> > Hi all,
> > 
> > Clang fails to compile the file gdb/testsuite/gdb.cp/try_catch.cc
> > with the following error:
> >   warning: result of comparison against a string literal is
> >   unspecified (use strncmp instead) [-Wstring-compare]
> > 
> > This patch replaces the string literal with a pointer, to avoid
> > the error.
> > 
> > Is it ok to commit?
> 
> Hmm, no, I don't think so.
[snip]
> invalid_argument stores a copy of the string, so with or
> without your patch, that (obj.what() != "gdb.1") comparison
> is returning false.  Thus the "test &= false;" statement is
> executing.

Ah, I had not realized the string was copied, I think I thought
the compiler was complaining about something else.

> IOW, that Clang warning found a bug in the test program...
> 
> The fix should be to use strcmp instead.
> 
> And also, please add a market at the "return 0;" line,
> let the program run to it, and then add a gdb_test checking
> that "test" is still true at that point.

Ok.  How about this?

--
Clang fails to compile the file gdb/testsuite/gdb.cp/try_catch.cc
with the following error:
  warning: result of comparison against a string literal is
  unspecified (use strncmp instead) [-Wstring-compare]

This commit fixes the error, replacing the pointer comparison with
a call to strcmp.  This commit also adds a final check: the test
program is run to the final return statement, and the value of
"test" is checked to ensure it is still "true" at that point.

gdb/testsuite/ChangeLog:

	* gdb.cp/try_catch.cc: Include string.h.
	(main): Replace comparison against string literal with
	strcmp, avoiding build failure with -Wstring-compare.
	Add "marker test-complete".
	* gdb.cp/try_catch.exp: Run the test to the above marker,
	then verify that the value of "test" is still true.
---
 gdb/testsuite/ChangeLog            | 9 +++++++++
 gdb/testsuite/gdb.cp/try_catch.cc  | 5 +++--
 gdb/testsuite/gdb.cp/try_catch.exp | 3 +++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.cp/try_catch.cc b/gdb/testsuite/gdb.cp/try_catch.cc
index 4c4add2..5f454f4 100644
--- a/gdb/testsuite/gdb.cp/try_catch.cc
+++ b/gdb/testsuite/gdb.cp/try_catch.cc
@@ -18,6 +18,7 @@
 #include <exception>
 #include <stdexcept>
 #include <string>
+#include <string.h>
 
 enum region { oriental, egyptian, greek, etruscan, roman };
 
@@ -129,8 +130,8 @@ int main()
     }
   catch (exception& obj)
     {
-      if (obj.what() != "gdb.1")	// marker 3-catch
+      if (strcmp (obj.what(), "gdb.1") != 0)	// marker 3-catch
 	test &= false;
     }
-  return 0;
+  return 0;	// marker test-complete
 }
diff --git a/gdb/testsuite/gdb.cp/try_catch.exp b/gdb/testsuite/gdb.cp/try_catch.exp
index fb53294..e4da885 100644
--- a/gdb/testsuite/gdb.cp/try_catch.exp
+++ b/gdb/testsuite/gdb.cp/try_catch.exp
@@ -63,5 +63,8 @@ gdb_continue_to_breakpoint "marker 3-throw"
 gdb_breakpoint [gdb_get_line_number "marker 3-catch"]
 gdb_continue_to_breakpoint "marker 3-catch"
 
+gdb_breakpoint [gdb_get_line_number "marker test-complete"]
+gdb_test "p test" "= true"
+
 gdb_exit
 return 0
-- 
1.8.3.1

Re: [PATCH v2] Fix -Wstring-compare testcase build failure

[Pedro Alves]

On 6/26/20 2:05 PM, Gary Benson wrote:

> diff --git a/gdb/testsuite/gdb.cp/try_catch.exp b/gdb/testsuite/gdb.cp/try_catch.exp
> index fb53294..e4da885 100644
> --- a/gdb/testsuite/gdb.cp/try_catch.exp
> +++ b/gdb/testsuite/gdb.cp/try_catch.exp
> @@ -63,5 +63,8 @@ gdb_continue_to_breakpoint "marker 3-throw"
>  gdb_breakpoint [gdb_get_line_number "marker 3-catch"]
>  gdb_continue_to_breakpoint "marker 3-catch"
>  
> +gdb_breakpoint [gdb_get_line_number "marker test-complete"]
> +gdb_test "p test" "= true"
> +

You need to continue the program to the breakpoint too.
I.e., you're missing a

  gdb_continue_to_breakpoint "marker test-complete"

line.

OK with that change.

Thanks,
Pedro Alves

Re: [PATCH v2] Fix -Wstring-compare testcase build failure

[Gary Benson]

Pedro Alves wrote:
> On 6/26/20 2:05 PM, Gary Benson wrote:
> 
> > diff --git a/gdb/testsuite/gdb.cp/try_catch.exp b/gdb/testsuite/gdb.cp/try_catch.exp
> > index fb53294..e4da885 100644
> > --- a/gdb/testsuite/gdb.cp/try_catch.exp
> > +++ b/gdb/testsuite/gdb.cp/try_catch.exp
> > @@ -63,5 +63,8 @@ gdb_continue_to_breakpoint "marker 3-throw"
> >  gdb_breakpoint [gdb_get_line_number "marker 3-catch"]
> >  gdb_continue_to_breakpoint "marker 3-catch"
> >  
> > +gdb_breakpoint [gdb_get_line_number "marker test-complete"]
> > +gdb_test "p test" "= true"
> > +
> 
> You need to continue the program to the breakpoint too.
> I.e., you're missing a
> 
>   gdb_continue_to_breakpoint "marker test-complete"
> 
> line.
> 
> OK with that change.

Fixed and pushed, thanks Pedro.

Cheers,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat