Skip to content

Commit

Permalink
Export of internal Abseil changes
Browse files Browse the repository at this point in the history
--
ac1df60490c9583e475e22de7adfc40023196fbf by Martijn Vels <mvels@google.com>:

Change Cord constructor(string_view) to explicit make_tree and Cordz tracking

This CL changes the ctor to use an easier to maintain model where Cord code explicitly invokes Cordz update or new / tree logic, which avoids the ambiguity of the 'branched' InlineRep::set_tree code. This removes the need to equip InlineRep with 'MethodIdentifier' or other necessary call info, and also is a cleaner model: InlineRep is carrying too much code now that should plainly sit in Cord, especially with all internal abstractions having moved to InlineData.

See child CL(s) for desired state

PiperOrigin-RevId: 369433619

--
b665af7f586e6c679a8b27d4f78d5a1d2b596058 by Abseil Team <absl-team@google.com>:

Rename the 'Compare' template type to 'LessThan', as the passed-in function is expected to act like operator<. It is worth avoiding confusion with std::compare, which returns an int (-1/0/1), as due to implicit casting this can lead to hard-to-spot bugs.

PiperOrigin-RevId: 369391118

--
c3c775269cad0f4982ec63f3616dd78bb9e52dca by Martijn Vels <mvels@google.com>:

Integrate CordzUpdateTracker into CordzInfo

PiperOrigin-RevId: 369348824

--
771d81ed357496c117179e1daec76eba5155932d by Martijn Vels <mvels@google.com>:

Replace mutex() with Lock() / Unlock() function

Mini design future tracking of CordzInfo sampled cords: CordzInfo holds a CordRep* reference without a reference count. Cord is responsible for synchronizing updates for sampled cords such that the CordRep* contained in CordzInfo is at all times valid. This is done by scoping Lock() and Unlock() calls around the code modifying the code of a sampled cord. For example (using the future CL CordzUpdateScope()):

CordzInfo* cordz_info = get_cordz_info();
CordzUpdateScope scope(cordz_info, CordzUpdateTracker::kRemovePrefix);
CordRep* rep = RemovePrefixImpl(root);
set_tree(rep);
if (cordz_info) {
  cordz_info->SetCordRep(rep);
}

On CordzInfo::Unlock(), if the internal rep is null, the cord is no longer sampled, and CordzInfo will be deleted. Thus any update resulting in the Cord being inlined will automatically no longer be sampled.

PiperOrigin-RevId: 369338802

--
5563c12df04a1e965a03b50bdd032739c55c0706 by Martijn Vels <mvels@google.com>:

Add UpdateTracker to CordzStatistics

PiperOrigin-RevId: 369318178

--
6b4d8463722a3e55a3e8f6cb3741a41055e7f83e by Martijn Vels <mvels@google.com>:

Add kClear, kConstructor* and kUnknown values and fix typo

PiperOrigin-RevId: 369297163

--
041adcbc929789d6d53371a8236840fc350e1eeb by Derek Mauro <dmauro@google.com>:

Switch from malloc to operator new in pool_urbg.cc
so it can only fail by throwing/aborting

PiperOrigin-RevId: 369274087

--
5d97a5f43e3f2d02d0a5bbe586d93b5751812981 by Benjamin Barenblat <bbaren@google.com>:

Correct Thumb function bound computation in the symbolizer

On 32-bit ARM, all functions are aligned to multiples of two bytes, and
the lowest-order bit in a function’s address is ignored by the CPU when
computing branch targets. That bit is still present in instructions and
ELF symbol tables, though; it’s repurposed to indicate whether the
function contains ARM or Thumb code. If the symbolizer doesn’t ignore
that bit, it will believe Thumb functions have boundaries that are off
by one byte, so instruct the symbolizer to null out the lowest-order bit
after retrieving it from the symbol table.

PiperOrigin-RevId: 369254082

--
462bb307c6cc332c1e2c3adb5f0cad51804bf937 by Derek Mauro <dmauro@google.com>:

Add a check for malloc failure in pool_urbg.cc
GitHub #940

PiperOrigin-RevId: 369238100
GitOrigin-RevId: ac1df60490c9583e475e22de7adfc40023196fbf
Change-Id: Ic6ec91c62cd3a0031f6a75a43a83da959ece2d25
  • Loading branch information
Abseil Team authored and dinord committed Apr 20, 2021
1 parent 732c654 commit 1ae9b71
Show file tree
Hide file tree
Showing 14 changed files with 382 additions and 155 deletions.
180 changes: 90 additions & 90 deletions absl/algorithm/container.h

Large diffs are not rendered by default.

10 changes: 10 additions & 0 deletions absl/debugging/symbolize_elf.inc
Expand Up @@ -701,6 +701,16 @@ static ABSL_ATTRIBUTE_NOINLINE FindSymbolResult FindSymbol(
const char *start_address =
ComputeOffset(original_start_address, relocation);

#ifdef __arm__
// ARM functions are always aligned to multiples of two bytes; the
// lowest-order bit in start_address is ignored by the CPU and indicates
// whether the function contains ARM (0) or Thumb (1) code. We don't care
// about what encoding is being used; we just want the real start address
// of the function.
start_address = reinterpret_cast<const char *>(
reinterpret_cast<uintptr_t>(start_address) & ~1);
#endif

if (deref_function_descriptor_pointer &&
InSection(original_start_address, opd)) {
// The opd section is mapped into memory. Just dereference
Expand Down
43 changes: 43 additions & 0 deletions absl/debugging/symbolize_test.cc
Expand Up @@ -477,6 +477,46 @@ void ABSL_ATTRIBUTE_NOINLINE TestWithReturnAddress() {
#endif
}

#if defined(__arm__) && ABSL_HAVE_ATTRIBUTE(target)
// Test that we correctly identify bounds of Thumb functions on ARM.
//
// Thumb functions have the lowest-order bit set in their addresses in the ELF
// symbol table. This requires some extra logic to properly compute function
// bounds. To test this logic, nudge a Thumb function right up against an ARM
// function and try to symbolize the ARM function.
//
// A naive implementation will simply use the Thumb function's entry point as
// written in the symbol table and will therefore treat the Thumb function as
// extending one byte further in the instruction stream than it actually does.
// When asked to symbolize the start of the ARM function, it will identify an
// overlap between the Thumb and ARM functions, and it will return the name of
// the Thumb function.
//
// A correct implementation, on the other hand, will null out the lowest-order
// bit in the Thumb function's entry point. It will correctly compute the end of
// the Thumb function, it will find no overlap between the Thumb and ARM
// functions, and it will return the name of the ARM function.

__attribute__((target("thumb"))) int ArmThumbOverlapThumb(int x) {
return x * x * x;
}

__attribute__((target("arm"))) int ArmThumbOverlapArm(int x) {
return x * x * x;
}

void ABSL_ATTRIBUTE_NOINLINE TestArmThumbOverlap() {
#if defined(ABSL_HAVE_ATTRIBUTE_NOINLINE)
const char *symbol = TrySymbolize((void *)&ArmThumbOverlapArm);
ABSL_RAW_CHECK(symbol != nullptr, "TestArmThumbOverlap failed");
ABSL_RAW_CHECK(strcmp("ArmThumbOverlapArm()", symbol) == 0,
"TestArmThumbOverlap failed");
std::cout << "TestArmThumbOverlap passed" << std::endl;
#endif
}

#endif // defined(__arm__) && ABSL_HAVE_ATTRIBUTE(target)

#elif defined(_WIN32)
#if !defined(ABSL_CONSUME_DLL)

Expand Down Expand Up @@ -551,6 +591,9 @@ int main(int argc, char **argv) {
TestWithPCInsideInlineFunction();
TestWithPCInsideNonInlineFunction();
TestWithReturnAddress();
#if defined(__arm__) && ABSL_HAVE_ATTRIBUTE(target)
TestArmThumbOverlap();
#endif
#endif

return RUN_ALL_TESTS();
Expand Down
7 changes: 3 additions & 4 deletions absl/random/internal/pool_urbg.cc
Expand Up @@ -194,11 +194,10 @@ RandenPoolEntry* PoolAlignedAlloc() {
// Not all the platforms that we build for have std::aligned_alloc, however
// since we never free these objects, we can over allocate and munge the
// pointers to the correct alignment.
void* memory = std::malloc(sizeof(RandenPoolEntry) + kAlignment);
auto x = reinterpret_cast<intptr_t>(memory);
intptr_t x = reinterpret_cast<intptr_t>(
new char[sizeof(RandenPoolEntry) + kAlignment]);
auto y = x % kAlignment;
void* aligned =
(y == 0) ? memory : reinterpret_cast<void*>(x + kAlignment - y);
void* aligned = reinterpret_cast<void*>(y == 0 ? x : (x + kAlignment - y));
return new (aligned) RandenPoolEntry();
}

Expand Down
4 changes: 4 additions & 0 deletions absl/strings/BUILD.bazel
Expand Up @@ -368,6 +368,7 @@ cc_library(
":cord_internal",
":cordz_handle",
":cordz_statistics",
":cordz_update_tracker",
"//absl/base:config",
"//absl/base:core_headers",
"//absl/debugging:stacktrace",
Expand Down Expand Up @@ -406,6 +407,7 @@ cc_library(
hdrs = ["internal/cordz_statistics.h"],
copts = ABSL_DEFAULT_COPTS,
deps = [
":cordz_update_tracker",
"//absl/base:config",
],
)
Expand Down Expand Up @@ -450,6 +452,8 @@ cc_test(
":cord_internal",
":cordz_handle",
":cordz_info",
":cordz_statistics",
":cordz_update_tracker",
":strings",
"//absl/base:config",
"//absl/debugging:stacktrace",
Expand Down
4 changes: 4 additions & 0 deletions absl/strings/CMakeLists.txt
Expand Up @@ -641,6 +641,7 @@ absl_cc_library(
DEPS
absl::config
absl::core_headers
absl::cordz_update_tracker
absl::synchronization
)

Expand Down Expand Up @@ -690,6 +691,7 @@ absl_cc_library(
absl::cord_internal
absl::cordz_handle
absl::cordz_statistics
absl::cordz_update_tracker
absl::core_headers
absl::span
absl::stacktrace
Expand All @@ -707,6 +709,8 @@ absl_cc_test(
absl::cord_test_helpers
absl::cordz_handle
absl::cordz_info
absl::cordz_statistics
absl::cordz_update_tracker
absl::span
absl::stacktrace
absl::symbolize
Expand Down
6 changes: 5 additions & 1 deletion absl/strings/cord.cc
Expand Up @@ -39,6 +39,7 @@
#include "absl/strings/internal/cord_rep_flat.h"
#include "absl/strings/internal/cord_rep_ring.h"
#include "absl/strings/internal/cordz_statistics.h"
#include "absl/strings/internal/cordz_update_tracker.h"
#include "absl/strings/internal/resize_uninitialized.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/str_format.h"
Expand Down Expand Up @@ -544,7 +545,10 @@ Cord::Cord(absl::string_view src) {
if (n <= InlineRep::kMaxInline) {
contents_.set_data(src.data(), n, false);
} else {
contents_.set_tree(NewTree(src.data(), n, 0));
contents_.data_.make_tree(NewTree(src.data(), n, 0));
if (ABSL_PREDICT_FALSE(absl::cord_internal::cordz_should_profile())) {
contents_.StartProfiling();
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions absl/strings/cord.h
Expand Up @@ -84,6 +84,7 @@
#include "absl/strings/internal/cordz_functions.h"
#include "absl/strings/internal/cordz_info.h"
#include "absl/strings/internal/cordz_statistics.h"
#include "absl/strings/internal/cordz_update_tracker.h"
#include "absl/strings/internal/resize_uninitialized.h"
#include "absl/strings/internal/string_constant.h"
#include "absl/strings/string_view.h"
Expand Down Expand Up @@ -668,6 +669,8 @@ class Cord {
explicit constexpr Cord(strings_internal::StringConstant<T>);

private:
using CordzUpdateTracker = cord_internal::CordzUpdateTracker;

friend class CordTestPeer;
friend bool operator==(const Cord& lhs, const Cord& rhs);
friend bool operator==(const Cord& lhs, absl::string_view rhs);
Expand Down
78 changes: 65 additions & 13 deletions absl/strings/internal/cordz_info.cc
Expand Up @@ -19,6 +19,7 @@
#include "absl/strings/internal/cord_internal.h"
#include "absl/strings/internal/cordz_handle.h"
#include "absl/strings/internal/cordz_statistics.h"
#include "absl/strings/internal/cordz_update_tracker.h"
#include "absl/synchronization/mutex.h"
#include "absl/types/span.h"

Expand All @@ -44,18 +45,15 @@ CordzInfo* CordzInfo::Next(const CordzSnapshot& snapshot) const {
return ci_next_unsafe();
}

CordzInfo* CordzInfo::TrackCord(CordRep* rep, const CordzInfo* src) {
CordzInfo* ci = new CordzInfo(rep);
if (src) {
ci->parent_stack_depth_ = src->stack_depth_;
memcpy(ci->parent_stack_, src->stack_, sizeof(void*) * src->stack_depth_);
}
CordzInfo* CordzInfo::TrackCord(CordRep* rep, const CordzInfo* src,
MethodIdentifier method) {
CordzInfo* ci = new CordzInfo(rep, src, method);
ci->Track();
return ci;
}

CordzInfo* CordzInfo::TrackCord(CordRep* rep) {
return TrackCord(rep, nullptr);
CordzInfo* CordzInfo::TrackCord(CordRep* rep, MethodIdentifier method) {
return TrackCord(rep, nullptr, method);
}

void CordzInfo::UntrackCord(CordzInfo* cordz_info) {
Expand All @@ -66,12 +64,35 @@ void CordzInfo::UntrackCord(CordzInfo* cordz_info) {
}
}

CordzInfo::CordzInfo(CordRep* rep)
CordzInfo::MethodIdentifier CordzInfo::GetParentMethod(const CordzInfo* src) {
if (src == nullptr) return MethodIdentifier::kUnknown;
return src->parent_method_ != MethodIdentifier::kUnknown ? src->parent_method_
: src->method_;
}

int CordzInfo::FillParentStack(const CordzInfo* src, void** stack) {
assert(stack);
if (src == nullptr) return 0;
if (src->parent_stack_depth_) {
memcpy(stack, src->parent_stack_, src->parent_stack_depth_ * sizeof(void*));
return src->parent_stack_depth_;
}
memcpy(stack, src->stack_, src->stack_depth_ * sizeof(void*));
return src->stack_depth_;
}

CordzInfo::CordzInfo(CordRep* rep, const CordzInfo* src,
MethodIdentifier method)
: rep_(rep),
stack_depth_(absl::GetStackTrace(stack_, /*max_depth=*/kMaxStackDepth,
/*skip_count=*/1)),
parent_stack_depth_(0),
create_time_(absl::Now()) {}
parent_stack_depth_(FillParentStack(src, parent_stack_)),
method_(method),
parent_method_(GetParentMethod(src)),
create_time_(absl::Now()),
size_(rep->length) {
update_tracker_.LossyAdd(method);
}

CordzInfo::~CordzInfo() {
// `rep_` is potentially kept alive if CordzInfo is included
Expand All @@ -96,7 +117,7 @@ void CordzInfo::Untrack() {
{
// TODO(b/117940323): change this to assuming ownership instead once all
// Cord logic is properly keeping `rep_` in sync with the Cord root rep.
absl::MutexLock lock(&mutex());
absl::MutexLock lock(&mutex_);
rep_ = nullptr;
}

Expand All @@ -120,9 +141,31 @@ void CordzInfo::Untrack() {
}
}

void CordzInfo::Lock(MethodIdentifier method)
ABSL_EXCLUSIVE_LOCK_FUNCTION(mutex_) {
mutex_.Lock();
update_tracker_.LossyAdd(method);
assert(rep_);
}

void CordzInfo::Unlock() ABSL_UNLOCK_FUNCTION(mutex_) {
bool tracked = rep_ != nullptr;
if (rep_) {
size_.store(rep_->length);
}
mutex_.Unlock();
if (!tracked) {
Untrack();
CordzHandle::Delete(this);
}
}

void CordzInfo::SetCordRep(CordRep* rep) {
mutex().AssertHeld();
mutex_.AssertHeld();
rep_ = rep;
if (rep) {
size_.store(rep->length);
}
}

absl::Span<void* const> CordzInfo::GetStack() const {
Expand All @@ -133,6 +176,15 @@ absl::Span<void* const> CordzInfo::GetParentStack() const {
return absl::MakeConstSpan(parent_stack_, parent_stack_depth_);
}

CordzStatistics CordzInfo::GetCordzStatistics() const {
CordzStatistics stats;
stats.method = method_;
stats.parent_method = parent_method_;
stats.update_tracker = update_tracker_;
stats.size = size_.load(std::memory_order_relaxed);
return stats;
}

} // namespace cord_internal
ABSL_NAMESPACE_END
} // namespace absl

0 comments on commit 1ae9b71

Please sign in to comment.