diff mbox series

[1/2] libphobos: fix CET for non-glibc targets

Message ID 20211220000831.332831-1-alex_y_xu@yahoo.ca
State New
Headers
Series [1/2] libphobos: fix CET for non-glibc targets |

Commit Message

Alex Xu \(Hello71\) Dec. 20, 2021, 12:08 a.m. UTC 
  On musl, linking against libphobos fails because it requires ucontext
but is not explicitly linked against it. This is caused by configure
assuming that it is implemented in assembly, but it is actually not
implemented. This silently works on other libcs because context API does
not require an external library.
---
 libphobos/m4/druntime/libraries.m4 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Li, Pan2 via Gcc-patches Dec. 20, 2021, 1:56 p.m. UTC | #1 
> On 20/12/2021 01:08 Alex Xu (Hello71) via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
>  
> On musl, linking against libphobos fails because it requires ucontext
> but is not explicitly linked against it. This is caused by configure
> assuming that it is implemented in assembly, but it is actually not
> implemented. This silently works on other libcs because context API does
> not require an external library.

Thanks.

Looks reasonable to me, also for backporting to gcc-11, which also has the same CET support.

Iain.
Alex Xu \(Hello71\) Dec. 20, 2021, 3:41 p.m. UTC | #2 
Excerpts from ibuclaw@gdcproject.org's message of December 20, 2021 8:56 am:
>> On 20/12/2021 01:08 Alex Xu (Hello71) via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>> 
>>  
>> On musl, linking against libphobos fails because it requires ucontext
>> but is not explicitly linked against it. This is caused by configure
>> assuming that it is implemented in assembly, but it is actually not
>> implemented. This silently works on other libcs because context API does
>> not require an external library.
> 
> Thanks.
> 
> Looks reasonable to me, also for backporting to gcc-11, which also has the same CET support.
> 
> Iain.
> 

Yes, we noticed this first on gcc 11. I tested that these patches fix 
the issue on gcc 11, and since nothing seems to have changed, I think 
the same problem exists and will be fixed by these patches on master.

Cheers,
Alex.
Li, Pan2 via Gcc-patches Dec. 20, 2021, 9 p.m. UTC | #3 
> On 20/12/2021 16:41 Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:
> 
>  
> Excerpts from ibuclaw@gdcproject.org's message of December 20, 2021 8:56 am:
> >> On 20/12/2021 01:08 Alex Xu (Hello71) via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> >> 
> >>  
> >> On musl, linking against libphobos fails because it requires ucontext
> >> but is not explicitly linked against it. This is caused by configure
> >> assuming that it is implemented in assembly, but it is actually not
> >> implemented. This silently works on other libcs because context API does
> >> not require an external library.
> > 
> > Thanks.
> > 
> > Looks reasonable to me, also for backporting to gcc-11, which also has the same CET support.
> > 
> > Iain.
> > 
> 
> Yes, we noticed this first on gcc 11. I tested that these patches fix 
> the issue on gcc 11, and since nothing seems to have changed, I think 
> the same problem exists and will be fixed by these patches on master.
> 

Do you need me to commit this?

Iain.
Alex Xu \(Hello71\) Dec. 28, 2021, 2:18 p.m. UTC | #4 
Excerpts from ibuclaw@gdcproject.org's message of December 20, 2021 4:00 pm:
>> On 20/12/2021 16:41 Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:
>> 
>>  
>> Excerpts from ibuclaw@gdcproject.org's message of December 20, 2021 8:56 am:
>> >> On 20/12/2021 01:08 Alex Xu (Hello71) via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>> >> 
>> >>  
>> >> On musl, linking against libphobos fails because it requires ucontext
>> >> but is not explicitly linked against it. This is caused by configure
>> >> assuming that it is implemented in assembly, but it is actually not
>> >> implemented. This silently works on other libcs because context API does
>> >> not require an external library.
>> > 
>> > Thanks.
>> > 
>> > Looks reasonable to me, also for backporting to gcc-11, which also has the same CET support.
>> > 
>> > Iain.
>> > 
>> 
>> Yes, we noticed this first on gcc 11. I tested that these patches fix 
>> the issue on gcc 11, and since nothing seems to have changed, I think 
>> the same problem exists and will be fixed by these patches on master.
>> 
> 
> Do you need me to commit this?
> 
> Iain.
> 

I wrote this patch for Alpine Linux and have submitted it for review 
and, since I do not have commit access to gcc, in the hope that someone 
can commit it so that non-Alpine users may benefit and also Alpine will 
not have to maintain the patch. Since I am not familiar with gcc 
development procedures, I don't know if that means I need something from 
you specifically, but if somebody could commit it, that would be 
appreciated.

Also, since I forgot to do so initially, please apply Signed-off-by: 
Alex Xu (Hello71) <alex_y_xu@yahoo.ca>.

Cheers,
Alex.
diff mbox series

Patch

diff --git a/libphobos/m4/druntime/libraries.m4 b/libphobos/m4/druntime/libraries.m4
index 45a56f6f76a..e11582e433a 100644
--- a/libphobos/m4/druntime/libraries.m4
+++ b/libphobos/m4/druntime/libraries.m4
@@ -223,10 +223,14 @@  AC_DEFUN([DRUNTIME_LIBRARIES_UCONTEXT],
   case "$target_cpu" in
     aarch64* | \
     arm* | \
-    i[[34567]]86|x86_64 | \
     powerpc)
       druntime_fiber_asm_external=yes
       ;;
+    i[[34567]]86|x86_64)
+      if test "$enable_cet" = no; then
+        druntime_fiber_asm_external=yes
+      fi
+      ;;
   esac
   if test "$druntime_fiber_asm_external" = no; then
     AC_SEARCH_LIBS([swapcontext], [c ucontext], [],