In D152405#4645719, @dyung wrote:

Hi @leonardchan your change was still causing a failure on the PS4 linux bot, so I reverted it in 070493ddbd9473499d6f00ca62bc6aa92808ed79. I noticed earlier that the failing test omit-rtti-component-without-no-rtti.cpp was failing on both the linux and Windows PS bots, and I don't think it is because it was on Windows, but rather, these bots use PS4 and PS5 as the default targets, on which rtti is not enabled by default. The run command on line 4 in your test tests the "default" behavior which on most platforms seems to have rtti enabled by default. If I remove that line, the test then passes on both of the PS targets. To reland the change, you might want to consider removing that line as it makes an assumption which does not seem to be true for all targets.

In D147832#4645512, @ldionne wrote:

After careful consideration, I am left wondering why -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -fvisibility=hidden even makes sense. I understand that you want to minimize the number of symbols exported from your program and I think that's a desirable goal. However, let's say you take a typical program and just compile it normally without using -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -fvisibility=hidden -- what's the problem? What symbols end up being exported that you wouldn't want to export?

We already mark the vast majority of symbols in libc++ as hidden (using _LIBCPP_HIDE_FROM_ABI), and the ones that are not marked as hidden basically need to be public for correctness purposes.

In D152405#4404742, @mcgrathr wrote:

In D152405#4404616, @leonardchan wrote:

Oh this is completely independent from relative vtables. I'll update the wording.

Great. I'd like to see us try some experiments with enabling both together in places like the Fuchsia kernel where we get benefit from relative-vtables today but it's all a single fungible internal ABI domain and there's no problem having a one-off ABI. It's also interesting of course for true embedded contexts, but those tend to be ILP32 where relative-vtables isn't a size savings anyway. In the Fuchsia kernel we should get some size savings from this and it would be nice to see what that is (probably not all that big, but interesting).

In D147832#4644272, @ldionne wrote:

In D147832#4256529, @leonardchan wrote:

You say you build with -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -fvisibility=hidden which AFAICT defeats the purpose of a shared library, since it wouldn't export a single symbol.

My bad, I should've clarified on this. For fuchsia, we build everything with hidden visibility by default and individual binaries need to explicitly mark their symbols with default visibility if they want to be exported. This also means that when a fuchsia binary uses libc++, we need to make sure it doesn't accidentally export any libc++ symbols. Fuchsia binaries can consume libc++ as either a shared lib or a hermetic static lib, but we don't know at compile time how libc++ will eventually be consumed. If used as a shared lib, then symbols will be defined libc++.so. If linked in as a hermetic static lib, then we need to ensure those symbols have hidden visibility. To cover both cases and prevent us from having to compile twice, we just compile everything with -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -fvisibility=hidden.

I still don't understand your setup. So you compile the program using LTO and passing -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -fvisibility=hidden, right? That way the program itself doesn't export any libc++ symbols and everything in the program has hidden visibility AND hidden LTO visibility. But then you compile libc++.so with -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -fvisibility=hidden, which causes everything in libc++.so to have hidden visibility. But since you compile libc++.so with LTO disabled, everything in libc++.so has default LTO visibility.

This was landed in various other smaller changes

ping for thoughts on this

ping

In D158767#4615015, @Chia-hungDuan wrote:

Sorry I'm not familiar with the 39 bit vma. Could you share more context behind this? Is this only causing issue on Fuchsia RISCV?

In D158550#4608476, @MaskRay wrote:

This can happen if the region size happens to be relatively small and we allocate a bunch of objects that fit into the largest size class.

Can you elaborate? RISC-V?

This was landed in fe195781ad2f1aaecd5def14f361f3590a17c79e

I think perhaps you can re-upload your diff to the original one.

Sorry I accidentally closed the revision. I think just re-opening it should be fine then when you commit it, you can still use this differential revision.

In rGfe195781ad2f1aaecd5def14f361f3590a17c79e#1235569, @dhoekwater wrote:

Looks like this lists the wrong differential revision.

You listed:
https://reviews.llvm.org/D157127

You probably meant:
https://reviews.llvm.org/D157128

This closes my active revision. Please fix!

It looks like this change causes an out of range exception to be thrown for this snippet:

Prior to this, it looks like we'd unconditionally mark pointers in the allocator cache as reachable. So we may find more leaks after this.

LGTM but probably give it a day before landing to give others time to respond

Hi. I think this is leading to the lldb test failures we're seeing on our linux builder (https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8778187770933863505/+/u/lldb/test/stdout?format=raw):

Woops. Forgot to add the differential revision: bit to the commit, but this was submitted in 17623e96b5b84c0868fe6b4a1456e3d7da275c95.

Looks like this is still causing the test to fail still (https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8778209096851671889/+/u/clang/test/stdout?format=raw):

Hi. I think this caused the override-layout.cpp test to fail on our windows builder (https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8778279637538184401/+/u/clang/test/stdout?format=raw):

Hi. This fails for me locally with:

Hi. It looks like this is failing to build for us on windows host https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8778294144728066337/+/u/clang/build/stdout?format=raw with:

Hi. Looks like the static-archive-collision.test test is failing on our builders at https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8778475674577642785/overview. Would you mind sending out a fix or revert?

Ok here's some of my findings. So there's a step in the attributor where it replaces some instructions with unreachable. One step is:

https://reviews.llvm.org/D152322 for qsort_r

Can we extend the test cases to exercise both with and without relative-vtables also enabled?