A quick architecture status report
diff mbox

Message ID CAKCAbMi5W_dGvaeMRdvjGrC9b09YK37TkCiyTkqL1F+5TPnpeg@mail.gmail.com
State New
Headers show

Commit Message

Zack Weinberg July 10, 2017, 3:53 p.m. UTC
On Mon, Jul 10, 2017 at 8:56 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Mon, 10 Jul 2017, Zack Weinberg wrote:
>
>> Alas, this is significantly more research than I have time for.  I was
>> hoping for a simple matter of adding .note.GNU-stack annotations to
>> our own .S files :)
>
> Since we build with -Wa,--noexecstack if the compiler puts .note.GNU-stack
> sections in its output, that's never necessary.  What's needed in GCC is
> (a) a TARGET_ASM_FILE_END hook that uses file_end_indicate_exec_stack, (b)
> any assembly sources in libgcc need .note.GNU-stack annotations.  Easy to
> add, but architecture maintainers are best placed to know if that's the
> right thing to do for the particular architecture.

Hm, what do you think of this patch?  It makes elf/check-execstack an
expected failure on any target where the compiler doesn't put
.note.GNU-stack sections in its output.  (UNSUPPORTED might be more
appropriate, but I don't see a way to trigger that from a makefile
conditional.)

zw

Comments

Joseph Myers July 10, 2017, 3:59 p.m. UTC | #1
On Mon, 10 Jul 2017, Zack Weinberg wrote:

> Hm, what do you think of this patch?  It makes elf/check-execstack an
> expected failure on any target where the compiler doesn't put
> .note.GNU-stack sections in its output.  (UNSUPPORTED might be more
> appropriate, but I don't see a way to trigger that from a makefile
> conditional.)

It's not clear it *should* be expected unless there's a good 
architecture-specific reason.
Andreas Schwab July 10, 2017, 4:08 p.m. UTC | #2
On Jul 10 2017, Zack Weinberg <zackw@panix.com> wrote:

> +# If the compiler does not support .note.GNU-stack for this
> +# architecture, check-execstack is expected to fail.
> +ifeq (,$(filter %noexecstack,$(ASFLAGS-config)))
> +test-xfail-check-execstack = yes
> +endif

An architecture that is noexec by default doesn't have -znoexecstack
either.

Andreas.
Zack Weinberg July 10, 2017, 4:13 p.m. UTC | #3
On Mon, Jul 10, 2017 at 11:59 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Mon, 10 Jul 2017, Zack Weinberg wrote:
>> Hm, what do you think of this patch?  It makes elf/check-execstack an
>> expected failure on any target where the compiler doesn't put
>> .note.GNU-stack sections in its output.  (UNSUPPORTED might be more
>> appropriate, but I don't see a way to trigger that from a makefile
>> conditional.)
>
> It's not clear it *should* be expected unless there's a good
> architecture-specific reason.

This is why I said UNSUPPORTED might be more appropriate.

Regardless of whether it *should* work, the fact is that it doesn't,
for reasons outside the control of code in glibc, and therefore
leaving this as a FAIL is inappropriate in my book.  Would you be
satisfied with a patch along these lines (and a more specific
configure check, because of what Andreas said downthread) if I also
filed bugs on GCC for all affected architectures?  I didn't find any
existing such bugs, but I didn't look very hard.

zw
Joseph Myers July 10, 2017, 4:40 p.m. UTC | #4
On Mon, 10 Jul 2017, Zack Weinberg wrote:

> On Mon, Jul 10, 2017 at 11:59 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> > On Mon, 10 Jul 2017, Zack Weinberg wrote:
> >> Hm, what do you think of this patch?  It makes elf/check-execstack an
> >> expected failure on any target where the compiler doesn't put
> >> .note.GNU-stack sections in its output.  (UNSUPPORTED might be more
> >> appropriate, but I don't see a way to trigger that from a makefile
> >> conditional.)
> >
> > It's not clear it *should* be expected unless there's a good
> > architecture-specific reason.
> 
> This is why I said UNSUPPORTED might be more appropriate.
> 
> Regardless of whether it *should* work, the fact is that it doesn't,
> for reasons outside the control of code in glibc, and therefore
> leaving this as a FAIL is inappropriate in my book.  Would you be

Well, if you (a new architecture, say) have executable stacks there's 
something wrong with your toolchain port unless there's a good reason, and 
I think it's desirable for there to be something immediately visible 
showing there's something wrong.

That is, we should decide case-by-case whether something should be fixed 
or XFAILed.  Which for this test should be fix if it's just a 
straightforward GCC change needed (ask architecture maintainers), XFAIL 
with comments otherwise.  Or obsolete the port in the absence of 
architecture maintainers (MicroBlaze, for which email to the maintainer 
bounces).

Patch
diff mbox

diff --git a/elf/Makefile b/elf/Makefile
index e758a4c960..f62f31a45b 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -1082,6 +1082,12 @@  $(objpfx)check-execstack.out:
$(..)scripts/check-execstack.awk \
        $(evaluate-test)
 generated += check-execstack.out

+# If the compiler does not support .note.GNU-stack for this
+# architecture, check-execstack is expected to fail.
+ifeq (,$(filter %noexecstack,$(ASFLAGS-config)))
+test-xfail-check-execstack = yes
+endif
+
 $(objpfx)tst-dlmodcount: $(libdl)
 $(objpfx)tst-dlmodcount.out: $(test-modules)