Alpine aarch64 build questions

[Downchuck]

Summary

Running cmake --build ./build on a stock Alpine Linux installation with clang 17 (and clang 18) resulted in two compile errors - I am not clear if there's a simple environment fix or if it just needs more actual code in the Hermes repo.

lseek has lseek64 capabilities, but neither lseek or lseek64 are in the namespace scope. (raw_ostream.cpp) thread-safety-analysis is not picking up capability annotations for std:mutex and so the analysis fails on the lock.

They're very simple and the patch just shows an inline workaround (e.g. calling lseek instead of lseek64, and for the sake of compiling, just disabling tsa as that source is already verified by other build environments)

Test Plan

This code presents the two issues and work-arounds, and is request for clarification as to the correct environment or appropriate workarounds:

lseek64 and sleek are not in the global namespace, and it may be that lseek is desired though the capability is 64. This may have some clue as to that evolution: https://reviews.llvm.org/D139752?id=484363

The failure of the TSA attribute is seen here:

/tmp/scratch/hermes/API/hermes/cdp/DomainAgent.h:67:39: warning: 'guarded_by' attribute requires arguments whose type is annotated with 'capability' attribute; type here is 'std::mutex' [-Wthread-safety-attributes]
   67 |     std::function<void(Args...)> func TSA_GUARDED_BY(mutex);
      |                                       ^
/tmp/scratch/hermes/API/hermes/../hermes/ThreadSafetyAnalysis.h:26:61: note: expanded from macro 'TSA_GUARDED_BY'
   26 | #define TSA_GUARDED_BY(x) TSA_THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x))
      |                                                             ^
/tmp/scratch/hermes/API/hermes/cdp/DomainAgent.h:47:25: error: reading variable 'func' requires holding mutex 'funcContainer_->mutex' [-Werror,-Wthread-safety-analysis]
   47 |     if (funcContainer_->func) {

While this couple line patch works fine to get things going - it would be nice to understand if there's an appropriate patch or change to the environment. I've not checked a separate aarch64 Linux distribution with LLVM. I've not checked for pre-built releases from Hermes for aarch64. React Native 0.76 does not supply aarch64.

With this small patch I believe I'm building just fine end-to-end for RN 0.76 on aarch64. This patch is not intended to be merged upstream, but rather illustrates the only two issues I hit in compiling.

[tmikov]

Hi, we are still looking into the lseek64() issue, but with regards to TSA, we think we should just turn it off by default. Would you please submit a PR changing HERMES_THREAD_SAFETY_ANALYSIS to OFF in the root CMakeLists.txt?

[dannysu]

hi @Downchuck, for the thread safety and for the changes under cdp, could you please make a dedicated PR? We'll review it.

For thread safety, rather than changing it to not error, could you please change https://github.com/facebook/hermes/blob/main/CMakeLists.txt to turn HERMES_THREAD_SAFETY_ANALYSIS to OFF by default?

Thanks

[Downchuck]

@dannysu @tmikov I think for lseek this one is fine for adding to the file:

#ifdef HAVE_LSEEK64
#ifndef lseek64
off64_t lseek64(int fd, off64_t offset, int whence) {
  return lseek(fd, offset, whence);
}
#endif
#endif

Per the lseek documentation. I'll put up the two separate PRs, one for external/llvh/lib/Support/raw_ostream.cpp and one for CMakeLists.txt