default_no_named_section bad default

So, on darwin, the new tools don't like FDE information when you have:

__Z24default_no_named_sectionPKcjP9tree_node:
LFB588:
LFE588:

in the object file.

$ dwarfdump --eh-frame --verify varasm.o
----------------------------------------------------------------------
 File: varasm.o (x86_64)
----------------------------------------------------------------------
Verifying EH Frame... error: FDE row for address 0x00000000000058f0 is not in the FDE address range.

0x000020e0: FDE
        length: 0x0000001c
   CIE_pointer: 0x00000000
    start_addr: 0x00000000000058f0 __Z24default_no_named_sectionPKcjP9tree_node
    range_size: 0x0000000000000000 (end_addr = 0x00000000000058f0)
                DW_CFA_nop
                DW_CFA_nop
                DW_CFA_nop
                DW_CFA_nop
                DW_CFA_nop
                DW_CFA_nop
                DW_CFA_nop
  Instructions: 0x00000000000058f0: CFA=rsp+8     rip=[rsp]

which, well, seems reasonable.  This causes the static linker to reject it.  Now, one can prune out the pair, but, I think really, we either need to have a default of 0 (to crash in a nice way), or an assert (to crash in a nice way).  Just falling off then end into space (the next function in the executable file), seems wrong.

Ok?

	PR57438
	* varasm.c (default_no_named_section): Assert instead.

On Fri, May 31, 2013 at 11:13 AM, Mike Stump <mikestump@comcast.net> wrote:
> So, on darwin, the new tools don't like FDE information when you have:
>
> __Z24default_no_named_sectionPKcjP9tree_node:
> LFB588:
> LFE588:
>
> in the object file.
>
> $ dwarfdump --eh-frame --verify varasm.o
> ----------------------------------------------------------------------
>  File: varasm.o (x86_64)
> ----------------------------------------------------------------------
> Verifying EH Frame... error: FDE row for address 0x00000000000058f0 is not in the FDE address range.
>
> 0x000020e0: FDE
>         length: 0x0000001c
>    CIE_pointer: 0x00000000
>     start_addr: 0x00000000000058f0 __Z24default_no_named_sectionPKcjP9tree_node
>     range_size: 0x0000000000000000 (end_addr = 0x00000000000058f0)
>                 DW_CFA_nop
>                 DW_CFA_nop
>                 DW_CFA_nop
>                 DW_CFA_nop
>                 DW_CFA_nop
>                 DW_CFA_nop
>                 DW_CFA_nop
>   Instructions: 0x00000000000058f0: CFA=rsp+8     rip=[rsp]
>
> which, well, seems reasonable.  This causes the static linker to reject it.  Now, one can prune out the pair, but, I think really, we either need to have a default of 0 (to crash in a nice way), or an assert (to crash in a nice way).  Just falling off then end into space (the next function in the executable file), seems wrong.
>
> Ok?
>
>         PR57438
>         * varasm.c (default_no_named_section): Assert instead.


This will only fix the GCC source but not other sources which does:
void f(void)
{
  __builtin_unreachable();
}

Thanks,
Andrew

>
> Index: varasm.c
> ===================================================================
> --- varasm.c    (revision 199270)
> +++ varasm.c    (working copy)
> @@ -6052,7 +6052,7 @@ default_no_named_section (const char *na
>  {
>    /* Some object formats don't support named sections at all.  The
>       front-end should already have flagged this as an error.  */
> -  gcc_unreachable ();
> +  gcc_assert (0);
>  }
>
>  #ifndef TLS_SECTION_ASM_FLAG
>

On May 31, 2013, at 2:56 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> This will only fix the GCC source but not other sources which does:
> void f(void)
> {
>  __builtin_unreachable();
> }

Yes.  Speaking of which, so how should this be handled?  Imagine we have asm("# no bytes") before the unreachable.  The compiler can't know the size (though, the linker can), and yet, a good solution handles this as well.  Hopefully a dwarf person can weigh in, as engineering a bad solution is worse than leaving it broken in my book.

On Sat, Jun 1, 2013 at 12:28 AM, Mike Stump <mikestump@comcast.net> wrote:
> On May 31, 2013, at 2:56 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>> This will only fix the GCC source but not other sources which does:
>> void f(void)
>> {
>>  __builtin_unreachable();
>> }
>
> Yes.  Speaking of which, so how should this be handled?  Imagine we have asm("# no bytes") before the unreachable.  The compiler can't know the size (though, the linker can), and yet, a good solution handles this as well.  Hopefully a dwarf person can weigh in, as engineering a bad solution is worse than leaving it broken in my book.

Let the assembler compute it as difference of two labels?

Richard.

On Jun 3, 2013, at 1:27 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>> Yes.  Speaking of which, so how should this be handled?  Imagine we have asm("# no bytes") before the unreachable.  The compiler can't know the size (though, the linker can), and yet, a good solution handles this as well.  Hopefully a dwarf person can weigh in, as engineering a bad solution is worse than leaving it broken in my book.
> 
> Let the assembler compute it as difference of two labels?

We do use two labels, one for the start, one for the end.