In D67908#1678816, @fjricci wrote:

Interesting - I had tried to add has_feature for lsan to clang in the past and couldn’t get the change accepted. Nice to see it works now :)

Once GCC will grow support for __SANITIZER_LEAK__ (already pending in review) there will be include as a separate patch support for LSan/GCC.

This is needed in the NetBSD kernel, more fine-grained checks would be acceptable too, but one global feature detection is what I need.

With these patches included there are still ~3 failing tests and I will mark them as XFAIL for now.

In D67331#1677564, @vitalybuka wrote:

Tests?

Looks good to me but I will leave the final decision to someone else.

• apply changes from review

• apply changes from review

• restrict __lsan::ScopedInterceptorDisabler to ASan and LSan

In D66361#1656037, @rsmith wrote:

In D66361#1655903, @krytarowski wrote:

This change broke on NetBSD.

http://lab.llvm.org:8011/builders/netbsd-amd64/builds/22003/steps/run%20unit%20tests/logs/FAIL%3A%20Clang%3A%3Astack-exhaustion.cpp

Test disabled for NetBSD in r370801. If you're interested in investigating why this isn't working there, feel free, but this is only a best-effort mitigation for the case where things have already gone wrong, so there are limits to how much effort it makes sense to resolve this.

Does NetBSD set a hard stack rlimit of less than 8MB by any chance?

This change broke on NetBSD.

This breaks NetBSD as it shall be paired with llvm::call_once.

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 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 mailbox and workaround it.

Maybe this is related to other reviews.llvm.org issues during the weekend where you couldn't update reviews with strange table full
errors?

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 mailbox and workaround it.

In D64647#1584340, @mgorny wrote:

In D64647#1583429, @krytarowski wrote:

Something that we do not cover here is that once a tracee reports a signal (like someone poked it with SIGUSR1) and we want to pass it over to the tracee, we will reset siginfo.

This scenario should be covered by a test and we should handle it properly..

The solution in NetBSD for passing over signals without changing siginfo, is to not calling PT_SET_SIGINFO as the default one will be kept by the kernel. Optionally pick old siginfo with PT_GET_SIGINFO and optionally change destination lwp.

How is 'properly'? Should we reject resuming with a signal when there's already another signal pending?

Something that we do not cover here is that once a tracee reports a signal (like someone poked it with SIGUSR1) and we want to pass it over to the tracee, we will reset siginfo.

I think it looks OK. there are some nits that could be optimized in future or handled additionally.. but for now it should be fine.

In D64057#1579974, @lebedev.ri wrote:

In D64057#1579961, @krytarowski wrote:

I've landed this version as it is good enough to land LLVM-9.

Unfortunately many tests fail as they seem to be tuned for Linux/Darwin.

Please see:

https://reviews.llvm.org/D64057#1565937

@vitalybuka how to address these problems?

Can you show how the tests *fail*>
I.e. what is the diff between expected and actual output?

I've landed this version as it is good enough to land LLVM-9.

• switch the protype of internal_ptrace() to the one matching NetBSD
• move the stop-the-world implementation for NetBSD into a separate file
• minor changes

Ping? There is planned LLVM branching soon and I would like to see this in LLVM 9.0.

Testing notification e-mails from phabricator.

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.

This code in general works.. however the tests are not compatible with NetBSD.

• Fixup style.

@ruiu ping?

I would follow the same kernel behavior here as Linux, but that can be done independently.

This check should contain additional check for uid==root. If we are root we can read and write to DB registers.

I would leave the NetBSD version as it is in this patch and let us to fix/test it later.

This change makes NetBSD buildbot flaky.

In general this code should be close to functional on NetBSD (if not already compatible).

In D59744#1529218, @mgorny wrote:

In D59744#1529182, @krytarowski wrote:

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 discussed in https://reviews.llvm.org/D60748) at present, I only fix the bug for Linux. If other system wants the fix, the only thing needed is to add a flag (like "IsLinuxABI" ) to enable it.

CC @mgorny and @joerg - do we want this for NetBSD?

Probably yes. FWICS, gcc uses %mm0 and %mm1 on NetBSD while clang doesn't.

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 discussed in https://reviews.llvm.org/D60748) at present, I only fix the bug for Linux. If other system wants the fix, the only thing needed is to add a flag (like "IsLinuxABI" ) to enable it.

This commit breaks the NetBSD buildbot node.

If we want to reject opening directories we shall do it during open(2)/fopen(3).

sysv abi is not only for UNIX but most non-Windows ones (BSDs, HAIKU, ...).

whereas others emit "is a directory

In D62499#1520654, @labath wrote:

In D62499#1520610, @krytarowski wrote:

One time just before it's loaded (so lldb can check which modules are loaded) and another right after it's loaded (so lldb can check again which ones are loaded and calculate the difference).

There is on NetBSD and on a selection of other OSs: _rtld_debug_state integrated as a part of ELF dynamic loader.

Is there something like that on Android that could be reused?

Yes, there is, and it's being used now. The question here is what do we do *after* we hit the dynamic loader breakpoint...

One time just before it's loaded (so lldb can check which modules are loaded) and another right after it's loaded (so lldb can check again which ones are loaded and calculate the difference).

It is probably fine.. but I will defer review to lit/libc++ maintainers.

I think there is a bug thought in the code.

In D60728#1469794, @hans wrote:

In D60728#1468713, @krytarowski wrote:

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.

I don't think that works. No one is reading through the test files of the repository.

In D60728#1468486, @hans wrote:

What's the value in checking in this xfail'ed test without an actual fix for the problem?

Adding @void, regression introduced with D55616.

@void hi, this broke assembly code on NetBSD for various archs and blocks upgrade of the toolchain.

I think depending on CMAKE_THREAD_LIBS_INIT is the way to go. If it is broken for some platform internally, such OS has more issues than building OpenMP.