[llvm] [llvm-objcopy] Add possibility to verify .note section format (PR #90458)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 10 14:27:23 PDT 2024
https://github.com/serge-sans-paille updated https://github.com/llvm/llvm-project/pull/90458
>From 7483002043b3c1e72d8005b2e7d417a5dd475bee Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Mon, 29 Apr 2024 13:27:04 +0200
Subject: [PATCH 1/8] [llvm-objcopy] Error with relevant message when adding
invalid notes
Also add a --no-verify-note-sections flag to make it possible to add
invalid sections if needs be.
---
llvm/include/llvm/ObjCopy/CommonConfig.h | 1 +
llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp | 71 +++++++++++++++++--
.../llvm-objcopy/ELF/add-invalid-note.test | 34 +++++++++
llvm/tools/llvm-objcopy/ObjcopyOptions.cpp | 2 +
llvm/tools/llvm-objcopy/ObjcopyOpts.td | 3 +
5 files changed, 105 insertions(+), 6 deletions(-)
create mode 100644 llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
diff --git a/llvm/include/llvm/ObjCopy/CommonConfig.h b/llvm/include/llvm/ObjCopy/CommonConfig.h
index ae08d4032736e..c77a286799ead 100644
--- a/llvm/include/llvm/ObjCopy/CommonConfig.h
+++ b/llvm/include/llvm/ObjCopy/CommonConfig.h
@@ -258,6 +258,7 @@ struct CommonConfig {
bool StripNonAlloc = false;
bool StripSections = false;
bool StripUnneeded = false;
+ bool VerifyNoteSections = true;
bool Weaken = false;
bool DecompressDebugSections = false;
diff --git a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
index f343d1447e055..bf36adba148e0 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
+++ b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
@@ -623,6 +623,58 @@ handleUserSection(const NewSectionInfo &NewSection,
return F(NewSection.SectionName, Data);
}
+static Error verifyNoteSection(StringRef Name, endianness Endianness,
+ ArrayRef<uint8_t> Data) {
+ // An ELF note has the following structure:
+ // Name Size: 4 bytes (integer)
+ // Desc Size: 4 bytes (integer)
+ // Type : 4 bytes
+ // Name : variable size, padded to a 4 byte boundary
+ // Desc : variable size, padded to a 4 byte boundary
+
+ if (Data.empty())
+ return Error::success();
+
+ if (Data.size() < 12) {
+ std::string msg;
+ raw_string_ostream(msg)
+ << Name << " data must be either empty or at least 12 bytes long";
+ return createStringError(errc::invalid_argument, msg);
+ }
+ if (Data.size() % 4 != 0) {
+ std::string msg;
+ raw_string_ostream(msg)
+ << Name << " data size must be a multiple of 4 bytes";
+ return createStringError(errc::invalid_argument, msg);
+ }
+ ArrayRef<uint8_t> NameSize = Data.slice(0, 4);
+ ArrayRef<uint8_t> DescSize = Data.slice(4, 4);
+
+ uint32_t NameSizeValue = *reinterpret_cast<const uint32_t *>(NameSize.data());
+ uint32_t DescSizeValue = *reinterpret_cast<const uint32_t *>(DescSize.data());
+
+ if (Endianness != llvm::endianness::native) {
+ NameSizeValue = byteswap(NameSizeValue);
+ DescSizeValue = byteswap(DescSizeValue);
+ }
+ uint64_t ExpectedDataSize =
+ /*NameSize=*/4 + /*DescSize=*/4 + /*Type=*/4 +
+ /*Name=*/(NameSizeValue + 3) / 4 * 4 +
+ /*Desc=*/(DescSizeValue + 3) / 4 * 4;
+ uint64_t ActualDataSize = Data.size();
+ if (ActualDataSize != ExpectedDataSize) {
+ std::string msg;
+ raw_string_ostream(msg)
+ << Name
+ << " data size is incompatible with the content of "
+ "the name and description size fields:"
+ << " expecting " << ExpectedDataSize << ", found " << ActualDataSize;
+ return createStringError(errc::invalid_argument, msg);
+ }
+
+ return Error::success();
+}
+
// This function handles the high level operations of GNU objcopy including
// handling command line options. It's important to outline certain properties
// we expect to hold of the command line operations. Any operation that "keeps"
@@ -631,7 +683,7 @@ handleUserSection(const NewSectionInfo &NewSection,
// depend a) on the order the options occur in or b) on some opaque priority
// system. The only priority is that keeps/copies overrule removes.
static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig,
- Object &Obj) {
+ ElfType OutputElfType, Object &Obj) {
if (Config.OutputArch) {
Obj.Machine = Config.OutputArch->EMachine;
Obj.OSABI = Config.OutputArch->OSABI;
@@ -675,12 +727,19 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig,
if (Sec.Flags & SHF_ALLOC && Sec.Type != SHT_NOTE)
Sec.Type = SHT_NOBITS;
+ endianness E = OutputElfType == ELFT_ELF32LE || OutputElfType == ELFT_ELF64LE
+ ? endianness::little
+ : endianness::big;
+
for (const NewSectionInfo &AddedSection : Config.AddSection) {
- auto AddSection = [&](StringRef Name, ArrayRef<uint8_t> Data) {
+ auto AddSection = [&](StringRef Name, ArrayRef<uint8_t> Data) -> Error {
OwnedDataSection &NewSection =
Obj.addSection<OwnedDataSection>(Name, Data);
- if (Name.starts_with(".note") && Name != ".note.GNU-stack")
+ if (Name.starts_with(".note") && Name != ".note.GNU-stack") {
NewSection.Type = SHT_NOTE;
+ if (Config.VerifyNoteSections)
+ return verifyNoteSection(Name, E, Data);
+ }
return Error::success();
};
if (Error E = handleUserSection(AddedSection, AddSection))
@@ -813,7 +872,7 @@ Error objcopy::elf::executeObjcopyOnIHex(const CommonConfig &Config,
const ElfType OutputElfType =
getOutputElfType(Config.OutputArch.value_or(MachineInfo()));
- if (Error E = handleArgs(Config, ELFConfig, **Obj))
+ if (Error E = handleArgs(Config, ELFConfig, OutputElfType, **Obj))
return E;
return writeOutput(Config, **Obj, Out, OutputElfType);
}
@@ -831,7 +890,7 @@ Error objcopy::elf::executeObjcopyOnRawBinary(const CommonConfig &Config,
// (-B<arch>).
const ElfType OutputElfType =
getOutputElfType(Config.OutputArch.value_or(MachineInfo()));
- if (Error E = handleArgs(Config, ELFConfig, **Obj))
+ if (Error E = handleArgs(Config, ELFConfig, OutputElfType, **Obj))
return E;
return writeOutput(Config, **Obj, Out, OutputElfType);
}
@@ -850,7 +909,7 @@ Error objcopy::elf::executeObjcopyOnBinary(const CommonConfig &Config,
? getOutputElfType(*Config.OutputArch)
: getOutputElfType(In);
- if (Error E = handleArgs(Config, ELFConfig, **Obj))
+ if (Error E = handleArgs(Config, ELFConfig, OutputElfType, **Obj))
return createFileError(Config.InputFilename, std::move(E));
if (Error E = writeOutput(Config, **Obj, Out, OutputElfType))
diff --git a/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
new file mode 100644
index 0000000000000..1cd2123782eed
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
@@ -0,0 +1,34 @@
+## Verify that --add-section warns on invalid note sections.
+
+# Add [namesz, descsz, type, name, desc] for a build id.
+#
+# RUN: echo -e -n "\x04\x00\x00\x00" > %t-miss-padding-note.bin
+# RUN: echo -e -n "\x07\x00\x00\x00" >> %t-miss-padding-note.bin
+# RUN: echo -e -n "\x03\x00\x00\x00" >> %t-miss-padding-note.bin
+# RUN: echo -e -n "GNU\x00" >> %t-miss-padding-note.bin
+# RUN: echo -e -n "\x0c\x0d\x0e" >> %t-miss-padding-note.bin
+#
+# RUN: echo -e -n "\x08\x00\x00\x00" > %t-invalid-size-note.bin
+# RUN: echo -e -n "\x07\x00\x00\x00" >> %t-invalid-size-note.bin
+# RUN: echo -e -n "\x03\x00\x00\x00" >> %t-invalid-size-note.bin
+# RUN: echo -e -n "GNU\x00" >> %t-invalid-size-note.bin
+# RUN: echo -e -n "\x0c\x0d\x0e\x00" >> %t-invalid-size-note.bin
+
+# RUN: echo -e -n "\x08\x00\x00\x00" > %t-short-note.bin
+# RUN: echo -e -n "\x07\x00\x00\x00" >> %t-short-note.bin
+
+# RUN: yaml2obj %s -o %t.o
+# RUN: not llvm-objcopy --add-section=.note.miss.padding=%t-miss-padding-note.bin %t.o %t-with-note.o 2>&1 | FileCheck --check-prefix=CHECK-MISS-PADDING %s
+# RUN: not llvm-objcopy --add-section=.note.invalid.size=%t-invalid-size-note.bin %t.o %t-with-note.o 2>&1 | FileCheck --check-prefix=CHECK-INVALID-SIZE %s
+# RUN: not llvm-objcopy --add-section=.note.short=%t-short-note.bin %t.o %t-with-note.o 2>&1 | FileCheck --check-prefix=CHECK-SHORT %s
+# RUN: llvm-objcopy --add-section=.note.short=%t-short-note.bin --no-verify-note-sections %t.o %t-with-note.o
+
+!ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_REL
+
+# CHECK-MISS-PADDING: .note.miss.padding data size must be a multiple of 4 bytes
+# CHECK-INVALID-SIZE: .note.invalid.size data size is incompatible with the content of the name and description size fields: expecting 28, found 20
+# CHECK-SHORT: .note.short data must be either empty or at least 12 bytes long
diff --git a/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp b/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
index 4ab3b7265f2f6..ceacfdcf9865e 100644
--- a/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ b/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -949,6 +949,8 @@ objcopy::parseObjcopyOptions(ArrayRef<const char *> RawArgsArr,
? DiscardType::All
: DiscardType::Locals;
}
+ Config.VerifyNoteSections =
+ !InputArgs.hasArg(OBJCOPY_no_verify_note_sections);
Config.OnlyKeepDebug = InputArgs.hasArg(OBJCOPY_only_keep_debug);
ELFConfig.KeepFileSymbols = InputArgs.hasArg(OBJCOPY_keep_file_symbols);
MachOConfig.KeepUndefined = InputArgs.hasArg(OBJCOPY_keep_undefined);
diff --git a/llvm/tools/llvm-objcopy/ObjcopyOpts.td b/llvm/tools/llvm-objcopy/ObjcopyOpts.td
index 4bc80eba05f8e..1db80e0f2c4e8 100644
--- a/llvm/tools/llvm-objcopy/ObjcopyOpts.td
+++ b/llvm/tools/llvm-objcopy/ObjcopyOpts.td
@@ -176,6 +176,9 @@ def G : JoinedOrSeparate<["-"], "G">,
Alias<keep_global_symbol>,
HelpText<"Alias for --keep-global-symbol">;
+def no_verify_note_sections : Flag<["--"], "no-verify-note-sections">,
+ HelpText<"Disable note section validation">;
+
defm keep_global_symbols
: Eq<"keep-global-symbols",
"Read a list of symbols from <filename> and run as if "
>From f3c0f538ab91cc430a75f84d9cd992bb59b80d5a Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Mon, 3 Jun 2024 11:15:46 +0200
Subject: [PATCH 2/8] fixup! [llvm-objcopy] Error with relevant message when
adding invalid notes
---
llvm/docs/CommandGuide/llvm-objcopy.rst | 6 +++++
llvm/include/llvm/ObjCopy/CommonConfig.h | 1 -
llvm/include/llvm/ObjCopy/ELF/ELFConfig.h | 1 +
llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp | 4 ++--
.../llvm-objcopy/ELF/add-invalid-note.test | 22 ++++++++++++++-----
llvm/tools/llvm-objcopy/ObjcopyOptions.cpp | 8 +++++--
llvm/tools/llvm-objcopy/ObjcopyOpts.td | 11 ++++++++--
7 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/llvm/docs/CommandGuide/llvm-objcopy.rst b/llvm/docs/CommandGuide/llvm-objcopy.rst
index a62acfc8fdcd8..29745848abf55 100644
--- a/llvm/docs/CommandGuide/llvm-objcopy.rst
+++ b/llvm/docs/CommandGuide/llvm-objcopy.rst
@@ -512,6 +512,12 @@ them.
specified format. See `SUPPORTED FORMATS`_ for a list of valid ``<format>``
values.
+.. option:: --verify-note-sections, --no-verify-note-sections
+
+ When adding note sections, verify if the section format is valid. Defaults to
+ ``--verify-note-sections``
+
+
.. option:: --weaken-symbol <symbol>, -W
Mark global symbols named ``<symbol>`` as weak symbols in the output. Can
diff --git a/llvm/include/llvm/ObjCopy/CommonConfig.h b/llvm/include/llvm/ObjCopy/CommonConfig.h
index c77a286799ead..ae08d4032736e 100644
--- a/llvm/include/llvm/ObjCopy/CommonConfig.h
+++ b/llvm/include/llvm/ObjCopy/CommonConfig.h
@@ -258,7 +258,6 @@ struct CommonConfig {
bool StripNonAlloc = false;
bool StripSections = false;
bool StripUnneeded = false;
- bool VerifyNoteSections = true;
bool Weaken = false;
bool DecompressDebugSections = false;
diff --git a/llvm/include/llvm/ObjCopy/ELF/ELFConfig.h b/llvm/include/llvm/ObjCopy/ELF/ELFConfig.h
index eafed92516c7d..59960b6530743 100644
--- a/llvm/include/llvm/ObjCopy/ELF/ELFConfig.h
+++ b/llvm/include/llvm/ObjCopy/ELF/ELFConfig.h
@@ -30,6 +30,7 @@ struct ELFConfig {
bool AllowBrokenLinks = false;
bool KeepFileSymbols = false;
bool LocalizeHidden = false;
+ bool VerifyNoteSections = true;
};
} // namespace objcopy
diff --git a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
index bf36adba148e0..26ca3e84166e6 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
+++ b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
@@ -653,7 +653,7 @@ static Error verifyNoteSection(StringRef Name, endianness Endianness,
uint32_t NameSizeValue = *reinterpret_cast<const uint32_t *>(NameSize.data());
uint32_t DescSizeValue = *reinterpret_cast<const uint32_t *>(DescSize.data());
- if (Endianness != llvm::endianness::native) {
+ if (Endianness != endianness::native) {
NameSizeValue = byteswap(NameSizeValue);
DescSizeValue = byteswap(DescSizeValue);
}
@@ -737,7 +737,7 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig,
Obj.addSection<OwnedDataSection>(Name, Data);
if (Name.starts_with(".note") && Name != ".note.GNU-stack") {
NewSection.Type = SHT_NOTE;
- if (Config.VerifyNoteSections)
+ if (ELFConfig.VerifyNoteSections)
return verifyNoteSection(Name, E, Data);
}
return Error::success();
diff --git a/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
index 1cd2123782eed..2f8116c51ce14 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
@@ -1,34 +1,46 @@
## Verify that --add-section warns on invalid note sections.
# Add [namesz, descsz, type, name, desc] for a build id.
-#
+
+## Notes should be padded on 8 bits
# RUN: echo -e -n "\x04\x00\x00\x00" > %t-miss-padding-note.bin
# RUN: echo -e -n "\x07\x00\x00\x00" >> %t-miss-padding-note.bin
# RUN: echo -e -n "\x03\x00\x00\x00" >> %t-miss-padding-note.bin
# RUN: echo -e -n "GNU\x00" >> %t-miss-padding-note.bin
# RUN: echo -e -n "\x0c\x0d\x0e" >> %t-miss-padding-note.bin
-#
+
+## The namesz field bit is incorrect
# RUN: echo -e -n "\x08\x00\x00\x00" > %t-invalid-size-note.bin
# RUN: echo -e -n "\x07\x00\x00\x00" >> %t-invalid-size-note.bin
# RUN: echo -e -n "\x03\x00\x00\x00" >> %t-invalid-size-note.bin
# RUN: echo -e -n "GNU\x00" >> %t-invalid-size-note.bin
# RUN: echo -e -n "\x0c\x0d\x0e\x00" >> %t-invalid-size-note.bin
+## Missing type field
# RUN: echo -e -n "\x08\x00\x00\x00" > %t-short-note.bin
# RUN: echo -e -n "\x07\x00\x00\x00" >> %t-short-note.bin
# RUN: yaml2obj %s -o %t.o
+
+## test each invalid note
# RUN: not llvm-objcopy --add-section=.note.miss.padding=%t-miss-padding-note.bin %t.o %t-with-note.o 2>&1 | FileCheck --check-prefix=CHECK-MISS-PADDING %s
+# CHECK-MISS-PADDING: .note.miss.padding data size must be a multiple of 4 bytes
+
# RUN: not llvm-objcopy --add-section=.note.invalid.size=%t-invalid-size-note.bin %t.o %t-with-note.o 2>&1 | FileCheck --check-prefix=CHECK-INVALID-SIZE %s
+# CHECK-INVALID-SIZE: .note.invalid.size data size is incompatible with the content of the name and description size fields: expecting 28, found 20
+
# RUN: not llvm-objcopy --add-section=.note.short=%t-short-note.bin %t.o %t-with-note.o 2>&1 | FileCheck --check-prefix=CHECK-SHORT %s
+# CHECK-SHORT: .note.short data must be either empty or at least 12 bytes long
+
+## test that verification can be disabled
# RUN: llvm-objcopy --add-section=.note.short=%t-short-note.bin --no-verify-note-sections %t.o %t-with-note.o
+## test that last argument has precedence
+# RUN: llvm-objcopy --add-section=.note.short=%t-short-note.bin --verify-note-section --no-verify-note-sections %t.o %t-with-note.o
+
!ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
Type: ET_REL
-# CHECK-MISS-PADDING: .note.miss.padding data size must be a multiple of 4 bytes
-# CHECK-INVALID-SIZE: .note.invalid.size data size is incompatible with the content of the name and description size fields: expecting 28, found 20
-# CHECK-SHORT: .note.short data must be either empty or at least 12 bytes long
diff --git a/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp b/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
index ceacfdcf9865e..5ba4ad1d662b6 100644
--- a/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ b/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -949,8 +949,12 @@ objcopy::parseObjcopyOptions(ArrayRef<const char *> RawArgsArr,
? DiscardType::All
: DiscardType::Locals;
}
- Config.VerifyNoteSections =
- !InputArgs.hasArg(OBJCOPY_no_verify_note_sections);
+
+ const bool ShouldVerifyNoteSectionsByDefault = true;
+ ELFConfig.VerifyNoteSections = InputArgs.hasFlag(
+ OBJCOPY_verify_note_sections, OBJCOPY_no_verify_note_sections,
+ ShouldVerifyNoteSectionsByDefault);
+
Config.OnlyKeepDebug = InputArgs.hasArg(OBJCOPY_only_keep_debug);
ELFConfig.KeepFileSymbols = InputArgs.hasArg(OBJCOPY_keep_file_symbols);
MachOConfig.KeepUndefined = InputArgs.hasArg(OBJCOPY_keep_undefined);
diff --git a/llvm/tools/llvm-objcopy/ObjcopyOpts.td b/llvm/tools/llvm-objcopy/ObjcopyOpts.td
index 1db80e0f2c4e8..86c109d70c737 100644
--- a/llvm/tools/llvm-objcopy/ObjcopyOpts.td
+++ b/llvm/tools/llvm-objcopy/ObjcopyOpts.td
@@ -1,5 +1,11 @@
include "CommonOpts.td"
+multiclass B<string name, string help1, string help2> {
+ def NAME: Flag<["--"], name>, HelpText<help1>;
+ def no_ # NAME: Flag<["--"], "no-" # name>, HelpText<help2>;
+}
+
+
defm binary_architecture
: Eq<"binary-architecture", "Ignored for compatibility">;
def B : JoinedOrSeparate<["-"], "B">,
@@ -176,8 +182,9 @@ def G : JoinedOrSeparate<["-"], "G">,
Alias<keep_global_symbol>,
HelpText<"Alias for --keep-global-symbol">;
-def no_verify_note_sections : Flag<["--"], "no-verify-note-sections">,
- HelpText<"Disable note section validation">;
+defm verify_note_sections : B<"verify-note-sections",
+ "Validate note sections",
+ "Don't validate note sections">;
defm keep_global_symbols
: Eq<"keep-global-symbols",
>From 1a4aa183ff66c8e0b5ea06b53872f34ef3a86fca Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Mon, 3 Jun 2024 12:07:43 +0200
Subject: [PATCH 3/8] fixup! [llvm-objcopy] Error with relevant message when
adding invalid notes
---
llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
index 2f8116c51ce14..2154e64f53349 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
@@ -36,7 +36,7 @@
# RUN: llvm-objcopy --add-section=.note.short=%t-short-note.bin --no-verify-note-sections %t.o %t-with-note.o
## test that last argument has precedence
-# RUN: llvm-objcopy --add-section=.note.short=%t-short-note.bin --verify-note-section --no-verify-note-sections %t.o %t-with-note.o
+# RUN: llvm-objcopy --add-section=.note.short=%t-short-note.bin --verify-note-sections --no-verify-note-sections %t.o %t-with-note.o
!ELF
FileHeader:
>From 2c82ed7d7b7dc2f7a9e03423a32ae06e1d0b75b9 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Tue, 4 Jun 2024 20:24:54 +0200
Subject: [PATCH 4/8] fixup! [llvm-objcopy] Error with relevant message when
adding invalid notes
---
llvm/docs/CommandGuide/llvm-objcopy.rst | 9 ++++++---
llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test | 10 +++++-----
llvm/tools/llvm-objcopy/ObjcopyOptions.cpp | 4 +---
llvm/tools/llvm-objcopy/ObjcopyOpts.td | 1 -
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/llvm/docs/CommandGuide/llvm-objcopy.rst b/llvm/docs/CommandGuide/llvm-objcopy.rst
index 29745848abf55..4036d1de48c6b 100644
--- a/llvm/docs/CommandGuide/llvm-objcopy.rst
+++ b/llvm/docs/CommandGuide/llvm-objcopy.rst
@@ -512,11 +512,14 @@ them.
specified format. See `SUPPORTED FORMATS`_ for a list of valid ``<format>``
values.
-.. option:: --verify-note-sections, --no-verify-note-sections
+.. option:: --verify-note-sections
- When adding note sections, verify if the section format is valid. Defaults to
- ``--verify-note-sections``
+ When adding note sections, verify if the section format is valid. This flag is
+ turned on by default for ELF files.
+.. option:: --no-verify-note-sections
+
+ When adding note sections, do not verify if the section format is valid.
.. option:: --weaken-symbol <symbol>, -W
diff --git a/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
index 2154e64f53349..23a45500b064e 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
@@ -1,28 +1,28 @@
## Verify that --add-section warns on invalid note sections.
-# Add [namesz, descsz, type, name, desc] for a build id.
+## Add [namesz, descsz, type, name, desc] for a build id.
-## Notes should be padded on 8 bits
+## Notes should be padded on 8 bytes.
# RUN: echo -e -n "\x04\x00\x00\x00" > %t-miss-padding-note.bin
# RUN: echo -e -n "\x07\x00\x00\x00" >> %t-miss-padding-note.bin
# RUN: echo -e -n "\x03\x00\x00\x00" >> %t-miss-padding-note.bin
# RUN: echo -e -n "GNU\x00" >> %t-miss-padding-note.bin
# RUN: echo -e -n "\x0c\x0d\x0e" >> %t-miss-padding-note.bin
-## The namesz field bit is incorrect
+## The namesz field bit is incorrect.
# RUN: echo -e -n "\x08\x00\x00\x00" > %t-invalid-size-note.bin
# RUN: echo -e -n "\x07\x00\x00\x00" >> %t-invalid-size-note.bin
# RUN: echo -e -n "\x03\x00\x00\x00" >> %t-invalid-size-note.bin
# RUN: echo -e -n "GNU\x00" >> %t-invalid-size-note.bin
# RUN: echo -e -n "\x0c\x0d\x0e\x00" >> %t-invalid-size-note.bin
-## Missing type field
+## Missing type field.
# RUN: echo -e -n "\x08\x00\x00\x00" > %t-short-note.bin
# RUN: echo -e -n "\x07\x00\x00\x00" >> %t-short-note.bin
# RUN: yaml2obj %s -o %t.o
-## test each invalid note
+## Test each invalid note.
# RUN: not llvm-objcopy --add-section=.note.miss.padding=%t-miss-padding-note.bin %t.o %t-with-note.o 2>&1 | FileCheck --check-prefix=CHECK-MISS-PADDING %s
# CHECK-MISS-PADDING: .note.miss.padding data size must be a multiple of 4 bytes
diff --git a/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp b/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
index 5ba4ad1d662b6..90cb924458181 100644
--- a/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ b/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -950,10 +950,8 @@ objcopy::parseObjcopyOptions(ArrayRef<const char *> RawArgsArr,
: DiscardType::Locals;
}
- const bool ShouldVerifyNoteSectionsByDefault = true;
ELFConfig.VerifyNoteSections = InputArgs.hasFlag(
- OBJCOPY_verify_note_sections, OBJCOPY_no_verify_note_sections,
- ShouldVerifyNoteSectionsByDefault);
+ OBJCOPY_verify_note_sections, OBJCOPY_no_verify_note_sections, true);
Config.OnlyKeepDebug = InputArgs.hasArg(OBJCOPY_only_keep_debug);
ELFConfig.KeepFileSymbols = InputArgs.hasArg(OBJCOPY_keep_file_symbols);
diff --git a/llvm/tools/llvm-objcopy/ObjcopyOpts.td b/llvm/tools/llvm-objcopy/ObjcopyOpts.td
index 86c109d70c737..75b6e5bfac1cc 100644
--- a/llvm/tools/llvm-objcopy/ObjcopyOpts.td
+++ b/llvm/tools/llvm-objcopy/ObjcopyOpts.td
@@ -5,7 +5,6 @@ multiclass B<string name, string help1, string help2> {
def no_ # NAME: Flag<["--"], "no-" # name>, HelpText<help2>;
}
-
defm binary_architecture
: Eq<"binary-architecture", "Ignored for compatibility">;
def B : JoinedOrSeparate<["-"], "B">,
>From 36daa119020f0cd2d57559738a2d1f36f6ee7096 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Thu, 13 Jun 2024 15:29:00 +0200
Subject: [PATCH 5/8] fixup! [llvm-objcopy] Error with relevant message when
adding invalid notes
---
llvm/docs/CommandGuide/llvm-objcopy.rst | 12 ++++----
llvm/docs/ReleaseNotes.rst | 4 +++
.../llvm-objcopy/ELF/add-invalid-note.test | 29 +++++++++----------
3 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/llvm/docs/CommandGuide/llvm-objcopy.rst b/llvm/docs/CommandGuide/llvm-objcopy.rst
index 4036d1de48c6b..3581839378403 100644
--- a/llvm/docs/CommandGuide/llvm-objcopy.rst
+++ b/llvm/docs/CommandGuide/llvm-objcopy.rst
@@ -422,6 +422,10 @@ them.
The default is `default`.
+.. option:: --no-verify-note-sections
+
+ When adding note sections, do not verify if the section format is valid.
+
.. option:: --output-target <format>, -O
Write the output as the specified format. See `SUPPORTED FORMATS`_ for a list
@@ -514,12 +518,8 @@ them.
.. option:: --verify-note-sections
- When adding note sections, verify if the section format is valid. This flag is
- turned on by default for ELF files.
-
-.. option:: --no-verify-note-sections
-
- When adding note sections, do not verify if the section format is valid.
+ When adding note sections, verify if the section format is valid. On by default
+ for ELF files.
.. option:: --weaken-symbol <symbol>, -W
diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index 97f445d498dd0..9692761c3302c 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -298,6 +298,10 @@ Changes to the LLVM tools
jumping in reverse direction with shift+L/R/B). (`#95662
<https://github.com/llvm/llvm-project/pull/95662>`).
+* llvm-objcopy now verifies format of ``.note`` sections for ELF input. This can
+ be disabled by ``--no-verify-note-sections``. (`#90458
+ <https://github.com/llvm/llvm-project/pull/90458>`).
+
Changes to LLDB
---------------------------------
diff --git a/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
index 23a45500b064e..528e7e53d25ff 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
@@ -3,22 +3,22 @@
## Add [namesz, descsz, type, name, desc] for a build id.
## Notes should be padded on 8 bytes.
-# RUN: echo -e -n "\x04\x00\x00\x00" > %t-miss-padding-note.bin
-# RUN: echo -e -n "\x07\x00\x00\x00" >> %t-miss-padding-note.bin
-# RUN: echo -e -n "\x03\x00\x00\x00" >> %t-miss-padding-note.bin
-# RUN: echo -e -n "GNU\x00" >> %t-miss-padding-note.bin
-# RUN: echo -e -n "\x0c\x0d\x0e" >> %t-miss-padding-note.bin
+# RUN: printf "\x04\x00\x00\x00" > %t-miss-padding-note.bin
+# RUN: printf "\x07\x00\x00\x00" >> %t-miss-padding-note.bin
+# RUN: printf "\x03\x00\x00\x00" >> %t-miss-padding-note.bin
+# RUN: printf "GNU\x00" >> %t-miss-padding-note.bin
+# RUN: printf "\x0c\x0d\x0e" >> %t-miss-padding-note.bin
## The namesz field bit is incorrect.
-# RUN: echo -e -n "\x08\x00\x00\x00" > %t-invalid-size-note.bin
-# RUN: echo -e -n "\x07\x00\x00\x00" >> %t-invalid-size-note.bin
-# RUN: echo -e -n "\x03\x00\x00\x00" >> %t-invalid-size-note.bin
-# RUN: echo -e -n "GNU\x00" >> %t-invalid-size-note.bin
-# RUN: echo -e -n "\x0c\x0d\x0e\x00" >> %t-invalid-size-note.bin
+# RUN: printf "\x08\x00\x00\x00" > %t-invalid-size-note.bin
+# RUN: printf "\x07\x00\x00\x00" >> %t-invalid-size-note.bin
+# RUN: printf "\x03\x00\x00\x00" >> %t-invalid-size-note.bin
+# RUN: printf "GNU\x00" >> %t-invalid-size-note.bin
+# RUN: printf "\x0c\x0d\x0e\x00" >> %t-invalid-size-note.bin
## Missing type field.
-# RUN: echo -e -n "\x08\x00\x00\x00" > %t-short-note.bin
-# RUN: echo -e -n "\x07\x00\x00\x00" >> %t-short-note.bin
+# RUN: printf "\x08\x00\x00\x00" > %t-short-note.bin
+# RUN: printf "\x07\x00\x00\x00" >> %t-short-note.bin
# RUN: yaml2obj %s -o %t.o
@@ -32,10 +32,10 @@
# RUN: not llvm-objcopy --add-section=.note.short=%t-short-note.bin %t.o %t-with-note.o 2>&1 | FileCheck --check-prefix=CHECK-SHORT %s
# CHECK-SHORT: .note.short data must be either empty or at least 12 bytes long
-## test that verification can be disabled
+## Test that verification can be disabled.
# RUN: llvm-objcopy --add-section=.note.short=%t-short-note.bin --no-verify-note-sections %t.o %t-with-note.o
-## test that last argument has precedence
+## Test that last argument has precedence.
# RUN: llvm-objcopy --add-section=.note.short=%t-short-note.bin --verify-note-sections --no-verify-note-sections %t.o %t-with-note.o
!ELF
@@ -43,4 +43,3 @@ FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
Type: ET_REL
-
>From a9e09b87c9091f99006b7475c3ba1b3ffb94efb9 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Tue, 18 Jun 2024 22:30:04 +0200
Subject: [PATCH 6/8] fixup! [llvm-objcopy] Error with relevant message when
adding invalid notes
---
llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp | 8 ++++++--
llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test | 1 +
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
index 26ca3e84166e6..460b1fa42ba12 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
+++ b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
@@ -657,10 +657,14 @@ static Error verifyNoteSection(StringRef Name, endianness Endianness,
NameSizeValue = byteswap(NameSizeValue);
DescSizeValue = byteswap(DescSizeValue);
}
+
+ // NameSizeValue and DescSizeValue need to be padded to 4.
+ auto iceil4 = [](auto v) { return (v + 3) / 4 * 4; };
+
uint64_t ExpectedDataSize =
/*NameSize=*/4 + /*DescSize=*/4 + /*Type=*/4 +
- /*Name=*/(NameSizeValue + 3) / 4 * 4 +
- /*Desc=*/(DescSizeValue + 3) / 4 * 4;
+ /*Name=*/iceil4(NameSizeValue) +
+ /*Desc=*/iceil4(DescSizeValue);
uint64_t ActualDataSize = Data.size();
if (ActualDataSize != ExpectedDataSize) {
std::string msg;
diff --git a/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
index 528e7e53d25ff..00d65a813b4d3 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
@@ -37,6 +37,7 @@
## Test that last argument has precedence.
# RUN: llvm-objcopy --add-section=.note.short=%t-short-note.bin --verify-note-sections --no-verify-note-sections %t.o %t-with-note.o
+# RUN: not llvm-objcopy --add-section=.note.short=%t-short-note.bin --no-verify-note-sections --verify-note-sections %t.o %t-with-note.o 2>&1 | FileCheck --check-prefix=CHECK-SHORT %s
!ELF
FileHeader:
>From bb4ca67091245a0282903644f6c3ee6f3bb0c5c9 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Wed, 10 Jul 2024 17:30:45 +0200
Subject: [PATCH 7/8] fixup! [llvm-objcopy] Error with relevant message when
adding invalid notes
---
llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
index 3af955e089fac..670bc45b4e0ec 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
+++ b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
@@ -658,13 +658,10 @@ static Error verifyNoteSection(StringRef Name, endianness Endianness,
DescSizeValue = byteswap(DescSizeValue);
}
- // NameSizeValue and DescSizeValue need to be padded to 4.
- auto iceil4 = [](auto v) { return (v + 3) / 4 * 4; };
-
uint64_t ExpectedDataSize =
/*NameSize=*/4 + /*DescSize=*/4 + /*Type=*/4 +
- /*Name=*/iceil4(NameSizeValue) +
- /*Desc=*/iceil4(DescSizeValue);
+ /*Name=*/alignTo(NameSizeValue, 4) +
+ /*Desc=*/alignTo(DescSizeValue, 4);
uint64_t ActualDataSize = Data.size();
if (ActualDataSize != ExpectedDataSize) {
std::string msg;
>From 8c4abe2478f8b7f34f1cd596aa571d7475456c54 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Wed, 10 Jul 2024 23:23:10 +0200
Subject: [PATCH 8/8] fixup! [llvm-objcopy] Error with relevant message when
adding invalid notes
---
llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp | 9 ++-------
llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test | 8 ++++++++
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
index 670bc45b4e0ec..075455c034154 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
+++ b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
@@ -650,13 +650,8 @@ static Error verifyNoteSection(StringRef Name, endianness Endianness,
ArrayRef<uint8_t> NameSize = Data.slice(0, 4);
ArrayRef<uint8_t> DescSize = Data.slice(4, 4);
- uint32_t NameSizeValue = *reinterpret_cast<const uint32_t *>(NameSize.data());
- uint32_t DescSizeValue = *reinterpret_cast<const uint32_t *>(DescSize.data());
-
- if (Endianness != endianness::native) {
- NameSizeValue = byteswap(NameSizeValue);
- DescSizeValue = byteswap(DescSizeValue);
- }
+ uint32_t NameSizeValue = support::endian::read32(NameSize.data(), Endianness);
+ uint32_t DescSizeValue = support::endian::read32(DescSize.data(), Endianness);
uint64_t ExpectedDataSize =
/*NameSize=*/4 + /*DescSize=*/4 + /*Type=*/4 +
diff --git a/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
index 00d65a813b4d3..eeec7eef320e2 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/add-invalid-note.test
@@ -20,6 +20,12 @@
# RUN: printf "\x08\x00\x00\x00" > %t-short-note.bin
# RUN: printf "\x07\x00\x00\x00" >> %t-short-note.bin
+## Test compatibility with .note.gnu.property
+# RUN: printf "\x04\x00\x00\x00\x40\x00\x00\x00\x05\x00\x00\x00\x47\x4e\x55\x00" > %t-gnu-note-property.bin
+# RUN: printf "\x02\x00\x00\xc0\x04\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00" >> %t-gnu-note-property.bin
+# RUN: printf "\x01\x00\x01\xc0\x04\x00\x00\x00\x0b\x00\x00\x00\x00\x00\x00\x00" >> %t-gnu-note-property.bin
+# RUN: printf "\x02\x00\x01\xc0\x04\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00" >> %t-gnu-note-property.bin
+
# RUN: yaml2obj %s -o %t.o
## Test each invalid note.
@@ -32,6 +38,8 @@
# RUN: not llvm-objcopy --add-section=.note.short=%t-short-note.bin %t.o %t-with-note.o 2>&1 | FileCheck --check-prefix=CHECK-SHORT %s
# CHECK-SHORT: .note.short data must be either empty or at least 12 bytes long
+# RUN: llvm-objcopy --add-section=.gnu.note.property=%t-gnu-note-property.bin %t.o %t-with-gnu-note-property.o
+
## Test that verification can be disabled.
# RUN: llvm-objcopy --add-section=.note.short=%t-short-note.bin --no-verify-note-sections %t.o %t-with-note.o
More information about the llvm-commits
mailing list