Be clearer about linker warnings.
Explicitly say "warning" for warnings, explicitly say what action
we're going to take (such as "(ignoring)"), always provide a link to
our documentation when there is one, explicitly say what API level the
behavior changes at, and explicitly say why we're allowing the misbehavior
for now.
Bug: http://b/71852862
Test: ran tests, looked at logcat
Change-Id: I1795a5af45deb904332b866d7d666690dae4340b
diff --git a/linker/linker.cpp b/linker/linker.cpp
index ff2a7e6..dd700fe 100644
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -1105,10 +1105,14 @@
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 @@
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 @@
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 @@
} 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 @@
// 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 @@
#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 @@
}
// 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 @@
&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());
diff --git a/linker/linker_config.cpp b/linker/linker_config.cpp
index 83c2f36..c00b734 100644
--- a/linker/linker_config.cpp
+++ b/linker/linker_config.cpp
@@ -194,14 +194,14 @@
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 @@
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 @@
}
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 @@
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 @@
(*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 @@
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 @@
}
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());
diff --git a/linker/linker_globals.cpp b/linker/linker_globals.cpp
index 155ebf4..bcc2a1e 100644
--- a/linker/linker_globals.cpp
+++ b/linker/linker_globals.cpp
@@ -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 @@
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());
+}
diff --git a/linker/linker_globals.h b/linker/linker_globals.h
index b470918..32aa09d 100644
--- a/linker/linker_globals.h
+++ b/linker/linker_globals.h
@@ -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); \
diff --git a/linker/linker_namespaces.cpp b/linker/linker_namespaces.cpp
index 9fdf0b5..fd72cdc 100644
--- a/linker/linker_namespaces.cpp
+++ b/linker/linker_namespaces.cpp
@@ -64,7 +64,8 @@
// 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;
}
diff --git a/linker/linker_phdr.cpp b/linker/linker_phdr.cpp
index a9873c4..a5eab44 100644
--- a/linker/linker_phdr.cpp
+++ b/linker/linker_phdr.cpp
@@ -268,8 +268,10 @@
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 @@
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 @@
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 @@
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 @@
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");
}
diff --git a/linker/linker_soinfo.cpp b/linker/linker_soinfo.cpp
index 54bfcf0..731e8f5 100644
--- a/linker/linker_soinfo.cpp
+++ b/linker/linker_soinfo.cpp
@@ -152,7 +152,7 @@
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());
}
diff --git a/linker/linker_utils.cpp b/linker/linker_utils.cpp
index 5bf88e7..661e7cb 100644
--- a/linker/linker_utils.cpp
+++ b/linker/linker_utils.cpp
@@ -209,31 +209,28 @@
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 @@
}
}
}
-