Page MenuHomePhabricator

smeenai (Shoaib Meenai)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 19 2016, 10:21 AM (301 w, 1 d)

Recent Activity

Thu, May 26

smeenai added a reverting change for rGec10ac750a8a: [runtimes] Detect changes to Tests.cmake: rGa831ce528fc0: Revert "[runtimes] Detect changes to Tests.cmake".
Thu, May 26, 9:39 AM · Restricted Project
smeenai committed rGa831ce528fc0: Revert "[runtimes] Detect changes to Tests.cmake" (authored by smeenai).
Revert "[runtimes] Detect changes to Tests.cmake"
Thu, May 26, 9:39 AM · Restricted Project, Restricted Project
smeenai added a reverting change for D121647: [runtimes] Detect changes to Tests.cmake: rGa831ce528fc0: Revert "[runtimes] Detect changes to Tests.cmake".
Thu, May 26, 9:39 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
smeenai committed rG0be0a53df65c: [libunwind] Use process_vm_readv to avoid potential segfaults (authored by smeenai).
[libunwind] Use process_vm_readv to avoid potential segfaults
Thu, May 26, 9:22 AM · Restricted Project
smeenai committed rG3d2b5b7b8785: [libunwind] Factor out sigreturn check condition. NFC (authored by smeenai).
[libunwind] Factor out sigreturn check condition. NFC
Thu, May 26, 9:21 AM · Restricted Project
smeenai closed D126343: [libunwind] Use process_vm_readv to avoid potential segfaults.
Thu, May 26, 9:21 AM · Restricted Project, Restricted Project, Restricted Project
smeenai closed D126342: [libunwind] Factor out sigreturn check condition. NFC.
Thu, May 26, 9:21 AM · Restricted Project, Restricted Project, Restricted Project

Wed, May 25

smeenai updated the diff for D126343: [libunwind] Use process_vm_readv to avoid potential segfaults.

Rebase

Wed, May 25, 9:49 PM · Restricted Project, Restricted Project, Restricted Project
smeenai updated the diff for D126342: [libunwind] Factor out sigreturn check condition. NFC.

Define macro to 1

Wed, May 25, 9:38 PM · Restricted Project, Restricted Project, Restricted Project
smeenai added inline comments to D126343: [libunwind] Use process_vm_readv to avoid potential segfaults.
Wed, May 25, 1:09 PM · Restricted Project, Restricted Project, Restricted Project
smeenai updated the diff for D126343: [libunwind] Use process_vm_readv to avoid potential segfaults.

Address review comments

Wed, May 25, 1:09 PM · Restricted Project, Restricted Project, Restricted Project
smeenai committed rG4baae166ce33: [pseudo] Fix pseudo-gen usage when cross-compiling (authored by smeenai).
[pseudo] Fix pseudo-gen usage when cross-compiling
Wed, May 25, 11:08 AM · Restricted Project, Restricted Project
smeenai closed D126397: [pseudo] Fix pseudo-gen usage when cross-compiling.
Wed, May 25, 11:08 AM · Restricted Project, Restricted Project
smeenai added a comment to D126397: [pseudo] Fix pseudo-gen usage when cross-compiling.

Thank you! I have been banging my head against this, I'm not entirely sure why this works and my version doesn't.

Wed, May 25, 10:45 AM · Restricted Project, Restricted Project
smeenai added a comment to D125667: [pseudo] A basic implementation of compiling cxx grammar at build time..

I'll see if I can fix it quickly, else will revert (there are a couple of fixes stacked on top already, so revert isn't quite clean)

Thank you! I'll have some time to look at this a bit later myself as well, and it should hopefully be a quick fix, so we can try to avoid the revert :)

Wed, May 25, 10:24 AM · Restricted Project, Restricted Project
smeenai requested review of D126397: [pseudo] Fix pseudo-gen usage when cross-compiling.
Wed, May 25, 10:24 AM · Restricted Project, Restricted Project
smeenai added a comment to D125667: [pseudo] A basic implementation of compiling cxx grammar at build time..

I'll see if I can fix it quickly, else will revert (there are a couple of fixes stacked on top already, so revert isn't quite clean)

Wed, May 25, 9:56 AM · Restricted Project, Restricted Project
smeenai added a comment to D125667: [pseudo] A basic implementation of compiling cxx grammar at build time..

This breaks cross-compilation, because we're building pseudo-gen for the target but then trying to run it on the build machine. You'll need to do something like TableGen does (see https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/CrossCompile.cmake and the uses of build_native_tool). That being said, I'm trying to figure out why our builds are attempting to build this in the first place, since as far as I can tell nothing external depends on it yet.

Wed, May 25, 9:37 AM · Restricted Project, Restricted Project

Tue, May 24

smeenai added inline comments to D126343: [libunwind] Use process_vm_readv to avoid potential segfaults.
Tue, May 24, 11:23 PM · Restricted Project, Restricted Project, Restricted Project
smeenai added inline comments to D126343: [libunwind] Use process_vm_readv to avoid potential segfaults.
Tue, May 24, 11:07 PM · Restricted Project, Restricted Project, Restricted Project
smeenai updated the diff for D126343: [libunwind] Use process_vm_readv to avoid potential segfaults.

Better MSAN suppression

Tue, May 24, 11:00 PM · Restricted Project, Restricted Project, Restricted Project
smeenai updated the diff for D126343: [libunwind] Use process_vm_readv to avoid potential segfaults.

XFAIL with MSAN, similar to other tests

Tue, May 24, 10:49 PM · Restricted Project, Restricted Project, Restricted Project
smeenai updated the diff for D126343: [libunwind] Use process_vm_readv to avoid potential segfaults.

Use sizeof instead of raw number

Tue, May 24, 4:51 PM · Restricted Project, Restricted Project, Restricted Project
smeenai added a comment to D123428: [libunwind] Add configuration to disable sigreturn frame check.

D126343 implements the process_vm_readv approach

Tue, May 24, 4:24 PM · Restricted Project, Restricted Project, Restricted Project
smeenai requested review of D126343: [libunwind] Use process_vm_readv to avoid potential segfaults.
Tue, May 24, 4:23 PM · Restricted Project, Restricted Project, Restricted Project
smeenai requested review of D126342: [libunwind] Factor out sigreturn check condition. NFC.
Tue, May 24, 4:22 PM · Restricted Project, Restricted Project, Restricted Project
smeenai added a comment to D125827: [cmake] Don't try creating an executable when detecting the linker.

LGTM, thanks.

I think this might potentially break cross-compilation on Windows. We're in a NOT WIN32 block here, but that means you're not targeting Windows; you could still be targeting e.g. Linux but building on a Windows machine. I think NUL is the Windows equivalent, and you could have a CMAKE_HOST_WIN32 block, but it'll depend on how CMake invokes an execute_process command. (I have a Windows machine handy and can experiment with that, if you'd like.) That being said

Hmm, I just tried it out on a Windows machine and it seems to work.

To be clear, were you building for Windows on a Windows machine, or targeting some other platform? The NOT WIN32 above would have already taken care of the building for Windows on Windows case.

Ugh, yeah, then I guess my testing wasn't sound. Do you have a setup to target Linux from a windows host?

Tue, May 24, 1:19 PM · Restricted Project, Restricted Project

Mon, May 23

smeenai accepted D126262: [lld-macho][nfc] Run clang-format on lld/MachO/*.{h,cpp}.

LGTM, thanks. I'm surprised these weren't caught by the lint running on the diffs where they were introduced.

Mon, May 23, 6:08 PM · Restricted Project, Restricted Project, Restricted Project

Fri, May 20

smeenai added a reviewer for D126072: [lld-macho] Stop crash when emitting personalities with -dead_strip: oontvoo.
Fri, May 20, 1:26 PM · Restricted Project, Restricted Project, Restricted Project

Thu, May 19

smeenai accepted D125827: [cmake] Don't try creating an executable when detecting the linker.

I think this might potentially break cross-compilation on Windows. We're in a NOT WIN32 block here, but that means you're not targeting Windows; you could still be targeting e.g. Linux but building on a Windows machine. I think NUL is the Windows equivalent, and you could have a CMAKE_HOST_WIN32 block, but it'll depend on how CMake invokes an execute_process command. (I have a Windows machine handy and can experiment with that, if you'd like.) That being said

Hmm, I just tried it out on a Windows machine and it seems to work.

Thu, May 19, 10:43 AM · Restricted Project, Restricted Project

Tue, May 17

smeenai resigned from D125800: [COFF] Add vfsoverlay flag.

In the LLVM Windows toolchain, we work around this by creating a directory of symlinks: https://github.com/llvm/llvm-project/blob/8527f32f0a16a77edb0b53a5ab8074e518eeff54/llvm/cmake/platforms/WinMsvc.cmake#L133-L157. A VFS overlay would be a nicer solution, and I'm excited to have this supported. I'll leave the review to @rnk and @mstorsjo though.

Tue, May 17, 5:59 PM · Restricted Project
smeenai requested changes to D125827: [cmake] Don't try creating an executable when detecting the linker.

Removing from my queue in the meantime

Tue, May 17, 5:51 PM · Restricted Project, Restricted Project
smeenai added a comment to D125827: [cmake] Don't try creating an executable when detecting the linker.

I think this might potentially break cross-compilation on Windows. We're in a NOT WIN32 block here, but that means you're not targeting Windows; you could still be targeting e.g. Linux but building on a Windows machine. I think NUL is the Windows equivalent, and you could have a CMAKE_HOST_WIN32 block, but it'll depend on how CMake invokes an execute_process command. (I have a Windows machine handy and can experiment with that, if you'd like.) That being said

Tue, May 17, 12:49 PM · Restricted Project, Restricted Project
smeenai added a comment to D125478: [llvm-objcopy][test] Add cmp after copy.

We're seeing an internal failure after this change as well :( Great catch, but it looks like it exposed some existing brokenness.

Tue, May 17, 11:51 AM · Restricted Project, Restricted Project

Mon, May 16

smeenai committed rGf20e6a6e61da: [test-suite][cmake] sort unit test targets (authored by gracejennings).
[test-suite][cmake] sort unit test targets
Mon, May 16, 4:57 PM · Restricted Project, Restricted Project
smeenai closed D124810: [test-suite][cmake] sort unit test targets.
Mon, May 16, 4:57 PM · Restricted Project, Restricted Project
smeenai added a comment to D124810: [test-suite][cmake] sort unit test targets.

LGTM

Thank you for the review! I don't yet have commit access. Would you commit the change on my behalf?

Mon, May 16, 3:32 PM · Restricted Project, Restricted Project

Fri, May 6

smeenai accepted D124810: [test-suite][cmake] sort unit test targets.

LGTM

Fri, May 6, 3:42 PM · Restricted Project, Restricted Project
smeenai planned changes to D124557: [compiler-rt][Darwin] Check for arm64 support directly.

Planning changes to remove from people's queues while I address the comments (which will take a bit, since I'm out next week). Thanks for the reviews!

Fri, May 6, 1:25 PM · Restricted Project, Restricted Project
smeenai abandoned D123428: [libunwind] Add configuration to disable sigreturn frame check.

https://github.com/abseil/abseil-cpp/blob/master/absl/debugging/internal/address_is_readable.cc has gone through several iterations. We can use rt_sigprocmask.

@rprichard, do you know if this would work for Android? It has the TOCTOU issue, but I imagine it's much simpler than having to manage and synchronize the pipe fd, and we could live with the TOCTOU in practice.

I'm glad @MaskRay found this -- I think it will probably work, and it seems better than assuming the PC is readable.

I see rt_procsigmask listed in bionic/libc/SYSCALLS.TXT, and I don't see it mentioned in any of the bionic/libc/SECCOMP*.txt files. I think that means seccomp is allowing the system call. I looked at kernel/signal.c, and AFAICT it's not doing any security checks that could be a problem. Bionic itself uses rt_sigprocmask for (at least) spawning new processes, creating/exiting threads, TLS-related locking, POSIX timers, and abort(). I think any seccomp-like blocking of rt_sigprocmask would have to be very targeted, so I think the syscall is probably allowed everywhere on Android.

It is assuming that the kernel will validate the address before validating the how. The kernel has a principle of not breaking userland -- is there a more specific guarantee we can rely on? e.g. The code has this comment:

// This strategy depends on Linux implementation details,
// so we rely on the test to alert us if it stops working.

The kernel source is structured as two wrappers around sigprocmask, SYSCALL_DEFINE4(rt_sigprocmask, ...) and COMPAT_SYSCALL_DEFINE4(rt_sigprocmask, ...). The wrappers copy user memory to/from kernel memory before calling sigprocmask, so it makes sense that they would validate the address first.

I talked with cferris today (the maintainer of Android's libunwindstack), and I think LLVM's libunwind could just use process_vm_readv, which is more straightforward and would also avoid the TOCTOU issue.

process_vm_readv is used by libunwindstack, and cferris wasn't aware of any situation where process_vm_readv was blocked.

process_vm_readv is presumably slower than rt_procsigmask, but that's probably acceptable? It's only used when the unwinder can't find DWARF unwind info for a PC.

process_vm_readv requires a 3.2 kernel or newer, but this is syscall only needed for arm64, and arm64 support wasn't added until a later kernel version (3.7, it appears?).

I think the kernel special-cases process_vm_readv so it doesn't need ptrace permissions when reading the calling process's address space. When mm == current->mm, the kernel skips the ptrace_may_access call:

Fri, May 6, 1:20 PM · Restricted Project, Restricted Project, Restricted Project

Thu, May 5

smeenai added inline comments to D122258: [MC] Omit DWARF unwind info if compact unwind is present where eligible.
Thu, May 5, 12:53 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
smeenai added inline comments to D122258: [MC] Omit DWARF unwind info if compact unwind is present where eligible.
Thu, May 5, 12:50 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
smeenai added inline comments to D122258: [MC] Omit DWARF unwind info if compact unwind is present where eligible.
Thu, May 5, 11:41 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Tue, May 3

smeenai resigned from D124489: Deprecate LLVM_BUILD_EXTERNAL_COMPILER_RT.

LGTM after changing to an error with a warning downgrade. Resigning to clear my queue, plus the Apple folks should get the final accept here :)

Tue, May 3, 3:58 PM · Restricted Project, Restricted Project

Mon, May 2

smeenai added a comment to D124810: [test-suite][cmake] sort unit test targets.

IIUC this only affects IDE project generation. This makes sense to me, though I'll give others a few days to take a look as well in case they have any thoughts on the organization.

Mon, May 2, 4:27 PM · Restricted Project, Restricted Project

Thu, Apr 28

smeenai resigned from D37375: [libcxx] Test case for the experimental library visibility macros.
Thu, Apr 28, 2:09 PM · Restricted Project
smeenai abandoned D39809: MSVC toolchain.
Thu, Apr 28, 2:09 PM · Restricted Project, Restricted Project
smeenai commandeered D39809: MSVC toolchain.

Commandeering to abandon, since this landed elsewhere

Thu, Apr 28, 2:08 PM · Restricted Project, Restricted Project
smeenai requested changes to D49587: [CMake] Support exporting runtimes using the CMake export.

Clearing my queue.

Thu, Apr 28, 2:08 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
smeenai resigned from D51174: Move Microsoft Demangler to Support.
Thu, Apr 28, 2:08 PM · Restricted Project, Restricted Project
smeenai resigned from D56906: Prototype of update to file headers for relicensing..
Thu, Apr 28, 2:07 PM · Restricted Project, Restricted Project, Restricted Project
smeenai resigned from D67321: Respect CLANG_LINK_CLANG_DYLIB=ON in libclang and c-index-test.
Thu, Apr 28, 2:06 PM · Restricted Project, Restricted Project
smeenai resigned from D88124: [windows-itanium] [WIP] Windows Itanium Build Recipe.
Thu, Apr 28, 2:05 PM · Restricted Project
smeenai resigned from D89492: [compiler-rt] Enable building builtins using top-level CMake file.
Thu, Apr 28, 2:05 PM · Restricted Project, Restricted Project, Restricted Project
smeenai resigned from D93351: [llvm-shlib] Build backend libraries as loadable modules.
Thu, Apr 28, 2:04 PM · Restricted Project, Restricted Project
smeenai resigned from D94364: [clang] Allow specifying the aapcs and aapcs-vfp for windows on arm.
Thu, Apr 28, 2:03 PM · Restricted Project, Restricted Project
smeenai resigned from D95727: llvm-shlib: Create object libraries for each component and link against them.
Thu, Apr 28, 2:00 PM · Restricted Project, Restricted Project
smeenai resigned from D99080: Normalize usage of StrBoolAttr.
Thu, Apr 28, 1:58 PM · Restricted Project, Restricted Project, Restricted Project
smeenai requested changes to D99155: [CMake] Support building libLLVM.a archive.

Is this still relevant? Requesting changes to clear my queue.

Thu, Apr 28, 1:58 PM · Restricted Project, Restricted Project
smeenai resigned from D105258: [llvm-readobj][MachO] Support option --unwind for __eh_frame.
Thu, Apr 28, 1:57 PM · Restricted Project, Restricted Project
smeenai resigned from D110018: [lld-macho] Speed up markLive().
Thu, Apr 28, 1:55 PM · Restricted Project, Restricted Project, Restricted Project
smeenai resigned from D113226: Support: Change FileOutputBuffer to return FileError.
Thu, Apr 28, 1:54 PM · Restricted Project, Restricted Project, Restricted Project
smeenai resigned from D116689: [libunwind][libcxxabi] Use object libraries in the build.
Thu, Apr 28, 1:54 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
smeenai accepted D117263: [CMake] Support runtimes targets without specifying triple.

Sorry, this fell off my radar! Your reasoning makes sense to me, and in particular, I'd forgotten that only compiler-t has the Darwin multi-arch setup (and you'd still need to handle libc++/libc++abi/libunwind differently).

Thu, Apr 28, 1:53 PM · Restricted Project, Restricted Project
smeenai resigned from D118331: [llvm-objdump][macho] Add initial --chained_fixups support.
Thu, Apr 28, 1:42 PM · Restricted Project, Restricted Project
smeenai requested changes to D122814: Functionality Added to CMAKE.

Requesting changes since there are unaddressed reviewer questions.

Thu, Apr 28, 1:41 PM · Restricted Project, Restricted Project
smeenai resigned from D121560: [clang][Opt] Add NoArgUnusedWith to not warn for unused redundant options.
Thu, Apr 28, 1:40 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
smeenai requested changes to D124315: lld: Bundle compact_unwind_encoding.h in lld to fix stand-alone builds.

While we're figuring this out ... @ldionne, @phosek, do the runtimes build changes (https://libcxx.llvm.org/BuildingLibcxx.html) also mean that we assume a full LLVM checkout is present? If so, we could move this header to LLVM and have both libunwind and LLD use it from there, which would sidestep the issue here.

Thu, Apr 28, 1:40 PM · Restricted Project, Restricted Project, Restricted Project
smeenai requested changes to D122931: [CMake][compiler-rt] Support for using in-tree libc++.

(clearing my queue while there are unaddressed comments)

Thu, Apr 28, 1:38 PM · Restricted Project, Restricted Project, Restricted Project
smeenai resigned from D123407: File Extension.

I'm not really sure about the ".md being more standard than .txt" in the summary, but I guess it's more standard with Github (where Markdown gets nice rendering), so this makes sense.

Thu, Apr 28, 1:30 PM · Restricted Project, Restricted Project
smeenai abandoned D57429: [docs] Document ignoring build directory.
Thu, Apr 28, 1:26 PM · Restricted Project, Restricted Project
smeenai abandoned D79904: [libcxx][libcxxabi] Link against libdl on Android.
Thu, Apr 28, 1:25 PM · Restricted Project, Restricted Project, Restricted Project
smeenai abandoned D47988: [CodeGen] Emit MSVC funclet IR for Obj-C exceptions.
Thu, Apr 28, 1:24 PM · Restricted Project
smeenai abandoned D53546: [WIP][private] Add Obj-C discriminator to MS ABI RTTI.
Thu, Apr 28, 1:23 PM · Restricted Project
smeenai abandoned D52674: [AST] Add Obj-C discriminator to MS ABI RTTI.
Thu, Apr 28, 1:23 PM · Restricted Project

Apr 27 2022

smeenai added inline comments to D123752: [lld] Implement safe icf for MachO.
Apr 27 2022, 5:14 PM · Restricted Project, Restricted Project, Restricted Project
smeenai added inline comments to D123752: [lld] Implement safe icf for MachO.
Apr 27 2022, 5:10 PM · Restricted Project, Restricted Project, Restricted Project
smeenai added a comment to D124563: Drop '* text=auto' from .gitattributes and normalize.

If I check out this commit and then check out the previous commit, clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp and clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected become modified in my working directory; their line endings are changed from CRLF to LF. That seems undesirable.

Apr 27 2022, 4:42 PM · Restricted Project, Restricted Project
smeenai added a comment to D124563: Drop '* text=auto' from .gitattributes and normalize.

So maybe we should simply remove

# These test input files rely on two-byte Windows (CRLF) line endings.
clang-apply-replacements/Inputs/crlf/crlf.cpp text eol=crlf
clang-apply-replacements/Inputs/crlf/crlf.cpp.expected text eol=crlf

from the .gitattributes file again. Then we can keep them stored with CRLF endings, and each file has a comment anyway that they rely on them. The files have been there for many years and never been a problem.

Apr 27 2022, 4:39 PM · Restricted Project, Restricted Project
smeenai added a comment to D124563: Drop '* text=auto' from .gitattributes and normalize.

The following files have their line endings (when checked out on disk) changed from CRLF to LF by this patch. Seems harmless, but I just wanted to confirm that it was expected:

Apr 27 2022, 4:25 PM · Restricted Project, Restricted Project
smeenai added a comment to rGac5f7be6a868: Move test/.gitattributes to clang-tools-extra/test.

Since this commit, no matter what I do, crlf.cpp and crlf.cpp.expected show up as modified. No git checkout --force nor git reset --hard changes this. git config.autocrlf is false and the files indeed have CRLF endings. git check-attr -a confirms the attributes are set (text: set eol: crlf).

That is, I have two checkouts, different versions of git showing different modified status.

Showing modified in checkout A, but not in checkout B:

  • git --version: git version 2.35.1 (from MSYS2)
  • git --version: git version 2.35.2.windows.1 (shipping with Visual Studio 2022)

Showing modified in checkout B, but not in checkout A:

  • git --version: git version 2.25.1 (from Ubuntu 22.04, running under WSL1)
Apr 27 2022, 3:54 PM · Restricted Project, Restricted Project
smeenai added a comment to D124557: [compiler-rt][Darwin] Check for arm64 support directly.

It seems that codes for tvos simulator also needs to be modified.

Apr 27 2022, 3:46 PM · Restricted Project, Restricted Project
smeenai updated the diff for D124557: [compiler-rt][Darwin] Check for arm64 support directly.

Adjust tvos simulator architecture detection as well

Apr 27 2022, 3:46 PM · Restricted Project, Restricted Project
smeenai updated the summary of D124557: [compiler-rt][Darwin] Check for arm64 support directly.
Apr 27 2022, 2:52 PM · Restricted Project, Restricted Project
smeenai requested review of D124557: [compiler-rt][Darwin] Check for arm64 support directly.
Apr 27 2022, 2:35 PM · Restricted Project, Restricted Project

Apr 26 2022

smeenai added a comment to D124433: [ELF] Prevent LTO stripping of wrapped script-referenced symbols.

in order for the wrapping logic to correctly set referencedAfterWrap and prevent LTO from eliminating them.

Perhaps elaborate which symbol's referencedAfterWrap is set (sym's).

Apr 26 2022, 6:49 PM · Restricted Project, Restricted Project
smeenai committed rG7cc328600e25: [ELF] Prevent LTO stripping of wrapped script-referenced symbols (authored by smeenai).
[ELF] Prevent LTO stripping of wrapped script-referenced symbols
Apr 26 2022, 6:49 PM · Restricted Project
smeenai closed D124433: [ELF] Prevent LTO stripping of wrapped script-referenced symbols.
Apr 26 2022, 6:49 PM · Restricted Project, Restricted Project
smeenai updated the diff for D124433: [ELF] Prevent LTO stripping of wrapped script-referenced symbols.

Address review comments

Apr 26 2022, 6:38 PM · Restricted Project, Restricted Project
smeenai added a comment to D124489: Deprecate LLVM_BUILD_EXTERNAL_COMPILER_RT.

I don't think many people are gonna notice a warning in the general spew of CMake configuration, tbh. Would it be too disruptive to make this an error instead and have a CMake option you can pass to get rid of the error?

Apr 26 2022, 5:43 PM · Restricted Project, Restricted Project
smeenai added a comment to D123428: [libunwind] Add configuration to disable sigreturn frame check.

Using /proc/self/maps would be subject to TOCTOU, but I think most methods wouldn't, e.g.:

  • Open /proc/self/mem and pread() the address. This seems strictly better than /proc/self/maps?
  • Create a pipe using pipe(), write() the bytes into the pipe buffer and read() them back out. I believe a Linux pipe buffer is guaranteed to be big enough (>= 8 bytes).
  • process_vm_readv

I wonder if security configurations are a problem. Maybe I should experiment on an Android build.

With all those methods, there's still the chance (however unlikely) that the address is readable at the time of the check but somehow becomes unreadable by the time we perform the actual read, right? I doubt it matters much in practice though.

The unwinder needs to read two aligned 4-byte words at the target PC. I was thinking that it could replace the ordinary reads with system calls. e.g. For the pipe trick, write the target 8 bytes into a pipe and then read them back out. If multiple threads share a pipe fd, then they'd need the synchronize their use of the pipe.

I wonder if we'd need to close the pipe fd for dlclose. On Android, the unwinder is linked statically into app shared objects.

For the pipe, since we'll be checking for validity multiple times, I imagine we'll need to empty out the buffer at some point. libunwind seems to use mincore if available and the pipe as a fallback, but I'm not really understanding how mincore would work here, since a page could be readable even though it's not currently resident.

Yeah, mincore doesn't seem to do what's needed here.

https://github.com/abseil/abseil-cpp/blob/master/absl/debugging/internal/address_is_readable.cc has gone through several iterations. We can use rt_sigprocmask.

Apr 26 2022, 12:30 PM · Restricted Project, Restricted Project, Restricted Project

Apr 25 2022

smeenai requested review of D124433: [ELF] Prevent LTO stripping of wrapped script-referenced symbols.
Apr 25 2022, 11:02 PM · Restricted Project, Restricted Project

Apr 22 2022

smeenai added a comment to D124315: lld: Bundle compact_unwind_encoding.h in lld to fix stand-alone builds.

To be clear, this is to support a build scenario where you only have the source tree under lld available?

Yes, that's correct.

Apr 22 2022, 6:48 PM · Restricted Project, Restricted Project, Restricted Project
smeenai added a comment to D124315: lld: Bundle compact_unwind_encoding.h in lld to fix stand-alone builds.

To be clear, this is to support a build scenario where you only have the source tree under lld available?

Apr 22 2022, 6:02 PM · Restricted Project, Restricted Project, Restricted Project
smeenai committed rG2a04f5c455c8: [ELF] Drop unused original symbol after wrapping if not defined (authored by smeenai).
[ELF] Drop unused original symbol after wrapping if not defined
Apr 22 2022, 4:47 PM · Restricted Project
smeenai closed D124065: [ELF] Drop unused original symbol after wrapping if not defined.
Apr 22 2022, 4:47 PM · Restricted Project, Restricted Project
smeenai committed rG1af25a986069: [ELF] Fix wrapping symbols produced during LTO codegen (authored by smeenai).
[ELF] Fix wrapping symbols produced during LTO codegen
Apr 22 2022, 4:46 PM · Restricted Project
smeenai closed D124056: [ELF] Fix wrapping symbols produced during LTO codegen.
Apr 22 2022, 4:46 PM · Restricted Project, Restricted Project
smeenai committed rGb862bcbf4455: [ELF] Move SymbolUnion assertions to source file (authored by smeenai).
[ELF] Move SymbolUnion assertions to source file
Apr 22 2022, 4:45 PM · Restricted Project
smeenai closed D124041: [ELF] Move SymbolUnion size assertion to source file.
Apr 22 2022, 4:45 PM · Restricted Project, Restricted Project