* default_no_named_section bad default
@ 2013-05-31 18:13 Mike Stump
2013-05-31 21:53 ` Mike Stump
2013-05-31 21:56 ` Andrew Pinski
0 siblings, 2 replies; 6+ messages in thread
From: Mike Stump @ 2013-05-31 18:13 UTC (permalink / raw)
To: gcc-patches
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.
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: default_no_named_section bad default
2013-05-31 18:13 default_no_named_section bad default Mike Stump
@ 2013-05-31 21:53 ` Mike Stump
2013-05-31 21:56 ` Andrew Pinski
1 sibling, 0 replies; 6+ messages in thread
From: Mike Stump @ 2013-05-31 21:53 UTC (permalink / raw)
To: gcc-patches
On May 31, 2013, at 11:13 AM, Mike Stump <mikestump@comcast.net> wrote:
> I think really, we either need to have a default of 0 (to crash in a nice way)
The previous patch does not fix it. I think we need to use 0 instead:
Ok?
New idea:
Index: target.def
===================================================================
--- target.def (revision 199270)
+++ target.def (working copy)
@@ -225,7 +225,7 @@ DEFHOOK
(named_section,
"",
void, (const char *name, unsigned int flags, tree decl),
- default_no_named_section)
+ 0)
/* Return preferred text (sub)section for function DECL.
Main purpose of this function is to separate cold, normal and hot
Index: varasm.c
===================================================================
--- varasm.c (revision 199270)
+++ varasm.c (working copy)
@@ -6042,19 +6042,6 @@ have_global_bss_p (void)
return bss_noswitch_section || targetm.have_switchable_bss_sections;
}
-/* Output assembly to switch to section NAME with attribute FLAGS.
- Four variants for common object file formats. */
-
-void
-default_no_named_section (const char *name ATTRIBUTE_UNUSED,
- unsigned int flags ATTRIBUTE_UNUSED,
- tree decl ATTRIBUTE_UNUSED)
-{
- /* Some object formats don't support named sections at all. The
- front-end should already have flagged this as an error. */
- gcc_unreachable ();
-}
-
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: default_no_named_section bad default
2013-05-31 18:13 default_no_named_section bad default Mike Stump
2013-05-31 21:53 ` Mike Stump
@ 2013-05-31 21:56 ` Andrew Pinski
2013-05-31 22:29 ` Mike Stump
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Pinski @ 2013-05-31 21:56 UTC (permalink / raw)
To: Mike Stump; +Cc: GCC Patches
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: default_no_named_section bad default
2013-05-31 21:56 ` Andrew Pinski
@ 2013-05-31 22:29 ` Mike Stump
2013-06-03 8:27 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Mike Stump @ 2013-05-31 22:29 UTC (permalink / raw)
To: Andrew Pinski; +Cc: Mike Stump, GCC Patches
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: default_no_named_section bad default
2013-05-31 22:29 ` Mike Stump
@ 2013-06-03 8:27 ` Richard Biener
2013-06-03 20:02 ` Mike Stump
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2013-06-03 8:27 UTC (permalink / raw)
To: Mike Stump; +Cc: Andrew Pinski, GCC Patches
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: default_no_named_section bad default
2013-06-03 8:27 ` Richard Biener
@ 2013-06-03 20:02 ` Mike Stump
0 siblings, 0 replies; 6+ messages in thread
From: Mike Stump @ 2013-06-03 20:02 UTC (permalink / raw)
To: Richard Biener; +Cc: Mike Stump, Andrew Pinski, GCC Patches
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-03 20:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-31 18:13 default_no_named_section bad default Mike Stump
2013-05-31 21:53 ` Mike Stump
2013-05-31 21:56 ` Andrew Pinski
2013-05-31 22:29 ` Mike Stump
2013-06-03 8:27 ` Richard Biener
2013-06-03 20:02 ` Mike Stump
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).