Re: gas -Werror compilation break [Re: [patch ld bfd gas]: Handle linkonce for cfi generated debug_frame and eh_frame data]

[Kai Tietz <ktietz@redhat.com>]

----- Original Message -----
From: "Dave Korn" <dave.korn.cygwin@gmail.com>
To: "Jan Kratochvil" <jan.kratochvil@redhat.com>
Cc: "Kai Tietz" <ktietz@redhat.com>, "Nick Clifton" <nickc@redhat.com>, "Kai Tietz" <ktietz70@googlemail.com>, "H.J. Lu" <hjl.tools@gmail.com>, "Binutils" <binutils@sourceware.org>, "Alan Modra" <amodra@gmail.com>
Sent: Thursday, May 5, 2011 2:29:20 AM
Subject: Re: gas -Werror compilation break  [Re: [patch ld bfd gas]: Handle linkonce for cfi generated debug_frame and eh_frame data]

On 27/04/2011 21:34, Jan Kratochvil wrote:
> Hi Kai,
> 
> On Tue, 26 Apr 2011 17:30:08 +0200, Nick Clifton wrote:
>>>>>  ChangeLog gas/
>>>>>  2011-04-18  Kai Tietz
>>>>>
>>>>>         * dw2gencfi.c (dwcfi_seg_list): New struct.
>>>>>         (dwcfi_hash): New static hash variable.
>>>>>         (get_debugseg_name): New.
>>>>>         (alloc_debugseg_item): New.
>>>>>         (make_debug_seg): New.
>>>>>         (dwcfi_hash_insert): New.
>>>>>         (dwcfi_hash_find): New.
>>>>>         (dwcfi_hash_find_or_make): New.
>>>>>         (cfi_insn_data): New member cur_seg.
>>>>>         (cie_entry): Likewise.
>>>>>         (fde_entry): New cseg and handled members.
>>>>>         (alloc_fde_entry): Initialize cseg member.
>>>>>         (alloc_cfi_insn_data): Initialize cur_seg member.
>>>>>         (dot_cfi_sections): Compare for beginning of
>>>>>         section names via strncmp.
>>>>>         (get_cfi_seg): New.
>>>>>         (cfi_finish): Treat link-once sections.
>>>>>         (is_now_linkonce_segment): New local helper.
>>>>>         (output_cie): Ignore cie entries not member of current
>>>>>         segment.
>>>>>         (output_fde): Likewise.
>>>>>         (select_cie_for_fde): Likewise.
>>>>>         (cfi_change_reg_numbers): Add new argument for current segment
>>>>>         and ignore insn elements, if not part of current segment.
>>>>>         * ehopt.c (get_cie_info): Use strncmp for
>>>>>         section name matching.
>>>>>         (heck_eh_frame): Likewise.
> 
> dw2gencfi.c:1870:8: error: variable ‘cfi_seg’ set but not used [-Werror=unused-but-set-variable]
> gcc-4.6.0-5.fc15.x86_64
> 
> Could you please provide a fix?

  This fixes the breakage, but doesn't address the issue that it looks to me
like there's an omitted call to md_fix_up_eh_frame() after the second
invocation of get_cfi_seg().

gas/ChangeLog:

	* dw2gencfi.c (dwcfi_hash): Wrap in #ifdef md_fix_up_eh_frame.
	(get_debugseg_name): Likewise.
	(alloc_debugseg_item): Likewise.
	(make_debug_seg): Likewise.
	(dwcfi_hash_insert): Likewise.
	(dwcfi_hash_find): Likewise.
	(dwcfi_hash_find_or_make): Likewise.
	(get_cfi_seg): Likewise.
	(cfi_finish): Wrap references to cfi_seg and calls to get_cfi_seg()
	in #ifdef md_fix_up_eh_frame likewise.

  Verified that it fixes the build on i686-pc-cygwin and
x86_64-unknown-linux-gnu, OK to install?

    cheers,
      DaveK

Hmm, I see the issue that cfi_seg might be just set, but not used. This can be fixed by simply adding - after setting of cfi_seg - an check for if (!cfi_seg) abort ();.  This check is superflous as cfi_seg never can be NULL, but it silence this warning.
The missing md_fix_up_eh_frame is used, as it was done in original code. Here just on case (IIRC it is the debug_frame one) uses it.  It might be necessary to add it here for the "eh_frame" case too. But I don't see actual a reason to add all this #ifdef guarding you do.

Regards,
Kai