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)

ARM
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: jseward, Assigned: jseward)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Depends on: 802240
Attached patch Patch (obsolete) (deleted) — Splinter Review
Attachment #736765 - Flags: review?(mh+mozilla)
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-
Attached patch Patch v2 (deleted) — Splinter Review
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 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+
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.

Attachment

General

Created:
Updated:
Size: