[compiler-rt][cmake] Provide empty version of enable_execute_stack for baremetal targets

[clm]

Summary

This patch allows compiler-rt to build for targets that don't have support for mprotect (ie. baremetal) by providing an empty version of enable_execute_stack.

Okay to commit?

Thanks,
Catherine

[clm]

The first version of the patch failed to include the empty version of enable_execute_stack. The diff is now updated.

[jroelofs]

lib/builtins/CMakeLists.txt

193

This blob of cmake goop would be simplified if, rather than having two files for enable_execute_stack{_empty,}.c, the flag just controlled the body's existence in enable_execute_stack.c.

lib/builtins/enable_execute_stack_empty.c

6

LLVM style nit: we don't put a space between function names and the opening paren.

[joerg]

Why do you call __enable_execute_stack in first place? I would just leave it and fix any potential callers.

[jroelofs]

In D33018#750300, @joerg wrote:

Why do you call __enable_execute_stack in first place? I would just leave it and fix any potential callers.

GCC inserts calls to it for nested functions.

[joerg]

To expand on what I said on IRC:

(1) I don't think anything should normally call it. For GCC, it depends on the target configuration whether it believes __enable_execute_stack is supported/required or not.
(2) We shouldn't fail to build compiler-rt for this.
(3) It is better to fail linking a user program with this when it is not expected / wasted code than silently hiding the problem.

As such, I'd prefer to just skip the whole default implementation for platforms where it shouldn't be used. This includes bare-metal.

[clm]

I've now updated the patch based on Joerg's comments. This version checks the setting of COMPILER_RT_BAREMETAL_BUILD before building enable_execute_stack.c.

CMakeLists.txt

 option(COMPILER_RT_BUILD_XRAY "Build xray" ON)
 mark_as_advanced(COMPILER_RT_BUILD_XRAY)
 
+set(COMPILER_RT_BAREMETAL_BUILD OFF CACHE BOOLEAN
+  "Build for a bare-metal target.")
+
 if (COMPILER_RT_STANDALONE_BUILD)
   load_llvm_config()
 

lib/builtins/CMakeLists.txt

   divti3.c
   divtf3.c
   divxc3.c
-  enable_execute_stack.c
   eprintf.c
   extendsfdf2.c
   extendhfsf2.c

   "Skip the atomic builtin (this may be needed if system headers are unavailable)"
   Off)
 
+if(COMPILER_RT_BAREMETAL_BUILD)
+  set(GENERIC_SOURCES
+    ${GENERIC_SOURCES}
+    enable_execute_stack_empty.c)
+else()
+  set(GENERIC_SOURCES
+    ${GENERIC_SOURCES}
+    enable_execute_stack.c)
+endif()
+
 if(COMPILER_RT_HAS_ATOMIC_KEYWORD AND NOT COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN)
   set(GENERIC_SOURCES
     ${GENERIC_SOURCES}

lib/builtins/README.txt

 
 // __enable_execute_stack() is used with nested functions when a trampoline
 // function is written onto the stack and that page range needs to be made
-// executable.
+// executable.  A dummy version is provided for baremetal targets that do
+// not have support for mprotect.
 void __enable_execute_stack(void* addr);
 
 // __gcc_personality_v0() is normally only called by the system unwinder.

lib/builtins/enable_execute_stack_empty.c

+/* Dummy implementation of __enable_execute_stack.  */
+
+extern void __enable_execute_stack (void *);
+
+void
+__enable_execute_stack (void *addr __attribute__((__unused__)))
+{
+}