[PATCH users/roland/osabi-assert] Do not crash on unrecognized GNU .note.ABI-tag values

[PATCH users/roland/osabi-assert] Do not crash on unrecognized GNU .note.ABI-tag values

[Roland McGrath]

Diagnosis of unexpected input (in this case, in an executable file)
should not crash as if it were a bug in GDB.  This one is especially
harmless, since the "osabi" has so little effect on anything.

OK for trunk and 7.9 branch?


Thanks,
Roland


gdb/
2015-05-28  Roland McGrath  <mcgrathr@google.com>

	* osabi.c (generic_elf_osabi_sniff_abi_tag_sections): Use warning
	rather than internal_error for an unrecognized value.

diff --git a/gdb/osabi.c b/gdb/osabi.c
index 9d90c55..abf4bd4 100644
--- a/gdb/osabi.c
+++ b/gdb/osabi.c
@@ -493,10 +493,9 @@ generic_elf_osabi_sniff_abi_tag_sections (bfd
*abfd, asection *sect, void *obj)
 	      break;

 	    default:
-	      internal_error (__FILE__, __LINE__,
-			      _("generic_elf_osabi_sniff_abi_tag_sections: "
-				"unknown OS number %d"),
-			      abi_tag);
+              warning ("GNU ABI tag value %u unrecognized.\n", abi_tag);
+              *osabi = GDB_OSABI_UNKNOWN;
+              break;
 	    }
 	  return;
 	}

Re: [PATCH users/roland/osabi-assert] Do not crash on unrecognized GNU .note.ABI-tag values

[Pedro Alves]

On 05/29/2015 01:21 AM, Roland McGrath wrote:
> Diagnosis of unexpected input (in this case, in an executable file)
> should not crash as if it were a bug in GDB.  

Indeed.

> This one is especially
> harmless, since the "osabi" has so little effect on anything.

It's used to pick the right gdbarch for the inferior.

> 
> OK for trunk and 7.9 branch?

In order to get this into a release branch, the protocol is to file
a PR and then once fixed, list the fix in the release page in the wiki:

  https://sourceware.org/gdb/wiki/GDB_7.9_Release

That helps the release manager build the release notes.

> gdb/
> 2015-05-28  Roland McGrath  <mcgrathr@google.com>
> 
> 	* osabi.c (generic_elf_osabi_sniff_abi_tag_sections): Use warning
> 	rather than internal_error for an unrecognized value.
> 
> diff --git a/gdb/osabi.c b/gdb/osabi.c
> index 9d90c55..abf4bd4 100644
> --- a/gdb/osabi.c
> +++ b/gdb/osabi.c
> @@ -493,10 +493,9 @@ generic_elf_osabi_sniff_abi_tag_sections (bfd
> *abfd, asection *sect, void *obj)
>  	      break;
> 
>  	    default:
> -	      internal_error (__FILE__, __LINE__,
> -			      _("generic_elf_osabi_sniff_abi_tag_sections: "
> -				"unknown OS number %d"),
> -			      abi_tag);
> +              warning ("GNU ABI tag value %u unrecognized.\n", abi_tag);

Wrap string in _().

Indentation looks odd.  tabs vs spaces?  (We use tabs in gdb.)

> +              *osabi = GDB_OSABI_UNKNOWN;

This line is not necessary.  It if were, we'd need to do the
same in the other two paths that don't match anything in
this function too.

OK with those fixed.

> +              break;
>  	    }
>  	  return;
>  	}

Thanks,
Pedro Alves

Re: [PATCH users/roland/osabi-assert] Do not crash on unrecognized GNU .note.ABI-tag values

[Doug Evans]

On Fri, May 29, 2015 at 3:07 AM, Pedro Alves <palves@redhat.com> wrote:
> On 05/29/2015 01:21 AM, Roland McGrath wrote:
>> Diagnosis of unexpected input (in this case, in an executable file)
>> should not crash as if it were a bug in GDB.
>>...
>> +              warning ("GNU ABI tag value %u unrecognized.\n", abi_tag);
>
> Wrap string in _().

No trailing \n too, right?

Re: [PATCH users/roland/osabi-assert] Do not crash on unrecognized GNU .note.ABI-tag values

[Pedro Alves]

On 05/29/2015 05:26 PM, Doug Evans wrote:
> On Fri, May 29, 2015 at 3:07 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 05/29/2015 01:21 AM, Roland McGrath wrote:
>>> Diagnosis of unexpected input (in this case, in an executable file)
>>> should not crash as if it were a bug in GDB.
>>> ...
>>> +              warning ("GNU ABI tag value %u unrecognized.\n", abi_tag);
>>
>> Wrap string in _().
> 
> No trailing \n too, right?

Right.

Thanks,
Pedro Alves

Re: [PATCH users/roland/osabi-assert] Do not crash on unrecognized GNU .note.ABI-tag values

[Roland McGrath]

On Fri, May 29, 2015 at 3:07 AM, Pedro Alves <palves@redhat.com> wrote:
> In order to get this into a release branch, the protocol is to file
> a PR and then once fixed, list the fix in the release page in the wiki:
>
>   https://sourceware.org/gdb/wiki/GDB_7.9_Release
>
> That helps the release manager build the release notes.

I've filed 18464 for it.  I'm not clear from what you said here whether I
should cherry-pick the commit onto the branch myself, or just list it in
the "Maybe" section on the wiki to wait for approval.

>> +              warning ("GNU ABI tag value %u unrecognized.\n", abi_tag);
>
> Wrap string in _().

I was following the model of the other warning call in that file (and the
end of gdbarch_init_osabi).  It seemed odd to me that it didn't have a
translation wrapper.  But looking around the rest of the code, I see now
that gdbarch_init_osabi is the outlier, and normal usage is to have a
translation wrapper and now trailing newline.  So I've fixed the new call.

> Indentation looks odd.  tabs vs spaces?  (We use tabs in gdb.)

I've found a lot of inconsistency as to that in this repository
(and even in this particular file).  I've fixed it to use tabs.

>> +              *osabi = GDB_OSABI_UNKNOWN;
>
> This line is not necessary.  It if were, we'd need to do the
> same in the other two paths that don't match anything in
> this function too.

OK.

Below is what I've committed to the trunk.
Can I cherry-pick it to the 7.9 branch too?


Thanks,
Roland


gdb/
2015-05-29  Roland McGrath  <mcgrathr@google.com>

	PR gdb/18464
	* osabi.c (generic_elf_osabi_sniff_abi_tag_sections): Use warning
	rather than internal_error for an unrecognized value.

diff --git a/gdb/osabi.c b/gdb/osabi.c
index 9d90c55..3581eb3 100644
--- a/gdb/osabi.c
+++ b/gdb/osabi.c
@@ -493,10 +493,8 @@ generic_elf_osabi_sniff_abi_tag_sections (bfd
*abfd, asection *sect, void *obj)
 	      break;

 	    default:
-	      internal_error (__FILE__, __LINE__,
-			      _("generic_elf_osabi_sniff_abi_tag_sections: "
-				"unknown OS number %d"),
-			      abi_tag);
+	      warning (_("GNU ABI tag value %u unrecognized."), abi_tag);
+	      break;
 	    }
 	  return;
 	}

Re: [PATCH users/roland/osabi-assert] Do not crash on unrecognized GNU .note.ABI-tag values

[Pedro Alves]

On 05/29/2015 05:32 PM, Roland McGrath wrote:
> On Fri, May 29, 2015 at 3:07 AM, Pedro Alves <palves@redhat.com> wrote:
>> In order to get this into a release branch, the protocol is to file
>> a PR and then once fixed, list the fix in the release page in the wiki:
>>
>>   https://sourceware.org/gdb/wiki/GDB_7.9_Release
>>
>> That helps the release manager build the release notes.
> 
> I've filed 18464 for it.

Thanks.

> I'm not clear from what you said here whether I
> should cherry-pick the commit onto the branch myself, or just list it in
> the "Maybe" section on the wiki to wait for approval.

I've added a new "Fixes in the GDB 7.9 branch" in case there's
another release.  Please push to the branch and list under "Done".

> Below is what I've committed to the trunk.
> Can I cherry-pick it to the 7.9 branch too?

Go ahead.

Thanks,
Pedro Alves

Re: [PATCH users/roland/osabi-assert] Do not crash on unrecognized GNU .note.ABI-tag values

[Roland McGrath]

On Fri, May 29, 2015 at 10:00 AM, Pedro Alves <palves@redhat.com> wrote:
> I've added a new "Fixes in the GDB 7.9 branch" in case there's
> another release.  Please push to the branch and list under "Done".

I can't seem to figure out how to edit the wiki.  (I only just created my
account there.)  The top line says "Immutable page".

>> Below is what I've committed to the trunk.
>> Can I cherry-pick it to the 7.9 branch too?
>
> Go ahead.

Done.

Re: [PATCH users/roland/osabi-assert] Do not crash on unrecognized GNU .note.ABI-tag values

[Pedro Alves]

On 05/29/2015 06:34 PM, Roland McGrath wrote:
> On Fri, May 29, 2015 at 10:00 AM, Pedro Alves <palves@redhat.com> wrote:
>> I've added a new "Fixes in the GDB 7.9 branch" in case there's
>> another release.  Please push to the branch and list under "Done".
> 
> I can't seem to figure out how to edit the wiki.  (I only just created my
> account there.)  The top line says "Immutable page".

Ah.  Just like in the glibc wiki, you need to be in the editors
group.  What's your account name?

> 
>>> Below is what I've committed to the trunk.
>>> Can I cherry-pick it to the 7.9 branch too?
>>
>> Go ahead.
> 
> Done.
> 


Thanks,
Pedro Alves

Re: [PATCH users/roland/osabi-assert] Do not crash on unrecognized GNU .note.ABI-tag values

[Roland McGrath]

On Fri, May 29, 2015 at 10:36 AM, Pedro Alves <palves@redhat.com> wrote:
> Ah.  Just like in the glibc wiki, you need to be in the editors
> group.

Yeah, but in glibc I have every kind of privilege and never kept track of
which ones I had or might need. ;-)

> What's your account name?

Shockingly enough, it's RolandMcGrath. :-)


Thanks,
Roland

Re: [PATCH users/roland/osabi-assert] Do not crash on unrecognized GNU .note.ABI-tag values

[Pedro Alves]

On 05/29/2015 07:21 PM, Roland McGrath wrote:

>> What's your account name?
> 
> Shockingly enough, it's RolandMcGrath. :-)

:-)  Added.

Thanks,
Pedro Alves