[OB PATCH] Fix uninitialized type_flags variable

[OB PATCH] Fix uninitialized type_flags variable

[Simon Marchi]

If uninitialized, type_flags can contain a random value. If the
arch-specific code doesn't set a value, this give unexpected results.

I believe this is an obvious change. Since I don't have commit access yet,
could anybody push this?

gdb/ChangeLog
2014-01-16  Simon Marchi  <simon.marchi@ericsson.com>

	* gdbtypes.c (address_space_name_to_int): Initialize type_flags to 0.

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 042c17d..7d9485a 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -531,7 +531,7 @@ lookup_function_type_with_arguments (struct type *type,
 int
 address_space_name_to_int (struct gdbarch *gdbarch, char *space_identifier)
 {
-  int type_flags;
+  int type_flags = 0;

   /* Check for known address space delimiters.  */
   if (!strcmp (space_identifier, "code"))

Re: [OB PATCH] Fix uninitialized type_flags variable

[Pedro Alves]

On 01/16/2014 02:38 PM, Simon Marchi wrote:
> If uninitialized, type_flags can contain a random value. If the
> arch-specific code doesn't set a value, this give unexpected results.

The whole point of that method is to get that value from the arch-specific
code.  If some arch-specific code is returning true but not setting
type_flags, then that's the bug to fix.  IOW, the method should only return
true if the type_flags was set.  If you're relying on this initialization,
you're papering over an arch-specific bug.

> 
> I believe this is an obvious change. Since I don't have commit access yet,
> could anybody push this?
> 
> gdb/ChangeLog
> 2014-01-16  Simon Marchi  <simon.marchi@ericsson.com>
> 
> 	* gdbtypes.c (address_space_name_to_int): Initialize type_flags to 0.
> 
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 042c17d..7d9485a 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -531,7 +531,7 @@ lookup_function_type_with_arguments (struct type *type,
>  int
>  address_space_name_to_int (struct gdbarch *gdbarch, char *space_identifier)
>  {
> -  int type_flags;
> +  int type_flags = 0;
> 
>    /* Check for known address space delimiters.  */
>    if (!strcmp (space_identifier, "code"))
> 


-- 
Pedro Alves

Re: [OB PATCH] Fix uninitialized type_flags variable

[Simon Marchi]

Oh ok, thanks for the clarification. It wasn't very clear what the contract of the function was.

On 14-01-16 11:38 AM, Pedro Alves wrote:
> On 01/16/2014 02:38 PM, Simon Marchi wrote:
>> If uninitialized, type_flags can contain a random value. If the
>> arch-specific code doesn't set a value, this give unexpected results.
> 
> The whole point of that method is to get that value from the arch-specific
> code.  If some arch-specific code is returning true but not setting
> type_flags, then that's the bug to fix.  IOW, the method should only return
> true if the type_flags was set.  If you're relying on this initialization,
> you're papering over an arch-specific bug.
> 
>>
>> I believe this is an obvious change. Since I don't have commit access yet,
>> could anybody push this?
>>
>> gdb/ChangeLog
>> 2014-01-16  Simon Marchi  <simon.marchi@ericsson.com>
>>
>> 	* gdbtypes.c (address_space_name_to_int): Initialize type_flags to 0.
>>
>> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
>> index 042c17d..7d9485a 100644
>> --- a/gdb/gdbtypes.c
>> +++ b/gdb/gdbtypes.c
>> @@ -531,7 +531,7 @@ lookup_function_type_with_arguments (struct type *type,
>>  int
>>  address_space_name_to_int (struct gdbarch *gdbarch, char *space_identifier)
>>  {
>> -  int type_flags;
>> +  int type_flags = 0;
>>
>>    /* Check for known address space delimiters.  */
>>    if (!strcmp (space_identifier, "code"))
>>
> 
> 

Re: [OB PATCH] Fix uninitialized type_flags variable

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> Oh ok, thanks for the clarification. It wasn't very clear what the
Simon> contract of the function was.

A gdbarch.sh patch to clarify this would be welcome :)

Tom

Re: [OB PATCH] Fix uninitialized type_flags variable

[Pedro Alves]

On 01/16/2014 06:13 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
> 
> Simon> Oh ok, thanks for the clarification. It wasn't very clear what the
> Simon> contract of the function was.
> 
> A gdbarch.sh patch to clarify this would be welcome :)

What Tromey said.  :-)

-- 
Pedro Alves