Page MenuHomePhabricator

jakubjelinek (Jakub Jelínek)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 20 2016, 10:07 AM (260 w, 4 d)

Recent Activity

Thu, Aug 26

jakubjelinek added a comment to D107819: Problem with realpath interceptor.

What kind of problems are you talking about? Is it that some non-glibc system wants also the NULL resolved_path handling and doesn't have it in the libc?
To be precise, the patch would work fine if only the INIT_REALPATH macro is changed to the COMMON_INTERCEPT_FUNCTION_GLIBC_VER_MIN macro, but no reordering will help,
the problem is that tsan disables the wrappers altogether at some points and then it only calls the original function. And it is crucial that the original function is the right one.

Thu, Aug 26, 12:35 AM · Restricted Project

Aug 10 2021

jakubjelinek added a comment to D107819: Problem with realpath interceptor.

I don't remember if you have commit access or not. Do you want me to land this?

Aug 10 2021, 7:18 AM · Restricted Project
jakubjelinek requested review of D107819: Problem with realpath interceptor.
Aug 10 2021, 5:24 AM · Restricted Project

Jun 7 2021

jakubjelinek added a comment to D102475: Prevent introduction of a dependency of libasan.a on libstdc++.

It used to be 3 times per thread, now it is 2 times per thread.

Jun 7 2021, 12:43 PM · Restricted Project
jakubjelinek added a comment to D102475: Prevent introduction of a dependency of libasan.a on libstdc++.

https://reviews.llvm.org/D100645 contains the details.

Jun 7 2021, 12:16 PM · Restricted Project
jakubjelinek added a comment to D102475: Prevent introduction of a dependency of libasan.a on libstdc++.

The reason why the previous change was done is because it is no longer a constant on Linux with recent glibc (2.34 to be released later this year). But it is still constant for older glibcs, or on non-Linux.

Jun 7 2021, 11:17 AM · Restricted Project
jakubjelinek added inline comments to D102475: Prevent introduction of a dependency of libasan.a on libstdc++.
Jun 7 2021, 10:56 AM · Restricted Project

May 14 2021

jakubjelinek added a comment to D102475: Prevent introduction of a dependency of libasan.a on libstdc++.

It will succeed once. But if another thread calls GetAltStackSize() while the first one just returned from __sync_bool_compare_and_swap(&kAltStackSize, 0, 1) but hasn't yet stored the SIGSTKSZ * 4 value (e.g. is inside of sysconf), or even has stored, but the value hasn't propagated to other CPU caches yet, then they can see kAltStackSize of 1 rather than the right value.

May 14 2021, 5:38 AM · Restricted Project
jakubjelinek added a comment to D102475: Prevent introduction of a dependency of libasan.a on libstdc++.

This is certainly wrong. It will happily return 1 when some other thread is changing it, and the store is non-atomic.
Perhaps you want something like:

static atomic_uint32_t  kAltStackSize;
uptr ret = atomic_load(&kAltStackSize, memory_order_relaxed);
if (ret == 0) {
  ret = SIGSTKSZ * 4;
  atomic_store(&kAltStackSize, ret, memory_order_relaxed);
}
return ret;

on the assumption that SIGSTKSZ will always evaluate to the same non-zero constant and it is ok to evaluate it more than once, but faster not to do that if possible.
Would be nice to get rid of the overhead if SIGSTKSZ is a constant expression though, maybe:

if (__builtin_constant_p (SIGSTKSZ * 4))
  return SIGSTKSZ * 4;
else {
  // Above atomic stuff.
}
May 14 2021, 3:25 AM · Restricted Project

May 10 2021

jakubjelinek added inline comments to D102059: libsanitizer: Guard cyclades inclusion in sanitizer.
May 10 2021, 3:50 AM · Restricted Project

May 7 2021

jakubjelinek added a comment to D102059: libsanitizer: Guard cyclades inclusion in sanitizer.

Alternative to this would be

May 7 2021, 2:52 AM · Restricted Project

Apr 16 2021

jakubjelinek added a comment to D100645: Sanitizer built against glibc 2.34 doesn't work.

As a minor optimization for the latter you could do in SetAlternateSignalStack() do

const uptr kAltStackSize = GetAltStackSize();
void *base = MmapOrDie(kAltStackSize, __func__);
altstack.ss_sp = (char*) base;
altstack.ss_flags = 0;
altstack.ss_size = kAltStackSize;
Apr 16 2021, 10:55 AM · Restricted Project
jakubjelinek added a comment to D100645: Sanitizer built against glibc 2.34 doesn't work.

LGTM. For older glibc it will be most likely inlined and optimized into const like it was before, for new glibc it will be slightly more costly at startup but will be thread-safe (the usual C++ local static init costs).

Apr 16 2021, 10:53 AM · Restricted Project
jakubjelinek added a reviewer for D100645: Sanitizer built against glibc 2.34 doesn't work: glider.
Apr 16 2021, 6:41 AM · Restricted Project
jakubjelinek requested review of D100645: Sanitizer built against glibc 2.34 doesn't work.
Apr 16 2021, 6:39 AM · Restricted Project

Dec 14 2020

jakubjelinek added a comment to D92052: [MC][ELF] Accept abbreviated form with sh_flags and sh_entsize.

OK my bad (I don't regularly see "See Also"... ) I've sent https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97827#c9 . I'll follow up with GCC folks there. I don't subscribe gcc-patches so it will be a bit inconvenient for me if most people comment on the mailing list instead...

Dec 14 2020, 11:40 AM · Restricted Project

Nov 23 2020

jakubjelinek added a comment to D87974: [Builtin] Add __builtin_zero_non_value_bits..

@jwakely It looks like UnsizedTail causes a crash.

Nov 23 2020, 5:42 AM · Restricted Project

Nov 25 2019

jakubjelinek added a comment to D70662: Fix sanitizer-common build with glibc 2.31.

Can somebody please commit it? I don't have access to llvm repo. Thanks.

Nov 25 2019, 1:42 PM · Restricted Project, Restricted Project
jakubjelinek created D70662: Fix sanitizer-common build with glibc 2.31.
Nov 25 2019, 2:26 AM · Restricted Project, Restricted Project

Nov 20 2019

jakubjelinek added a comment to D69104: [Arm][libsanitizer] Fix arm libsanitizer failure with bleeding edge glibc.

The change looks wrong, the upcoming glibc 2.31 changes aren't just about ARM, but actually most of the architectures, so adding a special case just for ARM is just weird.
Either we should change the structures to match what glibc 2.31 will have and disable this ipc_perm mode checking if not __GLIBC_PREREQ (2, 31) on SANITIZER_LINUX,
or the check needs to be disabled on most Linux architectures.
Affected are e.g. x86_64, i?86, riscv*, sparc 32-bit, s390 31-bit, arm. And, on arm big endian and s390 31-bit there is even an important ABI change, so shmctl interception
won't be able to use dlsym but will need to use dlvsym.
Completely untested patch would be something like:

  • sanitizer_common/sanitizer_platform_limits_posix.h 2019-11-07 17:56:23.530835549 +0100

+++ sanitizer_common/sanitizer_platform_limits_posix.h 2019-11-12 12:22:26.314511706 +0100
@@ -207,26 +207,13 @@ struct __sanitizer_ipc_perm {

u64 __unused1;                                                                                                                                  
u64 __unused2;

#elif defined(sparc)
-#if defined(arch64)

unsigned mode;
  • unsigned short __pad1;

-#else

  • unsigned short __pad1;
  • unsigned short mode; unsigned short __pad2;

-#endif

unsigned short __seq;                                                                                                                           
unsigned long long __unused1;                                                                                                                   
unsigned long long __unused2;

-#elif defined(mips) || defined(aarch64) || defined(s390x)

  • unsigned int mode;
  • unsigned short __seq;
  • unsigned short __pad1;
  • unsigned long __unused1;
  • unsigned long __unused2; #else
  • unsigned short mode;
  • unsigned short __pad1;

+ unsigned int mode;

unsigned short __seq;                                                                                                                           
unsigned short __pad2;

#if defined(x86_64) && !defined(_LP64)
---sanitizer_common/sanitizer_platform_limits_posix.cpp 2019-11-07 17:56:23.551835239 +0100
+++ sanitizer_common/sanitizer_platform_limits_posix.cpp 2019-11-12 12:23:42.959358844 +0100
@@ -1128,11 +1128,9 @@ CHECK_SIZE_AND_OFFSET(ipc_perm, uid);
CHECK_SIZE_AND_OFFSET(ipc_perm, gid);
CHECK_SIZE_AND_OFFSET(ipc_perm, cuid);
CHECK_SIZE_AND_OFFSET(ipc_perm, cgid);
-#if (!defined(aarch64) || !SANITIZER_LINUX || __GLIBC_PREREQ (2, 21)) && \

  • !defined(arm)

-/* On aarch64 glibc 2.20 and earlier provided incorrect mode field. */
-/* On Arm newer glibc provide a different mode field, it's hard to detect

  • so just disable the check. */

+#if !SANITIZER_LINUX || __GLIBC_PREREQ (2, 31)
+/* glibc 2.30 and earlier provided 16-bit mode field instead of 32-bit
+ on most architectures. */
CHECK_SIZE_AND_OFFSET(ipc_perm, mode);
#endif

Nov 20 2019, 2:17 AM · Restricted Project, Restricted Project

Jun 16 2018

jakubjelinek added inline comments to D48255: [sanitizer] Fix LSAN for 32-bit glibc before 2.27.
Jun 16 2018, 3:51 AM · Restricted Project

Jun 8 2018

jakubjelinek added a comment to D44623: [ASAN] Fix crash on i?86-linux (32-bit) against glibc 2.27 and later.

Could you push it @jakubjelinek? If nobody takes care of it I'll try to merge it later.

Jun 8 2018, 9:43 AM · Restricted Project

May 22 2018

jakubjelinek added inline comments to D44623: [ASAN] Fix crash on i?86-linux (32-bit) against glibc 2.27 and later.
May 22 2018, 6:06 AM · Restricted Project

May 9 2018

jakubjelinek added inline comments to D44623: [ASAN] Fix crash on i?86-linux (32-bit) against glibc 2.27 and later.
May 9 2018, 11:40 AM · Restricted Project
jakubjelinek updated the diff for D44623: [ASAN] Fix crash on i?86-linux (32-bit) against glibc 2.27 and later.
May 9 2018, 10:12 AM · Restricted Project
jakubjelinek added a comment to D44623: [ASAN] Fix crash on i?86-linux (32-bit) against glibc 2.27 and later.

Updated patch, which uses a template with 2 support classes to avoid some code duplication. I must say that I find

#ifndef __GLIBC_PREREQ
# define __GLIBC_PREREQ(x, y) 0
#endif

easier to read over the longer sequence of #ifs, and as I said earlier, the proposed

#if defined(__i386__) && (!defined(__GLIBC_PREREQ) || !__GLIBC_PREREQ(2, 27))

is not valid C/C++ if __GLIBC_PREREQ macro is not defined.

May 9 2018, 9:05 AM · Restricted Project
jakubjelinek updated the diff for D44623: [ASAN] Fix crash on i?86-linux (32-bit) against glibc 2.27 and later.
May 9 2018, 9:02 AM · Restricted Project
jakubjelinek added inline comments to D44623: [ASAN] Fix crash on i?86-linux (32-bit) against glibc 2.27 and later.
May 9 2018, 8:27 AM · Restricted Project
jakubjelinek added inline comments to D46638: [sanitizer] Fix runtime crash on 32-bit Linux with glibc 2.27.
May 9 2018, 8:04 AM · Restricted Project

Mar 21 2018

jakubjelinek added inline comments to D44623: [ASAN] Fix crash on i?86-linux (32-bit) against glibc 2.27 and later.
Mar 21 2018, 12:01 PM · Restricted Project
jakubjelinek added a comment to D44623: [ASAN] Fix crash on i?86-linux (32-bit) against glibc 2.27 and later.
In D44623#1042762, @kcc wrote:

Is this related to https://github.com/google/sanitizers/issues/914 or this is another problem?

Mar 21 2018, 11:48 AM · Restricted Project

Mar 19 2018

jakubjelinek added a comment to D44623: [ASAN] Fix crash on i?86-linux (32-bit) against glibc 2.27 and later.

glibc 2.27 has added the glob@@GLIBC_2.27 symbols approx. one month after these internal_function changes, so it is closer to that than e.g. trying to parse confstr for glibc 2.27 and later, and is also smaller than trying to parse the confstr string.

Mar 19 2018, 6:49 AM · Restricted Project
jakubjelinek created D44623: [ASAN] Fix crash on i?86-linux (32-bit) against glibc 2.27 and later.
Mar 19 2018, 6:47 AM · Restricted Project

Nov 29 2017

jakubjelinek added a comment to D40600: Enhance libsanitizer support for invalid-pointer-pair..

Can you or Konstantin please commit it? I don't have write access to llvm repo.
Thanks.

Nov 29 2017, 2:42 PM · Restricted Project
jakubjelinek added a comment to D38971: Enhance libsanitizer support for invalid-pointer-pair..

Phabriactor doesn't allow me to update this patch, so I've uploaded it to D40600 instead. If you have a way to update the diff here, please do.

Nov 29 2017, 5:09 AM
jakubjelinek created D40600: Enhance libsanitizer support for invalid-pointer-pair..
Nov 29 2017, 5:07 AM · Restricted Project

Nov 10 2017

jakubjelinek added a comment to D39889: Export __lsan_init.

Can you please commit it?

Nov 10 2017, 10:00 AM
jakubjelinek created D39889: Export __lsan_init.
Nov 10 2017, 1:33 AM
jakubjelinek added a reviewer for D39888: [Sanitizers, LSan, Darwin] Allow for lack of VM_MEMORY_OS_ALLOC_ONCE: kcc.
Nov 10 2017, 1:24 AM · Restricted Project

Oct 17 2017

jakubjelinek added a comment to D39018: -fsanitize=noreturn sanitization.

I have no idea how to CC those mailing lists (Change Subscribers doesn't seem to allow those) in this tool (or shall I just mail them normally)?
As for tests, the ones I have for GCC are:

Oct 17 2017, 11:47 PM
jakubjelinek created D39018: -fsanitize=noreturn sanitization.
Oct 17 2017, 12:42 PM
jakubjelinek added a comment to D38991: Unbreak libbacktrace symbolizer.
In D38991#899950, @kcc wrote:

LGTM
Do you need me to commit it?

Oct 17 2017, 11:40 AM
jakubjelinek added a comment to D38971: Enhance libsanitizer support for invalid-pointer-pair..

Shouldn't detect_invalid_pointer_pairs=1 be the default? I mean, the user already makes the decision to enable it through a compiler option/parameter and the generated code adds non-zero amount of .text space and some runtime cost, and there is no extra runtime costs for this if code isn't instrumented, so detect_invalid_pointer_pairs=0 by default only causes false assumption that code is error clean.

Oct 17 2017, 8:00 AM
jakubjelinek added a comment to D38971: Enhance libsanitizer support for invalid-pointer-pair..

Besides the patch, we'd like to coordinate on options enabling this in the compilers.
From what I can see, in clang it is currently an experimental asan-detect-invalid-pointer-pair param,
what Martin has implemented is handle it as another kind of sanitization (separate kinds for pointer compare and pointer subtract),
-fsanitize=pointer-subtract or -fsanitize=pointer-compare. One can then do:
-fsanitize=address,pointer-subtract,pointer-compare etc., if address is not specified but one of these is, then
right now we error out. I could imagine pointer-subtract or pointer-compare to be supported even without address
sanitization, where we'd link against libasan and add poisoning/unpoisoning/global variable registration etc., but not
memory dereference checks, the problem is that the compiler then doesn't know if it should do the registration etc.
in kernel-address or user address style.

Oct 17 2017, 2:37 AM
jakubjelinek created D38991: Unbreak libbacktrace symbolizer.
Oct 17 2017, 1:23 AM

Jul 13 2017

jakubjelinek added a comment to D35246: Fix sanitizer build against latest glibc.

Could somebody please commit it? Thanks.

Jul 13 2017, 10:44 AM

Jul 11 2017

jakubjelinek added a comment to D35246: Fix sanitizer build against latest glibc.

Using stack_t in the internal_sigaltstack prototype is problematic, because stack_t is only defined in the *.cc file which includes signal.h through sys/wait.h. But describing what it points to isn't really necessary, the function could be also declared with uptr arguments, or sanitizer_stack_t if we define it independently, etc.
Alternative to using struct
res_state * is using res_state, i.e.

res_state statp = (res_state)state;
Jul 11 2017, 4:22 AM
jakubjelinek created D35246: Fix sanitizer build against latest glibc.
Jul 11 2017, 4:18 AM

Feb 17 2017

jakubjelinek added a comment to D29992: Wrong sanitizer handling of PCs in backtrace.
In D29992#679484, @kcc wrote:

Please:

  • no more #ifdefs int this part of code, at least in the first two hunks
  • no code duplications (the first two hunks look similar)
  • no C-style comments, use //
  • a test

Also, if possible, please try to change the logic from "harm, then unharm" to "don't harm"

Feb 17 2017, 1:31 AM

Feb 15 2017

jakubjelinek created D29992: Wrong sanitizer handling of PCs in backtrace.
Feb 15 2017, 9:02 AM

Feb 10 2017

jakubjelinek updated the diff for D29825: s390 CVE-2016-2143 whitelist for RHEL kernels.
Feb 10 2017, 2:53 PM
jakubjelinek added a comment to D29825: s390 CVE-2016-2143 whitelist for RHEL kernels.
In D29825#674022, @kcc wrote:
  • please upload patches with full context, otherwise they are harder to review
Feb 10 2017, 2:38 PM
jakubjelinek added a comment to D29824: Fix AddressSanitizer on s390 (31-bit).
In D29824#674011, @kcc wrote:

Is this change testable?

Feb 10 2017, 2:34 PM
jakubjelinek created D29825: s390 CVE-2016-2143 whitelist for RHEL kernels.
Feb 10 2017, 5:31 AM
jakubjelinek added a comment to D29824: Fix AddressSanitizer on s390 (31-bit).

Note, in theory __builtin_extract_return_addr should be used on all architectures, but not sure if it can't break anything, so I'm proposing to use it for now just on s390 31-bit.
In GCC, the builtin returns something other than its unmodified arguments on:
alpha-vms in certain cases
arm (for 26-bit mode)
mips (always masks off lsb)
hppa (always masks off 2 ls bits)
s390 31-bit mode (always masks off msb)
sparc adds 8 or 12 bytes to it
microblaze adds 8 bytes to it

Feb 10 2017, 5:15 AM
jakubjelinek created D29824: Fix AddressSanitizer on s390 (31-bit).
Feb 10 2017, 5:07 AM
jakubjelinek added a comment to D29735: s390x __tls_get_addr_internal vs. __tls_get_offset.
In D29735#672682, @kcc wrote:

check-asan does not pass with this change due to lint"
/tmp/check_lint.XhMkOExnXU/tmp.TSPGZQsNod.sanitizer_common_interceptors.inc.cc:4611: Extra space before ( in function call [whitespace/parens] [4]
/tmp/check_lint.XhMkOExnXU/tmp.TSPGZQsNod.sanitizer_common_interceptors.inc.cc:4611: All parameters should be named in a function [readability/function] [3]
/tmp/check_lint.XhMkOExnXU/tmp.TSPGZQsNod.sanitizer_common_interceptors.inc.cc:4614: Extra space before ( in function call [whitespace/parens] [4]
/tmp/check_lint.XhMkOExnXU/tmp.TSPGZQsNod.sanitizer_common_interceptors.inc.cc:4615: Extra space before ( in function call [whitespace/parens] [4]

Feb 10 2017, 2:19 AM · Restricted Project
jakubjelinek updated the diff for D29735: s390x __tls_get_addr_internal vs. __tls_get_offset.
Feb 10 2017, 2:14 AM · Restricted Project

Feb 9 2017

jakubjelinek updated the summary of D29735: s390x __tls_get_addr_internal vs. __tls_get_offset.
Feb 9 2017, 10:19 AM · Restricted Project
jakubjelinek updated the diff for D29735: s390x __tls_get_addr_internal vs. __tls_get_offset.
Feb 9 2017, 10:18 AM · Restricted Project
jakubjelinek added a comment to D29735: s390x __tls_get_addr_internal vs. __tls_get_offset.

Shall I upload a new patch with that? What is the process now, can somebody check it in for me?

Feb 9 2017, 2:31 AM · Restricted Project
jakubjelinek updated the diff for D29735: s390x __tls_get_addr_internal vs. __tls_get_offset.

Updated patch for that.

Feb 9 2017, 12:42 AM · Restricted Project
jakubjelinek added a comment to D29735: s390x __tls_get_addr_internal vs. __tls_get_offset.
In D29735#671362, @kcc wrote:

This seems to be a S390-only change, right?

Feb 9 2017, 12:26 AM · Restricted Project

Feb 8 2017

jakubjelinek created D29735: s390x __tls_get_addr_internal vs. __tls_get_offset.
Feb 8 2017, 3:40 PM · Restricted Project

Oct 17 2016

jakubjelinek added a comment to D25509: tsan: intercept libatomic.

Defining the builtins directly is certainly not going to work very well.
E.g. GCC libatomic uses:

void libat_load (size_t, void *, void *, int) __asm ("__atomic_load");

or e.g.

void lbat_load (size_t, void *, void *, int);
...
typeof (libat_load) export_load __asm ("__atomic_load") __attribute__((alias ("libat_load")));

or ifuncs.

Oct 17 2016, 11:14 PM

Sep 20 2016

jakubjelinek added a comment to D24771: Add C++17 aligned new/delete entrypoints to libasan.

An interesting question is how to test it upstream.

In theory you could just test it even with C++11, with something like:

#include <new>
Sep 20 2016, 2:34 PM
jakubjelinek added a comment to D24769: tsan: support pie binaries on newer kernels.

Well want to merge all the libsanitizer changes in the next month or two at most, but sure, this can be also desirable for release branch backporting eventually.

Sep 20 2016, 10:22 AM
jakubjelinek retitled D24771: Add C++17 aligned new/delete entrypoints to libasan from to Add C++17 aligned new/delete entrypoints to libasan.
Sep 20 2016, 10:14 AM