Merge "Be clearer about linker warnings."

This commit is contained in:
Elliott Hughes 2018-03-01 00:26:11 +00:00 committed by Gerrit Code Review
commit 8edbb3eb45
9 changed files with 105 additions and 66 deletions

View file

@ -1105,10 +1105,14 @@ static int open_library(android_namespace_t* ns,
const char* fix_dt_needed(const char* dt_needed, const char* sopath __unused) {
#if !defined(__LP64__)
// Work around incorrect DT_NEEDED entries for old apps: http://b/21364029
if (get_application_target_sdk_version() < __ANDROID_API_M__) {
int app_target_api_level = get_application_target_sdk_version();
if (app_target_api_level < __ANDROID_API_M__) {
const char* bname = basename(dt_needed);
if (bname != dt_needed) {
DL_WARN("library \"%s\" has invalid DT_NEEDED entry \"%s\"", sopath, dt_needed);
DL_WARN_documented_change(__ANDROID_API_M__,
"invalid-dt_needed-entries-enforced-for-api-level-23",
"library \"%s\" has invalid DT_NEEDED entry \"%s\"",
sopath, dt_needed, app_target_api_level);
add_dlwarning(sopath, "invalid DT_NEEDED entry", dt_needed);
}
@ -1246,10 +1250,11 @@ static bool load_library(android_namespace_t* ns,
const soinfo* needed_or_dlopened_by = task->get_needed_by();
const char* sopath = needed_or_dlopened_by == nullptr ? "(unknown)" :
needed_or_dlopened_by->get_realpath();
DL_WARN("library \"%s\" (\"%s\") needed or dlopened by \"%s\" is not accessible for the namespace \"%s\""
" - the access is temporarily granted as a workaround for http://b/26394120, note that the access"
" will be removed in future releases of Android.",
name, realpath.c_str(), sopath, ns->get_name());
DL_WARN_documented_change(__ANDROID_API_N__,
"private-api-enforced-for-api-level-24",
"library \"%s\" (\"%s\") needed or dlopened by \"%s\" "
"is not accessible by namespace \"%s\"",
name, realpath.c_str(), sopath, ns->get_name());
add_dlwarning(sopath, "unauthorized access to", name);
}
} else {
@ -3363,7 +3368,9 @@ bool soinfo::prelink_image() {
set_dt_flags_1(d->d_un.d_val);
if ((d->d_un.d_val & ~SUPPORTED_DT_FLAGS_1) != 0) {
DL_WARN("\"%s\" has unsupported flags DT_FLAGS_1=%p", get_realpath(), reinterpret_cast<void*>(d->d_un.d_val));
DL_WARN("Warning: \"%s\" has unsupported flags DT_FLAGS_1=%p "
"(ignoring unsupported flags)",
get_realpath(), reinterpret_cast<void*>(d->d_un.d_val));
}
break;
#if defined(__mips__)
@ -3442,7 +3449,7 @@ bool soinfo::prelink_image() {
} else {
tag_name = "unknown";
}
DL_WARN("\"%s\" unused DT entry: %s (type %p arg %p)",
DL_WARN("Warning: \"%s\" unused DT entry: %s (type %p arg %p) (ignoring)",
get_realpath(),
tag_name,
reinterpret_cast<void*>(d->d_tag),
@ -3495,16 +3502,20 @@ bool soinfo::prelink_image() {
// Before M release linker was using basename in place of soname.
// In the case when dt_soname is absent some apps stop working
// because they can't find dt_needed library by soname.
// This workaround should keep them working. (applies only
// for apps targeting sdk version < M). Make an exception for
// the main executable and linker; they do not need to have dt_soname
// This workaround should keep them working. (Applies only
// for apps targeting sdk version < M.) Make an exception for
// the main executable and linker; they do not need to have dt_soname.
// TODO: >= O the linker doesn't need this workaround.
if (soname_ == nullptr &&
this != solist_get_somain() &&
(flags_ & FLAG_LINKER) == 0 &&
get_application_target_sdk_version() < __ANDROID_API_M__) {
soname_ = basename(realpath_.c_str());
DL_WARN("%s: is missing DT_SONAME will use basename as a replacement: \"%s\"",
get_realpath(), soname_);
DL_WARN_documented_change(__ANDROID_API_M__,
"missing-soname-enforced-for-api-level-23",
"\"%s\" has no DT_SONAME (will use %s instead)",
get_realpath(), soname_);
// Don't call add_dlwarning because a missing DT_SONAME isn't important enough to show in the UI
}
return true;
@ -3535,7 +3546,8 @@ bool soinfo::link_image(const soinfo_list_t& global_group, const soinfo_list_t&
#if !defined(__LP64__)
if (has_text_relocations) {
// Fail if app is targeting M or above.
if (get_application_target_sdk_version() >= __ANDROID_API_M__) {
int app_target_api_level = get_application_target_sdk_version();
if (app_target_api_level >= __ANDROID_API_M__) {
DL_ERR_AND_LOG("\"%s\" has text relocations (https://android.googlesource.com/platform/"
"bionic/+/master/android-changes-for-ndk-developers.md#Text-Relocations-"
"Enforced-for-API-level-23)", get_realpath());
@ -3543,13 +3555,13 @@ bool soinfo::link_image(const soinfo_list_t& global_group, const soinfo_list_t&
}
// Make segments writable to allow text relocations to work properly. We will later call
// phdr_table_protect_segments() after all of them are applied.
DL_WARN("\"%s\" has text relocations (https://android.googlesource.com/platform/"
"bionic/+/master/android-changes-for-ndk-developers.md#Text-Relocations-Enforced-"
"for-API-level-23)", get_realpath());
DL_WARN_documented_change(__ANDROID_API_M__,
"Text-Relocations-Enforced-for-API-level-23",
"\"%s\" has text relocations",
get_realpath());
add_dlwarning(get_realpath(), "text relocations");
if (phdr_table_unprotect_segments(phdr, phnum, load_bias) < 0) {
DL_ERR("can't unprotect loadable segments for \"%s\": %s",
get_realpath(), strerror(errno));
DL_ERR("can't unprotect loadable segments for \"%s\": %s", get_realpath(), strerror(errno));
return false;
}
}
@ -3739,7 +3751,7 @@ std::vector<android_namespace_t*> init_default_namespaces(const char* executable
&config,
&error_msg)) {
if (!error_msg.empty()) {
DL_WARN("error reading config file \"%s\" for \"%s\" (will use default configuration): %s",
DL_WARN("Warning: couldn't read \"%s\" for \"%s\" (using default configuration instead): %s",
config_file,
executable_path,
error_msg.c_str());

View file

@ -194,14 +194,14 @@ static bool parse_config_file(const char* ld_config_file_path,
std::string section_name;
while(true) {
while (true) {
std::string name;
std::string value;
std::string error;
int result = cp.next_token(&name, &value, &error);
if (result == ConfigParser::kError) {
DL_WARN("error parsing %s:%zd: %s (ignoring this line)",
DL_WARN("%s:%zd: warning: couldn't parse %s (ignoring this line)",
ld_config_file_path,
cp.lineno(),
error.c_str());
@ -214,7 +214,7 @@ static bool parse_config_file(const char* ld_config_file_path,
if (result == ConfigParser::kPropertyAssign) {
if (!android::base::StartsWith(name, "dir.")) {
DL_WARN("error parsing %s:%zd: unexpected property name \"%s\", "
DL_WARN("%s:%zd: warning: unexpected property name \"%s\", "
"expected format dir.<section_name> (ignoring this line)",
ld_config_file_path,
cp.lineno(),
@ -228,7 +228,7 @@ static bool parse_config_file(const char* ld_config_file_path,
}
if (value.empty()) {
DL_WARN("error parsing %s:%zd: property value is empty (ignoring this line)",
DL_WARN("%s:%zd: warning: property value is empty (ignoring this line)",
ld_config_file_path,
cp.lineno());
continue;
@ -275,7 +275,7 @@ static bool parse_config_file(const char* ld_config_file_path,
if (result == ConfigParser::kPropertyAssign) {
if (properties->find(name) != properties->end()) {
DL_WARN("%s:%zd: warning: property \"%s\" redefinition",
DL_WARN("%s:%zd: warning: redefining property \"%s\" (overriding previous value)",
ld_config_file_path,
cp.lineno(),
name.c_str());
@ -284,7 +284,7 @@ static bool parse_config_file(const char* ld_config_file_path,
(*properties)[name] = PropertyValue(std::move(value), cp.lineno());
} else if (result == ConfigParser::kPropertyAppend) {
if (properties->find(name) == properties->end()) {
DL_WARN("%s:%zd: warning: appending to property \"%s\" which isn't defined",
DL_WARN("%s:%zd: warning: appending to undefined property \"%s\" (treating as assignment)",
ld_config_file_path,
cp.lineno(),
name.c_str());
@ -299,7 +299,7 @@ static bool parse_config_file(const char* ld_config_file_path,
value = ":" + value;
(*properties)[name].append_value(std::move(value));
} else {
DL_WARN("%s:%zd: warning: += isn't allowed to property \"%s\". Ignoring.",
DL_WARN("%s:%zd: warning: += isn't allowed for property \"%s\" (ignoring)",
ld_config_file_path,
cp.lineno(),
name.c_str());
@ -308,7 +308,7 @@ static bool parse_config_file(const char* ld_config_file_path,
}
if (result == ConfigParser::kError) {
DL_WARN("error parsing %s:%zd: %s (ignoring this line)",
DL_WARN("%s:%zd: warning: couldn't parse %s (ignoring this line)",
ld_config_file_path,
cp.lineno(),
error.c_str());

View file

@ -26,10 +26,12 @@
* SUCH DAMAGE.
*/
#include "linker.h"
#include "linker_globals.h"
#include "linker_namespaces.h"
#include "android-base/stringprintf.h"
int g_argc = 0;
char** g_argv = nullptr;
char** g_envp = nullptr;
@ -48,3 +50,18 @@ size_t linker_get_error_buffer_size() {
return sizeof(__linker_dl_err_buf);
}
void DL_WARN_documented_change(int api_level, const char* doc_link, const char* fmt, ...) {
std::string result{"Warning: "};
va_list ap;
va_start(ap, fmt);
android::base::StringAppendV(&result, fmt, ap);
va_end(ap);
android::base::StringAppendF(&result,
" and will not work when the app moves to API level %d or later "
"(https://android.googlesource.com/platform/bionic/+/master/%s) "
"(allowing for now because this app's target API level is still %d)",
api_level, doc_link, get_application_target_sdk_version());
DL_WARN("%s", result.c_str());
}

View file

@ -49,6 +49,8 @@
async_safe_format_fd(2, "\n"); \
} while (false)
void DL_WARN_documented_change(int api_level, const char* doc_link, const char* fmt, ...);
#define DL_ERR_AND_LOG(fmt, x...) \
do { \
DL_ERR(fmt, x); \

View file

@ -64,7 +64,8 @@ bool android_namespace_t::is_accessible(soinfo* s) {
// This is workaround for apps hacking into soinfo list.
// and inserting their own entries into it. (http://b/37191433)
if (!si->has_min_version(3)) {
DL_WARN("invalid soinfo version for \"%s\"", si->get_soname());
DL_WARN("Warning: invalid soinfo version for \"%s\" (assuming inaccessible)",
si->get_soname());
return false;
}

View file

@ -268,8 +268,10 @@ bool ElfReader::VerifyElfHeader() {
name_.c_str(), header_.e_shentsize, sizeof(ElfW(Shdr)));
return false;
}
DL_WARN("\"%s\" has unsupported e_shentsize: 0x%x (expected 0x%zx)",
name_.c_str(), header_.e_shentsize, sizeof(ElfW(Shdr)));
DL_WARN_documented_change(__ANDROID_API_O__,
"invalid-elf-header_section-headers-enforced-for-api-level-26",
"\"%s\" has unsupported e_shentsize 0x%x (expected 0x%zx)",
name_.c_str(), header_.e_shentsize, sizeof(ElfW(Shdr)));
add_dlwarning(name_.c_str(), "has invalid ELF header");
}
@ -280,7 +282,9 @@ bool ElfReader::VerifyElfHeader() {
return false;
}
DL_WARN("\"%s\" has invalid e_shstrndx", name_.c_str());
DL_WARN_documented_change(__ANDROID_API_O__,
"invalid-elf-header_section-headers-enforced-for-api-level-26",
"\"%s\" has invalid e_shstrndx", name_.c_str());
add_dlwarning(name_.c_str(), "has invalid ELF header");
}
@ -395,11 +399,13 @@ bool ElfReader::ReadDynamicSection() {
pt_dynamic_offset);
return false;
}
DL_WARN("\"%s\" .dynamic section has invalid offset: 0x%zx, "
"expected to match PT_DYNAMIC offset: 0x%zx",
name_.c_str(),
static_cast<size_t>(dynamic_shdr->sh_offset),
pt_dynamic_offset);
DL_WARN_documented_change(__ANDROID_API_O__,
"invalid-elf-header_section-headers-enforced-for-api-level-26",
"\"%s\" .dynamic section has invalid offset: 0x%zx "
"(expected to match PT_DYNAMIC offset 0x%zx)",
name_.c_str(),
static_cast<size_t>(dynamic_shdr->sh_offset),
pt_dynamic_offset);
add_dlwarning(name_.c_str(), "invalid .dynamic section");
}
@ -412,11 +418,13 @@ bool ElfReader::ReadDynamicSection() {
pt_dynamic_filesz);
return false;
}
DL_WARN("\"%s\" .dynamic section has invalid size: 0x%zx, "
"expected to match PT_DYNAMIC filesz: 0x%zx",
name_.c_str(),
static_cast<size_t>(dynamic_shdr->sh_size),
pt_dynamic_filesz);
DL_WARN_documented_change(__ANDROID_API_O__,
"invalid-elf-header_section-headers-enforced-for-api-level-26",
"\"%s\" .dynamic section has invalid size: 0x%zx "
"(expected to match PT_DYNAMIC filesz 0x%zx)",
name_.c_str(),
static_cast<size_t>(dynamic_shdr->sh_size),
pt_dynamic_filesz);
add_dlwarning(name_.c_str(), "invalid .dynamic section");
}
@ -651,10 +659,13 @@ bool ElfReader::LoadSegments() {
if ((prot & (PROT_EXEC | PROT_WRITE)) == (PROT_EXEC | PROT_WRITE)) {
// W + E PT_LOAD segments are not allowed in O.
if (get_application_target_sdk_version() >= __ANDROID_API_O__) {
DL_ERR_AND_LOG("\"%s\": W + E load segments are not allowed", name_.c_str());
DL_ERR_AND_LOG("\"%s\": W+E load segments are not allowed", name_.c_str());
return false;
}
DL_WARN("\"%s\": W + E load segments are not allowed", name_.c_str());
DL_WARN_documented_change(__ANDROID_API_O__,
"writable-and-executable-segments-enforced-for-api-level-26",
"\"%s\" has load segments that are both writable and executable",
name_.c_str());
add_dlwarning(name_.c_str(), "W+E load segments");
}

View file

@ -152,7 +152,7 @@ static bool is_symbol_global_and_defined(const soinfo* si, const ElfW(Sym)* s) {
ELF_ST_BIND(s->st_info) == STB_WEAK) {
return s->st_shndx != SHN_UNDEF;
} else if (ELF_ST_BIND(s->st_info) != STB_LOCAL) {
DL_WARN("unexpected ST_BIND value: %d for \"%s\" in \"%s\"",
DL_WARN("Warning: unexpected ST_BIND value: %d for \"%s\" in \"%s\" (ignoring)",
ELF_ST_BIND(s->st_info), si->get_string(s->st_name), si->get_realpath());
}

View file

@ -209,31 +209,28 @@ void resolve_paths(std::vector<std::string>& paths,
const char* original_path = path.c_str();
if (realpath(original_path, resolved_path) != nullptr) {
struct stat s;
if (stat(resolved_path, &s) == 0) {
if (S_ISDIR(s.st_mode)) {
resolved_paths->push_back(resolved_path);
} else {
DL_WARN("Warning: \"%s\" is not a directory (excluding from path)", resolved_path);
continue;
}
} else {
DL_WARN("Warning: cannot stat file \"%s\": %s", resolved_path, strerror(errno));
if (stat(resolved_path, &s) == -1) {
DL_WARN("Warning: cannot stat file \"%s\": %s (ignoring)", resolved_path, strerror(errno));
continue;
}
if (!S_ISDIR(s.st_mode)) {
DL_WARN("Warning: \"%s\" is not a directory (ignoring)", resolved_path);
continue;
}
resolved_paths->push_back(resolved_path);
} else {
std::string normalized_path;
if (!normalize_path(original_path, &normalized_path)) {
DL_WARN("Warning: unable to normalize \"%s\" (ignoring)", original_path);
continue;
}
std::string zip_path;
std::string entry_path;
std::string normalized_path;
if (!normalize_path(original_path, &normalized_path)) {
DL_WARN("Warning: unable to normalize \"%s\"", original_path);
continue;
}
if (parse_zip_path(normalized_path.c_str(), &zip_path, &entry_path)) {
if (realpath(zip_path.c_str(), resolved_path) == nullptr) {
DL_WARN("Warning: unable to resolve \"%s\": %s", zip_path.c_str(), strerror(errno));
DL_WARN("Warning: unable to resolve \"%s\": %s (ignoring)",
zip_path.c_str(), strerror(errno));
continue;
}
@ -242,4 +239,3 @@ void resolve_paths(std::vector<std::string>& paths,
}
}
}

View file

@ -1503,7 +1503,7 @@ TEST(dlfcn, dlopen_invalid_rw_load_segment) {
"/libtest_invalid-rw_load_segment.so";
void* handle = dlopen(libpath.c_str(), RTLD_NOW);
ASSERT_TRUE(handle == nullptr);
std::string expected_dlerror = std::string("dlopen failed: \"") + libpath + "\": W + E load segments are not allowed";
std::string expected_dlerror = std::string("dlopen failed: \"") + libpath + "\": W+E load segments are not allowed";
ASSERT_STREQ(expected_dlerror.c_str(), dlerror());
}