[PATCH] D142888: [compiler-rt] Fix scudo build on ARM

Leandro Lupori via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 3 06:13:54 PDT 2023


luporl added inline comments.


================
Comment at: compiler-rt/lib/scudo/standalone/CMakeLists.txt:157
 
 if (COMPILER_RT_HAS_GWP_ASAN)
   add_dependencies(scudo_standalone gwp_asan)
----------------
hctim wrote:
> luporl wrote:
> > hctim wrote:
> > > Maybe instead, just:
> > > 
> > > ```
> > > set(SCUDO_LINK_LIBS)
> > > 
> > > if (COMPILER_RT_HAS_GWP_ASAN)
> > >   if(COMPILER_RT_USE_LLVM_UNWINDER)
> > >     list(APPEND SCUDO_LINK_LIBS ${COMPILER_RT_UNWINDER_LINK_LIBS} dl)
> > >   elseif (COMPILER_RT_HAS_GCC_S_LIB)
> > >     list(APPEND SCUDO_LINK_LIBS gcc_s)
> > >   elseif (COMPILER_RT_HAS_GCC_LIB)
> > >     list(APPEND SCUDO_LINK_LIBS gcc)
> > >   else()
> > >     message(FATAL_ERROR "No suitable unwinder library")
> > >   endif()
> > > 
> > >   ... other stuff already in this branch
> > > endif()
> > > ```
> > >   
> > Just to confirm, is it ok to make this change for all architectures?
> > Non-ARM32 architectures will start to get an an error when `COMPILER_RT_USE_LLVM_UNWINDER` is OFF and libgcc is not found, even if they don't need to link against an unwind library.
> I'm kinda guessing that most people aren't missing an unwinder. Just checking the synthetic command line from a clang-link invocation on my machine, it just includes `-lunwind` by default.
> 
> So it's only the fancy-schmancy people who are using `COMPILER_RT_USE_LLVM_UNWINDER` who will get their own unwinder. Everyone else gets libunwind from gcc.
> 
> If people complain though, we can just disable GWP-ASan if there's no platform unwinder, and `message(WARNING ...` that it's happening.
Right, it makes sense. I'll proceed with the suggested changes, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142888/new/

https://reviews.llvm.org/D142888



More information about the llvm-commits mailing list