Closed
Bug 861141
Opened 12 years ago
Closed 12 years ago
Connect Breakpad on Android to faulty.lib's mmap interface
Categories
(Core :: Gecko Profiler, defect)
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: jseward, Assigned: jseward)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
Bug 802240 gives faulty.lib the ability to mmap a file in an APK, for
arbitrary reading. Use this to allow Breakpad to read unwind info out
of such files.
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #736765 -
Flags: review?(mh+mozilla)
Comment 2•12 years ago
|
||
Comment on attachment 736765 [details] [diff] [review]
Patch
Review of attachment 736765 [details] [diff] [review]:
-----------------------------------------------------------------
::: toolkit/crashreporter/google-breakpad/src/common/linux/dump_symbols.h
@@ +70,5 @@
> +bool ReadSymbolDataInternal(const uint8_t* obj_file,
> + const string& obj_filename,
> + const std::vector<string>& debug_dirs,
> + SymbolData symbol_data,
> + Module** module);
You need to add the patch to toolkit/crashreporter/breakpad-patches/
::: tools/profiler/local_debug_info_symbolizer.cc
@@ +55,5 @@
> +// Find out where the installation's lib directory is, since we'll
> +// have to look in there to get hold of libmozglue.so.
> +static const char* get_installation_lib_dir ( void )
> +{
> + nsCOMPtr<nsIProperties>
trailing whitespace.
@@ +65,5 @@
> + if (NS_FAILED(rv)) return NULL;
> + nsCString path;
> + rv = greDir->GetNativePath(path);
> + if (NS_FAILED(rv)) return NULL;
> + return path.get();
That will lead to a use-after-free.
@@ +86,5 @@
> +// "/system/lib/" to the path. So as in (1), we can give it
> +// as-is to faulty.lib.
> +//
> +// Hence only (2) requires special-casing here.
> +//
Trailing whitespace.
@@ +107,5 @@
> + // faulty.lib to map it, then call ReadSymbolDataInternal, then
> + // unmap and dlclose it.
> + void* hdl = dlopen(obj_file_to_use.c_str(), RTLD_GLOBAL | RTLD_LAZY);
> + if (!hdl) {
> + BPLOG(INFO) << "ReadSymbolData_APK: Failed to get handle for ELF file \'"
Trailing whitespace.
@@ +115,5 @@
> +
> + size_t sz = __dl_get_mappable_length(hdl);
> + if (sz == 0) {
> + dlclose(hdl);
> + BPLOG(INFO) << "ReadSymbolData_APK: Unable to get size for ELF file \'"
Trailing whitespace.
@@ +123,5 @@
> +
> + void* image = __dl_mmap(hdl, NULL, sz, 0);
> + if (image == MAP_FAILED) {
> + dlclose(hdl);
> + BPLOG(INFO) << "ReadSymbolData_APK: Failed to mmap ELF file \'"
Trailing whitespace.
@@ +130,5 @@
> + }
> +
> + bool ok = ReadSymbolDataInternal((const uint8_t*)image,
> + obj_file_to_use, debug_dirs,
> + symbol_data, module);
Trailing whitespace.
@@ +147,5 @@
> + const std::vector<string>& debug_dirs,
> + SymbolData symbol_data,
> + Module** module)
> +{
> + return ReadSymbolData(obj_filename, debug_dirs, symbol_data, module);
Why not use ReadSymbolData directly below?
Attachment #736765 -
Flags: review?(mh+mozilla) → review-
Assignee | ||
Comment 3•12 years ago
|
||
Addresses all points in comment 2, and adds a build fix for B2G.
https://tbpl.mozilla.org/?tree=Try&rev=6c5a28a10df3
Attachment #736765 -
Attachment is obsolete: true
Attachment #737307 -
Flags: review?(mh+mozilla)
Comment 4•12 years ago
|
||
Comment on attachment 737307 [details] [diff] [review]
Patch v2
Review of attachment 737307 [details] [diff] [review]:
-----------------------------------------------------------------
::: tools/profiler/local_debug_info_symbolizer.cc
@@ +52,5 @@
>
> +#if defined(SPS_OS_android) && !defined(MOZ_WIDGET_GONK)
> +
> +// Find out where the installation's lib directory is, since we'll
> +// have to look in there to get hold of libmozglue.so. Returned
trailing whitespace
@@ +54,5 @@
> +
> +// Find out where the installation's lib directory is, since we'll
> +// have to look in there to get hold of libmozglue.so. Returned
> +// C string is heap allocated and the caller must deallocate it.
> +static const char* get_installation_lib_dir ( void )
static char*
@@ +100,5 @@
> + if (obj_file_to_use == "libmozglue.so") {
> + const char* libdir = get_installation_lib_dir();
> + if (libdir) {
> + obj_file_to_use = string(libdir) + "/lib/" + obj_file_to_use;
> + free((void*)libdir);
no need for a cast here if you change get_installation_lib_dir.
@@ +169,5 @@
> + ok = ReadSymbolData_ANDROID(module->code_file(), debug_dirs_,
> + ONLY_CFI, &debug_info_module);
> +# else
> + ok = ReadSymbolData(module->code_file(), debug_dirs_,
> + ONLY_CFI, &debug_info_module);
trailing whitespace
Attachment #737307 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 5•12 years ago
|
||
Comment 6•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in
before you can comment on or make changes to this bug.
Description
•