[PATCH] D60728: [clang] [test] Add a (xfailing) test for PR41027

krytarowski added a comment. In D60728#1468486 , @hans wrote: > What's the value in checking in this xfail'ed test without an actual fix for > the problem? Raise awareness about the problem. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60728/

[PATCH] D60728: [clang] [test] Add a (xfailing) test for PR41027

krytarowski added a comment. In D60728#1469794 , @hans wrote: > In D60728#1468713 , @krytarowski > wrote: > > > In D60728#1468486 , @hans wrote: > > > > > What's the value i

[PATCH] D57592: Replace uses of %T with %t in from previous frontend test differential

krytarowski added a comment. The NetBSD buildbot breaks in these tests now: http://lab.llvm.org:8011/builders/lldb-amd64-ninja-netbsd8/builds/18721 Both tests break in a similar way: Command Output (stderr): -- + : 'RUN: at line 1' + not /home/motus/netbsd8/netbsd8/build/bin/clang -cc1

[PATCH] D58592: [clang] [ToolChains/NetBSD] Support relative libc++ header path

krytarowski added inline comments. Comment at: clang/lib/Driver/ToolChains/NetBSD.cpp:430 +// system install from src +getDriver().SysRoot + "/usr/include/c++", + }; I propose to go for: `getDriver().SysRoot + "/usr/include/c++/v1",` with a fallback for

[PATCH] D58592: [clang] [ToolChains/NetBSD] Support relative libc++ header path

krytarowski added inline comments. Comment at: clang/lib/Driver/ToolChains/NetBSD.cpp:430 +// system install from src +getDriver().SysRoot + "/usr/include/c++", + }; mgorny wrote: > krytarowski wrote: > > I propose to go for: > > > > `getDriver().SysRoo

[PATCH] D58592: [clang] [ToolChains/NetBSD] Support relative libc++ header path

krytarowski added inline comments. Comment at: clang/lib/Driver/ToolChains/NetBSD.cpp:430 +// system install from src +getDriver().SysRoot + "/usr/include/c++", + }; mgorny wrote: > krytarowski wrote: > > mgorny wrote: > > > krytarowski wrote: > > > > I

[PATCH] D58592: [clang] [ToolChains/NetBSD] Support relative libc++ header path

krytarowski accepted this revision. krytarowski added a comment. This revision is now accepted and ready to land. This will make life much more easier now with this change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58592/new/ https://reviews.llvm.org/D58592 __

[PATCH] D59744: Fix i386 ABI "__m64" type bug

krytarowski added a comment. sysv abi is not only for UNIX but most non-Windows ones (BSDs, HAIKU, ...). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59744/new/ https://reviews.llvm.org/D59744 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

krytarowski added a comment. This commit breaks the NetBSD buildbot node. http://lab.llvm.org:8011/builders/netbsd-amd64/builds/20359 The problem is that this patch hardcodes python specific binary name, while it has to be detected dynamically or ideally passed from CMake. Afair lit has a know

[PATCH] D59744: Fix i386 ABI "__m64" type bug

krytarowski added subscribers: mgorny, joerg. krytarowski added a comment. In D59744#1527412 , @wxiao3 wrote: > Consider other Systems (e.g Darwin, PS4 and FreeBSD) don't want to spend any > effort dealing with the ramifications of ABI breaks (as discusse

[PATCH] D59744: Fix i386 ABI "__m64" type bug

krytarowski added a comment. In D59744#1529218 , @mgorny wrote: > In D59744#1529182 , @krytarowski > wrote: > > > In D59744#1527412 , @wxiao3 wrote: > > > > > Consider other

[PATCH] D64488: [Driver] Support -fsanitize=function on Solaris/x86

krytarowski added a comment. Something is broken between reviews. and my mailbox as I am not receiving any e-mails so pinging does not make any effect... no LLVM admin replied to my questions on this to validate whether my mail was blacklisted or similar. I have decided to change server of my

[PATCH] D64488: [Driver] Support -fsanitize=function on Solaris/x86

krytarowski added a comment. In D64488#1606758 , @ro wrote: > In D64488#1606716 , @krytarowski > wrote: > > > Something is broken between reviews. and my mailbox as I am not receiving > > any e-mails so pinging do

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

krytarowski added a comment. Herald added a subscriber: MaskRay. @ruiu ping? LLVM 9 is branching soon and we would like to unearth from local patches. From the internal discussions it was decided that we want to maintain our current behavior that differs to Linux. CHANGES SINCE LAST ACTION

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

krytarowski added a comment. > Personally I am opposed to this change. The compiler driver (gcc,clang) has a > set of arch/OS dependent defaults. It seems weird/redundant to add another > sets of defaults on the linker side. The NetBSD toolchain relies on internal knowledge in a linker that can

[PATCH] D62005: [libunwind] [test] Fix inferring source paths

krytarowski resigned from this revision. krytarowski added a comment. Herald added a subscriber: krytarowski. It is probably fine.. but I will defer review to lit/libc++ maintainers. Repository: rUNW libunwind CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62005/new/ https://reviews.l

[PATCH] D60748: Fix i386 struct and union parameter alignment

krytarowski added subscribers: mgorny, joerg. krytarowski added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:1501 +// +// Exclude other System V OS (e.g Darwin, PS4 and FreeBSD) since we don't +// want to spend any effort dealing with the ramifications of A

[PATCH] D55657: [libcxx] [regex] Use distinct __regex_word on NetBSD

krytarowski added a comment. Where and what defines 0x2000, what header and what symbol name? Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55657/new/ https://reviews.llvm.org/D55657 ___ cfe-commits mailing list c

[PATCH] D55657: [libcxx] [regex] Use distinct __regex_word on NetBSD

krytarowski accepted this revision. krytarowski added a comment. This revision is now accepted and ready to land. OK, I got it: `#define _CTYPE_Q0x2000 /* Phonogram */` from /usr/include/sys/ctype_bits.h I propose to mention the symbol name in the description and the header. In the de

[PATCH] D55654: [clang] [Driver] [NetBSD] Add -D_REENTRANT when using sanitizers

krytarowski added a comment. Please normalize the description to resemble a typical commit message. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55654/new/ https://reviews.llvm.org/D55654 ___ cfe-commits mailing lis

[PATCH] D55654: [clang] [Driver] [NetBSD] Add -D_REENTRANT when using sanitizers

krytarowski added inline comments. Comment at: lib/Driver/ToolChains/NetBSD.cpp:465 + const SanitizerArgs &SanArgs = getSanitizerArgs(); + if (SanArgs.needsAsanRt() || SanArgs.needsHwasanRt() || + SanArgs.needsDfsanRt() || SanArgs.needsLsanRt() || There sh

[PATCH] D55916: [clang] Replace getOS() == llvm::Triple::*BSD with isOS*BSD() [NFCI]

krytarowski added inline comments. Comment at: lib/Basic/Targets/ARM.cpp:285 default: - if (Triple.getOS() == llvm::Triple::NetBSD) + if (Triple.isOSNetBSD()) setABI("apcs-gnu"); Actually we could use IsNetBSD and IsOpenBSD here. CHANGES

[PATCH] D56000: [compiler-rt] [xray] Disable alignas() for thread_local objects on NetBSD

krytarowski added inline comments. Comment at: lib/xray/xray_basic_logging.cc:58 -struct alignas(64) ThreadLocalData { +struct +/* TLD is not aligned properly on NetBSD, resulting in segfault */ Can we introduce a macro like: ``` #if SANITIZER_NETBSD #define X

[PATCH] D56000: [compiler-rt] [xray] Disable alignas() for thread_local objects on NetBSD

krytarowski added inline comments. Comment at: lib/xray/xray_basic_logging.cc:58 -struct alignas(64) ThreadLocalData { +struct +/* TLD is not aligned properly on NetBSD, resulting in segfault */ mgorny wrote: > krytarowski wrote: > > Can we introduce a macro li

[PATCH] D56000: [compiler-rt] [xray] Disable alignas() for thread_local objects on NetBSD

krytarowski added inline comments. Comment at: lib/xray/xray_basic_logging.cc:58 -struct alignas(64) ThreadLocalData { +struct +/* TLD is not aligned properly on NetBSD, resulting in segfault */ krytarowski wrote: > mgorny wrote: > > krytarowski wrote: > > > Ca

[PATCH] D56000: [compiler-rt] [xray] Disable alignas() for thread_local objects on NetBSD

krytarowski added inline comments. Comment at: lib/xray/xray_defs.h:22 +#if !SANITIZER_NETBSD +#define XRAY_TLS_ALIGNAS(x) alignas(x) I would switch the order, in order to remove unneeded negation. ``` #if SANITIZER_NETBSD ... #else ... #endif ``` `#define XR

[PATCH] D56049: [compiler-rt] [xray] Detect MPROTECT and error out when it's enabled (on NetBSD)

krytarowski accepted this revision. krytarowski added inline comments. This revision is now accepted and ready to land. Comment at: lib/xray/xray_init.cc:70 + // XRAY is not compatible with pax MPROTECT + CheckMPROTECT(); PaX Repository: rCRT Compiler Run

[PATCH] D56047: [Driver] Disable -faddrsig on Gentoo by default

krytarowski added a comment. It was disabled on NetBSD for the same reason - GNU strip(1) is breaking: http://netbsd.org/~kamil/llvm/strip.txt Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56047/new/ https://reviews.llvm.org/D56047 ___

[PATCH] D47817: [compiler-rt] [sanitizer_common] Fix using libtirpc on Linux

krytarowski added a comment. If libtirpc is from an external library we shall not sanitize it. If it is a part of libc (on glibc? on solaris?) it shall be sanitized only there. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47817/new/ https://reviews.llvm.org/D47817 _

[PATCH] D56109: [sanitizer_common] Define __sanitizer_FILE on NetBSD

krytarowski added inline comments. Comment at: lib/sanitizer_common/sanitizer_platform_limits_netbsd.cc:2249 +CHECK_SIZE_AND_OFFSET(FILE, _offset); +CHECK_SIZE_AND_OFFSET(FILE, _flags); + Duplicate with L2231 Repository: rCRT Compiler Runtime CHANGES SINCE L

[PATCH] D56109: [sanitizer_common] Define __sanitizer_FILE on NetBSD

krytarowski added a comment. As a subtask please add missing calls to `unpoison_file` in interceptors (in sanitizer_common_interceptors.inc) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56109/new/ https://reviews.llvm.org/D56109 ___ cfe-c

[PATCH] D47817: [compiler-rt] [sanitizer_common] Remove support for tirpc/rpc/xdr.h

krytarowski added a comment. Looks good to me, but I will let a Linux person to accept it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47817/new/ https://reviews.llvm.org/D47817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D56109: [sanitizer_common] Define __sanitizer_FILE on NetBSD

krytarowski added inline comments. Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:5698 +#if SANITIZER_NETBSD + if (fp->_bf._base) +COMMON_INTERCEPTOR_INITIALIZE_RANGE(fp->_bf._base, maybe `if (fp->_bf._base && fp->_bf._size)` CHANGES SI

[PATCH] D56136: [compiler-rt] [sanitizer_common] Add tests for more stdio.h functions

krytarowski added inline comments. Comment at: test/sanitizer_common/TestCases/Posix/feof_fileno_ferror.cc:7 +int main(int argc, char **argv) { + FILE *fp; + int fd; Please move local variable declaration to the first line used (whenever possible) ==

[PATCH] D56109: [sanitizer_common] Define __sanitizer_FILE on NetBSD

krytarowski added a subscriber: dvyukov. krytarowski added a comment. If we understand it correctly, unpoision FILE would be only for inlined routines accessing FILE buffer(s) directly. On NetBSD we enforce _REENTRANT for all sanitizers in order to support only _REENTRANT variations of calls tha

[PATCH] D56109: [sanitizer_common] Define __sanitizer_FILE on NetBSD

krytarowski added inline comments. Comment at: lib/tsan/rtl/tsan_interceptors.cc:45 #define dirfd(dirp) (*(int *)(dirp)) #define fileno_unlocked fileno We might want to go for emulating `_unlocked`: ``` #define fileno_unlocked(p) \ 488 ((p)->_fi

[PATCH] D56136: [compiler-rt] [sanitizer_common] Add tests for more stdio.h functions

krytarowski accepted this revision. krytarowski added inline comments. This revision is now accepted and ready to land. Comment at: test/sanitizer_common/TestCases/Posix/feof_fileno_ferror.cc:38 + + fclose(fp); + return 0; `assert(!fclose(fp));` =

[PATCH] D56136: [compiler-rt] [sanitizer_common] Add tests for more stdio.h functions

krytarowski added inline comments. Comment at: test/sanitizer_common/TestCases/Posix/feof_fileno_ferror.cc:38 + + fclose(fp); + return 0; krytarowski wrote: > `assert(!fclose(fp));` alternatively `!= EOF` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D56136: [compiler-rt] [sanitizer_common] Add tests for more stdio.h functions

krytarowski added inline comments. Comment at: test/sanitizer_common/TestCases/Posix/feof_fileno_ferror.cc:38 + + fclose(fp); + return 0; mgorny wrote: > krytarowski wrote: > > krytarowski wrote: > > > `assert(!fclose(fp));` > > alternatively `!= EOF` > It will

[PATCH] D56149: [sanitizer_common] Rewrite more Posix tests to use asserts

krytarowski added inline comments. Comment at: test/sanitizer_common/TestCases/Posix/devname_r.cc:21 #if defined(__NetBSD__) - if (devname_r(st.st_rdev, type, name, sizeof(name))) -exit(1); + assert(!devname_r(st.st_rdev, type, name, sizeof(name))); #else

[PATCH] D56150: [sanitizer_common] Fix devname_r() return type on !NetBSD

krytarowski added a comment. Looks fine, but according to newer style of newer interceptors we prefer: `DEVNAME_R_RETTYPE res = REAL(devname_r)(dev, type, path, len);` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56150/new/ https://reviews.llvm.org/D56150 __

[PATCH] D56153: [sanitizer_common] Add test for popen()

krytarowski added a comment. It looks fine, but we don't have interceptors for popen(3), popenve(3), pclose(3). Could you include them together with this patch? Add add a test for popenve(3). Repository: rCRT Compiler Runtime CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56153/new/

[PATCH] D56152: [sanitizer_common] Add tests for remaining *putc and *getc variants

krytarowski added a comment. Are all the functions available for FreeBSD, NetBSD, Linux, Android, Solaris an Darwin? I would include a test for `getchar_unlocked()` anyway, even if it's an alias in most/all implementations. Repository: rCRT Compiler Runtime CHANGES SINCE LAST ACTION http

[PATCH] D56152: [sanitizer_common] Add tests for remaining *putc and *getc variants

krytarowski added a comment. Does this depend on D56109 ? It looks like we shall install interceptors for a lot of routines that are FILE* users. Repository: rCRT Compiler Runtime CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56152/new/ https://revi

[PATCH] D56154: [sanitizer_common] Add tests for NetBSD funopen*()

krytarowski added inline comments. Comment at: test/sanitizer_common/TestCases/NetBSD/funopen2.cc:2 +// RUN: %clangxx -g %s -o %t && %run %t | FileCheck %s + +// CHECK: READ CALLED; len={{[0-9]*}} // UNSUPPORTED: linux, freebsd, solaris, darwin Repository: rC

[PATCH] D56154: [sanitizer_common] Add tests for NetBSD funopen*()

krytarowski accepted this revision. krytarowski added inline comments. This revision is now accepted and ready to land. Comment at: test/sanitizer_common/TestCases/NetBSD/funopen2.cc:2 +// RUN: %clangxx -g %s -o %t && %run %t | FileCheck %s + +// CHECK: READ CALLED; len={{[0-9]*}

[PATCH] D56153: [sanitizer_common] Add test for popen()

krytarowski accepted this revision. krytarowski added a comment. This revision is now accepted and ready to land. But of course new code can be added as a new revision. Repository: rCRT Compiler Runtime CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56153/new/ https://reviews.llvm.org

[PATCH] D56157: [sanitizer_common] Implement popen, popenve, pclose interceptors

krytarowski added a subscriber: dvyukov. krytarowski added a comment. Looks good to me, but it would be nice to have opinion of @vitalybuka and/or @dvyukov (TSan). Repository: rCRT Compiler Runtime CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56157/new/ https://reviews.llvm.org/D56

[PATCH] D56152: [sanitizer_common] Add tests for remaining *putc and *getc variants

krytarowski accepted this revision. krytarowski added a comment. This revision is now accepted and ready to land. In D56152#1342372 , @mgorny wrote: > In D56152#1342331 , @krytarowski > wrote: > > > Are all the fun

[PATCH] D56158: [sanitizer_common] Implement funopen*() interceptors for NetBSD

krytarowski added a comment. Shouldn't this follow SANITIZER_INTERCEPT_FOPENCOOKIE and implement a wrapper for operations? Repository: rCRT Compiler Runtime CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56158/new/ https://reviews.llvm.org/D56158 __

[PATCH] D56158: [sanitizer_common] Implement funopen*() interceptors for NetBSD

krytarowski added inline comments. Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:9142 + +struct WrappedCookie { + void *real_cookie; I would call it WrappedFunopenCookie to be less generic, similarly for funopen2. CHANGES SINCE LAST ACTION

[PATCH] D56158: [sanitizer_common] Implement funopen*() interceptors for NetBSD

krytarowski added inline comments. Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:9150 + +static int wrapped_read(void *cookie, char *buf, int len) { + COMMON_INTERCEPTOR_UNPOISON_PARAM(3); How about `wrapped_funopen_read` etc? It won't clash

[PATCH] D56044: [Driver] Support object files in addition to static and shared libraries in compiler-rt

krytarowski added a comment. What's the rationale for this? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56044/new/ https://reviews.llvm.org/D56044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://list

[PATCH] D56157: [sanitizer_common] Implement popen, popenve, pclose interceptors

krytarowski added inline comments. Comment at: lib/esan/esan_interceptors.cpp:90 } while (false) +#define COMMON_INTERCEPTOR_PIPE_OPEN(ctx, file) \ + do { \ --

[PATCH] D56044: [Driver] Support object files in addition to static and shared libraries in compiler-rt

krytarowski added a comment. Sorry, but this rather belongs to libc(abi) of Fuchsia. Why to push it into LLVM? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56044/new/ https://reviews.llvm.org/D56044 ___ cfe-commits

[PATCH] D56044: [Driver] Support object files in addition to static and shared libraries in compiler-rt

krytarowski added a comment. I don't know the original rationale of GNU.. but the final result (probably not out of interest of FSF) is that it's a vendor lock of glibc&gcc. Please implement your crt* in your libc. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D5

[PATCH] D56044: [Driver] Support object files in addition to static and shared libraries in compiler-rt

krytarowski added a reviewer: joerg. krytarowski added a comment. Adding joerg who raised concerns in the original change and is bypassed with this change. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56044/new/ https://reviews.llvm.org/D56044 __

[PATCH] D56044: [Driver] Support object files in addition to static and shared libraries in compiler-rt

krytarowski added a comment. This patch attempts to add support for .o/object files from compiler-rt and right now there are no users and the only potential ones are crt* according to my guess. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56044/new/ https://re

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. In D56215#1344233 , @ruiu wrote: > lld's driver is intentionally designed to be agnostic of the host that the > linker is running for the reasons described at the beginning of Driver.cpp: > https://github.com/llvm-project/lld

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. In D56215#1344317 , @joerg wrote: > In D56215#1344279 , @krytarowski > wrote: > > > In D56215#1344183 , @joerg wrote: > > > > > This doesn't see

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. I assume that we need to reimplement this in the context of lld: NetBSD::NetBSD(const Driver &D, const llvm::Triple &Triple, const ArgList &Args) : Generic_ELF(D, Triple, Args) { if (!Args.hasArg(options::OPT_nostdlib)) { // When targeting a 32-bit

[PATCH] D56109: [sanitizer_common] Define __sanitizer_FILE on NetBSD

krytarowski added a comment. In D56109#1344521 , @vitalybuka wrote: > LGTM Thanks! Any feedback regarding the raised comments in https://reviews.llvm.org/D56109#1341967 and https://reviews.llvm.org/D56109#1342059 ? CHANGES SINCE LAST ACTION https:/

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. In D56215#1345326 , @ruiu wrote: > In D56215#1344279 , @krytarowski > wrote: > > > In D56215#1344233 , @ruiu wrote: > > > > > lld's driver is in

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. It's an option but Joerg insists on using lld standalone. Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56215/new/ https://reviews.llvm.org/D56215 ___ cfe-commits mailing list cfe

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. Actually I find it frustrating and unfair as GNU ld doesn't have builtin knowledge either.. it's passed with gross 'super hack' comments from build scripts... but we are forced to push it to lld in order to move on. Repository: rLLD LLVM Linker CHANGES SINCE LAS

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. In D56215#1345400 , @ruiu wrote: > Joerg, what is special about NetBSD? Nothing. But the result is that we don't have GNU gold either and are stuck with 40min linking times of LLVM. It's destructive to productivity and dama

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. We will prepare a patch for i386 x86_64 only for now. Until we will get lld fully functional on NetBSD/amd64 we will skip other architectures. Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56215/new/ https://reviews.llvm.org/

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added inline comments. Comment at: ELF/Driver.cpp:377 + break; +case EM_MIPS: + if (Config->EKind == ELF64LEKind || Config->EKind == ELF64BEKind) Please drop MIPS/PPC for now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56215

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added inline comments. Comment at: ELF/Driver.cpp:369 +void LinkerDriver::appendDefaultSearchPaths() { +#if defined(__NetBSD__) + // NetBSD driver relies on the linker knowing the default search paths. is it possible to use some Triple NetBSD target

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. On 03.01.2019 21:19, Joerg Sonnenberger wrote: > On Thu, Jan 03, 2019 at 06:34:22PM +0000, Kamil Rytarowski via Phabricator > via cfe-commits wrote: > >> krytarowski added a comment. >> >> Actually I find it frustrating and unfair as

[PATCH] D56288: [ELF] Do not enable 'new dtags' on NetBSD by default

krytarowski added a comment. We want to keep it disabled by default on NetBSD.. but it would be to keep dynamic detection of target NetBSD rather than hardcoded ifdef. GNU ld sets it by default to false, following it would be the easiest option. Repository: rLLD LLVM Linker CHANGES SINCE LA

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. I think that ifdefining the linker options with `__NetBSD__` is no-go. In D56215#1345563 , @mgorny wrote: > We've discussed this a bit and given other changes we need to do, and I see > pretty much three options here: > > 1.

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. On Thu, Jan 03, 2019 at 08:31:53PM +, Kamil Rytarowski via Phabricator via cfe-commits wrote: > krytarowski added a comment. > > On 03.01.2019 21:19, Joerg Sonnenberger wrote: > >> On Thu, Jan 03, 2019 at 06:34:22PM +, Ka

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. On 03.01.2019 23:15, Joerg Sonnenberger wrote: > On Thu, Jan 03, 2019 at 09:38:49PM +0000, Kamil Rytarowski via Phabricator via cfe-commits wrote: >> I think that this place is not the right place to bash GNU ld for performance issues. >

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. In D56215#1345695 , @ruiu wrote: > In D56215#1345417 , @joerg wrote: > > > Talking from the perspective of having had to deal with thousands of > > packages in pkgsrc over the years: it

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. Actually I think that we can handle two cases witch a combination of proposals: - native mode - cross mode For the native mode we go for something like: #if defined(__NetBSD__) #define NATIVE_TARGET LLD_NETBSD #else ... and for cross mode read `argv[0]` to

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. In D56215#1345845 , @ruiu wrote: > Not sure what I understand the point, but as to make lld work on/for NetBSD, > I wonder if you can just add -L to the command line in the compiler > driver. Isn't a NetBSD triple passed to t

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. In D56215#1346342 , @arichardson wrote: > In D56215#1346061 , @mgorny wrote: > > > For the record, another option is to actually fix other software not to > > call LD directly. > > > Or

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. I've grepped FreeBSD, OpenBSD and pkgsrc. OpenBSD ports = Total 2 packages there have issues with LLD. $ grep -r USE_LLD . ./devel/libcoap/Makefile:USE_LLD= No ./sysutils/firmware/vmm/Makefile:USE_LLD= No FreeBSD ports =

[PATCH] D52610: [Esan] Port cache frag to FreeBSD

krytarowski added a comment. Can you try to port this mutant thread-process in ESan to FreeBSD? I don't want to see changes in generic LLVM code. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52610/new/ https://reviews.llvm.org/D52610

[PATCH] D56288: [ELF] Do not enable 'new dtags' on NetBSD by default

krytarowski added a comment. In D56288#1348148 , @mgorny wrote: > Ok, maybe I'm being silly but since clang driver has to pass > `--enable-new-dtags` for GNU ld compatibility anyway, wouldn't it make sense > to keep the default as disabled in order to ma

[PATCH] D56288: [ELF] Do not enable 'new dtags' on NetBSD by default

krytarowski added a comment. On NetBSD the 'new' semantics was already implemented with 'old' RPATH. I don't know the timing whether there already existed RUNPATH, but the result is that we never implemented it and never needed it. The core of NetBSD was convinced to add an alias as it was ver

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. In D56215#1350134 , @joerg wrote: > Thanks, this looks like a good starting point. What's the final state you want? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56215/new/ https://reviews.llvm.org/D56215 __

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added inline comments. Comment at: ELF/Driver.cpp:781 + } +} + There is need to add a fallback for a native triple. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56215/new/ https://reviews.llvm.org/D56215 ___

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

krytarowski added a comment. @joerg if you think that this patch is OK, please click accept so we can be aware that this is what you want. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56215/new/ https://reviews.llvm.org/D56215 ___ cfe-com

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added inline comments. Comment at: docs/ld.lld.1:515 +.Dv PT_GNU_STACK +segment. .It Cm norelro section? Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 __

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. Looks good, we don't GNU STACK on NetBSD. Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added inline comments. Comment at: ELF/Writer.cpp:1980 + if (!Config->ZNognustack) { +// PT_GNU_STACK is a special section to tell the loader to make the +// pages for the stack non-executable. If you really want an executable section -> segm

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added inline comments. Comment at: ELF/Driver.cpp:874 Config->ZGlobal = hasZOption(Args, "global"); + Config->ZNognustack = hasZOption(Args, "nognustack"); Config->ZHazardplt = hasZOption(Args, "hazardplt"); I would add `gnustack` vs `nognustac

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. In D56554#1353368 , @ruiu wrote: > The absence of PT_GNU_STACK segment makes stack area executable on systems > that recognizes PT_GNU_STACK segment. So, I think if `-z execstack` is > specified, we should omit PT_GNU_STACK s

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. In D56554#1353513 , @ruiu wrote: > In D56554#1353487 , @krytarowski > wrote: > > > In D56554#1353368 , @ruiu wrote: > > > > > The absence of PT_

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. Actually looking at GNU ld(1) source code, the logic is a little bit more complex and it depends on more aspects like relocations. Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 __

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. Shouldn't the logic of emitting/not-emitting be based on `findSection(".note.GNU-stack")`? @mgorny can you please investigate it? Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 ___

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. I understand the concerns, but lld is biased with being a Linux linker. We need to customize it (restore defaults?) for NetBSD. Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56554/new/ https://reviews.llvm.org/D56554 _

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

krytarowski added a comment. What do you think about this new logic: 1. specified `-z execstack` -> do not emit GNU STACK segment 2. specified `-z noexecstack` -> emit GNU STACK segment 3. no option specified -> detect `findSection(".note.GNU-stack")` (I might miss offhand some details here to b

[PATCH] D67719: [clang] [Basic] Enable __has_feature(leak_sanitizer)

krytarowski added a comment. Looks good to me but I will leave the final decision to someone else. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67719/new/ https://reviews.llvm.org/D67719 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature

krytarowski added a comment. Herald added a project: LLVM. This is needed in the NetBSD kernel, more fine-grained checks would be acceptable too, but one global feature detection is what I need. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52386/new/ https://rev

[PATCH] D41240: [Solaris] __float128 is supported on Solaris/x86

krytarowski added subscribers: joerg, krytarowski. krytarowski added a comment. Does it work for you for 32-bit? @joerg opposed adoption of 32-bit version as it seemed to be unimplemented.. Repository: rL LLVM https://reviews.llvm.org/D41240 ___

[PATCH] D45149: MallocChecker, adding specific BSD calls

krytarowski added a comment. These calls are not 'BSD' specific.. but OpenBSD. https://reviews.llvm.org/D45149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47814: Teach libc++ to use native NetBSD's max_align_t

krytarowski added a comment. ping^2 Repository: rL LLVM https://reviews.llvm.org/D47814 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

<    1   2   3   >