Page MenuHomePhabricator

srhines (Stephen Hines)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 23 2012, 7:46 PM (553 w, 36 m)

Recent Activity

Wed, Mar 22

srhines updated subscribers of D146664: Apply the same fallbacks as runtimes search for stdlib search.
Wed, Mar 22, 3:17 PM · Restricted Project, Restricted Project

Tue, Mar 21

srhines accepted D146565: [clang] Remove mips target triple for Android.

Thank you for taking the time to properly remove this.

Tue, Mar 21, 6:16 PM · Restricted Project, Restricted Project, Restricted Project
srhines requested changes to D146565: [clang] Remove mips target triple for Android.

You probably need to remove the tests that use these triples (or switch them to a non-Android variant).

Tue, Mar 21, 2:39 PM · Restricted Project, Restricted Project, Restricted Project
srhines accepted D146560: [PATCH] [PATCH] Enable targeting riscv64-linux-android.
Tue, Mar 21, 2:06 PM · Restricted Project, Restricted Project, Restricted Project

Mon, Mar 20

srhines accepted D146355: [compiler-rt] Allow finding LLVMConfig if CMAKE_FIND_ROOT_PATH_MODE_PACKAGE is set to ONLY.

LGTM, but @phosek should also be able to confirm this is ok.

Mon, Mar 20, 12:04 PM · Restricted Project

Thu, Mar 16

srhines added a comment to D146266: gn build: Fix Android build..

yeah this was very confusing when I was looking at this

$ fd i386 ~/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/
/usr/local/google/home/aeubanks/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/12.0.5/lib/linux/i386/
$ fd i686 ~/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/
...
/usr/local/google/home/aeubanks/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/12.0.5/lib/linux/libclang_rt.ubsan_standalone_cxx-i686-android.a # and lots of other compiler-rt libs
/usr/local/google/home/aeubanks/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/i686-linux-android/
/usr/local/google/home/aeubanks/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/i686-linux-android/
...

building with this patch, I still get ld.lld: error: unable to find library -l:libunwind.a for all the different arches

perhaps the Android NDK in Chromium is not the standard one? are you using the one bundled in Chromium?

Thu, Mar 16, 6:10 PM · Restricted Project, Restricted Project

Feb 14 2023

srhines accepted D143983: Remove Renderscript support from LLDB.

Thanks for cleaning this up, @kongyi.

Feb 14 2023, 12:32 PM · Restricted Project, Restricted Project

Feb 6 2023

srhines added a comment to D142309: [LLDB][NFC] Fix a incorrect use of shared_ptr in RenderScriptRuntime.cpp.

LGTM, but I wonder when we'll be able to get rid of RenderScript completely (CC @labath)

I am not going to stand in your way. I usually redirect these questions to @srhines, but I understand he's moved on to other things as well.

Thanks, no problem.

Feb 6 2023, 10:17 AM · Restricted Project, Restricted Project

Sep 30 2022

srhines accepted D125142: [clang][auto-init] Deprecate -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang.

Thank you, Kees and reviewers, for helping to make progress here.

Sep 30 2022, 2:05 PM · Restricted Project, Restricted Project

Sep 14 2022

srhines accepted D132984: Set HOME for tests that use module cache path.
Sep 14 2022, 12:07 PM · Restricted Project, Restricted Project

Aug 24 2022

srhines accepted D132514: [lldb] Remove obsolete Android-specific definitions.

Thanks for cleaning this up!

Aug 24 2022, 1:39 AM · Restricted Project, Restricted Project

Aug 11 2022

srhines accepted D131656: [Support] Remove Log2 workaround for Android API level < 18.

+Dan as FYI, but https://github.com/android/ndk/wiki/Changelog-r24 shows that the NDK has moved forward to at least a minimum target API of 19, so this code is indeed obsolete. We might want to go and update Clang to also forbid setting a target API less than 19 too.

Aug 11 2022, 12:50 AM · Restricted Project, Restricted Project
srhines added a reviewer for D131656: [Support] Remove Log2 workaround for Android API level < 18: danalbert.
Aug 11 2022, 12:49 AM · Restricted Project, Restricted Project

Jun 24 2022

srhines added a comment to D128453: Automate checking for "command that takes no arguments" being passed arguments in CommandObjectParsed.

This is great, it both guarantees consistently and enforces command objects registering their arguments. LGTM.

For the RenderScript plugin, I remember this at an LLVM social when @labath was in town. At the time, we were very close to being able to get rid of it, which is now several years ago. Pavel, do you remember who we spoke to and if we've reached the point where this can go away?

I don't know what's the state of renderscript, but I think @srhines is the person you have in mind.

Jun 24 2022, 2:57 PM · Restricted Project, Restricted Project

Jun 23 2022

srhines updated subscribers of D128382: [LLD] Two tweaks to symbol ordering scheme.

This patch tries to adjust D44969 heuristics a bit.

This is not necessary if less than two input sections contain anything that is executable, for example, when user specifies an ordering of read-only data (instead of function) symbols. It is also not necessary if total size of output section is small such

that no branch thunk would ever be required, which is common for mobile apps.

I agree that the mentioned cases do not unnecessarily need reordering. It doesn't matter either way, then why change the behavior with more code?

You should also add the motivation for this change to the description (which I believe is to minimize the number of pages occupied by hot code).

Such a motivation is fine. Perhaps @srhines can test this for other applications.

Jun 23 2022, 3:26 AM · Restricted Project, Restricted Project
srhines updated subscribers of D128382: [LLD] Two tweaks to symbol ordering scheme.
Jun 23 2022, 3:25 AM · Restricted Project, Restricted Project

Jun 22 2022

srhines added inline comments to D128029: [AArch64] Add target feature "all".
Jun 22 2022, 1:32 AM · Restricted Project, Restricted Project

Jun 14 2022

srhines added a comment to D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64.

We had some internal Google folks hit this exact issue recently. I don't think that the same "default" CPU should be used for debugging tools like llvm-objdump, and that is the crux of the matter here. Perhaps updating the test to specify "generic" as the CPU when passed to llvm-objdump for those tests is the right pattern? Then the default for disassembly should be to have all attributes/features enabled unless the developer specifies on the command line that they want a different target.

Jun 14 2022, 6:02 PM · Restricted Project, Restricted Project, Restricted Project
srhines added inline comments to D127528: [Clang] Let the linker choose shared or static libunwind unless specified.
Jun 14 2022, 12:04 PM · Restricted Project, Restricted Project

May 31 2022

srhines added a reviewer for D126702: [lldb] Fix TCPSocket::Connect when getaddrinfo returns multiple addrs: labath.
May 31 2022, 10:53 AM · Restricted Project, Restricted Project

Mar 15 2022

srhines accepted D121683: Emit a warning if -xc/-xc++ is after the last input file.

Looks great. Thanks for noticing and adding this.

Mar 15 2022, 8:27 AM · Restricted Project, Restricted Project

Feb 9 2022

srhines accepted D119354: [ARM] Patterns for vector conversion between half and float.

Thanks for getting this fixed quickly!

Feb 9 2022, 2:44 PM · Restricted Project

Feb 3 2022

srhines accepted D118942: [Driver][Android] Removed obsoleted --warn-shared-textrel.
Feb 3 2022, 3:02 PM · Restricted Project

Jan 26 2022

srhines accepted D116753: [Driver] Default to -fno-math-errno for musl too.

Verified with the docs from musl.

Jan 26 2022, 9:38 AM · Restricted Project

Dec 14 2021

srhines committed rGcce4a7258b81: [compiler-rt][AArch64] Add a workaround for Exynos 9810 (authored by srhines).
[compiler-rt][AArch64] Add a workaround for Exynos 9810
Dec 14 2021, 7:52 PM
srhines closed D114523: [compiler-rt][AArch64] Add a workaround for Exynos 9810.
Dec 14 2021, 7:52 PM · Restricted Project, Restricted Project, Restricted Project
srhines added a comment to D114523: [compiler-rt][AArch64] Add a workaround for Exynos 9810.

Stephen, could you land this patch? It can help fix downstream issues like https://crbug.com/1272795.

Dec 14 2021, 5:19 PM · Restricted Project, Restricted Project, Restricted Project

Dec 8 2021

srhines accepted D114523: [compiler-rt][AArch64] Add a workaround for Exynos 9810.

Ryan's comment above is correct that this won't affect the Android 12 platform image. The more important task will be backporting this patch for any necessary NDK, since apps compiled with outlined atomics would start causing problems otherwise. I will take care of that part once this is merged.

Dec 8 2021, 1:31 AM · Restricted Project, Restricted Project, Restricted Project

Nov 29 2021

srhines added a reviewer for D114523: [compiler-rt][AArch64] Add a workaround for Exynos 9810: rprichard.
Nov 29 2021, 3:39 PM · Restricted Project, Restricted Project, Restricted Project

Nov 18 2021

srhines accepted D114111: Remove a useless temporary of a base class type..

Per https://developer.android.com/guide/topics/renderscript/migrate, we are starting to turn things down as part of the deprecation. I reached out internally to the folks helping with that to make sure that we don't cause problems by deleting upstream code. I suspect that we'll be able to make more progress with adapting upstream soon. In the meantime, this CL is fine.

Nov 18 2021, 12:00 PM · Restricted Project, Restricted Project

Nov 16 2021

srhines accepted D114036: Add Android test case for -Wpartial-availability. Also update Android availability tests to match on the whole string, so we can distinguish between "Android 16" and "Android 16.0.0" at the end of warning messages..

Thanks for the expanded (and more specific) tests.

Nov 16 2021, 2:56 PM · Restricted Project

Aug 5 2021

srhines accepted D107526: Clean up instcombine stpcpy test.

LGTM from the Android side. Thanks for finding and fixing the original issue, as well as adding this stronger test. :)

Aug 5 2021, 12:18 PM · Restricted Project

Jun 11 2021

srhines committed rG6455418d3d2a: [compiler-rt] [builtins] [AArch64] Add missing AArch64 data synchronization… (authored by srhines).
[compiler-rt] [builtins] [AArch64] Add missing AArch64 data synchronization…
Jun 11 2021, 2:16 AM
srhines closed D104094: Add missing AArch64 data synchronization barrier (dsb) to __clear_cache.
Jun 11 2021, 2:15 AM · Restricted Project
srhines updated subscribers of D104094: Add missing AArch64 data synchronization barrier (dsb) to __clear_cache.
Jun 11 2021, 12:24 AM · Restricted Project
srhines requested review of D104094: Add missing AArch64 data synchronization barrier (dsb) to __clear_cache.
Jun 11 2021, 12:23 AM · Restricted Project

Jun 8 2021

srhines added a comment to D103906: Do not generate calls to the 128-bit function __multi3() on 32-bit ARM..

Right, ok, this is true for compiler-rt, but not necessarily other runtime libraries.

There is an open discussion if we should assume compiler-rt is the default and use what we have, but this is a different issue.

If the call to __multi3 is there by accident (leaked from other target's default), then it's ok to remove it. If not, we may need some more complex solution.

@compnerd @joerg @t.p.northover @srhines, do you know of any Arm32 runtime library that implements 128-bit maths with Clang?

I think Android does, so perhaps we can add a condition there on AndroidEABI?

Jun 8 2021, 11:39 AM · Restricted Project

Apr 28 2021

srhines updated subscribers of D101473: [compiler-rt] Move to cxx17..

@srhines, @mcgrathr, @phosek, @echristo - please raise your hand if you foresee any problems with bumping compiler-rt to c++17. The rest of LLVM seems to use it just fine.

Apr 28 2021, 4:56 PM · Restricted Project

Apr 7 2021

srhines added a comment to D99996: [Driver] Drop $DEFAULT_TRIPLE-$name as a fallback program name.

Adding Dan as an FYI. While this doesn't impact Android platform or regular NDK users, I suppose that someone could have some esoteric build rules that are relying on this, but they should probably be more explicit about what they're running in those cases anyways.

Apr 7 2021, 10:41 AM · Restricted Project
srhines updated subscribers of D99996: [Driver] Drop $DEFAULT_TRIPLE-$name as a fallback program name.
Apr 7 2021, 10:38 AM · Restricted Project

Mar 19 2021

srhines added a comment to D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt.

With NDK r22, I only see libunwind.a for armv7. Will it be provided for other architectures in future NDK versions?

NDK r23 should have a libunwind.a for all architectures. This change isn't in the r23 beta 2 that was just released, but it is in the current NDK canary build at https://ci.android.com/builds/branches/aosp-master-ndk/grid. (e.g. build 7220482).

Got it, thanks.

Is there a public repository for viewing the libunwind source state that the NDK r23 libunwind is built from? I have the AOSP source tree checked out (following the instructions in https://source.android.com/setup/build/downloading) on the llvm-toolchain manifest branch. toolchain/llvm-project inside that checkout (https://android.googlesource.com/toolchain/llvm-project) has a branch for ndk-release-r23, but the merge base of that branch and upstream LLVM main is from April 29 2020, and I don't see many newer changes to libunwind after that, so I wasn't sure I was looking at the right place. I know you've done a bunch more libunwind work since then (e.g. D87750, D87881, and D90898), so I was curious if those were gonna be part of r23's libunwind.

So libunwind is now produced as a prebuilt with the rest of the toolchain build, and not built from that copy of toolchain/llvm_project from the NDK packaging/release branch. Thus if you want to rebuild/examine the sources, you need to look at the toolchain that was included for NDK r23, which is clang-r399163b (from https://android.googlesource.com/platform/ndk/+/refs/tags/ndk-r23-beta2/ndk/toolchains.py#25). Given that, you can get the manifest for the toolchain directly here. You can use that manifest to check out the exact sources that were used to build everything (following instructions from here).

^ PROCESS is correct, but I picked the wrong version (not beta2). I'll update and post a better reply in a few minutes.

Mar 19 2021, 7:52 PM · Restricted Project
srhines added a comment to D96403: [Android] Use -l:libunwind.a with --rtlib=compiler-rt.

With NDK r22, I only see libunwind.a for armv7. Will it be provided for other architectures in future NDK versions?

NDK r23 should have a libunwind.a for all architectures. This change isn't in the r23 beta 2 that was just released, but it is in the current NDK canary build at https://ci.android.com/builds/branches/aosp-master-ndk/grid. (e.g. build 7220482).

Got it, thanks.

Is there a public repository for viewing the libunwind source state that the NDK r23 libunwind is built from? I have the AOSP source tree checked out (following the instructions in https://source.android.com/setup/build/downloading) on the llvm-toolchain manifest branch. toolchain/llvm-project inside that checkout (https://android.googlesource.com/toolchain/llvm-project) has a branch for ndk-release-r23, but the merge base of that branch and upstream LLVM main is from April 29 2020, and I don't see many newer changes to libunwind after that, so I wasn't sure I was looking at the right place. I know you've done a bunch more libunwind work since then (e.g. D87750, D87881, and D90898), so I was curious if those were gonna be part of r23's libunwind.

Mar 19 2021, 7:08 PM · Restricted Project

Mar 6 2021

srhines added inline comments to D97993: [Driver] Suppress GCC detection under -B.
Mar 6 2021, 1:36 AM · Restricted Project, Restricted Project

Mar 4 2021

srhines updated subscribers of D97993: [Driver] Suppress GCC detection under -B.
Mar 4 2021, 5:42 PM · Restricted Project, Restricted Project

Feb 26 2021

srhines accepted D91841: [builtins] Define fmax and scalbn inline.

+2 for relanding this.

Feb 26 2021, 3:04 PM · Restricted Project

Feb 9 2021

srhines added a comment to D46429: [LLD][ELF][AArch64] Add aarch64_elf64_le_vec emulation.

aarch64_elf64_le_vec is a variable name in BFD (extern const bfd_target aarch64_elf64_le_vec;), instead of a BFD emulation.
@srhines Can this workaround be dropped now?

% aarch64-linux-gnu-ld -m aarch64_elf64_le_vec a.o 
aarch64-linux-gnu-ld: unrecognised emulation mode: aarch64_elf64_le_vec
Supported emulations: aarch64linux aarch64elf aarch64elf32 aarch64elf32b aarch64elfb armelf armelfb aarch64linuxb aarch64linux32 aarch64linux32b armelfb_linux_eabi armelf_linux_eabi
Feb 9 2021, 12:33 AM

Jan 21 2021

srhines accepted D95166: Disable rosegment for old Android versions..
Jan 21 2021, 1:37 PM · Restricted Project

Dec 1 2020

srhines accepted D91664: Add a less ambiguous macro for Android version..
Dec 1 2020, 4:54 PM · Restricted Project

Nov 3 2020

srhines added a comment to D90720: Use LLD for Android compiler-rt.

Would it be better/easier if we just change the default in the Clang driver at this point to use LLD for Android triples? I believe Dan has patches for doing that which we hadn't submitted yet (because we only recently made it the NDK default).

Nov 3 2020, 5:52 PM · Restricted Project
srhines updated subscribers of D90720: Use LLD for Android compiler-rt.
Nov 3 2020, 5:51 PM · Restricted Project

Oct 22 2020

srhines updated subscribers of D89924: [llvm-zorg] Use ndk21 for buildbot.
Oct 22 2020, 5:24 PM

Oct 21 2020

srhines added inline comments to D89615: Disable emulated-tls and use LLD for compiler-rt+tests on Android.
Oct 21 2020, 3:41 PM · Restricted Project
srhines updated subscribers of D89615: Disable emulated-tls and use LLD for compiler-rt+tests on Android.

Why we can't just switch to elf TLS?

It seems to me that ideally the NDK clang should be defaulting to -fno-emulated-tls for android with a new enough target platform version, and should also be defaulting to use lld. I think that's been the plan for a while -- do you know if it's going to actually happen soon?

That might then make some of these changes unnecessary.

Oct 21 2020, 12:07 PM · Restricted Project
srhines updated subscribers of D89570: [Arm][Unwind][libc++abi] Add _Unwind_ForcedUnwind to EHABI..
Oct 21 2020, 9:06 AM · Restricted Project, Restricted Project

Oct 20 2020

srhines added inline comments to D89251: Reland [lsan] Enable LSAN for Android.
Oct 20 2020, 5:03 PM · Restricted Project

Oct 13 2020

srhines accepted D89251: Reland [lsan] Enable LSAN for Android.

I think this is ok, but I'd like @eugenis to confirm that he is fine with it too. Thanks for explaining the complexity across the different API levels.

Oct 13 2020, 10:20 AM · Restricted Project

Oct 12 2020

srhines added inline comments to D89251: Reland [lsan] Enable LSAN for Android.
Oct 12 2020, 12:09 PM · Restricted Project
srhines requested changes to D89251: Reland [lsan] Enable LSAN for Android.
Oct 12 2020, 9:42 AM · Restricted Project

Sep 15 2020

srhines committed rG516a01b5f36d: Implement __isOSVersionAtLeast for Android (authored by srhines).
Implement __isOSVersionAtLeast for Android
Sep 15 2020, 12:56 PM
srhines closed D86596: Implement __isOSVersionAtLeast for Android.
Sep 15 2020, 12:55 PM · Restricted Project

Sep 11 2020

srhines added a comment to D83144: Allow to specify macro names for android-comparison-in-temp-failure-retry..

Thanks for the review! Should I wait for Alex to take a look, or is it fine like this?

Sep 11 2020, 12:31 PM · Restricted Project
srhines accepted D83144: Allow to specify macro names for android-comparison-in-temp-failure-retry..

Thanks for creating the new test, and for making this more flexible. Everything else looks good here.

Sep 11 2020, 11:43 AM · Restricted Project

Sep 10 2020

srhines added inline comments to D83144: Allow to specify macro names for android-comparison-in-temp-failure-retry..
Sep 10 2020, 1:41 PM · Restricted Project

Sep 9 2020

srhines accepted D86596: Implement __isOSVersionAtLeast for Android.

Sorry, Jiyong, I had forgotten to subscribe llvm-commits when I first uploaded this (because it's part of compiler-rt and doesn't seem to have a default rule to add them). In any case, this patch LGTM, and I am going to go ahead and approve it since this only affects Android. Let's give it 24 hours for anyone else to comment before committing though (especially since I messed up with the original upload/review).

Sep 9 2020, 1:57 AM · Restricted Project
srhines removed a reviewer for D86596: Implement __isOSVersionAtLeast for Android: llvm-commits.
Sep 9 2020, 1:53 AM · Restricted Project
srhines added a reviewer for D86596: Implement __isOSVersionAtLeast for Android: llvm-commits.
Sep 9 2020, 1:52 AM · Restricted Project

Sep 3 2020

srhines added inline comments to D87067: [llvm-symbolizer] Add back --use-symbol-table=true.
Sep 3 2020, 3:17 PM · Restricted Project

Aug 26 2020

srhines requested review of D86596: Implement __isOSVersionAtLeast for Android.
Aug 26 2020, 1:31 AM · Restricted Project

Aug 24 2020

srhines accepted D86321: Fix test for D77924..

Yes, let's just submit this now. If it breaks further, then we can consider other options. I'm not sure why there is a separate libc++abi reviewer listed here.

Aug 24 2020, 3:00 PM · Restricted Project

Aug 14 2020

srhines added a comment to D67689: [llvm-objcopy] Add support for --gap-fill and --pad-to options.

@asmith, are you still working on this patch? The gap-fill feature is definitely needed for parity with existing objcopy implementations.

Aug 14 2020, 8:22 AM · Restricted Project

Jul 13 2020

srhines committed rG9d5a8b7edb28: Fix a missing update that C compiles default to gnu17. (authored by srhines).
Fix a missing update that C compiles default to gnu17.
Jul 13 2020, 4:36 PM
srhines closed D83726: Fix a missing update that C compiles default to gnu17..
Jul 13 2020, 4:36 PM · Restricted Project
Herald added a project to D83726: Fix a missing update that C compiles default to gnu17.: Restricted Project.
Jul 13 2020, 3:43 PM · Restricted Project

Jun 15 2020

srhines accepted D81622: [Clang] Search computed sysroot for libc++ header paths.

Thanks, Ryan!

Jun 15 2020, 1:14 PM · Restricted Project

Jun 10 2020

srhines added a comment to D81622: [Clang] Search computed sysroot for libc++ header paths.

Thanks, Ryan, for diagnosing and addressing this bug.

Jun 10 2020, 9:02 PM · Restricted Project

Jun 1 2020

srhines accepted D80828: [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer.

Thanks, Nick, for cleaning this up and always striving to make things more compatible.

Jun 1 2020, 12:25 PM · Restricted Project, Restricted Project
srhines added inline comments to D80828: [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer.
Jun 1 2020, 11:18 AM · Restricted Project, Restricted Project

May 29 2020

srhines added inline comments to D80828: [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer.
May 29 2020, 4:25 PM · Restricted Project, Restricted Project

Apr 30 2020

srhines accepted D78787: [libcxx][libcxxabi][libunwind] Use libgcc on Android.

I'm also satisfied with this from the Android side. Thank you for discussing this with Ryan.

Apr 30 2020, 3:09 PM · Restricted Project, Restricted Project, Restricted Project

Apr 24 2020

srhines accepted D78837: [lld] Remove special cases from default ld driver mode..

LGTM on the Android side. Thanks Dan for handling this.

Apr 24 2020, 5:21 PM · Restricted Project, lld

Apr 22 2020

srhines added a comment to D77330: Consider increasing the default ARM32 max page size to 64k..

Hmm...seems to have regressed ARMv5 Linux kernel boots according to a bisection from @nathanchance (https://groups.google.com/g/clang-built-linux/c/aKDK-N6JN4g)

I don't think that URL is publicly visible.

I don't know why that started happening :/

I think we saw such issue before. That time I think the URL of a page from incognito mode worked (i.e. a different URL to the same page).

Apr 22 2020, 2:08 AM · Restricted Project, lld

Apr 16 2020

srhines added a comment to D77168: Add a flag to debug automatic variable initialization.
In D77168#1988122, @jfb wrote:

pragma clang attribute is interesting, but how do you apply that in a selective fashion to local variables (especially in a way that can be automated)? At first, I didn't think the goal for this should be to create a frequently used option for most end users, but I do think that it could be quite useful for more folks when debugging, especially if it is easy to automate (which optimization-fuel approaches are, while pragmas are not).

__attribute__((uninitialized)) for more selectiveness :)

Of course, not automated. In general we don't really automate compiler things of this sort, say UBSan. OptRemarks is the best we really have to dig into these things. One other option would be to truly randomize the init pattern: emit a handful of different byte patterns, and then see the crash caused by a different pattern, and track it back to which local variable got that byte pattern.

Apr 16 2020, 9:06 PM · Restricted Project
srhines added a comment to D77168: Add a flag to debug automatic variable initialization.

pragma clang attribute is interesting, but how do you apply that in a selective fashion to local variables (especially in a way that can be automated)? At first, I didn't think the goal for this should be to create a frequently used option for most end users, but I do think that it could be quite useful for more folks when debugging, especially if it is easy to automate (which optimization-fuel approaches are, while pragmas are not).

Apr 16 2020, 6:57 PM · Restricted Project

Apr 8 2020

srhines accepted D77746: [Driver] Default arm-linux-androideabi to -z max-page-size=4096.

Thank you for making this clear.

Apr 8 2020, 11:57 AM · Restricted Project
srhines added a comment to D77330: Consider increasing the default ARM32 max page size to 64k..

I agree with Ryan. Please update the Clang driver for this to pass this same flag for 32-bit ARM as we already pass for AArch64.

Apr 8 2020, 1:35 AM · Restricted Project, lld

Apr 3 2020

srhines added a comment to D77330: Consider increasing the default ARM32 max page size to 64k..

I should have looked more closely at the AOSP sources, because we already hard-code the expected 4KB max-page-size. We do have some crufty flags that can be cleaned up, so I'll do that instead (https://android-review.googlesource.com/c/platform/build/soong/+/1278815). :) I'll also go ahead and create/test a CL to revert the Clang driver change forcing 4KB max-page-size next week.

Apr 3 2020, 8:34 PM · Restricted Project, lld
srhines updated subscribers of D77330: Consider increasing the default ARM32 max page size to 64k..

So it turns out that the ld shipped in the NDK actually did have the 64KB max-page-size patch, so not taking this patch would change current user behavior. Thanks to @MaskRay for sharing the exact binutils patch we were looking for to verify that it has it. For that reason, this patch should be fine for our 32-bit ARM NDK users. For the platform, we'll see if we should just set the max-page-size back to 4KB explicitly in the build rules, but that shouldn't affect this CL.

Apr 3 2020, 8:02 PM · Restricted Project, lld
srhines added inline comments to D77283: scudo: Add support for diagnosing memory errors when memory tagging is enabled..
Apr 3 2020, 9:40 AM · Restricted Project
srhines added a comment to D77330: Consider increasing the default ARM32 max page size to 64k..

We (Android Google folks) have an internal thread going on about what implications this might have, but I can say that I am concerned about making this kind of change for ARM Android targets. Even though we manually set a 4KB max page size in our platform builds for 32-bit, we also have to consider the needs of NDK developers (i.e. those folks making native applications/games/etc.). Some of their build configurations might behave very differently with this change, so my current thinking is that 64KB should not be the default for ARM Android targets.

Apr 3 2020, 1:34 AM · Restricted Project, lld

Apr 1 2020

srhines added a comment to D77168: Add a flag to debug automatic variable initialization.
In D77168#1955312, @jfb wrote:

Do you not think pragma is a more general approach? That's what's used in a bunch of other cases, and I'd like to see it attempted here.
If not, I agree with John that just counting up isn't a good bisection experience. I'd rather see a begin / end bound.

Apr 1 2020, 11:31 AM · Restricted Project

Mar 31 2020

srhines added a comment to D77168: Add a flag to debug automatic variable initialization.
In D77168#1953635, @jfb wrote:

I'm not sure this is a good idea at all. We want to keep the codepath as simple as possible to avoid introducing bugs. If a codebase sees a crash then it's easier to bisect one function at a time than doing something like this. I'd much rather see bisection using pragma to apply the uninitialized attribute to multiple declarations.

Mar 31 2020, 6:12 PM · Restricted Project

Mar 30 2020

srhines accepted D77100: [SelectionDAGISel] small cleanup to INLINEASM_BR selection. NFC.
Mar 30 2020, 3:51 PM · Restricted Project

Mar 26 2020

srhines added a comment to D76452: Use LLD by default for Android..

Another approach will be -DCLANG_DEFAULT_LINKER=lld. It provides a default value for -fuse-ld=.

How does that work for Darwin builds? Is lld fully supported for MachO at this point, or will Clang still choose to use the preferred Apple linker?

I am not clear about your use case. lld can be used as ELF/Mach-O/COFF/WebAssembly linker. If you want to build ELF objects, -fuse-ld=lld (via clangDriver) or configure clang with -DCLANG_DEFAULT_LINKER=lld.
If you want to build Mach-O objects, lld's current Mach-O port lacks features (and will be removed). The new Mach-O port is still under development.

Mar 26 2020, 3:14 PM · Restricted Project
srhines added a comment to D76452: Use LLD by default for Android..

Another approach will be -DCLANG_DEFAULT_LINKER=lld. It provides a default value for -fuse-ld=.

Mar 26 2020, 1:02 PM · Restricted Project

Mar 20 2020

srhines accepted D76452: Use LLD by default for Android..

Thanks, Dan, for setting this up.

Mar 20 2020, 1:00 PM · Restricted Project

Feb 27 2020

srhines added inline comments to D75238: [DAGCombine] Fix alias analysis for unaligned accesses.
Feb 27 2020, 11:46 AM · Restricted Project

Feb 14 2020

srhines added inline comments to D74375: [ELF] Support INSERT [AFTER|BEFORE] for orphan sections.
Feb 14 2020, 7:55 AM · Restricted Project

Dec 10 2019

srhines added a reviewer for D70800: Fix AArch64 AAPCS frame record chain: cferris.
Dec 10 2019, 9:23 AM · Restricted Project

Nov 6 2019

srhines added a comment to D69925: remove redundant LLVM version from version string when setting CLANG_VENDOR.

I think the reason it's written this way is that CLANG_VERSION_STRING might not correspond to an LLVM version? For example, Apple has its own versioning scheme.

Nov 6 2019, 6:53 PM · Restricted Project

Oct 25 2019

srhines added inline comments to D69418: [llvm-ar] Add output option for extract operation.
Oct 25 2019, 3:35 PM · Restricted Project, Restricted Project