In D158375#4617637, @MaskRay wrote:

Given how late we are in the release process, I'll go ahead, apply the changes, and land this patch.
I've landed other fixes for @raj.khem and hope that he doesn't mind:)

This is also needed on 17.0.0 branch too.

In D142880#4538652, @kito-cheng wrote:

@raj.khem ooops, thanks for report that, I am happy to debug that, so...it would be appreciate if you can provide files by -Wl,--reproduce=xx.tar :)

In D142880#4537432, @raj.khem wrote:

In D142880#4534918, @asb wrote:

Just cross-linking to https://github.com/llvm/llvm-project/issues/64102 - as we discussed at the last RISC-V LLVM sync-up we really would like to land this and get it into the LLVM 17 branch if possible, to avoid compatibility issues with the next binutils release.

For cross reference. this review https://reviews.llvm.org/D142879 also needs to land together with this.

In D142880#4534918, @asb wrote:

Just cross-linking to https://github.com/llvm/llvm-project/issues/64102 - as we discussed at the last RISC-V LLVM sync-up we really would like to land this and get it into the LLVM 17 branch if possible, to avoid compatibility issues with the next binutils release.

In D154852#4534868, @labrinea wrote:

Thanks. Reverted. Looks like an infinite loop.

I am seeing a crash in libLLVM-17.so when compiling socat on aarch64 and have narrowed it down to this change. here is testcase
https://uclibc.org/~kraj/nestlex.zip

indeed, broke my CI with same issue.

In D154555#4475661, @alexfh wrote:

Thanks for the quick fix! However, I'm not convinced this is a trivial fix for https://github.com/llvm/llvm-project/issues/63661, thus it may introduce other problems and make reverting to a known good state even more complex. As I mentioned on https://reviews.llvm.org/rG003b58f65bdd5d9c7d0c1b355566c9ef430c0e7d#1223579, I'd prefer a revert to green before this fix gets tested and the commits starting from 003b58f65bdd5d9c7d0c1b355566c9ef430c0e7d are recommitted.

I am late to party I know. I am trying to enable lld as default system linker for one of yocto based distros and this option turned on by default now breaks quite a few packages on linux with this error

This would fix https://github.com/llvm/llvm-project/issues/62192

In D144514#4144195, @ro wrote:

• You still haven't mentioned which target is affected.
• There needs to be a comment explaining why _TIME_BITS is #undefed.
• Please pass the patch through clang-format-diff.py: there's a TAB before !defined which should just be a blank.

In D144514#4142654, @ro wrote:

Please state what target the problem occurs on and how/where the patch was tested. _TIME_BITS doesn't exist on Solaris 11.4 and, AFAICS, neither on Illumos.

If the patch goes in, please add a comment why that #undef is needed.

That said, I wonder why sanitizer_platform_limits_posix.cpp doesn't have the same issue. It seems weird to me to work around a non-Solaris issue in a Solaris-specific file. ISTM that <features.h> is included in sanitizer_platform.h only to get a definition of __GLIBC__. I'd find it way more logical to handle the issue there, rather than in sanitizer_procmaps_solaris.cpp.

In D139752#4091098, @srj wrote:

In D139752#4089118, @MaskRay wrote:

In D139752#4074965, @raj.khem wrote:

In D139752#4074691, @MaskRay wrote:

@raj.khem would you be able to figure out the issue? :) https://llvm.org/ release/16.x will be created soon...

I have been trying to reproduce it on my end but have not succeeded, I don't have same setup as @srj, I would also look for some alternative ways to do it so we can avoid the issue at hand which seems some weird cmake thing.

@srj It will be nice to figure out the 32-bit glibc Halide issue and fix this for just just-branched-release/16.x so that llvm-project 16.0 can be built with next musl release, otherwise many musl based distributions will have to carry this local patch...

I wonder if this could be a bug in a specific version(s) of CMake? What version(s) have you tried replicating with? Our buildbots are using v3.22.6 (Halide requires a minimum of 3.22).

In D139752#4074691, @MaskRay wrote:

@raj.khem would you be able to figure out the issue? :) https://llvm.org/ release/16.x will be created soon...

In D141487#4067417, @aeubanks wrote:

this causes opt -passes=loop-unroll-full to crash on

define void @foo() {
bb:
  br label %bb1
    
bb1:                                              ; preds = %bb1, %bb1, %bb
  switch i1 true, label %bb1 [
    i1 true, label %bb2
    i1 false, label %bb1
  ]
    
 bb2:                                              ; preds = %bb1
  ret void
}

so I've reverted this for now

In D140875#4065824, @hokein wrote:

In D140875#4065763, @ckandeler wrote:

With this, I now get:
FAILED: bin/clangd-fuzzer
: && /usr/lib/icecream/libexec/icecc/bin/c++ -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-redundant-move -Wno-pessimizing-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wno-misleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O2 -g -DNDEBUG -fuse-ld=lld -Wl,--color-diagnostics -Wl,--gc-sections tools/clang/tools/extra/clangd/fuzzer/CMakeFiles/clangd-fuzzer.dir/FuzzerClangdMain.cpp.o tools/clang/tools/extra/clangd/fuzzer/CMakeFiles/clangd-fuzzer.dir/clangd-fuzzer.cpp.o -o bin/clangd-fuzzer -Wl,-rpath,"\$ORIGIN/../lib" lib/libclangDaemon.a lib/libclangdSupport.a lib/libclangPseudo.a lib/libclangPseudoGrammar.a lib/libclangTidyAndroidModule.a lib/libclangTidyAbseilModule.a lib/libclangTidyAlteraModule.a lib/libclangTidyBoostModule.a lib/libclangTidyCERTModule.a lib/libclangTidyConcurrencyModule.a lib/libclangTidyDarwinModule.a lib/libclangTidyFuchsiaModule.a lib/libclangTidyHICPPModule.a lib/libclangTidyBugproneModule.a lib/libclangTidyCppCoreGuidelinesModule.a lib/libclangTidyGoogleModule.a lib/libclangTidyLinuxKernelModule.a lib/libclangTidyLLVMModule.a lib/libclangTidyLLVMLibcModule.a lib/libclangTidyMiscModule.a lib/libclangAnalysis.a lib/libclangASTMatchers.a lib/libclangAST.a lib/libclangLex.a lib/libclangBasic.a lib/libclangTidyModernizeModule.a lib/libclangTidyObjCModule.a lib/libclangTidyOpenMPModule.a lib/libclangTidyPerformanceModule.a lib/libclangTidyPortabilityModule.a lib/libclangTidyReadabilityModule.a lib/libclangTidyZirconModule.a lib/libclangTidyMPIModule.a lib/libclangTidyUtils.a lib/libclangTidy.a lib/libclang-cpp.so.16git lib/libLLVM-16git.so && :
ld.lld: error: undefined symbol: clang::include_cleaner::walkUsed(llvm::ArrayRef<clang::Decl*>, llvm::ArrayRef<clang::include_cleaner::SymbolReference>, clang::include_cleaner::PragmaIncludes const*, clang::SourceManager const&, llvm::function_ref<void (clang::include_cleaner::SymbolReference const&, llvm::ArrayRef<clang::include_cleaner::Header>)>)

referenced by IncludeCleaner.cpp:504 (/sda/home/christian/dev/llvm/clang-tools-extra/clangd/IncludeCleaner.cpp:504)

IncludeCleaner.cpp.o:(clang::clangd::computeUnusedIncludesExperimental(clang::clangd::ParsedAST&) (.localalias)) in archive lib/libclangDaemon.a

sorry, should be fixed in https://github.com/llvm/llvm-project/commit/e84d69f52d9a9fab9162128d8fe8ebec99ea60da.

In D140948#4026982, @MaskRay wrote:

Is mips o32 still worth spending efforts? For mips perhaps we should only care about newer ABIs. Sanitizers are optional features anyway...

The patch looks good to me. I have tested it in yocto world builds, it fixes the problem and there are no new regressions.

In D129749#3783318, @xry111 wrote:

In D129749#3783314, @dmilosevic141 wrote:

In D129749#3783137, @raj.khem wrote:

I think this patch has created regression when using largefile support. So try adding -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 to compiler cmdline on mips (o32 ABI) and the size of this struct is bumped to 160 on linux/glibc see definition of stat for mips in glibc

https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/mips/bits/struct_stat.h;h=71594e456244d1e4a746c859048793385c5be376;hb=HEAD#l28

Thanks for pointing this out @raj.khem.

As far as I can see, -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 is expected by default for the O32 ABI in LLVM - see https://github.com/llvm/llvm-project/blob/main/compiler-rt/cmake/base-config-ix.cmake#L224. GCC, however, doesn't follow this - please see the following discussion https://gcc.gnu.org/pipermail/gcc-patches/2022-July/597617.html.

Rather than working with CMake, shouldn't we be able to just add a check if _LARGEFILE_SOURCE is defined and if _FILE_OFFSET_BITS is defined and equal to 64? If it is so, struct_kernel_stat_sz would be equal to 160, rather than 144 (for the O32 ABI). This change would then be cherry-picked into GCC. Thus we would cover every possible case for both LLVM and GCC.

The problem is we need to ensure libsanitizer functional properly, not just papering over the issue and make it build. That's the point of why we define our own value of struct_kernel_stat_sz and check it with COMPILER_CHECK, instead of just using sizeof(struct stat).

In D129749#3783186, @xry111 wrote:

In D129749#3783137, @raj.khem wrote:

I think this patch has created regression when using largefile support. So try adding -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 to compiler cmdline on mips (o32 ABI) and the size of this struct is bumped to 160 on linux/glibc see definition of stat for mips in glibc

https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/mips/bits/struct_stat.h;h=71594e456244d1e4a746c859048793385c5be376;hb=HEAD#l28

Hmm, so should we force -D_FILE_OFFSET_BITS=64 in the C compiler (clang and gcc) when -fsanitize=address is used? AFAIK There is nothing we can do to support both cases (with or w/o -D_FILE_OFFSET_BITS=64) in one libsanitizer build.

I think this patch has created regression when using largefile support. So try adding -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 to compiler cmdline on mips (o32 ABI) and the size of this struct is bumped to 160 on linux/glibc see definition of stat for mips in glibc

In D71387#3755056, @lewis-revill wrote:

I believe this patch is still relevant/necessary when using LTO for RISCV, so can I ask if @khchen is able to update it to rebase/address the feedback? If not, are there are any objections to me commandeering this revision to get it landed?

My local testing on clang-15 shows no problems with this patch so lgtm

I have tested this revision and it fixes the problem I was seeing

I am getting a compiler error on clang-15, can you confirm if this will work with clang-15 too

I built this on top if 14.x branch and it fixes the problems, I was seeing and the warning is still emitted as expected.

I have submitted a github issue which seems to be related here https://github.com/llvm/llvm-project/issues/54456

rebased

@MaskRay can you help in moving this forward ?

rebased

@peter.smith Can you help landing this changeset ? there has been no objections on it thus far.

In D121379#3373679, @MaskRay wrote:

Note: powerpc64 defines __powerpc__ as well. You need to check whether powerpc64 needs this as well.

In D119590#3316050, @JonChesterfield wrote:

Cross compilers are a hazard here. I'd expect there to be a fairly long list of magic flags you need to pass to clang to get it to find the right libraries. Can you add fno-openmp-implicit-rpath to that list instead?

I tested this patch and it fixes lldb build. Thanks for taking care of this,.

In D119058#3301524, @MaskRay wrote:

Move LLVM_ENABLE_ZLIB

I verified both patches together on latest 13.x branch and it fixes the issue I was seeing. So please apply both the fixes suggested in description to 13.x branch.

In D107156#2970143, @JonChesterfield wrote:

The current trunk cmake guards whether to build this with:

if (LLVM_DIR)
  # Builds that use pre-installed LLVM have LLVM_DIR set.                                                                                                                                                          
  find_program(CLANG_TOOL clang PATHS ${LLVM_TOOLS_BINARY_DIR} NO_DEFAULT_PATH)
  find_program(LINK_TOOL llvm-link PATHS ${LLVM_TOOLS_BINARY_DIR}
    NO_DEFAULT_PATH)
  find_program(OPT_TOOL opt PATHS ${LLVM_TOOLS_BINARY_DIR} NO_DEFAULT_PATH)
  libomptarget_say("Building AMDGCN device RTL. Using clang: ${CLANG_TOOL}")
elseif (LLVM_TOOL_CLANG_BUILD AND NOT CMAKE_CROSSCOMPILING AND NOT OPENMP_STANDALONE_BUILD)
  # LLVM in-tree builds may use CMake target names to discover the tools.                                                                                                                                          
  set(CLANG_TOOL $<TARGET_FILE:clang>)
  set(LINK_TOOL $<TARGET_FILE:llvm-link>)
  set(OPT_TOOL $<TARGET_FILE:opt>)
  libomptarget_say("Building AMDGCN device RTL. Using clang from in-tree build")
else()
  libomptarget_say("Not building AMDGCN device RTL. No appropriate clang found")
  return()
endif()

standalone cross-build

Which path are you hitting? Can grep for 'Building AMDGCN device RTL' to see which.

This breaks a case where opt is used from installed locations especially in standalone cross-build environment we pass -DOPT_TOOL=${STAGING_BINDIR_NATIVE}/opt and it worked ok but now its expecting opt to be built along with openmp build and fails like below

https://reviews.llvm.org/D104788 fixes it as well.

In D104821#2837568, @kkwli0 wrote:

Does D104788 take care of it?

In D104183#2816953, @peter.smith wrote:

For ELF targets I think this should work fine. However looking at the previous change I think this may break the Darwin build. This change looks like it reverts:

commit 84610ba9b38d917317ac997074acacd99962d767
Author: Steven Wu <stevenwu@apple.com>
Date:   Sat Oct 4 00:18:59 2014 +0000

    Fix the armv7 thumb builtins on darwin

    The arm builtins converted into thumb in r213481 are not working
    on darwin. On apple platforms, .thumb_func directive is required
    to generated correct symbols for thumb functions.

    <rdar://problem/18523605>

    llvm-svn: 219040

I think we'll need a way to get .thumb_func added for Darwin or potentially call DEFINE_COMPILERRT_FUNCTION for Armv6.

In D104183#2815292, @compnerd wrote:

Some platforms (e.g. WoA) do require thumb-only compilation. Can we conditionalize this on whether we are building for ARMv6 or not?

ping,

http://sprunge.us/FJzZXL is a file from harfbuzz and it warns

In D102872#2773106, @DavidSpickett wrote:

I assume this is caused by the removal of the specific NativeRegisterContextLinux_mips and you are building lldb on MIPS but you're going to connect to another debug server (gdb, custom stub etc.).

The problem with doing it this way is then if you build on a supported platform e.g. AArch64 NativeRegisterContextLinux_arm64 we then have two copies of CreateHostNativeRegisterContextLinux. Somehow this isn't an error on my system, and it picks up the one you've added. Meaning that you can't debug a simple hello world.

See the comment in lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h. (though again I'd have expect a hard linker error but anyway) Also the reason CI didn't fail is that lldb is disabled for both of the builds.

I can see the utility in allowing people to build at least lldb (client) on any platform. Perhaps we surround this in the #ifdef that the old mips register context used? Or modify our cmake to allow lldb-server to be disabled. (easier said than done)

LGTM

In D102535#2760944, @MaskRay wrote:

In D102535#2760911, @jrtc27 wrote:

In D102535#2760899, @MaskRay wrote:

In D102535#2760807, @jrtc27 wrote:

I view this as a bug in whatever calls the assembler; it should be passing -mno-relax, just as it needs to pass -fPIC as appropriate.

I'll use the -g analogy. clang -S -g emits debug info and leaves a .debug_line marker so that the assembler knows it needs to synthesize debug info and should not synthesize .debug_info from ELF symbols.
The assembler doesn't need -g to function.

For .option nopic, I agree it is not useful.

The two aren't comparable. Relaxation affects assembling and not code generation. Debug info affects code generation to emit all the information that's otherwise lost (the assembly doesn't have a copy of your source code...). Even PIC is more useful than .option norelax, as it affects what instructions get used to lower global addresses etc.

I think the -Wa,-mno-relax argument is justified if code generation cannot affect relaxation (which I am unsure).

Can GCC base assembly emission decisions on -mno-relax?

@MaskRay I dont find this in main branch yet, anything pending on me ?

once you fix the clang-format issue reported this looks good to me.

@MaskRay how should be proceeed here

0073c293a401774ac96b4b3d27f05e13f379f98e has fixed it.

Can someone review this please?

this is because warning were disabled during cmake tests

it works with python3 too, perhaps it would be good to ask for python3 instead

this is no longer needed.

I am still seeing build failures in NATIVE/llvm-config when I am cross building clang for say aarch64 on a x86_64 host. Even when I specify -DLLVM_ENABLE_TERMINFO=OFF, it still goes and pokes for these ncurses libs on build host and adds them to linker cmd. Since my target rootfs is set to not have these libs the final link fails to find -lterminfo rightly. Surprisingly, non NATIVE parts work ok and seem to respect LLVM_ENABLE_TERMINFO settings. Any insights ?

Can we get this on clang 12 release branch as well ?

In D85095#2400738, @ldionne wrote:

I don't understand why this has its place in libc++. Shouldn't this be in glibc instead?

it should have been after including syscall.h

Add comments in source

In D85095#2279476, @apazos wrote:

Can you clarify on the errors you are talking about? Are you building LLVM libc++with glibc?
I have been building LLVM libc++ with MUSl and I do not see a build issue.

this revision is not riscv32 specific. Let me know if this is ok

Fix formatting errors

In D70764#1807405, @smeenai wrote:

In D70764#1803559, @raj.khem wrote:

this is now in master, and I am seeing build failures in cross-building clang, e.g. when building clang for arm on a x86_64 host. its resorting to finding, libz from buildhost instead of target sysroot ( using --sysroot) and failing in link step. e.g.

FAILED: bin/llvm-config
...
.... -o bin/llvm-config -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMSupport.a /usr/lib/libz.so -lrt -ldl -ltinfo -lm lib/libLLVMDemangle.a
...

aarch64-yoe-linux-musl/aarch64-yoe-linux-musl-ld: /usr/lib/libz.so: error adding symbols: file in wrong format
clang-10: error: linker command failed with exit code 1 (use -v to see invocation)

you can see that its adding /usr/lib/libz.so to linker cmdline while cross linking.

Have you set CMAKE_SYSROOT appropriately?

this is now in master, and I am seeing build failures in cross-building clang, e.g. when building clang for arm on a x86_64 host. its resorting to finding, libz from buildhost instead of target sysroot ( using --sysroot) and failing in link step. e.g.

LGTM now. I am testing it in yocto multilib SDK builds.