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

MemoryMte.cpp arm64 linux build failure #209

Closed
omnium21 opened this issue Dec 10, 2021 · 4 comments
Closed

MemoryMte.cpp arm64 linux build failure #209

omnium21 opened this issue Dec 10, 2021 · 4 comments

Comments

@omnium21
Copy link

omnium21 commented Dec 10, 2021

I'm having a problem building perfetto for linux on arm64.
I've tried building on my x86_64 Ubuntu 20.04 host machine and from inside both Ubuntu 18.04 and 20.04 docker containers with the same results.

I'm currently at this commit:
2724a3642 (HEAD -> master, origin/master, origin/HEAD) Merge "Avoid busy empty loop"

Here are the steps I've distilled from the docs:

git clone https://android.googlesource.com/platform/external/perfetto/
cd perfetto/
tools/install-build-deps --linux-arm
tools/gn gen --args="target_os = \"linux\" target_cpu=\"arm64\" is_debug=false is_clang=true" out/linux
tools/ninja -C out/linux

The failure is here:

$ tools/ninja -C out/linux
ninja: Entering directory `out/linux'
[226/3131] compile ../../buildtools/android-unwinding/libunwindstack/MemoryMte.cpp
FAILED: obj/buildtools/android-unwinding/libunwindstack/libunwindstack.MemoryMte.o 
../../buildtools/linux64/clang/bin/clang++ --target=aarch64-linux-gnu --sysroot=../../buildtools/debian_sid_arm64-sysroot -MMD -MF obj/buildtools/android-unwinding/libunwindstack/libunwindstack.MemoryMte.o.d -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS -DNDEBUG -DNO_LIBDEXFILE_SUPPORT -DPERFETTO_IMPLEMENTATION -I../../buildtools/android-unwinding/libunwindstack/include -I../../buildtools/android-unwinding/libunwindstack -I../../buildtools/android-libbase/include -I../../buildtools/android-logging/liblog/include -I../../buildtools/android-libprocinfo/include -I../../buildtools/android-core/include -I../../buildtools/android-core/libcutils/include -I../../buildtools/bionic/libc/async_safe/include -I../../buildtools/bionic/libc/platform -I../../buildtools/lzma/C -I../.. -I../../src/profiling/memory/include -I../../include -Igen/build_config -Igen -DFAKE_LOG_DEVICE=1 -fstrict-aliasing -Wformat -g -fPIC -fstack-protector-strong -Werror -fcolor-diagnostics -fdiagnostics-show-template-tree -Wno-c99-designator -fno-omit-frame-pointer -O3 -fdata-sections -ffunction-sections -isystem ../../buildtools/android-unwinding/libunwindstack/include -isystem ../../buildtools/android-libprocinfo/include -isystem ../../buildtools/android-libbase/include -isystem ../../buildtools/android-core/demangle/include -fno-exceptions -fno-rtti -nostdinc++ -isystem../../buildtools/libcxx/include -isystem../../buildtools/libcxxabi/include -std=c++17      -c ../../buildtools/android-unwinding/libunwindstack/MemoryMte.cpp -o obj/buildtools/android-unwinding/libunwindstack/libunwindstack.MemoryMte.o
../../buildtools/android-unwinding/libunwindstack/MemoryMte.cpp:33:14: error: use of undeclared identifier 'PTRACE_PEEKMTETAGS'
  if (ptrace(PTRACE_PEEKMTETAGS, pid_, reinterpret_cast<void*>(addr), &iov) != 0 ||
             ^
../../buildtools/android-unwinding/libunwindstack/MemoryMte.cpp:49:8: error: use of undeclared identifier 'mte_supported'
  if (!mte_supported() || !Read(addr, &data, 1)) {
       ^
2 errors generated.
[235/3131] compile ../../buildtools/googletest/googletest/src/gtest-all.cc
ninja: build stopped: subcommand failed.

A more complete log can be found here: https://pastebin.com/yivwmaN7

If I hack around the PTRACE_PEEKMTETAGS error, then next error is about ptrace:

../../buildtools/android-unwinding/libunwindstack/MemoryMte.cpp:33:7: error: no matching function for call to 'ptrace'
  if (ptrace(0, pid_, reinterpret_cast<void*>(addr), &iov) != 0 ||
      ^~~~~~
../../buildtools/debian_sid_arm64-sysroot/usr/include/aarch64-linux-gnu/bits/ptrace-shared.h:132:17: note: candidate function not viable: no known conversion from 'int' to 'enum __ptrace_request' for 1st argument
extern long int ptrace (enum __ptrace_request __request, ...) __THROW;
                ^

Any help would be appreciated.

@primiano
Copy link
Collaborator

Hmm weird. mte_supported is defined in buildtools/bionic/libc/platform/bionic/mte.h

These errors are relative to the unwinder, which is only for callstack sampling and heap profiling.

As a workaround, can you try passing enable_perfetto_traced_perf=false enable_perfetto_heapprofd=false ? That should stop building the unwinding library.

@rsavitski @ddiproietto in case they have more thoughts. Not sure why this would fail only when targeting arm64 (likely something sysroot related)

@primiano
Copy link
Collaborator

okay I can repro as well.
I think the real issue is that MemoryMte.cpp in libunwindstack does:

#ifdef __BIONIC__
#include <bionic/mte.h>
#endif
...
#if defined(__aarch64__)
...
  if (!mte_supported()

Those ifdefs should really be

#if __BIONIC__ && defined(__aarch64__)

@ddiproietto can you send a patch upstream to libunwindstack (and roll it in perfetto?) when you have some spare time?

@omnium21
Copy link
Author

Thanks for investigating so quickly.
I can confirm that your suggestion fixes the build.
Your fix sounds about right too.

primiano pushed a commit that referenced this issue Dec 19, 2021
Reported-at: #209
Change-Id: I2808617a51e1a0838fb557f40781434e6f4f486f
@ddiproietto
Copy link
Collaborator

https://r.android.com/1918189 and https://r.android.com/1926318 have been submitted. This should be fixed now.

dpallotti pushed a commit to dpallotti/orbitprofiler that referenced this issue Apr 26, 2022
PTRACE_PEEKMTETAGS has been introduced in glibc very recently in
https://sourceware.org/git/?p=glibc.git;a=commit;h=0ff786226c03456bef332950ecf51793205a4f5d
and is not yet available in major distributions.

mte_supported() is only defined in bionic.

Let's disable mte support in non bionic linux arm64 to fix the build
there.

Test: All unit tests pass.
Reported-at: google/perfetto#209
Change-Id: Iaac59978ff1c83b26a8e0e8fc92c0f4b5769e615
(cherry picked from commit 4c1a5dd4f91e06ec982902d3ab1c560fe01bac6a)
dpallotti pushed a commit to dpallotti/orbitprofiler that referenced this issue Apr 26, 2022
PTRACE_PEEKMTETAGS has been introduced in glibc very recently in
https://sourceware.org/git/?p=glibc.git;a=commit;h=0ff786226c03456bef332950ecf51793205a4f5d
and is not yet available in major distributions.

mte_supported() is only defined in bionic.

Let's disable mte support in non bionic linux arm64 to fix the build
there.

Test: All unit tests pass.
Reported-at: google/perfetto#209
Change-Id: Iaac59978ff1c83b26a8e0e8fc92c0f4b5769e615
(cherry picked from commit 4c1a5dd4f91e06ec982902d3ab1c560fe01bac6a)
dpallotti pushed a commit to dpallotti/orbitprofiler that referenced this issue Apr 26, 2022
PTRACE_PEEKMTETAGS has been introduced in glibc very recently in
https://sourceware.org/git/?p=glibc.git;a=commit;h=0ff786226c03456bef332950ecf51793205a4f5d
and is not yet available in major distributions.

mte_supported() is only defined in bionic.

Let's disable mte support in non bionic linux arm64 to fix the build
there.

Test: All unit tests pass.
Reported-at: google/perfetto#209
Change-Id: Iaac59978ff1c83b26a8e0e8fc92c0f4b5769e615
(cherry picked from commit 4c1a5dd4f91e06ec982902d3ab1c560fe01bac6a)
dpallotti pushed a commit to dpallotti/orbitprofiler that referenced this issue Apr 26, 2022
PTRACE_PEEKMTETAGS has been introduced in glibc very recently in
https://sourceware.org/git/?p=glibc.git;a=commit;h=0ff786226c03456bef332950ecf51793205a4f5d
and is not yet available in major distributions.

mte_supported() is only defined in bionic.

Let's disable mte support in non bionic linux arm64 to fix the build
there.

Test: All unit tests pass.
Reported-at: google/perfetto#209
Change-Id: Iaac59978ff1c83b26a8e0e8fc92c0f4b5769e615
(cherry picked from commit 4c1a5dd4f91e06ec982902d3ab1c560fe01bac6a)
dpallotti pushed a commit to dpallotti/orbitprofiler that referenced this issue Apr 27, 2022
PTRACE_PEEKMTETAGS has been introduced in glibc very recently in
https://sourceware.org/git/?p=glibc.git;a=commit;h=0ff786226c03456bef332950ecf51793205a4f5d
and is not yet available in major distributions.

mte_supported() is only defined in bionic.

Let's disable mte support in non bionic linux arm64 to fix the build
there.

Test: All unit tests pass.
Reported-at: google/perfetto#209
Change-Id: Iaac59978ff1c83b26a8e0e8fc92c0f4b5769e615
(cherry picked from commit 4c1a5dd4f91e06ec982902d3ab1c560fe01bac6a)
dpallotti added a commit to google/orbit that referenced this issue Apr 27, 2022
#3621)

In particular, I'm interested in bringing `Maps` and `MapsInfo` up to date
before making too many changes there.

This is not the latest commit but the next one is fairly large so I want to
proceed in two or more steps.

Bug: http://b/230436550

Test:
- Unit tests.
- Capture Trata and verify unwinding.
- Try mixed unwinding prototype on top of this.

Cherry-picked commits below.



* Fix all known race conditions in the tests.

Sometimes the DexFiles tests fail in the memory size, so increase the
size slightly to avoid this problem.

Increase the timeout in the ThreadEntry from 5 to 10 seconds.
If running on a slower config, give a little more time.

Update the attach code to be a little more error tolerant.
Specifically, increase the time allowed for a quiesce. Also try
and kick the process if it gets into a group-stop state.

Display the Unwinder object error code when a failure occurs in
an UnwindTest.

Retry the thread unwinds if they timeout since this can happen on
slower systems.

Bug: 203224453

Test: Ran unit tests for hours on cuttlefish and flame hwasan config.
Change-Id: Iefa660007e067b521d45f59067f5603d63a5a1fa
(cherry picked from commit eab6451b1a114d15c07be52d3436df4eea2421c5)

* Maps contains shared_ptr MapInfo objects now.

In order to make it easier to change the map data, make the individual
MapInfo objects shared_ptr objects.

This also changes the LocalUpdatableMaps object so that all of the
reparsing is in the object itself. This means this object can be
used with the normal unwinder unchanged and I can delete the
LocalUnwinder completely in a future cl.

Add a method to share a maps object between ThreadUnwinder objects.

Add new unit tests to cover the changes.

Bug: 120606663

Test: Unit tests pass.
Test: Did a performance run before and after to verify no
Test: performance issues.
Change-Id: I074dc925c8e5ff8b3d75779052aefe4801b2c7bf
Merged-In: I074dc925c8e5ff8b3d75779052aefe4801b2c7bf
(cherry picked from commit bf98d2a784c8181ba508f978e123459e9a8c610b)
(cherry picked from commit 88367cfde862f4bdd1da64d691b8a3854026f439)

* Remove the next_real_map and prev_real_map.

It's a pain to keep track of these pointers when doing a reparse.
Rather than keeping these in sync, remove them completely and add
a next_map. The next_real_map and prev_real_map variables are only
needed in very specific circumstances that are very unlikely to
occur. In those cases, derive the values when they are needed.

Also, make the Get{Next,Prev}RealMap functions verify that the
name is the same so that those checks can be removed from the
places that use those values.

Add new unit tests to verify the new behavior, and verify the
pointers are set properly.

Test: All unit tests pass.
Change-Id: I8fd784df71d7b39890c763755bec5904c0bed8a3
(cherry picked from commit 734cc2f1d45d5e0269654cb5b8fdecfc93dd2ad3)

* Fix Elf caching.

There were a few places where Elf caching didn't work when trying to
cache apks. There were also a few places where caching of a read-only
and read-execute map that represents a single elf wasn't working
properly either. In addition, the elf_start_offset and elf_offset
was not getting set properly in many cases.

Fix these and change the caching strategy from file:offset to a
map of maps, first map is the filename, the second is the elf_start_offset.

Replace all of the elf cache tests with better tests that actually
test all of the varied paths.

There is also a potential bug where the Elf object was being modified
outside of the MapInfo lock. Fixed by making sure the modification is
set within the lock.

Bug: 193408481
Test: Unit tests pass.
Change-Id: I517ae316151b2b608afa62a5863cf632752f29b2
(cherry picked from commit b1812005de85da6cc3adef6b0affc9e4f8787ebd)

* Add GetPrintableBuildID function to Elf object.

Bug: 197981919

Test: Unit tests pass.
Change-Id: I6373920ad09d005ffa93d08c573f865bae3c070e
(cherry picked from commit d452e57da25d144a41634c8f91068e6028365ef8)

* Ignore /dev/* MapInfo for GetBuildID

Similarly to CreateMemory, do no try to access the backing file if this
is a device file.

Bug: 68319037
Test: atest --host libunwindstack_unit_test
Change-Id: I23fb7c449069bcbc4893a87f8d3a2a38b11c9eba
(cherry picked from commit 5e34c2a1be6460d0e888fddc192e59c90639c19f)

* Fix linux (non bionic) arm64 build

PTRACE_PEEKMTETAGS has been introduced in glibc very recently in
https://sourceware.org/git/?p=glibc.git;a=commit;h=0ff786226c03456bef332950ecf51793205a4f5d
and is not yet available in major distributions.

mte_supported() is only defined in bionic.

Let's disable mte support in non bionic linux arm64 to fix the build
there.

Test: All unit tests pass.
Reported-at: google/perfetto#209
Change-Id: Iaac59978ff1c83b26a8e0e8fc92c0f4b5769e615
(cherry picked from commit 4c1a5dd4f91e06ec982902d3ab1c560fe01bac6a)

Co-authored-by: Christopher Ferris <cferris@google.com>
Co-authored-by: Thiébaud Weksteen <tweek@google.com>
Co-authored-by: Daniele Di Proietto <ddiproietto@google.com>
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

4 participants