Page MenuHomePhabricator

rprichard (Ryan Prichard)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 7 2017, 9:53 PM (167 w, 3 d)

Recent Activity

Thu, Sep 24

rprichard added inline comments to D85005: [libunwind] Support DW_CFA_remember/restore_state without heap allocation..
Thu, Sep 24, 2:49 PM

Wed, Sep 23

rprichard committed rG881aba7071c6: [libunwind] Optimize dl_iterate_phdr's findUnwindSectionsByPhdr (authored by rprichard).
[libunwind] Optimize dl_iterate_phdr's findUnwindSectionsByPhdr
Wed, Sep 23, 3:42 PM
rprichard committed rGb16d6653c01c: [libunwind] Combine dl_iterate_phdr codepaths for DWARF and EHABI (authored by rprichard).
[libunwind] Combine dl_iterate_phdr codepaths for DWARF and EHABI
Wed, Sep 23, 3:42 PM
rprichard closed D87881: [libunwind] Optimize dl_iterate_phdr's findUnwindSectionsByPhdr.
Wed, Sep 23, 3:42 PM · Restricted Project, Restricted Project
rprichard closed D87880: [libunwind] Combine dl_iterate_phdr codepaths for DWARF and EHABI.
Wed, Sep 23, 3:42 PM · Restricted Project, Restricted Project

Sep 21 2020

rprichard added a comment to D87880: [libunwind] Combine dl_iterate_phdr codepaths for DWARF and EHABI.

Thanks for the review. It looks like I still need someone from the libunwind group to accept it, though.

Sep 21 2020, 4:03 PM · Restricted Project, Restricted Project
rprichard added a comment to D87881: [libunwind] Optimize dl_iterate_phdr's findUnwindSectionsByPhdr.

Thanks for the review. It looks like I still need someone from the libunwind group to accept it, though.

Sep 21 2020, 4:03 PM · Restricted Project, Restricted Project

Sep 17 2020

rprichard added a comment to D87881: [libunwind] Optimize dl_iterate_phdr's findUnwindSectionsByPhdr.

I uploaded the Android benchmark I used: https://gist.github.com/rprichard/1688de36f0132a40294adb0409189533.

Sep 17 2020, 9:31 PM · Restricted Project, Restricted Project
rprichard requested review of D87881: [libunwind] Optimize dl_iterate_phdr's findUnwindSectionsByPhdr.
Sep 17 2020, 9:19 PM · Restricted Project, Restricted Project
rprichard added a reviewer for D87880: [libunwind] Combine dl_iterate_phdr codepaths for DWARF and EHABI: saugustine.

This change enables the FrameHeaderCache for EHABI.

Sep 17 2020, 9:18 PM · Restricted Project, Restricted Project
rprichard updated the diff for D87880: [libunwind] Combine dl_iterate_phdr codepaths for DWARF and EHABI.

Apply clang-format fix.

Sep 17 2020, 9:17 PM · Restricted Project, Restricted Project
rprichard requested review of D87880: [libunwind] Combine dl_iterate_phdr codepaths for DWARF and EHABI.
Sep 17 2020, 9:16 PM · Restricted Project, Restricted Project
rprichard added a comment to D83573: [libunwind] Support for leaf function unwinding..

I think these libunwind tests are relying on the signal trampoline having "good enough" unwind info, but AFAIK, that's not the case with the signal trampoline (__kernel_rt_sigreturn) in the AArch64 Linux vDSO. There was a commit recently adjusting .cfi_startproc to let the unwinder find the vDSO's FDE, but then this was quickly reverted. The FDE change allowed libgcc to find the FDE, and that broke glibc pthread cancellation somehow, so the FDE was then removed:

Sep 17 2020, 5:01 PM · Restricted Project, Restricted Project

Sep 16 2020

rprichard abandoned D86256: [libunwind] Make .eh_frame scanning/caching optional.

Abandoned in favor of D87750.

Sep 16 2020, 7:03 PM · Restricted Project, Restricted Project
rprichard committed rGfb1abe00635c: [libunwind][DWARF] Fix end of .eh_frame calculation (authored by rprichard).
[libunwind][DWARF] Fix end of .eh_frame calculation
Sep 16 2020, 7:02 PM
rprichard closed D87750: [libunwind][DWARF] Fix end of .eh_frame calculation.
Sep 16 2020, 7:02 PM · Restricted Project, Restricted Project
rprichard added a comment to D87750: [libunwind][DWARF] Fix end of .eh_frame calculation.

This seems like a bad name.

Sep 16 2020, 4:37 PM · Restricted Project, Restricted Project
rprichard updated the diff for D87750: [libunwind][DWARF] Fix end of .eh_frame calculation.

Rename dso_length to text_segment_length.

Sep 16 2020, 4:29 PM · Restricted Project, Restricted Project
rprichard updated subscribers of D87750: [libunwind][DWARF] Fix end of .eh_frame calculation.

Adding @saugustine -- this patch makes the frame header cache one word larger per entry.

Sep 16 2020, 2:29 AM · Restricted Project, Restricted Project
rprichard updated the summary of D87750: [libunwind][DWARF] Fix end of .eh_frame calculation.
Sep 16 2020, 2:26 AM · Restricted Project, Restricted Project
rprichard updated the diff for D87750: [libunwind][DWARF] Fix end of .eh_frame calculation.

Stylistic change.

Sep 16 2020, 2:26 AM · Restricted Project, Restricted Project
rprichard added a comment to D86256: [libunwind] Make .eh_frame scanning/caching optional.

I uploaded D87750, an alternative to this patch.

Sep 16 2020, 2:24 AM · Restricted Project, Restricted Project
rprichard added a comment to D87750: [libunwind][DWARF] Fix end of .eh_frame calculation.

An alternative to D86256.

Sep 16 2020, 2:24 AM · Restricted Project, Restricted Project
rprichard requested review of D87750: [libunwind][DWARF] Fix end of .eh_frame calculation.
Sep 16 2020, 2:04 AM · Restricted Project, Restricted Project

Sep 15 2020

rprichard added a comment to D86256: [libunwind] Make .eh_frame scanning/caching optional.

Just want to comment "Or use LLD, which automatically inserts the terminator" - the zero terminator CIE is to work around a glibc issue D46566

Sep 15 2020, 7:54 PM · Restricted Project, Restricted Project

Sep 14 2020

rprichard added a comment to D86256: [libunwind] Make .eh_frame scanning/caching optional.

The commit description talks about making the caching optional, but I don't really see that aspect in the patch - or maybe I'm just not studying it closely enough?

Sep 14 2020, 5:17 PM · Restricted Project, Restricted Project
rprichard added a comment to D75795: [libc++abi] Change __cxa_finalize return type to void.

I think I'm fine with this revision. I was wondering if the two functions should be removed from the GNU libsupc++'s cxxabi.h as well, which also declares both of the functions with an int return type. (i.e. It has the same issue as LLVM's cxxabi.h.)

Sep 14 2020, 2:44 PM · Restricted Project, Restricted Project, Restricted Project

Sep 9 2020

rprichard committed rG09d492902f17: [libunwind] Bare-metal DWARF: set dso_base to 0 (authored by rprichard).
[libunwind] Bare-metal DWARF: set dso_base to 0
Sep 9 2020, 3:44 PM
rprichard closed D86748: [libunwind] Bare-metal DWARF: set dso_base to 0.
Sep 9 2020, 3:44 PM · Restricted Project, Restricted Project
rprichard updated the summary of D86748: [libunwind] Bare-metal DWARF: set dso_base to 0.
Sep 9 2020, 3:38 PM · Restricted Project, Restricted Project
rprichard added inline comments to D86748: [libunwind] Bare-metal DWARF: set dso_base to 0.
Sep 9 2020, 3:34 PM · Restricted Project, Restricted Project

Sep 8 2020

rprichard updated the diff for D86748: [libunwind] Bare-metal DWARF: set dso_base to 0.

Use -1 to indicate "search all cache entries".

Sep 8 2020, 8:40 PM · Restricted Project, Restricted Project
rprichard added a comment to D86748: [libunwind] Bare-metal DWARF: set dso_base to 0.

Why not use -1 as the indicator instead of 0?

Sep 8 2020, 8:39 PM · Restricted Project, Restricted Project
rprichard committed rG88bf133c99c3: [libunwind] Replace chain-of-ifdefs for dl_iterate_phdr (authored by rprichard).
[libunwind] Replace chain-of-ifdefs for dl_iterate_phdr
Sep 8 2020, 3:50 PM
rprichard closed D86768: [libunwind] Replace chain-of-ifdefs for dl_iterate_phdr.
Sep 8 2020, 3:50 PM · Restricted Project, Restricted Project

Sep 3 2020

rprichard added a comment to D86256: [libunwind] Make .eh_frame scanning/caching optional.

The Linux ABI Extensions spec describes the binary search table of .eh_frame_hdr as optional, so maybe falling back to .eh_frame scanning could be necessary to accommodate some ELF files? I wonder what situation results in an omitted search table. ld.bfd does have code for outputting .eh_frame_hdr without a search table.

Sep 3 2020, 10:05 PM · Restricted Project, Restricted Project
rprichard committed rG673484b34189: [libunwind] Minor SJLJ config cleanup. NFCI. (authored by rprichard).
[libunwind] Minor SJLJ config cleanup. NFCI.
Sep 3 2020, 4:00 PM
rprichard closed D86767: [libunwind] Minor SJLJ config cleanup. NFCI..
Sep 3 2020, 4:00 PM · Restricted Project, Restricted Project

Sep 2 2020

rprichard added a comment to D86766: [libunwind] Fix a few libunwind includes.

On second thought, maybe I don't actually need this change, and the includes still seem like a mess in libunwind. Maybe it makes more sense to do a larger revision eventually.

Sep 2 2020, 10:52 PM · Restricted Project, Restricted Project
rprichard updated the diff for D86768: [libunwind] Replace chain-of-ifdefs for dl_iterate_phdr.

I am going to accept this, since no one else seems to be able to review it. But the problem here is that no one really knows what a simplified ifdef chain with the same functionality looks like. I tried a couple of times and failed miserably.

So if this works, great! If not, we will need to revert it quickly.

Sep 2 2020, 10:13 PM · Restricted Project, Restricted Project
rprichard added reviewers for D86768: [libunwind] Replace chain-of-ifdefs for dl_iterate_phdr: phosek, jgorbe.
Sep 2 2020, 4:36 PM · Restricted Project, Restricted Project
rprichard added a reviewer for D86767: [libunwind] Minor SJLJ config cleanup. NFCI.: phosek.

Do I need an approval from a libunwind group member?

Sep 2 2020, 3:59 PM · Restricted Project, Restricted Project

Aug 28 2020

rprichard updated the summary of D86767: [libunwind] Minor SJLJ config cleanup. NFCI..
Aug 28 2020, 5:57 PM · Restricted Project, Restricted Project
rprichard updated the diff for D86767: [libunwind] Minor SJLJ config cleanup. NFCI..

Remove most of the previous change. Add a comment about Apple/armv7k.

Aug 28 2020, 5:57 PM · Restricted Project, Restricted Project
rprichard added a comment to D86767: [libunwind] Minor SJLJ config cleanup. NFCI..

Sorry for the inconvenience that libunwind is not really buildable for armv7 from iOS SDK. You can build a single threaded version without referring to those interfaces. If this is really troublesome for you, feel free to file a bug report and we can see what can be done here.

Aug 28 2020, 5:26 PM · Restricted Project, Restricted Project
rprichard updated the summary of D86256: [libunwind] Make .eh_frame scanning/caching optional.
Aug 28 2020, 3:04 AM · Restricted Project, Restricted Project
rprichard added a comment to D86256: [libunwind] Make .eh_frame scanning/caching optional.

If the unwinder uses dl_iterate_phdr, then entries that are automatically added to DwarfFDECache would become invalid if the module containing the entry were unloaded. (On Apple systems, DwarfFDECache registers dyldUnloadHook to remove unloaded entries.)

Aug 28 2020, 2:29 AM · Restricted Project, Restricted Project
rprichard added a reviewer for D86768: [libunwind] Replace chain-of-ifdefs for dl_iterate_phdr: saugustine.
Aug 28 2020, 1:33 AM · Restricted Project, Restricted Project
rprichard updated subscribers of D86256: [libunwind] Make .eh_frame scanning/caching optional.

FWIW, when compact unwinding defers to DWARF (compactSaysUseDwarf), the unwinder begins a scan of .eh_frame at a given offset. Is that really the right behavior, or will the offset always point directly at the appropriate FDE (as with the DWARF index .eh_frame_hdr)?

Aug 28 2020, 1:27 AM · Restricted Project, Restricted Project
rprichard updated subscribers of D86767: [libunwind] Minor SJLJ config cleanup. NFCI..

FWIW, I compiled libunwind for Darwin, and I had trouble getting it to compile for armv7. Unwind-sjlj.c needs these things:

  • #include <System/pthread_machdep.h>
  • _pthread_getspecific_direct(__PTK_LIBC_DYLD_Unwind_SjLj_Key)
Aug 28 2020, 1:16 AM · Restricted Project, Restricted Project
rprichard retitled D86256: [libunwind] Make .eh_frame scanning/caching optional from [libunwind] Refactor DWARF FDE lookup to [libunwind] Make .eh_frame scanning/caching optional.
Aug 28 2020, 12:56 AM · Restricted Project, Restricted Project
rprichard updated the diff for D86257: [libunwind] Split out UnwindInfoSections and dl_iterate_phdr lookup.

Rebase this change.

Aug 28 2020, 12:54 AM · Restricted Project, Restricted Project
rprichard updated the diff for D86256: [libunwind] Make .eh_frame scanning/caching optional.

Split out the chain-of-ifdefs change into D86768.

Aug 28 2020, 12:53 AM · Restricted Project, Restricted Project
rprichard requested review of D86768: [libunwind] Replace chain-of-ifdefs for dl_iterate_phdr.
Aug 28 2020, 12:53 AM · Restricted Project, Restricted Project
rprichard requested review of D86767: [libunwind] Minor SJLJ config cleanup. NFCI..
Aug 28 2020, 12:52 AM · Restricted Project, Restricted Project
rprichard requested review of D86766: [libunwind] Fix a few libunwind includes.
Aug 28 2020, 12:50 AM · Restricted Project, Restricted Project

Aug 27 2020

rprichard requested review of D86748: [libunwind] Bare-metal DWARF: set dso_base to 0.
Aug 27 2020, 4:22 PM · Restricted Project, Restricted Project

Aug 26 2020

rprichard committed rG3071d5ffba23: [libunwind] Factor out getInfoFromFdeCie. NFCI. (authored by rprichard).
[libunwind] Factor out getInfoFromFdeCie. NFCI.
Aug 26 2020, 6:24 PM
rprichard committed rG7a457593efec: [libunwind] Minor fixes in libunwind (authored by rprichard).
[libunwind] Minor fixes in libunwind
Aug 26 2020, 6:24 PM
rprichard closed D86255: [libunwind] Factor out getInfoFromFdeCie. NFCI..
Aug 26 2020, 6:24 PM · Restricted Project, Restricted Project
rprichard closed D86254: [libunwind] Minor fixes in libunwind.
Aug 26 2020, 6:24 PM · Restricted Project, Restricted Project

Aug 25 2020

rprichard added a comment to D86372: [libunwind] Make findUnwindSectionsByPhdr static.

From the discussion on D86254, this should be cherry-picked to 11.x right?

Correct. This findUnwindSectionsByPhdr change (D86372) is needed for 11.

Aug 25 2020, 4:17 AM · Restricted Project, Restricted Project

Aug 24 2020

rprichard added a comment to D86411: [libunwind] Remove static_assert / __has_feature macros.

Oh, ok, I see - maybe CMake deduced that the option wasn't needed in my case, when building with a compiler that defaults to a new enough version.

Aug 24 2020, 4:15 PM · Restricted Project, Restricted Project
rprichard committed rG9e32d7b6e7e6: [libunwind] Remove static_assert / __has_feature macros (authored by rprichard).
[libunwind] Remove static_assert / __has_feature macros
Aug 24 2020, 2:08 PM
rprichard closed D86411: [libunwind] Remove static_assert / __has_feature macros.
Aug 24 2020, 2:08 PM · Restricted Project, Restricted Project
rprichard added a comment to D86372: [libunwind] Make findUnwindSectionsByPhdr static.

From the discussion on D86254, this should be cherry-picked to 11.x right?

Aug 24 2020, 12:35 PM · Restricted Project, Restricted Project
rprichard added a comment to D86254: [libunwind] Minor fixes in libunwind.

Please ensure that the fix goes into 11 before release (CC: @hans), but the remainder of the change is fine.

Just to make sure I get it right, the fix need for 11 is https://reviews.llvm.org/D86372, right?

Aug 24 2020, 12:33 PM · Restricted Project, Restricted Project
rprichard added a comment to D86411: [libunwind] Remove static_assert / __has_feature macros.

I guess this fix makes sense - I'd be surprised if we don't already require C++11 for building in general.

Aug 24 2020, 4:55 AM · Restricted Project, Restricted Project

Aug 22 2020

rprichard added a comment to D86411: [libunwind] Remove static_assert / __has_feature macros.

This change fixes this GCC build failure, https://reviews.llvm.org/D86102#2232290.

Aug 22 2020, 5:30 PM · Restricted Project, Restricted Project
rprichard requested review of D86411: [libunwind] Remove static_assert / __has_feature macros.
Aug 22 2020, 5:24 PM · Restricted Project, Restricted Project
rprichard added a comment to D86372: [libunwind] Make findUnwindSectionsByPhdr static.

(This change was previously accepted when it was part of D86254.)

Aug 22 2020, 5:14 PM · Restricted Project, Restricted Project
rprichard committed rG3c1b2e338dfd: [libunwind] Make findUnwindSectionsByPhdr static (authored by rprichard).
[libunwind] Make findUnwindSectionsByPhdr static
Aug 22 2020, 5:13 PM
rprichard closed D86372: [libunwind] Make findUnwindSectionsByPhdr static.
Aug 22 2020, 5:13 PM · Restricted Project, Restricted Project
rprichard updated the summary of D86372: [libunwind] Make findUnwindSectionsByPhdr static.
Aug 22 2020, 5:12 PM · Restricted Project, Restricted Project
rprichard added a comment to D86102: [libunwind] Ensure enough alignment for unw_cursor_t for SEH build configurations .

After this change, I see a compile error with GCC. There's something broken with libunwind's static_assert emulation. I'd guess the fix is to remove that emulation and simply use the C++11 static_assert.

Aug 22 2020, 5:07 PM · Restricted Project, Restricted Project

Aug 21 2020

rprichard updated the diff for D86254: [libunwind] Minor fixes in libunwind.

Update summary.

Aug 21 2020, 2:31 PM · Restricted Project, Restricted Project
rprichard updated the diff for D86254: [libunwind] Minor fixes in libunwind.

Break the "Make findUnwindSectionsByPhdr static" change into its own patch.

Aug 21 2020, 2:28 PM · Restricted Project, Restricted Project
rprichard requested review of D86372: [libunwind] Make findUnwindSectionsByPhdr static.
Aug 21 2020, 2:27 PM · Restricted Project, Restricted Project

Aug 20 2020

rprichard updated the diff for D86256: [libunwind] Make .eh_frame scanning/caching optional.

Stylistic changes.

Aug 20 2020, 5:02 PM · Restricted Project, Restricted Project
rprichard updated the diff for D86255: [libunwind] Factor out getInfoFromFdeCie. NFCI..

Stylistic changes. Also, only a single caller to getInfoFromFdeCie adds the result to the DwarfFDECache, so leave the cache addition code in getInfoFromDwarfSection.

Aug 20 2020, 5:02 PM · Restricted Project, Restricted Project
rprichard added a comment to D86254: [libunwind] Minor fixes in libunwind.

There is a libunwind::findUnwindSectionsByPhdr as of 11.x. When I build 11.x libunwind.so for glibc (I tested arm and x86_64), the symbol is exposed:

Aug 20 2020, 3:55 PM · Restricted Project, Restricted Project

Aug 19 2020

rprichard requested review of D86257: [libunwind] Split out UnwindInfoSections and dl_iterate_phdr lookup.
Aug 19 2020, 5:30 PM · Restricted Project, Restricted Project
rprichard requested review of D86256: [libunwind] Make .eh_frame scanning/caching optional.
Aug 19 2020, 5:21 PM · Restricted Project, Restricted Project
rprichard requested review of D86255: [libunwind] Factor out getInfoFromFdeCie. NFCI..
Aug 19 2020, 5:19 PM · Restricted Project, Restricted Project
rprichard added a reviewer for D86254: [libunwind] Minor fixes in libunwind: compnerd.
Aug 19 2020, 5:19 PM · Restricted Project, Restricted Project
rprichard updated the diff for D86254: [libunwind] Minor fixes in libunwind.

Revert edit that crept in from a later patch: SECTION -> UNWIND.

Aug 19 2020, 5:18 PM · Restricted Project, Restricted Project
rprichard requested review of D86254: [libunwind] Minor fixes in libunwind.
Aug 19 2020, 5:10 PM · Restricted Project, Restricted Project
rprichard added a comment to D86163: Default to disabling the libunwind frameheader cache.

Should we cherry-pick this change to 11.x to help with https://bugs.llvm.org/show_bug.cgi?id=46743? The cache also relies on the dlpi_adds / dlpi_subs fields that are only added in Android R (the upcoming version).

Aug 19 2020, 1:48 AM · Restricted Project, Restricted Project

Aug 6 2020

rprichard added a comment to D85005: [libunwind] Support DW_CFA_remember/restore_state without heap allocation..

The change seems alright to me.

Aug 6 2020, 5:35 PM

Jul 22 2020

rprichard added a comment to D83742: [libunwind] Fix getSLEB128 on large values.

AFAIK, there's no regression, and no one has hit this issue in practice, so maybe it can wait until the next release? It's probably only an issue when the size of something exceeds 2**32?

Jul 22 2020, 5:39 PM · Restricted Project, Restricted Project

Jul 21 2020

rprichard added a comment to D83741: [libunwind] Fix CIE v1 return address parsing.

There's only an issue if the return address register is in the range [128..255], and my impression is that probably never happens. It's also not a regression.

Jul 21 2020, 5:20 PM · Restricted Project, Restricted Project

Jul 16 2020

rprichard committed rG15b37e1cfa5f: [builtins] Omit 80-bit builtins on Android and MSVC (authored by rprichard).
[builtins] Omit 80-bit builtins on Android and MSVC
Jul 16 2020, 3:11 PM
rprichard closed D82153: [builtins] Omit 80-bit builtins on Android and MSVC.
Jul 16 2020, 3:11 PM · Restricted Project

Jul 15 2020

rprichard committed rGfd802cc4dea4: [libunwind] Fix getSLEB128 on large values (authored by rprichard).
[libunwind] Fix getSLEB128 on large values
Jul 15 2020, 7:14 PM
rprichard committed rG52d0a78b8315: [libunwind] Fix CIE v1 return address parsing (authored by rprichard).
[libunwind] Fix CIE v1 return address parsing
Jul 15 2020, 7:14 PM
rprichard closed D83742: [libunwind] Fix getSLEB128 on large values.
Jul 15 2020, 7:14 PM · Restricted Project, Restricted Project
rprichard closed D83741: [libunwind] Fix CIE v1 return address parsing.
Jul 15 2020, 7:14 PM · Restricted Project, Restricted Project

Jul 13 2020

rprichard added a comment to D83741: [libunwind] Fix CIE v1 return address parsing.

For reference, here's where Clang selects the CIE version:
https://github.com/llvm/llvm-project/blob/74c14202d90b46dda64a2542602855727b7d7f60/llvm/lib/MC/MCDwarf.cpp#L1592-L1605

Jul 13 2020, 10:34 PM · Restricted Project, Restricted Project
rprichard updated the diff for D83741: [libunwind] Fix CIE v1 return address parsing.

Stylistic changes.

Jul 13 2020, 10:28 PM · Restricted Project, Restricted Project
rprichard updated the summary of D83742: [libunwind] Fix getSLEB128 on large values.
Jul 13 2020, 10:20 PM · Restricted Project, Restricted Project