Skip to content

Commit 986cd71

Browse files
Stylie777MaskRay
authored andcommitted
Respond to review comments
Summary of changes: * Removed sectionName parameter from parseGnuPropertyNote function. * For the PAUTH error messages, parseGnuPropertyNote will now select data based on off data is present, rather than skipping the errors is data is a nullptr. If data is not present, `desc` is used instead. * Use of the section header when finding the `.note.gnu.property` section in a Shared File has been removed. They are not always present, so should solely rely on a program header here. * Changed to using the `p_offset` value when finding the Note header for the `.note.gnu.property` section. `p_vaddr` may not always be present, or a reliable source, whereas `p_offset` will always point to the section.
1 parent 718e71c commit 986cd71

File tree

2 files changed

+14
-21
lines changed

2 files changed

+14
-21
lines changed

lld/ELF/InputFiles.cpp

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -922,11 +922,10 @@ template <typename ELFT>
922922
static void parseGnuPropertyNote(Ctx &ctx, uint32_t &featureAndType,
923923
ArrayRef<uint8_t> &desc, ELFFileBase *f,
924924
const uint8_t *base,
925-
ArrayRef<uint8_t> *data = nullptr,
926-
StringRef sectionName = ".note.gnu.property") {
925+
ArrayRef<uint8_t> *data = nullptr) {
927926
auto err = [&](const uint8_t *place) -> ELFSyncStream {
928927
auto diag = Err(ctx);
929-
diag << f->getName() << ":(" << sectionName << "+0x"
928+
diag << f->getName() << ":(" << ".note.gnu.property+0x"
930929
<< Twine::utohexstr(place - base) << "): ";
931930
return diag;
932931
};
@@ -954,13 +953,14 @@ static void parseGnuPropertyNote(Ctx &ctx, uint32_t &featureAndType,
954953
// the data variable as there is no InputSection to collect the
955954
// data from. As such, these are ignored. They are needed either
956955
// when loading a shared library oject.
957-
if (!f->aarch64PauthAbiCoreInfo.empty() && data != nullptr) {
956+
ArrayRef<uint8_t> contents = data ? *data : desc;
957+
if (!f->aarch64PauthAbiCoreInfo.empty()) {
958958
return void(
959-
err(data->data())
959+
err(contents.data())
960960
<< "multiple GNU_PROPERTY_AARCH64_FEATURE_PAUTH entries are "
961961
"not supported");
962-
} else if (size != 16 && data != nullptr) {
963-
return void(err(data->data())
962+
} else if (size != 16) {
963+
return void(err(contents.data())
964964
<< "GNU_PROPERTY_AARCH64_FEATURE_PAUTH entry "
965965
"is invalid: expected 16 bytes, but got "
966966
<< size);
@@ -1010,8 +1010,7 @@ static void readGnuProperty(Ctx &ctx, const InputSection &sec,
10101010
// Read a body of a NOTE record, which consists of type-length-value fields.
10111011
ArrayRef<uint8_t> desc = note.getDesc(sec.addralign);
10121012
const uint8_t *base = sec.content().data();
1013-
parseGnuPropertyNote<ELFT>(ctx, featureAndType, desc, &f, base, &data,
1014-
sec.name);
1013+
parseGnuPropertyNote<ELFT>(ctx, featureAndType, desc, &f, base, &data);
10151014

10161015
// Go to next NOTE record to look for more FEATURE_1_AND descriptions.
10171016
data = data.slice(nhdr->getSize(sec.addralign));
@@ -1448,9 +1447,8 @@ std::vector<uint32_t> SharedFile::parseVerneed(const ELFFile<ELFT> &obj,
14481447
// be collected from this is irrelevant for a dynamic object.
14491448
template <typename ELFT>
14501449
void SharedFile::parseGnuAndFeatures(const uint8_t *base,
1451-
const typename ELFT::PhdrRange headers,
1452-
const typename ELFT::Shdr *sHeader) {
1453-
if (numElfPhdrs == 0 || sHeader == nullptr)
1450+
const typename ELFT::PhdrRange headers) {
1451+
if (numElfPhdrs == 0)
14541452
return;
14551453
uint32_t featureAndType = ctx.arg.emachine == EM_AARCH64
14561454
? GNU_PROPERTY_AARCH64_FEATURE_1_AND
@@ -1461,12 +1459,12 @@ void SharedFile::parseGnuAndFeatures(const uint8_t *base,
14611459
continue;
14621460
const typename ELFT::Note note(
14631461
*reinterpret_cast<const typename ELFT::Nhdr *>(base +
1464-
headers[i].p_vaddr));
1462+
headers[i].p_offset));
14651463
if (note.getType() != NT_GNU_PROPERTY_TYPE_0 || note.getName() != "GNU")
14661464
continue;
14671465

14681466
// Read a body of a NOTE record, which consists of type-length-value fields.
1469-
ArrayRef<uint8_t> desc = note.getDesc(sHeader->sh_addralign);
1467+
ArrayRef<uint8_t> desc = note.getDesc(headers[i].p_align);
14701468
parseGnuPropertyNote<ELFT>(ctx, featureAndType, desc, this, base);
14711469
}
14721470
}
@@ -1520,7 +1518,6 @@ template <class ELFT> void SharedFile::parse() {
15201518
const Elf_Shdr *versymSec = nullptr;
15211519
const Elf_Shdr *verdefSec = nullptr;
15221520
const Elf_Shdr *verneedSec = nullptr;
1523-
const Elf_Shdr *noteSec = nullptr;
15241521
symbols = std::make_unique<Symbol *[]>(numSymbols);
15251522

15261523
// Search for .dynsym, .dynamic, .symtab, .gnu.version and .gnu.version_d.
@@ -1541,9 +1538,6 @@ template <class ELFT> void SharedFile::parse() {
15411538
case SHT_GNU_verneed:
15421539
verneedSec = &sec;
15431540
break;
1544-
case SHT_NOTE:
1545-
noteSec = &sec;
1546-
break;
15471541
}
15481542
}
15491543

@@ -1590,7 +1584,7 @@ template <class ELFT> void SharedFile::parse() {
15901584

15911585
verdefs = parseVerdefs<ELFT>(obj.base(), verdefSec);
15921586
std::vector<uint32_t> verneeds = parseVerneed<ELFT>(obj, verneedSec);
1593-
parseGnuAndFeatures<ELFT>(obj.base(), getELFPhdrs<ELFT>(), noteSec);
1587+
parseGnuAndFeatures<ELFT>(obj.base(), getELFPhdrs<ELFT>());
15941588

15951589
// Parse ".gnu.version" section which is a parallel array for the symbol
15961590
// table. If a given file doesn't have a ".gnu.version" section, we use

lld/ELF/InputFiles.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,8 +375,7 @@ class SharedFile : public ELFFileBase {
375375
const typename ELFT::Shdr *sec);
376376
template <typename ELFT>
377377
void parseGnuAndFeatures(const uint8_t *base,
378-
const typename ELFT::PhdrRange headers,
379-
const typename ELFT::Shdr *sHeader);
378+
const typename ELFT::PhdrRange headers);
380379
};
381380

382381
class BinaryFile : public InputFile {

0 commit comments

Comments
 (0)