tequilaOS/platform_bionic
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge "Be clearer about linker warnings."

Browse source
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
Show stats Download patch file Download diff file Expand all files Collapse all files

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

16
linker/linker_config.cpp
View file

@ -194,14 +194,14 @@ static bool parse_config_file(const char* ld_config_file_path,
std::string section_name; std::string section_name;
while(true) { while (true) {
std::string name; std::string name;
std::string value; std::string value;
std::string error; std::string error;
int result = cp.next_token(&name, &value, &error); int result = cp.next_token(&name, &value, &error);
if (result == ConfigParser::kError) { 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, ld_config_file_path,
cp.lineno(), cp.lineno(),
error.c_str()); error.c_str());
@ -214,7 +214,7 @@ static bool parse_config_file(const char* ld_config_file_path,
if (result == ConfigParser::kPropertyAssign) { if (result == ConfigParser::kPropertyAssign) {
if (!android::base::StartsWith(name, "dir.")) { 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)", "expected format dir.<section_name> (ignoring this line)",
ld_config_file_path, ld_config_file_path,
cp.lineno(), cp.lineno(),
@ -228,7 +228,7 @@ static bool parse_config_file(const char* ld_config_file_path,
} }
if (value.empty()) { 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, ld_config_file_path,
cp.lineno()); cp.lineno());
continue; continue;
@ -275,7 +275,7 @@ static bool parse_config_file(const char* ld_config_file_path,
if (result == ConfigParser::kPropertyAssign) { if (result == ConfigParser::kPropertyAssign) {
if (properties->find(name) != properties->end()) { 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, ld_config_file_path,
cp.lineno(), cp.lineno(),
name.c_str()); 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()); (*properties)[name] = PropertyValue(std::move(value), cp.lineno());
} else if (result == ConfigParser::kPropertyAppend) { } else if (result == ConfigParser::kPropertyAppend) {
if (properties->find(name) == properties->end()) { 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, ld_config_file_path,
cp.lineno(), cp.lineno(),
name.c_str()); name.c_str());
@ -299,7 +299,7 @@ static bool parse_config_file(const char* ld_config_file_path,
value = ":" + value; value = ":" + value;
(*properties)[name].append_value(std::move(value)); (*properties)[name].append_value(std::move(value));
} else { } 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, ld_config_file_path,
cp.lineno(), cp.lineno(),
name.c_str()); name.c_str());
@ -308,7 +308,7 @@ static bool parse_config_file(const char* ld_config_file_path,
} }
if (result == ConfigParser::kError) { 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, ld_config_file_path,
cp.lineno(), cp.lineno(),
error.c_str()); error.c_str());

19
linker/linker_globals.cpp
View file

@ -26,10 +26,12 @@
* SUCH DAMAGE. * SUCH DAMAGE.
*/ */
#include "linker.h"
#include "linker_globals.h" #include "linker_globals.h"
#include "linker_namespaces.h" #include "linker_namespaces.h"
#include "android-base/stringprintf.h"
int g_argc = 0; int g_argc = 0;
char** g_argv = nullptr; char** g_argv = nullptr;
char** g_envp = nullptr; char** g_envp = nullptr;
@ -48,3 +50,18 @@ size_t linker_get_error_buffer_size() {
return sizeof(__linker_dl_err_buf); 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());
}

2
linker/linker_globals.h
View file

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

3
linker/linker_namespaces.cpp
View file

@ -64,7 +64,8 @@ bool android_namespace_t::is_accessible(soinfo* s) {
// This is workaround for apps hacking into soinfo list. // This is workaround for apps hacking into soinfo list.
// and inserting their own entries into it. (http://b/37191433) // and inserting their own entries into it. (http://b/37191433)
if (!si->has_min_version(3)) { 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; return false;
} }

41
linker/linker_phdr.cpp
View file

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

2
linker/linker_soinfo.cpp
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) { ELF_ST_BIND(s->st_info) == STB_WEAK) {
return s->st_shndx != SHN_UNDEF; return s->st_shndx != SHN_UNDEF;
} else if (ELF_ST_BIND(s->st_info) != STB_LOCAL) { } 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()); ELF_ST_BIND(s->st_info), si->get_string(s->st_name), si->get_realpath());
} }

34
linker/linker_utils.cpp
View file

@ -209,31 +209,28 @@ void resolve_paths(std::vector<std::string>& paths,
const char* original_path = path.c_str(); const char* original_path = path.c_str();
if (realpath(original_path, resolved_path) != nullptr) { if (realpath(original_path, resolved_path) != nullptr) {
struct stat s; struct stat s;
if (stat(resolved_path, &s) == 0) { if (stat(resolved_path, &s) == -1) {
if (S_ISDIR(s.st_mode)) { DL_WARN("Warning: cannot stat file \"%s\": %s (ignoring)", resolved_path, strerror(errno));
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));
continue; 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 { } 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 zip_path;
std::string entry_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 (parse_zip_path(normalized_path.c_str(), &zip_path, &entry_path)) {
if (realpath(zip_path.c_str(), resolved_path) == nullptr) { 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; continue;
} }
@ -242,4 +239,3 @@ void resolve_paths(std::vector<std::string>& paths,
} }
} }
} }

2
tests/dlfcn_test.cpp
View file

@ -1503,7 +1503,7 @@ TEST(dlfcn, dlopen_invalid_rw_load_segment) {
"/libtest_invalid-rw_load_segment.so"; "/libtest_invalid-rw_load_segment.so";
void* handle = dlopen(libpath.c_str(), RTLD_NOW); void* handle = dlopen(libpath.c_str(), RTLD_NOW);
ASSERT_TRUE(handle == nullptr); 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()); ASSERT_STREQ(expected_dlerror.c_str(), dlerror());
} }