Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to run sanitized executables on ppc64le Ubuntu and SLES #933

Closed
diameter opened this issue Apr 9, 2018 · 30 comments
Closed

Unable to run sanitized executables on ppc64le Ubuntu and SLES #933

diameter opened this issue Apr 9, 2018 · 30 comments

Comments

@diameter
Copy link

diameter commented Apr 9, 2018

all executables built with asan fail to start on ppc64le machines.

simplest program to reproduce:

int main(int, char**) {
  return 0;
}

build:
gcc-5 -fsanitize=address main.cpp -o test

run:

zzz@linux:~/asan-test$ ASAN_OPTIONS="debug=1:verbosity=3" ./test 
==24573==Parsed ASAN_OPTIONS: debug=1:verbosity=3
==24573==AddressSanitizer: failed to intercept '__isoc99_printf'
==24573==AddressSanitizer: failed to intercept '__isoc99_sprintf'
==24573==AddressSanitizer: failed to intercept '__isoc99_snprintf'
==24573==AddressSanitizer: failed to intercept '__isoc99_fprintf'
==24573==AddressSanitizer: failed to intercept '__isoc99_vprintf'
==24573==AddressSanitizer: failed to intercept '__isoc99_vsprintf'
==24573==AddressSanitizer: failed to intercept '__isoc99_vsnprintf'
==24573==AddressSanitizer: failed to intercept '__isoc99_vfprintf'
==24573==AddressSanitizer: failed to intercept '__cxa_throw'
==24573==AddressSanitizer: libc interceptors initialized
|| `[0x120000000000, 0x7fffffffffff]` || HighMem    ||
|| `[0x044000000000, 0x11ffffffffff]` || HighShadow ||
|| `[0x024000000000, 0x043fffffffff]` || ShadowGap  ||
|| `[0x020000000000, 0x023fffffffff]` || LowShadow  ||
|| `[0x000000000000, 0x01ffffffffff]` || LowMem     || 
MemToShadow(shadow): 0x024000000000 0x0247ffffffff 0x028800000000 0x043fffffffff
redzone=16 
max_redzone=2048
quarantine_size=256M
malloc_context_size=30
SHADOW_SCALE: 3
SHADOW_GRANULARITY: 8
SHADOW_OFFSET: 20000000000
==24573==Installed the sigaction for signal 11
==24573==AddressSanitizer CHECK failed: ../../../../libsanitizer/asan/asan_poisoning.cc:24 " ((AddrIsInMem(addr))) != (0)" (0x0, 0x0)
   <empty stack>

environments tested:

  1. SUSE Linux Enterprise Server 12 SP3 (Linux linux 4.4.120-94.17-default Upgrade the LLVM code to avoid creating AVX instructions instead of SSE2 instructions #1 SMP Wed Mar 14 17:23:00 UTC 2018 (cf3a7bb) ppc64le ppc64le ppc64le GNU/Linux)
  2. Ubuntu 16.04.3 LTS (Linux ci-agent 4.13.0-16-generic asan false positives caused by dlcose #19~16.04.3-Ubuntu SMP Mon Oct 16 18:58:28 UTC 2017 ppc64le ppc64le ppc64le GNU/Linux)

compilers tested:
gcc-5, gcc-6, gcc-7

@kcc
Copy link
Contributor

kcc commented Apr 9, 2018

Does this work with clang?
Also, I'm afraid none of the Power maintainers are here. :(

@marxin
Copy link

marxin commented Apr 10, 2018

I can confirm that on SLE12 with GCC 7. Let me try to debug that.

@marxin
Copy link

marxin commented Apr 10, 2018

So root of the problem is that kHighMemEnd is initialized during runtime in libsanitizer/asan/asan_rtl.cc:

kHighMemEnd = GetMaxVirtualAddress()

And on CentOS 7 it has 46bits:

|| `[0x0a0000000000, 0x3fffffffffff]` || HighMem    ||
|| `[0x034000000000, 0x09ffffffffff]` || HighShadow ||
|| `[0x024000000000, 0x033fffffffff]` || ShadowGap  ||
|| `[0x020000000000, 0x023fffffffff]` || LowShadow  ||
|| `[0x000000000000, 0x01ffffffffff]` || LowMem     ||

While on SLE12 it has 48 bits and looks as follows:

|| `[0x120000000000, 0x7fffffffffff]` || HighMem    ||
|| `[0x044000000000, 0x11ffffffffff]` || HighShadow ||
|| `[0x024000000000, 0x043fffffffff]` || ShadowGap  ||
|| `[0x020000000000, 0x023fffffffff]` || LowShadow  ||
|| `[0x000000000000, 0x01ffffffffff]` || LowMem     ||

And issue is that allocator has defined range:

struct AP64 {  // Allocator64 parameters. Deliberately using a short name.
...
const uptr kAllocatorSpace =  0xa0000000000ULL;
 const uptr kAllocatorSize  =  0x20000000000ULL;  // 2T.

and kAllocatorSpace + kAllocatorSize does not fit in any of shadow regions, thus AddrIsInMem
return false and assert occurs.

Solution is to set kAllocatorSpace to address in HighMem:

diff --git a/libsanitizer/asan/asan_allocator.h b/libsanitizer/asan/asan_allocator.h
index 63260ff9895..e6a892f28fc 100644
--- a/libsanitizer/asan/asan_allocator.h
+++ b/libsanitizer/asan/asan_allocator.h
@@ -122,7 +122,7 @@ const uptr kAllocatorSpace = ~(uptr)0;
 const uptr kAllocatorSize  =  0x40000000000ULL;  // 4T.
 typedef DefaultSizeClassMap SizeClassMap;
 # elif defined(__powerpc64__)
-const uptr kAllocatorSpace =  0xa0000000000ULL;
+const uptr kAllocatorSpace =  0x120000000000ULL;
 const uptr kAllocatorSize  =  0x20000000000ULL;  // 2T.
 typedef DefaultSizeClassMap SizeClassMap;
 # elif defined(__aarch64__) && SANITIZER_ANDROID

This works, but extension of VA space to 48 will again break that. Kostya is it acceptable fix?

@kcc
Copy link
Contributor

kcc commented Apr 10, 2018

I don't mind, as long as someone from IBM reviews this upstream.
Note that you can also use kAllocatorSpace = ~(uptr)0 which will use dynamic base for the allocator region. More expensive, but more flexible.

@BillSeurer
Copy link

BillSeurer commented Apr 12, 2018

I tried both 0x120000000000ULL and ~(uptr)0 as values for kAllocatorSpace and tested on 4 different powerpc64 systems with various kernel levels and only ~(uptr)0 worked on all of them. I tried this with llvm, too, and it was similar.

@kcc
Copy link
Contributor

kcc commented Apr 12, 2018

Ok, let's switch powerpc64 to the dynamic shadow base then (kAllocatorSpace = ~(uptr)0).
You may want to get some performance measurements, just in case.

@BillSeurer
Copy link

Are there some suggested performance tests to run for asan?

@kcc
Copy link
Contributor

kcc commented Apr 12, 2018

For public consumption I usually run Spec 2006, it's good enough for this kind of measurement.
But Power is your platform, and so I don't insist on any particular benchmark -- just want you to run something to ensure that dynamic shadow is not outrageously expensive (it shouldn't be).

@kcc
Copy link
Contributor

kcc commented Apr 12, 2018

(check not just for the CPU overhead, but also for code size)

@jakubjelinek
Copy link

jakubjelinek commented Apr 13, 2018

I confirm that

--- libsanitizer/asan/asan_allocator.h	2017-10-19 13:20:58.926958939 +0200
+++ libsanitizer/asan/asan_allocator.h	2018-04-13 00:24:53.331985820 +0200
@@ -122,7 +122,7 @@ const uptr kAllocatorSpace = ~(uptr)0;
 const uptr kAllocatorSize  =  0x40000000000ULL;  // 4T.
 typedef DefaultSizeClassMap SizeClassMap;
 # elif defined(__powerpc64__)
-const uptr kAllocatorSpace =  0xa0000000000ULL;
+const uptr kAllocatorSpace = ~(uptr)0;
 const uptr kAllocatorSize  =  0x20000000000ULL;  // 2T.
 typedef DefaultSizeClassMap SizeClassMap;
 # elif defined(__aarch64__) && SANITIZER_ANDROID

fixed all the failing powerpc64 asan tests for me on a box where they have been failing before.

@marxin
Copy link

marxin commented Apr 13, 2018

I have nothing against switching to dynamic shadow. I'm just curious @BillSeurer what's difference in between the machines? Can you mention what's VM size and how does memory ranges look like:

gcc empty.c -fsanitize=address && ASAN_OPTIONS="verbosity=1" ./a.out
...
|| `[0x10007fff8000, 0x7fffffffffff]` || HighMem    ||
|| `[0x02008fff7000, 0x10007fff7fff]` || HighShadow ||
|| `[0x00008fff7000, 0x02008fff6fff]` || ShadowGap  ||
|| `[0x00007fff8000, 0x00008fff6fff]` || LowShadow  ||
|| `[0x000000000000, 0x00007fff7fff]` || LowMem     ||
...

Thanks.

@BillSeurer
Copy link

OK, I will check spec2006 then.

@BillSeurer
Copy link

power7 BE machine:
kernel: 2.6.32-696.13.2.el6.ppc64
distro: Red Hat Enterprise Linux Server release 6.9 (Santiago)
vma size = 44
Mem: 127115456
|| [0x040000000000, 0x0fffffffffff] || HighMem ||
|| [0x028000000000, 0x03ffffffffff] || HighShadow ||
|| [0x024000000000, 0x027fffffffff] || ShadowGap ||
|| [0x020000000000, 0x023fffffffff] || LowShadow ||
|| [0x000000000000, 0x01ffffffffff] || LowMem ||

power8 BE machine:
kernel: 3.10.0-693.5.2.el7.ppc64
distro: Red Hat Enterprise Linux Server 7.4 (Maipo)
vma size = 46
Mem: 254258112
|| [0x0a0000000000, 0x3fffffffffff] || HighMem ||
|| [0x034000000000, 0x09ffffffffff] || HighShadow ||
|| [0x024000000000, 0x033fffffffff] || ShadowGap ||
|| [0x020000000000, 0x023fffffffff] || LowShadow ||
|| [0x000000000000, 0x01ffffffffff] || LowMem ||

power8 LE machine:
kernel: 4.4.0-105-generic
distro: 16.04.4 LTS (Xenial Xerus)
vma size = 46
Mem: 267620608
|| [0x0a0000000000, 0x3fffffffffff] || HighMem ||
|| [0x034000000000, 0x09ffffffffff] || HighShadow ||
|| [0x024000000000, 0x033fffffffff] || ShadowGap ||
|| [0x020000000000, 0x023fffffffff] || LowShadow ||
|| [0x000000000000, 0x01ffffffffff] || LowMem ||

power8 LE machine:
kernel: 4.13.0-19-generic
distro: Ubuntu 17.10
vma size = 47
Mem: 267687424
|| [0x120000000000, 0x7fffffffffff] || HighMem ||
|| [0x044000000000, 0x11ffffffffff] || HighShadow ||
|| [0x024000000000, 0x043fffffffff] || ShadowGap ||
|| [0x020000000000, 0x023fffffffff] || LowShadow ||
|| [0x000000000000, 0x01ffffffffff] || LowMem ||

@jakubjelinek
Copy link

I'm afraid there is no constant value that can work everywhere (at least not if we want to go it into HighMem chunk). Because the 47-bit VA needs at least 0x120000000000, and 44-bit VA needs at most 0xe0000000000 (and even that would be too high, because there is also stack in there). And LowMem is too small in the 44-bit VA space, it is exactly the 2TB we need, but there would be no space for the executable etc.

@marxin
Copy link

marxin commented Apr 14, 2018

That would mean would have to sacrifice 44-bit VA seen on the power7 machine. It's running quite old Linux kernel. How long will you support the RHEL 6.9 version @jakubjelinek?

@jakubjelinek
Copy link

Until 2024.

@marxin
Copy link

marxin commented Apr 14, 2018

I see. There are another questions that I see:

  • why is range of 2TB requested for ppc64? There are targets with significantly smaller size.
  • as the memory map is done dynamically (getting size of VA seen), the kAllocatorSize could be also set dynamically respecting VA size?

@jakubjelinek
Copy link

as the memory map is done dynamically (getting size of VA seen), the kAllocatorSize could
be also set dynamically respecting VA size?

Isn't that exactly what ~(uptr) 0 does?

// Space: a portion of address space of kSpaceSize bytes starting at SpaceBeg.
// If kSpaceBeg is ~0 then SpaceBeg is chosen dynamically my mmap.
// Otherwise SpaceBeg=kSpaceBeg (fixed address).

@marxin
Copy link

marxin commented Apr 14, 2018

Sounds good, if @BillSeurer will not see any significant slow down, then let's install the patch.

@BillSeurer
Copy link

Ugh. Some of the spec2006 tests trigger asan errors when I tried it. I am going to see if there are a few that run OK and just use those.

@jakubjelinek
Copy link

Some SPEC2k6 sources are indeed invalid, see e.g. http://gcc.gnu.org/PR53073

@kcc
Copy link
Contributor

kcc commented Apr 16, 2018

See also: https://github.com/google/sanitizers/wiki/AddressSanitizerRunningSpecBenchmarks
You probably don't need to fix them, it would be enough to see the numbers on non-failing ones.

@BillSeurer
Copy link

The total run time for all the successful spec2006 tests was almost identical with the kAllocatorSpace = ~(uptr)0 change as without it. I could dive into the details but I think this is good enough for our purposes.

@kcc
Copy link
Contributor

kcc commented Apr 18, 2018

If it's good for you it's good for me, thanks for checking!

@marxin
Copy link

marxin commented Apr 18, 2018

Should I create a phabricator review? Or has anybody else done that?

@BillSeurer
Copy link

In order for this to work in llvm you may also need to change the value
kPPC64_ShadowOffset64 from 1ULL << 44 to 1ULL << 41 (in two places).

@kcc
Copy link
Contributor

kcc commented Apr 18, 2018

Should I create a phabricator review? Or has anybody else done that?

Yes, please.

@BillSeurer
Copy link

Never mind. I just tried some llvm test runs and the allocator patch above works fine by itself without any other changes.

@marxin
Copy link

marxin commented Apr 23, 2018

Review request created:
https://reviews.llvm.org/D45950

jyknight pushed a commit to jyknight/llvm-monorepo that referenced this issue Apr 23, 2018
@morehouse
Copy link
Contributor

https://reviews.llvm.org/D45950 landed. I think this can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants