[RFA] gdb/13483

[RFA] gdb/13483

[Keith Seitz]

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

Hi,

The bug in $SUBJECT talks about changing gdb to allow certain 
conversions to boolean during function calls.

The attached patch changes rank_one_type to fully implement the C++ 
standard [as of n3290 draft] for boolean conversions (section 4.12.1, 
conv.bool). I've re-purposed the BOOL_PTR_CONVERSION_BADNESS and changed 
that to a more generic BOOL_CONVERSION_BADNESS which includes all of 
these cases.

[No regressions on x86_64 native and native-gdbserver.]

Comments/questions/concerns?
Keith

ChangeLog
2012-08-31  Keith Seitz  <keiths@redhat.com>

	PR gdb/13483
	* gdbtypes.h (BOOL_PTR_CONVERSION_BADNESS): Rename to ...
	(BOOL_CONVERSION_BADNESS): ... this.
	* gdbtypes.c (BOOL_PTR_CONVERSION_BADNESS): Likewise.
	(rank_one_type): Allow all boolean conversions
	permitted by the standard.

testsuite/ChangeLog
2012-08-31  Keith Seitz  <keiths@redhat.com>

	PR gdb/13483
	* gdb.cp/converts.cc (A::A): Add ctor.
	(A::member_): Add member.
	(enum my_enum): New enumeration.
	(main): Add calls to foo1_7 with various
	permitted arguments.
	* gdb.cp/converts.exp: Add tests for boolean
	conversions permitted by the standard.


[-- Attachment #2: 13483.patch --]
[-- Type: text/x-patch, Size: 4679 bytes --]

diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 991026b..73fcbb1 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1603,8 +1603,8 @@ extern const struct rank FLOAT_CONVERSION_BADNESS;
 extern const struct rank INT_FLOAT_CONVERSION_BADNESS;
 /* Badness of conversion of pointer to void pointer.  */
 extern const struct rank VOID_PTR_CONVERSION_BADNESS;
-/* Badness of conversion of pointer to boolean.  */
-extern const struct rank BOOL_PTR_CONVERSION_BADNESS;
+/* Badness of conversion to boolean.  */
+extern const struct rank BOOL_CONVERSION_BADNESS;
 /* Badness of converting derived to base class.  */
 extern const struct rank BASE_CONVERSION_BADNESS;
 /* Badness of converting from non-reference to reference.  */
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 21d9043..8ac8799 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -54,7 +54,7 @@ const struct rank INTEGER_CONVERSION_BADNESS = {2,0};
 const struct rank FLOAT_CONVERSION_BADNESS = {2,0};
 const struct rank INT_FLOAT_CONVERSION_BADNESS = {2,0};
 const struct rank VOID_PTR_CONVERSION_BADNESS = {2,0};
-const struct rank BOOL_PTR_CONVERSION_BADNESS = {3,0};
+const struct rank BOOL_CONVERSION_BADNESS = {3,0};
 const struct rank BASE_CONVERSION_BADNESS = {2,0};
 const struct rank REFERENCE_CONVERSION_BADNESS = {2,0};
 const struct rank NULL_POINTER_CONVERSION_BADNESS = {2,0};
@@ -2718,14 +2718,23 @@ rank_one_type (struct type *parm, struct type *arg, struct value *value)
     case TYPE_CODE_BOOL:
       switch (TYPE_CODE (arg))
 	{
+	  /* n3290 draft, section 4.12.1 (conv.bool):
+
+	     "A prvalue of arithmetic, unscoped enumeration, pointer, or
+	     pointer to member type can be converted to a prvalue of type
+	     bool.  A zero value, null pointer value, or null member pointer
+	     value is converted to false; any other value is converted to
+	     true.  A prvalue of type std::nullptr_t can be converted to a
+	     prvalue of type bool; the resulting value is false."  */
 	case TYPE_CODE_INT:
 	case TYPE_CODE_CHAR:
-	case TYPE_CODE_RANGE:
 	case TYPE_CODE_ENUM:
 	case TYPE_CODE_FLT:
-	  return INCOMPATIBLE_TYPE_BADNESS;
+	case TYPE_CODE_MEMBERPTR:
 	case TYPE_CODE_PTR:
-	  return BOOL_PTR_CONVERSION_BADNESS;
+	  return BOOL_CONVERSION_BADNESS;
+	case TYPE_CODE_RANGE:
+	  return INCOMPATIBLE_TYPE_BADNESS;
 	case TYPE_CODE_BOOL:
 	  return EXACT_MATCH_BADNESS;
 	default:
diff --git a/gdb/testsuite/gdb.cp/converts.cc b/gdb/testsuite/gdb.cp/converts.cc
index ecebbf8..1cda43d 100644
--- a/gdb/testsuite/gdb.cp/converts.cc
+++ b/gdb/testsuite/gdb.cp/converts.cc
@@ -1,10 +1,17 @@
-class A {};
+class A
+{
+public:
+  A() : member_ (0) {};
+  int member_;
+};
 class B : public A {};
 
 typedef A TA1;
 typedef A TA2;
 typedef TA2 TA3;
 
+enum my_enum {MY_A, MY_B, MY_C, MY_D};
+
 int foo0_1 (TA1)  { return 1; }
 int foo0_2 (TA3)  { return 2; }
 int foo0_3 (A***) { return 3; }
@@ -62,6 +69,21 @@ int main()
   foo2_3 (b);        // ..array of pointers
   foo2_4 ((int**)b); // ..array of wrong pointers
 
+  // X to boolean conversions allowed by the standard
+  int integer = 0;
+  long long_int = 1;
+  float fp = 1.0;
+  double dp = 1.0;
+  foo1_7 (integer);		// integer to boolean
+  foo1_7 (long_int);		// long to boolean
+  foo1_7 (*a);			// char to boolean
+  foo1_7 (MY_A);		// unscoped enum to boolean
+  foo1_7 (&foo1_7);		// pointer to boolean
+  foo1_7 (&A::member_);		// pointer to member to boolean
+  foo1_7 (a);			// pointer to boolean
+  foo1_7 (fp);			// float to boolean
+  foo1_7 (dp);			// double  to boolean
+
   foo3_1 (0, 0);
   foo3_2 (0, static_cast<char const**> (0));
   foo3_2 (0, 0);
diff --git a/gdb/testsuite/gdb.cp/converts.exp b/gdb/testsuite/gdb.cp/converts.exp
index 414e2d0..148ac0f 100644
--- a/gdb/testsuite/gdb.cp/converts.exp
+++ b/gdb/testsuite/gdb.cp/converts.exp
@@ -77,6 +77,21 @@ gdb_test "p foo3_1 (0, (const char**) 1)" " = 31"
 gdb_test "p foo3_2 (0, 0)" "= 32"
 gdb_test "p foo3_2 (0, (char const**) 0)" " = 320"
 
+# Tests for boolean conversions allowed by the standard
+gdb_test "p foo1_7(0)" " = 17"
+gdb_test "p foo1_7(integer)" " = 17"
+gdb_test "p foo1_7(long_int)" " = 17"
+gdb_test "p foo1_7(*a)" " = 17"
+gdb_test "p foo1_7(MY_A)" " = 17"
+gdb_test "p foo1_7(&foo1_7)" " = 17"
+gdb_test "p foo1_7(&A::member_)" " = 17"
+gdb_test "p foo1_7(a)" " = 17"
+gdb_test "p foo1_7(fp)" " = 17"
+gdb_test "p foo1_7(dp)" " = 17"
+gdb_test "p foo1_7(-1.23)" " = 17"
+gdb_test "p foo1_7(ta)" \
+    "Cannot resolve function foo1_7 to any overloaded instance"
+
 # Test for strict type checking
 set error_str "Cannot resolve function %s to any overloaded instance"
 gdb_test "show check type" "Strict type checking is on\."

Re: [RFA] gdb/13483

[Tom Tromey]

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> 2012-08-31  Keith Seitz  <keiths@redhat.com>
Keith> 	PR gdb/13483
Keith> 	* gdbtypes.h (BOOL_PTR_CONVERSION_BADNESS): Rename to ...
Keith> 	(BOOL_CONVERSION_BADNESS): ... this.
Keith> 	* gdbtypes.c (BOOL_PTR_CONVERSION_BADNESS): Likewise.
Keith> 	(rank_one_type): Allow all boolean conversions
Keith> 	permitted by the standard.

Keith> 2012-08-31  Keith Seitz  <keiths@redhat.com>
Keith> 	PR gdb/13483
Keith> 	* gdb.cp/converts.cc (A::A): Add ctor.
Keith> 	(A::member_): Add member.
Keith> 	(enum my_enum): New enumeration.
Keith> 	(main): Add calls to foo1_7 with various
Keith> 	permitted arguments.
Keith> 	* gdb.cp/converts.exp: Add tests for boolean
Keith> 	conversions permitted by the standard.

This is ok.  Thanks.

Tom

Re: [RFA] gdb/13483

[Keith Seitz]

On 09/10/2012 08:31 AM, Tom Tromey wrote:
>>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
>
> Keith> 2012-08-31  Keith Seitz  <keiths@redhat.com>
> Keith> 	PR gdb/13483
> Keith> 	* gdbtypes.h (BOOL_PTR_CONVERSION_BADNESS): Rename to ...
> Keith> 	(BOOL_CONVERSION_BADNESS): ... this.
> Keith> 	* gdbtypes.c (BOOL_PTR_CONVERSION_BADNESS): Likewise.
> Keith> 	(rank_one_type): Allow all boolean conversions
> Keith> 	permitted by the standard.
>
> Keith> 2012-08-31  Keith Seitz  <keiths@redhat.com>
> Keith> 	PR gdb/13483
> Keith> 	* gdb.cp/converts.cc (A::A): Add ctor.
> Keith> 	(A::member_): Add member.
> Keith> 	(enum my_enum): New enumeration.
> Keith> 	(main): Add calls to foo1_7 with various
> Keith> 	permitted arguments.
> Keith> 	* gdb.cp/converts.exp: Add tests for boolean
> Keith> 	conversions permitted by the standard.
>
> This is ok.  Thanks.
>

Thank you for reviewing this. It has been committed.

Keith

Re: Testsuite regression gdb.cp/converts.exp [Re: [RFA] gdb/13483]

[Keith Seitz]

On 09/10/2012 10:45 PM, Jan Kratochvil wrote:
> on CentOS-5:
>
>   Running gdb/testsuite/gdb.cp/converts.exp ...
> -PASS: gdb.cp/converts.exp: continue to breakpoint: end of main
> [...]
> -PASS: gdb.cp/converts.exp: p foo3_2 (1,1)
> +gdb compile failed, gdb/testsuite/gdb.cp/converts.cc: In function 'int main()':
> +gdb/testsuite/gdb.cp/converts.cc:81: warning: the address of 'int foo1_7(bool)', will always evaluate as 'true'
> +UNTESTED: gdb.cp/converts.exp: converts.exp

My guess is an older compiler. Since none of these function calls is 
really necessary (well, one probably is necessary to make sure foo1_7 
isn't optimized away), I propose just deleting the offending line.

What do you think?
Keith

Re: Testsuite regression gdb.cp/converts.exp [Re: [RFA] gdb/13483]

[Jan Kratochvil]

On Wed, 12 Sep 2012 21:38:12 +0200, Keith Seitz wrote:
> On 09/10/2012 10:45 PM, Jan Kratochvil wrote:
> >on CentOS-5:
> >
> >  Running gdb/testsuite/gdb.cp/converts.exp ...
> >-PASS: gdb.cp/converts.exp: continue to breakpoint: end of main
> >[...]
> >-PASS: gdb.cp/converts.exp: p foo3_2 (1,1)
> >+gdb compile failed, gdb/testsuite/gdb.cp/converts.cc: In function 'int main()':
> >+gdb/testsuite/gdb.cp/converts.cc:81: warning: the address of 'int foo1_7(bool)', will always evaluate as 'true'
> >+UNTESTED: gdb.cp/converts.exp: converts.exp
> 
> My guess is an older compiler. Since none of these function calls is
> really necessary (well, one probably is necessary to make sure
> foo1_7 isn't optimized away), I propose just deleting the offending
> line.

Yes, I agree.  Although rather to replace it in .c by an explanatory comment
like

/* .exp file contains line
     gdb_test "p foo1_7(&foo1_7)" " = 17"
   which is not compiled here for verification as older GCCs (~4.1) fail the
   compilation due to:
     warning: the address of 'int foo1_7(bool)', will always evaluate as 'true'
   */
  

Thanks,
Jan

Re: Testsuite regression gdb.cp/converts.exp [Re: [RFA] gdb/13483]

[Keith Seitz]

On 09/12/2012 12:58 PM, Jan Kratochvil wrote:
> Yes, I agree.  Although rather to replace it in .c by an explanatory comment
> like
>
> /* .exp file contains line
>       gdb_test "p foo1_7(&foo1_7)" " = 17"
>     which is not compiled here for verification as older GCCs (~4.1) fail the
>     compilation due to:
>       warning: the address of 'int foo1_7(bool)', will always evaluate as 'true'
>     */

Sure. For the record, here is what I committed:

2012-09-12  Keith Seitz  <keiths@redhat.com>

	* gdb.cp/converts.cc (main): Comment out the pointer to boolean
	conversion statement.

Index: gdb.cp/converts.cc
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/converts.cc,v
retrieving revision 1.5
diff -u -p -r1.5 converts.cc
--- gdb.cp/converts.cc	10 Sep 2012 17:12:52 -0000	1.5
+++ gdb.cp/converts.cc	12 Sep 2012 23:27:03 -0000
@@ -78,7 +78,15 @@ int main()
    foo1_7 (long_int);		// long to boolean
    foo1_7 (*a);			// char to boolean
    foo1_7 (MY_A);		// unscoped enum to boolean
+  /* converts.exp tests the next statement directly.  It is not compiled
+     here for verification because older versions of GCC (~4.1) fail to
+     compile it:
+
+     warning: the address of 'int foo1_7(bool)' will always evaluate as 
true
+
    foo1_7 (&foo1_7);		// pointer to boolean
+  */
+
    foo1_7 (&A::member_);		// pointer to member to boolean
    foo1_7 (a);			// pointer to boolean
    foo1_7 (fp);			// float to boolean

Keith

[commit] gdb.cp/converts.exp FAIL on gcc-4.1 [Re: Testsuite regression gdb.cp/converts.exp [Re: [RFA] gdb/13483]]

[Jan Kratochvil]

On Thu, 13 Sep 2012 01:29:20 +0200, Keith Seitz wrote:
> Sure. For the record, here is what I committed:
> 
> 2012-09-12  Keith Seitz  <keiths@redhat.com>
> 
> 	* gdb.cp/converts.cc (main): Comment out the pointer to boolean
> 	conversion statement.

It compiles OK now but it FAILs on:
	p foo1_7(MY_A)^M
	No symbol "MY_A" in current context.^M
	(gdb) FAIL: gdb.cp/converts.exp: p foo1_7(MY_A)

Checked in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2012-09/msg00074.html

--- src/gdb/testsuite/ChangeLog	2012/09/14 21:23:45	1.3373
+++ src/gdb/testsuite/ChangeLog	2012/09/16 17:30:47	1.3374
@@ -1,3 +1,8 @@
+2012-09-16  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	Fix compatibility with old GCC (~4.1).
+	* gdb.cp/converts.cc (my_enum_var): New variable.
+
 2012-09-14  Andrew Burgess  <aburgess@broadcom.com>
 
 	* gdb.xml/tdesc-regs.exp: Update expected output for new
--- src/gdb/testsuite/gdb.cp/converts.cc	2012/09/12 23:29:05	1.6
+++ src/gdb/testsuite/gdb.cp/converts.cc	2012/09/16 17:30:48	1.7
@@ -12,6 +12,10 @@
 
 enum my_enum {MY_A, MY_B, MY_C, MY_D};
 
+/* Without this variable older 'enum my_enum' incl. its 'MY_A' would be omitted
+   by older versions of GCC (~4.1) failing the testcase using it below.  */
+enum my_enum my_enum_var;
+
 int foo0_1 (TA1)  { return 1; }
 int foo0_2 (TA3)  { return 2; }
 int foo0_3 (A***) { return 3; }