Small segfault fix when there is no python

Message ID CAF5HaEU+9uQn=P37F=c8WxausmA3BPL9SKXoTVZh+2SNA1V1cw@mail.gmail.com
State Committed
Headers

Commit Message

Daniel Gutson March 20, 2014, 4:33 p.m. UTC
  Hi,

   the small attached patch prevents gdb to segfault when an extension
language definition has no ops,
which e.g. occurs when HAVE_PYTHON is not defined so
extension_language_python remains with ops in NULL.
This causes the line
   if (extlang->ops->eval_from_control_command != NULL)
(in eval_ext_lang_from_control_command) to dereference a null pointer.

Please commit it for me if approved since I don't have write access.

Thanks,

   Daniel.


    2014-03-20    Daniel Gutson (daniel.gutson@tallertechnologies.com)

gdb/
    * extension.c: (eval_ext_lang_from_control_command) Added check to
prevent dereference of null pointer.
  

Comments

Sergio Durigan Junior March 20, 2014, 5:31 p.m. UTC | #1
On Thursday, March 20 2014, Daniel Gutson wrote:

> Hi,
>
>    the small attached patch prevents gdb to segfault when an extension
> language definition has no ops,
> which e.g. occurs when HAVE_PYTHON is not defined so
> extension_language_python remains with ops in NULL.
> This causes the line
>    if (extlang->ops->eval_from_control_command != NULL)
> (in eval_ext_lang_from_control_command) to dereference a null pointer.

Hi Daniel,

Thanks for the patch.  This is a simple patch so it doesn't need a
copyright assignment from you.  However, if you intend to continue
contributing to GDB, please e-mail me offlist and I can send you the
papers to obtain the assignment.

Just a few comments about formatting issues.

>     2014-03-20    Daniel Gutson (daniel.gutson@tallertechnologies.com)
>
> gdb/
>     * extension.c: (eval_ext_lang_from_control_command) Added check to
> prevent dereference of null pointer.

The ChangeLog format is wrong.  Take a look at the ChangeLog file for
lots of examples, but basically you need to write:

2014-03-20  Your Name  <your@email>

	* file.c (function): Added check to prevent blabla...

Pay attention to the 2 spaces between the date, the name and the e-mail,
and also to the TAB character indenting the description.

> diff --git a/gdb/extension.c b/gdb/extension.c
> index c2f502b..8357ee8 100644
> --- a/gdb/extension.c
> +++ b/gdb/extension.c
> @@ -342,11 +342,14 @@ eval_ext_lang_from_control_command (struct command_line *cmd)
>      {
>        if (extlang->cli_control_type == cmd->control_type)
>  	{
> -	  if (extlang->ops->eval_from_control_command != NULL)
> -	    {
> -	      extlang->ops->eval_from_control_command (extlang, cmd);
> -	      return;
> -	    }
> +      if (extlang->ops != NULL)
> +        {  
> +	    if (extlang->ops->eval_from_control_command != NULL)
> +	      {
> +	        extlang->ops->eval_from_control_command (extlang, cmd);
> +	        return;
> +	      }
> +        }

You could simplify this by writing:

  if (extlang->ops != NULL && extlang->ops->eval_from_control_command != NULL)

If you don't want to join the two "if"s, then you don't need to put the
braces on the outter "if", because it has one single statement.

The patch looks good to me, but I'm not a maintainer and can't approve
it.

Thanks,
  
Doug Evans March 20, 2014, 5:52 p.m. UTC | #2
On Thu, Mar 20, 2014 at 10:31 AM, Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
> On Thursday, March 20 2014, Daniel Gutson wrote:
>
>> Hi,
>>
>>    the small attached patch prevents gdb to segfault when an extension
>> language definition has no ops,
>> which e.g. occurs when HAVE_PYTHON is not defined so
>> extension_language_python remains with ops in NULL.
>> This causes the line
>>    if (extlang->ops->eval_from_control_command != NULL)
>> (in eval_ext_lang_from_control_command) to dereference a null pointer.
>
> Hi Daniel,
>
> Thanks for the patch.  This is a simple patch so it doesn't need a
> copyright assignment from you.  However, if you intend to continue
> contributing to GDB, please e-mail me offlist and I can send you the
> papers to obtain the assignment.
>
> Just a few comments about formatting issues.
>
>>     2014-03-20    Daniel Gutson (daniel.gutson@tallertechnologies.com)
>>
>> gdb/
>>     * extension.c: (eval_ext_lang_from_control_command) Added check to
>> prevent dereference of null pointer.
>
> The ChangeLog format is wrong.  Take a look at the ChangeLog file for
> lots of examples, but basically you need to write:
>
> 2014-03-20  Your Name  <your@email>
>
>         * file.c (function): Added check to prevent blabla...
>
> Pay attention to the 2 spaces between the date, the name and the e-mail,
> and also to the TAB character indenting the description.
>
>> diff --git a/gdb/extension.c b/gdb/extension.c
>> index c2f502b..8357ee8 100644
>> --- a/gdb/extension.c
>> +++ b/gdb/extension.c
>> @@ -342,11 +342,14 @@ eval_ext_lang_from_control_command (struct command_line *cmd)
>>      {
>>        if (extlang->cli_control_type == cmd->control_type)
>>       {
>> -       if (extlang->ops->eval_from_control_command != NULL)
>> -         {
>> -           extlang->ops->eval_from_control_command (extlang, cmd);
>> -           return;
>> -         }
>> +      if (extlang->ops != NULL)
>> +        {
>> +         if (extlang->ops->eval_from_control_command != NULL)
>> +           {
>> +             extlang->ops->eval_from_control_command (extlang, cmd);
>> +             return;
>> +           }
>> +        }
>
> You could simplify this by writing:
>
>   if (extlang->ops != NULL && extlang->ops->eval_from_control_command != NULL)
>
> If you don't want to join the two "if"s, then you don't need to put the
> braces on the outter "if", because it has one single statement.
>
> The patch looks good to me, but I'm not a maintainer and can't approve
> it.
>
> Thanks,
>
> --
> Sergio


I did an audit of all the uses of ->ops and think this is the only one I missed.
I will commit with the needed changes.
Thanks for the patch!
  
Daniel Gutson March 21, 2014, 4:24 p.m. UTC | #3
Thanks Doug and Sergio!

   Daniel.


On Thu, Mar 20, 2014 at 2:52 PM, Doug Evans <dje@google.com> wrote:
> On Thu, Mar 20, 2014 at 10:31 AM, Sergio Durigan Junior
> <sergiodj@redhat.com> wrote:
>> On Thursday, March 20 2014, Daniel Gutson wrote:
>>
>>> Hi,
>>>
>>>    the small attached patch prevents gdb to segfault when an extension
>>> language definition has no ops,
>>> which e.g. occurs when HAVE_PYTHON is not defined so
>>> extension_language_python remains with ops in NULL.
>>> This causes the line
>>>    if (extlang->ops->eval_from_control_command != NULL)
>>> (in eval_ext_lang_from_control_command) to dereference a null pointer.
>>
>> Hi Daniel,
>>
>> Thanks for the patch.  This is a simple patch so it doesn't need a
>> copyright assignment from you.  However, if you intend to continue
>> contributing to GDB, please e-mail me offlist and I can send you the
>> papers to obtain the assignment.
>>
>> Just a few comments about formatting issues.
>>
>>>     2014-03-20    Daniel Gutson (daniel.gutson@tallertechnologies.com)
>>>
>>> gdb/
>>>     * extension.c: (eval_ext_lang_from_control_command) Added check to
>>> prevent dereference of null pointer.
>>
>> The ChangeLog format is wrong.  Take a look at the ChangeLog file for
>> lots of examples, but basically you need to write:
>>
>> 2014-03-20  Your Name  <your@email>
>>
>>         * file.c (function): Added check to prevent blabla...
>>
>> Pay attention to the 2 spaces between the date, the name and the e-mail,
>> and also to the TAB character indenting the description.
>>
>>> diff --git a/gdb/extension.c b/gdb/extension.c
>>> index c2f502b..8357ee8 100644
>>> --- a/gdb/extension.c
>>> +++ b/gdb/extension.c
>>> @@ -342,11 +342,14 @@ eval_ext_lang_from_control_command (struct command_line *cmd)
>>>      {
>>>        if (extlang->cli_control_type == cmd->control_type)
>>>       {
>>> -       if (extlang->ops->eval_from_control_command != NULL)
>>> -         {
>>> -           extlang->ops->eval_from_control_command (extlang, cmd);
>>> -           return;
>>> -         }
>>> +      if (extlang->ops != NULL)
>>> +        {
>>> +         if (extlang->ops->eval_from_control_command != NULL)
>>> +           {
>>> +             extlang->ops->eval_from_control_command (extlang, cmd);
>>> +             return;
>>> +           }
>>> +        }
>>
>> You could simplify this by writing:
>>
>>   if (extlang->ops != NULL && extlang->ops->eval_from_control_command != NULL)
>>
>> If you don't want to join the two "if"s, then you don't need to put the
>> braces on the outter "if", because it has one single statement.
>>
>> The patch looks good to me, but I'm not a maintainer and can't approve
>> it.
>>
>> Thanks,
>>
>> --
>> Sergio
>
>
> I did an audit of all the uses of ->ops and think this is the only one I missed.
> I will commit with the needed changes.
> Thanks for the patch!
  

Patch

diff --git a/gdb/extension.c b/gdb/extension.c
index c2f502b..8357ee8 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -342,11 +342,14 @@  eval_ext_lang_from_control_command (struct command_line *cmd)
     {
       if (extlang->cli_control_type == cmd->control_type)
 	{
-	  if (extlang->ops->eval_from_control_command != NULL)
-	    {
-	      extlang->ops->eval_from_control_command (extlang, cmd);
-	      return;
-	    }
+      if (extlang->ops != NULL)
+        {  
+	    if (extlang->ops->eval_from_control_command != NULL)
+	      {
+	        extlang->ops->eval_from_control_command (extlang, cmd);
+	        return;
+	      }
+        }
 	  /* The requested extension language is not supported in this GDB.  */
 	  throw_ext_lang_unsupported (extlang);
 	}