The parameter in value_entirely_optimized_out is NULL

Message ID BLU181-W693D4F7D046CA8E692959AE1480@phx.gbl
State New, archived
Headers

Commit Message

建明 乔 May 17, 2016, 2:45 p.m. UTC
  Hi:



The GDB(v7.7 onwards) will crash at value_entirely_optimized_out (value=0x0) in some cases.



These cases are reported in GDB Database as Bug 20020,17076,17685 in X86 platform



and other reports that use cross-compiled GDB host(ARM & MIPS) from our side. 




This bug is introduced when the patch https://sourceware.org/ml/gdb-patches/2013-10/msg00353.html  is added.



The code from this patch that caused the regression is listed below:








Thanks
Jmqiao
  

Comments

Pedro Alves May 18, 2016, 11:44 a.m. UTC | #1
On 05/17/2016 03:45 PM, 建明 乔 wrote:
> Hi:
> 

FYI, something is odd with your emails -- lots of seemingly spurious empty lines:

 https://sourceware.org/ml/gdb-patches/2016-05/msg00259.html

Also, the follow up reply you sent was quite garbled, and it doesn't look
like it managed to reach the list.

> 
> 
> The GDB(v7.7 onwards) will crash at value_entirely_optimized_out (value=0x0) in some cases.
> 
> 
> 
> These cases are reported in GDB Database as Bug 20020,17076,17685 in X86 platform
> 
> 
> 
> and other reports that use cross-compiled GDB host(ARM & MIPS) from our side. 

I tried a couple tests from those bug reports, and I couldn't reproduce the problem
with current master.  Can you reproduce it reliably?  It'd be great to have a testcase
in the testsuite for this.  Best would probably be to write it using the Dwarf::assemble
mechanism (gdb/testsuite/gdb.dwarf2/).


> 
> 
> 
> 
> This bug is introduced when the patch https://sourceware.org/ml/gdb-patches/2013-10/msg00353.html  is added.
> 
> 
> 
> The code from this patch that caused the regression is listed below:
> 
> 
> 
> 
> diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
> index 1d7147c..4b625d1 100644
> --- a/gdb/cp-valprint.c
> +++ b/gdb/cp-valprint.c
> @@ -333,12 +333,9 @@ cp_print_value_fields (struct type *type, struct type *real_type,
>        fprintf_filtered (stream,
>            _("<error reading variable: %s>"),
>            ex.message);
> -    else if (v == NULL)
> -      val_print_optimized_out (NULL, stream);
> -    else
> -      cp_print_static_field (TYPE_FIELD_TYPE (type, i),
> -        v, stream, recurse + 1,
> -        options);
> +    cp_print_static_field (TYPE_FIELD_TYPE (type, i),
> +      v, stream, recurse + 1,
> +      options);
>    }
> 
> 
> 
> 
> Therefore, I propose to partly revert the previous patch and apply the change below.
> 
> 
> 
> 
> Is it acceptable ?

Can't tell without a more expanded rationale for the change.  E.g.,:

- Why is 'v' NULL here?

- Why is it OK for 'v' to be NULL here?

Thanks,
Pedro Alves
  
建明 乔 May 18, 2016, 4:18 p.m. UTC | #2
Yes, I can duplicate this issue reliably. it is cross-compiled(X86/ARM) gdb 7.11 version.
and It is really easy to duplicate this issue with a special application core file(Application is not
open source) on X86.

It is possible that the error can occur between TRY and CATCH block therefore leaving the value
of v still be NULL.

For example, In my failure case,it shows 

<error reading variable: Missing ELF symbol "std::basic_string<unsigned char, std::char_traits<unsigned char>, std::allocator<unsigned char>>::npos".>
Segmentation fault (core dumped)

Thanks
Jmqiao
----------------------------------------
> Subject: Re: [PATCH] The parameter in value_entirely_optimized_out is NULL
> To: kiki-good@hotmail.com; gdb-patches@sourceware.org
> From: palves@redhat.com
> Date: Wed, 18 May 2016 12:44:20 +0100
>
> On 05/17/2016 03:45 PM, 建明 乔 wrote:
>> Hi:
>>
>
> FYI, something is odd with your emails -- lots of seemingly spurious empty lines:
>
> https://sourceware.org/ml/gdb-patches/2016-05/msg00259.html
>
> Also, the follow up reply you sent was quite garbled, and it doesn't look
> like it managed to reach the list.
>
>>
>>
>> The GDB(v7.7 onwards) will crash at value_entirely_optimized_out (value=0x0) in some cases.
>>
>>
>>
>> These cases are reported in GDB Database as Bug 20020,17076,17685 in X86 platform
>>
>>
>>
>> and other reports that use cross-compiled GDB host(ARM & MIPS) from our side.
>
> I tried a couple tests from those bug reports, and I couldn't reproduce the problem
> with current master. Can you reproduce it reliably? It'd be great to have a testcase
> in the testsuite for this. Best would probably be to write it using the Dwarf::assemble
> mechanism (gdb/testsuite/gdb.dwarf2/).
>
>
>>
>>
>>
>>
>> This bug is introduced when the patch https://sourceware.org/ml/gdb-patches/2013-10/msg00353.html is added.
>>
>>
>>
>> The code from this patch that caused the regression is listed below:
>>
>>
>>
>>
>> diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
>> index 1d7147c..4b625d1 100644
>> --- a/gdb/cp-valprint.c
>> +++ b/gdb/cp-valprint.c
>> @@ -333,12 +333,9 @@ cp_print_value_fields (struct type *type, struct type *real_type,
>> fprintf_filtered (stream,
>> _("<error reading variable: %s>"),
>> ex.message);
>> - else if (v == NULL)
>> - val_print_optimized_out (NULL, stream);
>> - else
>> - cp_print_static_field (TYPE_FIELD_TYPE (type, i),
>> - v, stream, recurse + 1,
>> - options);
>> + cp_print_static_field (TYPE_FIELD_TYPE (type, i),
>> + v, stream, recurse + 1,
>> + options);
>> }
>>
>>
>>
>>
>> Therefore, I propose to partly revert the previous patch and apply the change below.
>>
>>
>>
>>
>> Is it acceptable ?
>
> Can't tell without a more expanded rationale for the change. E.g.,:
>
> - Why is 'v' NULL here?
>
> - Why is it OK for 'v' to be NULL here?
>
> Thanks,
> Pedro Alves
>
  

Patch

diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index 1d7147c..4b625d1 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -333,12 +333,9 @@  cp_print_value_fields (struct type *type, struct type *real_type,
       fprintf_filtered (stream,
           _("<error reading variable: %s>"),
           ex.message);
-    else if (v == NULL)
-      val_print_optimized_out (NULL, stream);
-    else
-      cp_print_static_field (TYPE_FIELD_TYPE (type, i),
-        v, stream, recurse + 1,
-        options);
+    cp_print_static_field (TYPE_FIELD_TYPE (type, i),
+      v, stream, recurse + 1,
+      options);
   }




Therefore, I propose to partly revert the previous patch and apply the change below.




Is it acceptable ?




diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index effce30..aaf36d1 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -328,9 +328,12 @@  cp_print_value_fields (struct type *type, struct type *real_type,
                    }
                  END_CATCH
 
-                 cp_print_static_field (TYPE_FIELD_TYPE (type, i),
-                                        v, stream, recurse + 1,
-                                        options);
+                 if (v == NULL)
+                     val_print_optimized_out(NULL, stream);
+                 else
+                     cp_print_static_field (TYPE_FIELD_TYPE (type, i),
+                                            v, stream, recurse + 1,
+                                            options);
                }
              else if (i == vptr_fieldno && type == vptr_basetype)
                {