public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v1] Fix of default lookup for "this" symbol.
@ 2016-03-09 14:32 Walfred Tedeschi
  2016-04-05 11:32 ` Yao Qi
  0 siblings, 1 reply; 5+ messages in thread
From: Walfred Tedeschi @ 2016-03-09 14:32 UTC (permalink / raw)
  To: palves, brobecker; +Cc: gdb-patches, Walfred Tedeschi

Using the default lookup for the symbol "this" might lead to segmentation
fault in GDB.
Some languages, e.g. Fortran, use as default lookup routine the C++
routines.
For those languages "this" can be the instance of a class or even the
definition of a class.
When an instance of a class having the name "this" is evaluated
in GDB a segmentation fault was observed.

As example of the issue take into consideration the Fortran code:
  type foo
    real :: a
    type(bar) :: x
    character*7 :: b
  end type foo
  type(foo) :: this

Issue appears when evaluating the variable "this" in GDB.

Within the language definition structure there is a field that represents
the name of the special symbol used for the C++ "this" for the language
being described.
The fix presented here takes into account the aforementioned field. In the
case the aforementioned field is NULL "this" is not represented in the
language described and the lookup should return a null_block_symbol.

2016-03-09  Walfred Tedeschi  <walfred.tedeschi@intel.com>

gdb/ChangeLog:

	* cp_lookup_bare_symbol (cp_lookup_bare_symbol): Add check for the
	name_of_this in order to return a null symbol when looking up
	for "this".

gdb/testsuite/ChangeLog:

	* gdb.fortran/derived-types.exp (result_line, result_line_2):
	New variables.
	(print this%a, print this%b, print this): New tests.
	* gdb.fortran/derived-types.f90 (this): New object and
	initialization.

---
 gdb/cp-namespace.c                         |  7 ++++++
 gdb/testsuite/gdb.fortran/derived-type.exp | 35 ++++++++++++++++++++++++++++--
 gdb/testsuite/gdb.fortran/derived-type.f90 |  7 +++++-
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 72002d6..b9d7dbe 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -210,6 +210,13 @@ cp_lookup_bare_symbol (const struct language_defn *langdef,
       if (lang_this.symbol == NULL)
 	return null_block_symbol;
 
+      /* This whole routine is also the default path for several languages.
+         In some languages "this" is not a special symbol,
+         i.e. LA_NAME_OF_THIS is NULL.
+         For those cases we should return a null_block_symbol.  */
+      if (langdef != NULL && langdef->la_name_of_this == NULL)
+	return null_block_symbol;
+
       type = check_typedef (TYPE_TARGET_TYPE (SYMBOL_TYPE (lang_this.symbol)));
       /* If TYPE_NAME is NULL, abandon trying to find this symbol.
 	 This can happen for lambda functions compiled with clang++,
diff --git a/gdb/testsuite/gdb.fortran/derived-type.exp b/gdb/testsuite/gdb.fortran/derived-type.exp
index f650352..acccf3b 100644
--- a/gdb/testsuite/gdb.fortran/derived-type.exp
+++ b/gdb/testsuite/gdb.fortran/derived-type.exp
@@ -74,14 +74,45 @@ gdb_test_multiple $test $test {
 gdb_test "print q%x%c" "\\$\[0-9\]+ = 1"
 gdb_test "print q%x%d" "\\$\[0-9\]+ = 2\\.375"
 
+set result_line "= \\( a = 3.125, x = \\( c = 1, d = 2\\.375 \\),\
+b = 'abcdefg' \\)\r\n$gdb_prompt $"
+
+# Used in case compiler generates an array of characters.
+set result_line_2 " = \\( a = 3.125, x = \\( 1, 2\\.375 \\),\
+b = \\('abcdefg'\\) \\)\r\n$gdb_prompt $"
+
 set test "print q"
 gdb_test_multiple $test $test {
-    -re "\\$\[0-9\]+ = \\( a = 3.125, x = \\( c = 1, d = 2\\.375 \\), b = 'abcdefg' \\)\r\n$gdb_prompt $" {
+    -re $result_line {
 	pass $test
     }
-    -re "\\$\[0-9\]+ = \\( a = 3.125, x = \\( 1, 2\\.375 \\), b = \\('abcdefg'\\) \\)\r\n$gdb_prompt $" {
+    -re $result_line_2 {
 	# Compiler should produce string, not an array of characters.
 	setup_xfail "*-*-*"
 	fail $test
     }
 }
+
+gdb_test "print this%a" " = 3\\.125"
+
+set test "print this%b"
+gdb_test_multiple $test $test {
+    -re " = 'abcdefg'\r\n$gdb_prompt $" {
+        pass $test
+    }
+    -re $result_line_2 {
+        setup_xfail "*-*-*"
+        fail $test
+    }
+}
+
+set test "print this"
+gdb_test_multiple $test $test {
+    -re $result_line {
+        pass $test
+    }
+    -re $result_line_2 {
+         setup_xfail "*-*-*"
+         fail $test
+    }
+}
diff --git a/gdb/testsuite/gdb.fortran/derived-type.f90 b/gdb/testsuite/gdb.fortran/derived-type.f90
index 2cb2339..aad1553 100644
--- a/gdb/testsuite/gdb.fortran/derived-type.f90
+++ b/gdb/testsuite/gdb.fortran/derived-type.f90
@@ -29,12 +29,17 @@ program main
   end type foo
   type(foo) :: q
   type(bar) :: p
+  type(foo) :: this
 
   p = bar(1, 2.375)
   q%a = 3.125
   q%b = "abcdefg"
   q%x%c = 1
   q%x%d = 2.375
-  print *,p,q 
+  this%a = 3.125
+  this%b = "abcdefg"
+  this%x%c = 1
+  this%x%d = 2.375
+  print *,p,q,this
 
 end program main
-- 
2.1.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] Fix of default lookup for "this" symbol.
  2016-03-09 14:32 [PATCH v1] Fix of default lookup for "this" symbol Walfred Tedeschi
@ 2016-04-05 11:32 ` Yao Qi
  0 siblings, 0 replies; 5+ messages in thread
From: Yao Qi @ 2016-04-05 11:32 UTC (permalink / raw)
  To: Walfred Tedeschi; +Cc: palves, brobecker, gdb-patches

Walfred Tedeschi <walfred.tedeschi@intel.com> writes:

> 2016-03-09  Walfred Tedeschi  <walfred.tedeschi@intel.com>
>
> gdb/ChangeLog:
>
> 	* cp_lookup_bare_symbol (cp_lookup_bare_symbol): Add check for the

        * cp-namespace.c (cp_lookup_bare_symbol):

> 	name_of_this in order to return a null symbol when looking up
> 	for "this".
>
>
> ---
>  gdb/cp-namespace.c                         |  7 ++++++
>  gdb/testsuite/gdb.fortran/derived-type.exp | 35 ++++++++++++++++++++++++++++--
>  gdb/testsuite/gdb.fortran/derived-type.f90 |  7 +++++-
>  3 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
> index 72002d6..b9d7dbe 100644
> --- a/gdb/cp-namespace.c
> +++ b/gdb/cp-namespace.c
> @@ -210,6 +210,13 @@ cp_lookup_bare_symbol (const struct language_defn *langdef,
>        if (lang_this.symbol == NULL)
>  	return null_block_symbol;
>  
> +      /* This whole routine is also the default path for several languages.
> +         In some languages "this" is not a special symbol,
> +         i.e. LA_NAME_OF_THIS is NULL.
> +         For those cases we should return a null_block_symbol.  */
> +      if (langdef != NULL && langdef->la_name_of_this == NULL)
> +	return null_block_symbol;
> +

The better fix would be pass "langdef" to lookup_language_this rather
than language_def (language_cplus).  The patch is like this:

--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -206,7 +206,7 @@ cp_lookup_bare_symbol (const struct language_defn *langdef,
       struct block_symbol lang_this;
       struct type *type;
 
-      lang_this = lookup_language_this (language_def (language_cplus), block);
+      lang_this = lookup_language_this (langdef, block);
       if (lang_this.symbol == NULL)
        return null_block_symbol;
 
>        type = check_typedef (TYPE_TARGET_TYPE (SYMBOL_TYPE (lang_this.symbol)));
>        /* If TYPE_NAME is NULL, abandon trying to find this symbol.
>  	 This can happen for lambda functions compiled with clang++,
> diff --git a/gdb/testsuite/gdb.fortran/derived-type.exp b/gdb/testsuite/gdb.fortran/derived-type.exp
> index f650352..acccf3b 100644
> --- a/gdb/testsuite/gdb.fortran/derived-type.exp
> +++ b/gdb/testsuite/gdb.fortran/derived-type.exp
> @@ -74,14 +74,45 @@ gdb_test_multiple $test $test {
>  gdb_test "print q%x%c" "\\$\[0-9\]+ = 1"
>  gdb_test "print q%x%d" "\\$\[0-9\]+ = 2\\.375"
>  
> +set result_line "= \\( a = 3.125, x = \\( c = 1, d = 2\\.375 \\),\
> +b = 'abcdefg' \\)\r\n$gdb_prompt $"
> +
> +# Used in case compiler generates an array of characters.
> +set result_line_2 " = \\( a = 3.125, x = \\( 1, 2\\.375 \\),\
> +b = \\('abcdefg'\\) \\)\r\n$gdb_prompt $"
> +
>  set test "print q"
>  gdb_test_multiple $test $test {
> -    -re "\\$\[0-9\]+ = \\( a = 3.125, x = \\( c = 1, d = 2\\.375 \\), b = 'abcdefg' \\)\r\n$gdb_prompt $" {

Your patch can't be applied cleanly, because the pattern in mainline is
"\\$\[0-9\]+ = \\( 3.125, \\( 1, 2\\.375 \\), 'abcdefg' \\)\r\n$gdb_prompt $", 
it doesn't have " a =" or "x = " things.

> +    -re $result_line {
>  	pass $test
>      }
> -    -re "\\$\[0-9\]+ = \\( a = 3.125, x = \\( 1, 2\\.375 \\), b = \\('abcdefg'\\) \\)\r\n$gdb_prompt $" {
> +    -re $result_line_2 {
>  	# Compiler should produce string, not an array of characters.
>  	setup_xfail "*-*-*"
>  	fail $test
>      }
>  }
> +

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH V1] Fix of default lookup for "this" symbol.
  2016-06-21  8:46 ` Yao Qi
@ 2016-06-21  8:51   ` Tedeschi, Walfred
  0 siblings, 0 replies; 5+ messages in thread
From: Tedeschi, Walfred @ 2016-06-21  8:51 UTC (permalink / raw)
  To: Yao Qi; +Cc: palves, gdb-patches

Hi Yao,

Sorry I had in mind that I haven't had submitted this one.

I will address all your comments. They look the right way to go.

Thanks again,
Fred

-----Original Message-----
From: Yao Qi [mailto:qiyaoltc@gmail.com] 
Sent: Tuesday, June 21, 2016 9:46 AM
To: Tedeschi, Walfred <walfred.tedeschi@intel.com>
Cc: palves@redhat.com; qiyaoltc@gmail.com; gdb-patches@sourceware.org
Subject: Re: [PATCH V1] Fix of default lookup for "this" symbol.

Walfred Tedeschi <walfred.tedeschi@intel.com> writes:

Hi,
Looks you post the same patch as you posted in March.
https://sourceware.org/ml/gdb-patches/2016-03/msg00128.html and I reviewed it https://sourceware.org/ml/gdb-patches/2016-04/msg00068.html

I don't see my comments are addressed in this version.

--
Yao (齐尧)
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V1] Fix of default lookup for "this" symbol.
  2016-06-20 14:17 [PATCH V1] " Walfred Tedeschi
@ 2016-06-21  8:46 ` Yao Qi
  2016-06-21  8:51   ` Tedeschi, Walfred
  0 siblings, 1 reply; 5+ messages in thread
From: Yao Qi @ 2016-06-21  8:46 UTC (permalink / raw)
  To: Walfred Tedeschi; +Cc: palves, qiyaoltc, gdb-patches

Walfred Tedeschi <walfred.tedeschi@intel.com> writes:

Hi,
Looks you post the same patch as you posted in March.
https://sourceware.org/ml/gdb-patches/2016-03/msg00128.html and I
reviewed it
https://sourceware.org/ml/gdb-patches/2016-04/msg00068.html

I don't see my comments are addressed in this version.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH V1] Fix of default lookup for "this" symbol.
@ 2016-06-20 14:17 Walfred Tedeschi
  2016-06-21  8:46 ` Yao Qi
  0 siblings, 1 reply; 5+ messages in thread
From: Walfred Tedeschi @ 2016-06-20 14:17 UTC (permalink / raw)
  To: palves, qiyaoltc; +Cc: gdb-patches, Walfred Tedeschi

Using the default lookup for the symbol "this" might lead to segmentation
fault in GDB.
Some languages, e.g. Fortran, use as default lookup routine the C++
routines.
For those languages "this" can be the instance of a class or even the
definition of a class.
When an instance of a class having the name "this" is evaluated
in GDB a segmentation fault was observed.

As example of the issue take into consideration the Fortran code:
  type foo
    real :: a
    type(bar) :: x
    character*7 :: b
  end type foo
  type(foo) :: this

Issue appears when evaluating the variable "this" in GDB.

Within the language definition structure there is a field that represents
the name of the special symbol used for the C++ "this" for the language
being described.
The fix presented here takes into account the aforementioned field. In the
case the aforementioned field is NULL "this" is not represented in the
language described and the lookup should return a null_block_symbol.

Tests: Performed tests with gfortran and ifort.

2016-03-09  Walfred Tedeschi  <walfred.tedeschi@intel.com>

gdb/ChangeLog:

	* cp_lookup_bare_symbol (cp_lookup_bare_symbol): Add check for the
	name_of_this in order to return a null symbol when looking up
	for "this".

gdb/testsuite/ChangeLog:

	* gdb.fortran/derived-types.exp (result_line, result_line_2):
	New variables.
	(print this%a, print this%b, print this): New tests.
	* gdb.fortran/derived-types.f90 (this): New object and
	initialization.

---
 gdb/cp-namespace.c                         |  7 ++++++
 gdb/testsuite/gdb.fortran/derived-type.exp | 35 ++++++++++++++++++++++++++++--
 gdb/testsuite/gdb.fortran/derived-type.f90 |  7 +++++-
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 016a42f..7eb3a13 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -210,6 +210,13 @@ cp_lookup_bare_symbol (const struct language_defn *langdef,
       if (lang_this.symbol == NULL)
 	return null_block_symbol;
 
+      /* This whole routine is also the default path for several languages.
+         In some languages "this" is not a special symbol,
+         i.e. LA_NAME_OF_THIS is NULL.
+         For those cases we should return a null_block_symbol.  */
+      if (langdef != NULL && langdef->la_name_of_this == NULL)
+	return null_block_symbol;
+
       type = check_typedef (TYPE_TARGET_TYPE (SYMBOL_TYPE (lang_this.symbol)));
       /* If TYPE_NAME is NULL, abandon trying to find this symbol.
 	 This can happen for lambda functions compiled with clang++,
diff --git a/gdb/testsuite/gdb.fortran/derived-type.exp b/gdb/testsuite/gdb.fortran/derived-type.exp
index 32431bc..2781850 100644
--- a/gdb/testsuite/gdb.fortran/derived-type.exp
+++ b/gdb/testsuite/gdb.fortran/derived-type.exp
@@ -73,14 +73,45 @@ gdb_test_multiple $test $test {
 gdb_test "print q%x%c" "\\$\[0-9\]+ = 1"
 gdb_test "print q%x%d" "\\$\[0-9\]+ = 2\\.375"
 
+set result_line "= \\( a = 3.125, x = \\( c = 1, d = 2\\.375 \\),\
+b = 'abcdefg' \\)\r\n$gdb_prompt $"
+
+# Used in case compiler generates an array of characters.
+set result_line_2 " = \\( a = 3.125, x = \\( 1, 2\\.375 \\),\
+b = \\('abcdefg'\\) \\)\r\n$gdb_prompt $"
+
 set test "print q"
 gdb_test_multiple $test $test {
-    -re "\\$\[0-9\]+ = \\( a = 3.125, x = \\( c = 1, d = 2\\.375 \\), b = 'abcdefg' \\)\r\n$gdb_prompt $" {
+    -re $result_line {
 	pass $test
     }
-    -re "\\$\[0-9\]+ = \\( a = 3.125, x = \\( 1, 2\\.375 \\), b = \\('abcdefg'\\) \\)\r\n$gdb_prompt $" {
+    -re $result_line_2 {
 	# Compiler should produce string, not an array of characters.
 	setup_xfail "*-*-*"
 	fail $test
     }
 }
+
+gdb_test "print this%a" " = 3\\.125"
+
+set test "print this%b"
+gdb_test_multiple $test $test {
+    -re " = 'abcdefg'\r\n$gdb_prompt $" {
+        pass $test
+    }
+    -re $result_line_2 {
+        setup_xfail "*-*-*"
+        fail $test
+    }
+}
+
+set test "print this"
+gdb_test_multiple $test $test {
+    -re $result_line {
+        pass $test
+    }
+    -re $result_line_2 {
+         setup_xfail "*-*-*"
+         fail $test
+    }
+}
diff --git a/gdb/testsuite/gdb.fortran/derived-type.f90 b/gdb/testsuite/gdb.fortran/derived-type.f90
index 2cb2339..aad1553 100644
--- a/gdb/testsuite/gdb.fortran/derived-type.f90
+++ b/gdb/testsuite/gdb.fortran/derived-type.f90
@@ -29,12 +29,17 @@ program main
   end type foo
   type(foo) :: q
   type(bar) :: p
+  type(foo) :: this
 
   p = bar(1, 2.375)
   q%a = 3.125
   q%b = "abcdefg"
   q%x%c = 1
   q%x%d = 2.375
-  print *,p,q 
+  this%a = 3.125
+  this%b = "abcdefg"
+  this%x%c = 1
+  this%x%d = 2.375
+  print *,p,q,this
 
 end program main
-- 
2.7.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-06-21  8:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-09 14:32 [PATCH v1] Fix of default lookup for "this" symbol Walfred Tedeschi
2016-04-05 11:32 ` Yao Qi
2016-06-20 14:17 [PATCH V1] " Walfred Tedeschi
2016-06-21  8:46 ` Yao Qi
2016-06-21  8:51   ` Tedeschi, Walfred

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).