Patchwork Fix gdb crash when trying to print the address of a synthetic pointer.

login
register
mail settings
Submitter Martin Galvan
Date April 7, 2015, 2:09 p.m.
Message ID <1428415783-15199-1-git-send-email-martin.galvan@tallertechnologies.com>
Download mbox | patch
Permalink /patch/6068/
State New
Headers show

Comments

Martin Galvan - April 7, 2015, 2:09 p.m.
Trying to print the address of a synthetic pointer (such as a C++ reference after O3 optimization) will cause gdb to crash with the following message:

../gdb/dwarf2loc.c:1625: internal-error: Should not be able to create a lazy value with an enclosing type

This patch fixes that by doing a check for synthetic pointers in value_addr and printing an error message.

I have a company-wide copyright assignment. I don't have commit access, though, so it would be great if anyone could commit this for me.

gdb/
2015-04-07 Martin Galvan <martin.galvan@tallertechnologies.com>

	* valops.c (value_addr): Don't try to get the address of a synthetic pointer.

---
 gdb/valops.c | 7 +++++++
 1 file changed, 7 insertions(+)

--
2.3.5
Pedro Alves - April 7, 2015, 2:33 p.m.
Hi Martin,

On 04/07/2015 03:09 PM, Martin Galvan wrote:
> Trying to print the address of a synthetic pointer (such as a C++ reference after O3 optimization) will cause gdb to crash with the following message:
> 
> ../gdb/dwarf2loc.c:1625: internal-error: Should not be able to create a lazy value with an enclosing type
> 
> This patch fixes that by doing a check for synthetic pointers in value_addr and printing an error message.
> 
> I have a company-wide copyright assignment. I don't have commit access, though, so it would be great if anyone could commit this for me.
> 

Does this pass the testsuite?  I seem to recall that the
TYPE_CODE_REF path below is used by synthetic pointers, but
I could well be wrong.

Could you add some test to cover this?  We have some <synthetic
pointer> tests in gdb.dwarf2/implptr.exp.  Maybe add something there?

> gdb/
> 2015-04-07 Martin Galvan <martin.galvan@tallertechnologies.com>

Two spaces before and after name.

> 
> 	* valops.c (value_addr): Don't try to get the address of a synthetic pointer.

Please wrap this at 80 cols.

> 
> ---
>  gdb/valops.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/gdb/valops.c b/gdb/valops.c
> index 66c63c1..66e2c9d 100644
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -1474,6 +1474,13 @@ value_addr (struct value *arg1)
>    struct value *arg2;
>    struct type *type = check_typedef (value_type (arg1));
> 
> +  if (value_bits_synthetic_pointer(arg1, value_embedded_offset (arg1),

Missing space before parens.

> +      TARGET_CHAR_BIT * TYPE_LENGTH (type)))
> +    {
> +      error (_("Attempt to take address of a synthetic pointer."));
> +      return NULL;

This "return" is never reached, as "error" throws.  Please remove it,
and then remove the then unnecessary braces.

Thanks,
Pedro Alves
Martin Galvan - April 7, 2015, 2:40 p.m.
On Tue, Apr 7, 2015 at 11:33 AM, Pedro Alves <palves@redhat.com> wrote:
> Hi Martin,
>
> On 04/07/2015 03:09 PM, Martin Galvan wrote:
>> Trying to print the address of a synthetic pointer (such as a C++ reference after O3 optimization) will cause gdb to crash with the following message:
>>
>> ../gdb/dwarf2loc.c:1625: internal-error: Should not be able to create a lazy value with an enclosing type
>>
>> This patch fixes that by doing a check for synthetic pointers in value_addr and printing an error message.
>>
>> I have a company-wide copyright assignment. I don't have commit access, though, so it would be great if anyone could commit this for me.
>>
>
> Does this pass the testsuite?  I seem to recall that the
> TYPE_CODE_REF path below is used by synthetic pointers, but
> I could well be wrong.

For some reason this problem doesn't happen with some synthetic
pointers, such as the "this" pointer in C++ (I assume those are
handled by the TYPE_CODE_REF path). However, I can confirm this
happens with C++ references if optimizing with gcc -O3.

> Could you add some test to cover this?  We have some <synthetic
> pointer> tests in gdb.dwarf2/implptr.exp.  Maybe add something there?

Ok, will do it.

>> gdb/
>> 2015-04-07 Martin Galvan <martin.galvan@tallertechnologies.com>
>
> Two spaces before and after name.
>
>>
>>       * valops.c (value_addr): Don't try to get the address of a synthetic pointer.
>
> Please wrap this at 80 cols.
>
>>
>> ---
>>  gdb/valops.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/gdb/valops.c b/gdb/valops.c
>> index 66c63c1..66e2c9d 100644
>> --- a/gdb/valops.c
>> +++ b/gdb/valops.c
>> @@ -1474,6 +1474,13 @@ value_addr (struct value *arg1)
>>    struct value *arg2;
>>    struct type *type = check_typedef (value_type (arg1));
>>
>> +  if (value_bits_synthetic_pointer(arg1, value_embedded_offset (arg1),
>
> Missing space before parens.
>
>> +      TARGET_CHAR_BIT * TYPE_LENGTH (type)))
>> +    {
>> +      error (_("Attempt to take address of a synthetic pointer."));
>> +      return NULL;
>
> This "return" is never reached, as "error" throws.  Please remove it,
> and then remove the then unnecessary braces.

Will do. Thanks a lot!

Patch

diff --git a/gdb/valops.c b/gdb/valops.c
index 66c63c1..66e2c9d 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1474,6 +1474,13 @@  value_addr (struct value *arg1)
   struct value *arg2;
   struct type *type = check_typedef (value_type (arg1));

+  if (value_bits_synthetic_pointer(arg1, value_embedded_offset (arg1),
+      TARGET_CHAR_BIT * TYPE_LENGTH (type)))
+    {
+      error (_("Attempt to take address of a synthetic pointer."));
+      return NULL;
+    }
+
   if (TYPE_CODE (type) == TYPE_CODE_REF)
     {
       /* Copy the value, but change the type from (T&) to (T*).  We