This is an archive of the discontinued LLVM Phabricator instance.

[demangler] Ensure proper alignment in BumpPointerAllocator
ClosedPublic

Authored by sepavloff on Jul 3 2018, 11:50 AM.

Details

Summary

This change slightly modifies algorithm of BumpPointerAllocator so that it
provides alignment of allocated memory blocks in runtime without specific
compiler support. It allows getting rid of the specifier alignas(16) in the
declaration of BumpPointerAllocator::InitialBuffer. Object of the class
BumpPointerAllocator is created as a part of another object allocated in heap,
so this specifier actually cannot guarantee proper alignment. Microsoft
compiler emits a warning in this case.

It fixes https://bugs.llvm.org/show_bug.cgi?id=37944.

Diff Detail

Repository
rL LLVM

Event Timeline

sepavloff created this revision.Jul 3 2018, 11:50 AM

Hi, thanks for working on this!

We don't need 16-byte alignment on MSVC, we only need 8 because alignof(max_align_t) is 8 on that platform, and malloc is required to provide memory that's aligned to that boundary. We can't portable use max_align_t in LLVM (see the commit message in r336162), but I think we can get away with using alignof(long double). Have you considered this option instead of fixing up the alignment dynamically?

We don't need 16-byte alignment on MSVC, we only need 8 because alignof(max_align_t) is 8 on that platform, and malloc is required to provide memory that's aligned to that boundary.

If allignment provided by malloc is sufficient, there is no need in runtime alignment. and alignas(alignof(long double)) should be OK.

sepavloff updated this revision to Diff 154108.Jul 4 2018, 7:18 AM

Updated patch

erik.pilkington accepted this revision.Jul 4 2018, 7:29 AM

LGTM, thanks again! Please also commit this to the version of this file in libcxxabi/src/cxa_demangle.cpp.

lib/Demangle/ItaniumDemangle.cpp
1953 ↗(On Diff #154108)

This can be spelt alignas(long double).

This revision is now accepted and ready to land.Jul 4 2018, 7:29 AM
sepavloff added inline comments.Jul 4 2018, 11:25 PM
lib/Demangle/ItaniumDemangle.cpp
1953 ↗(On Diff #154108)

Thanks!

This revision was automatically updated to reflect the committed changes.