This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: AW: Software Quality Binutils
On 08/23/2018 06:34 AM, Nick Clifton wrote:
> Hi John,
>
>> I looked deeper at some of the cases that cppcheck threw up. Some are
>> clearly false, others I cannot judge, but this one
>>
>> [elf64-hppa.c:3575] -> [elf64-hppa.c:3561]: (warning) Either the condition hh==0 is redundant or there is possible null pointer dereference: hh.
>>
>> unless I am very much mistaken is guaanteed to crash if the relevant
>> line is ever reached.
>
> Indeed you are right. I am checking the patch below to remove the
> block of code, since it can never have been executed, and replace
> it with an assertion.
>
> Cheers
> Nick
>
> bfd/ChangeLog
> 2018-08-23 Nick Clifton <nickc@redhat.com>
>
> * elf64-hppa.c (elf_hppa_final_link_relocate): Replace unworkable
> code with an assertion.
>
> diff --git a/bfd/elf64-hppa.c b/bfd/elf64-hppa.c
> index 2e66c92bfb..b4f047fa64 100644
> --- a/bfd/elf64-hppa.c
> +++ b/bfd/elf64-hppa.c
> @@ -3556,33 +3556,12 @@ elf_hppa_final_link_relocate (Elf_Internal_Rela *rel,
>
> case R_PARISC_LTOFF_FPTR32:
> {
> - /* We may still need to create the FPTR itself if it was for
> - a local symbol. */
> - if (hh == NULL)
> - {
> - /* The first two words of an .opd entry are zero. */
> - memset (hppa_info->opd_sec->contents + hh->opd_offset, 0, 16);
> -
> - /* The next word is the address of the function. */
> - bfd_put_64 (hppa_info->opd_sec->owner, value + addend,
> - (hppa_info->opd_sec->contents
> - + hh->opd_offset + 16));
> -
> - /* The last word is our local __gp value. */
> - value = _bfd_get_gp_value
> - (hppa_info->opd_sec->output_section->owner);
> - bfd_put_64 (hppa_info->opd_sec->owner, value,
> - hppa_info->opd_sec->contents + hh->opd_offset + 24);
> -
> - /* The DLT value is the address of the .opd entry. */
> - value = (hh->opd_offset
> - + hppa_info->opd_sec->output_offset
> - + hppa_info->opd_sec->output_section->vma);
> -
> - bfd_put_64 (hppa_info->dlt_sec->owner,
> - value,
> - hppa_info->dlt_sec->contents + hh->dlt_offset);
> - }
> + /* FIXME: There used to be code here to create the FPTR itself if
> + the relocation was against a local symbol. But the code could
> + never have worked. If the assert below is ever triggered then
> + the code will need to be reinstated and fixed so that it does
> + what is needed. */
> + BFD_ASSERT (hh != NULL);
My involvement with the PA has diminished greatly, but I think this code
is to support indirect calls to local functions. The big question is
did the 32-bit ELF ABI eliminate the need for fptr relocations -- I
vaguely recall they wanted to clean this up because the cost of indirect
calls was considered high, but I simply don't remember if it changed.
The way to try and trigger would be to have an indirect call to a local
function. At least with the SOM 32bit ABI that should trigger the need
for an FPTR that references a local function.
Jeff