Commit 8581e363 authored by Khushal's avatar Khushal Committed by Commit Bot

Revert "PR_SET_DUMPABLE 1 briefly if opening pagemap fails"

This reverts commit c63cb6f2.

Reason for revert: crbug.com/1130358

Original change's description:
> PR_SET_DUMPABLE 1 briefly if opening pagemap fails
> 
> This is often necessary on Android to read /proc/self/pagemap. Without
> prctl(PR_SET_DUMPABLE) opening this file fails in 94% of cases on
> Canary. This potentially leads to bias in reporting
> MappedAndResidentMemoryFootprint2.
> 
> Bug: 1070618
> Change-Id: Ic7f965099c4b3ba822baf248eb80d33678996188
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2416377
> Commit-Queue: Egor Pasko <pasko@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Reviewed-by: ssid <ssid@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#808298}

TBR=pasko@chromium.org,ssid@chromium.org,rsesek@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1070618
Change-Id: Icae38145675e215c8ba1b685308d63b0a60b1d28
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2420776Reviewed-by: default avatarKhushal <khushalsagar@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808758}
parent 034005d3
......@@ -5,8 +5,6 @@
#include <dlfcn.h>
#include <fcntl.h>
#include <stdint.h>
#include <sys/prctl.h>
#include <memory>
#include "base/android/library_loader/anchor_functions.h"
......@@ -230,39 +228,6 @@ uint32_t ReadLinuxProcSmapsFile(FILE* smaps_file,
return num_valid_regions;
}
// RAII class making the current process dumpable via prctl(PR_SET_DUMPABLE, 1),
// in case it is not currently dumpable as described in proc(5) and prctl(2).
// Noop if the original dumpable state could not be determined.
class ScopedProcessSetDumpable {
public:
ScopedProcessSetDumpable() {
int result = prctl(PR_GET_DUMPABLE, 0, 0, 0, 0);
if (result < 0) {
PLOG(ERROR) << "prctl";
was_dumpable_ = true; // Avoids prctl on destruction.
return;
}
was_dumpable_ = result > 0;
if (!was_dumpable_) {
result = prctl(PR_SET_DUMPABLE, 1, 0, 0, 0);
PLOG_IF(ERROR, result != 0) << "prctl";
}
}
ScopedProcessSetDumpable(const ScopedProcessSetDumpable&) = delete;
ScopedProcessSetDumpable& operator=(const ScopedProcessSetDumpable&) = delete;
~ScopedProcessSetDumpable() {
if (!was_dumpable_) {
PCHECK(prctl(PR_SET_DUMPABLE, 0, 0, 0, 0));
}
}
private:
bool was_dumpable_;
};
} // namespace
FILE* g_proc_smaps_for_testing = nullptr;
......@@ -363,14 +328,8 @@ OSMetrics::MappedAndResidentPagesDumpState OSMetrics::GetMappedAndResidentPages(
base::ScopedFILE pagemap_file(fopen(kPagemap, "r"));
if (!pagemap_file.get()) {
{
ScopedProcessSetDumpable set_dumpable;
pagemap_file.reset(fopen(kPagemap, "r"));
}
if (!pagemap_file.get()) {
DLOG(WARNING) << "Could not open " << kPagemap;
return OSMetrics::MappedAndResidentPagesDumpState::kAccessPagemapDenied;
}
DLOG(WARNING) << "Could not open " << kPagemap;
return OSMetrics::MappedAndResidentPagesDumpState::kAccessPagemapDenied;
}
const size_t kPageSize = base::GetPageSize();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment