Move function info word into its own data structure #153627
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The sframe generator needs to construct this word separately from FDEs themselves, so split them into a separate data structure.
|
@llvm/pr-subscribers-llvm-binary-utilities Author: None (Sterling-Augustine) ChangesThe sframe generator needs to construct this word separately from FDEs themselves, so split them into a separate data structure. Full diff: https://github.com/llvm/llvm-project/pull/153627.diff 4 Files Affected:
diff --git a/llvm/include/llvm/BinaryFormat/SFrame.h b/llvm/include/llvm/BinaryFormat/SFrame.h
index 74e47ea8acca9..a02b1589f81c9 100644
--- a/llvm/include/llvm/BinaryFormat/SFrame.h
+++ b/llvm/include/llvm/BinaryFormat/SFrame.h
@@ -104,14 +104,8 @@ template <endianness E> struct Header {
detail::packed<uint32_t, E> FREOff;
};
-template <endianness E> struct FuncDescEntry {
- detail::packed<int32_t, E> StartAddress;
- detail::packed<uint32_t, E> Size;
- detail::packed<uint32_t, E> StartFREOff;
- detail::packed<uint32_t, E> NumFREs;
- detail::packed<uint8_t, E> Info;
- detail::packed<uint8_t, E> RepSize;
- detail::packed<uint16_t, E> Padding2;
+struct FDEInfo {
+ uint8_t Info;
uint8_t getPAuthKey() const { return (Info >> 5) & 1; }
FDEType getFDEType() const { return static_cast<FDEType>((Info >> 4) & 1); }
@@ -125,6 +119,16 @@ template <endianness E> struct FuncDescEntry {
}
};
+template <endianness E> struct FuncDescEntry {
+ detail::packed<int32_t, E> StartAddress;
+ detail::packed<uint32_t, E> Size;
+ detail::packed<uint32_t, E> StartFREOff;
+ detail::packed<uint32_t, E> NumFREs;
+ FDEInfo Info;
+ detail::packed<uint8_t, E> RepSize;
+ detail::packed<uint16_t, E> Padding2;
+};
+
template <endianness E> struct FREInfo {
detail::packed<uint8_t, E> Info;
diff --git a/llvm/lib/Object/SFrameParser.cpp b/llvm/lib/Object/SFrameParser.cpp
index 847067a7ae989..c49a02907367f 100644
--- a/llvm/lib/Object/SFrameParser.cpp
+++ b/llvm/lib/Object/SFrameParser.cpp
@@ -173,10 +173,10 @@ iterator_range<typename SFrameParser<E>::fre_iterator>
SFrameParser<E>::fres(const sframe::FuncDescEntry<E> &FDE, Error &Err) const {
uint64_t Offset = getFREBase() + FDE.StartFREOff;
fre_iterator BeforeBegin = make_fallible_itr(
- FallibleFREIterator(Data, FDE.getFREType(), -1, FDE.NumFREs, Offset),
+ FallibleFREIterator(Data, FDE.Info.getFREType(), -1, FDE.NumFREs, Offset),
Err);
fre_iterator End = make_fallible_end(
- FallibleFREIterator(Data, FDE.getFREType(), FDE.NumFREs, FDE.NumFREs,
+ FallibleFREIterator(Data, FDE.Info.getFREType(), FDE.NumFREs, FDE.NumFREs,
/*Offset=*/0));
return {++BeforeBegin, End};
}
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index ed29e0bf3f7cd..731250532402a 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -6505,12 +6505,13 @@ void ELFDumper<ELFT>::printSFrameFDEs(
{
DictScope InfoScope(W, "Info");
- W.printEnum("FRE Type", It->getFREType(), sframe::getFRETypes());
- W.printEnum("FDE Type", It->getFDEType(), sframe::getFDETypes());
+ W.printEnum("FRE Type", It->Info.getFREType(), sframe::getFRETypes());
+ W.printEnum("FDE Type", It->Info.getFDEType(), sframe::getFDETypes());
switch (Parser.getHeader().ABIArch) {
case sframe::ABI::AArch64EndianBig:
case sframe::ABI::AArch64EndianLittle:
- W.printEnum("PAuth Key", sframe::AArch64PAuthKey(It->getPAuthKey()),
+ W.printEnum("PAuth Key",
+ sframe::AArch64PAuthKey(It->Info.getPAuthKey()),
sframe::getAArch64PAuthKeys());
break;
case sframe::ABI::AMD64EndianLittle:
@@ -6518,12 +6519,13 @@ void ELFDumper<ELFT>::printSFrameFDEs(
break;
}
- W.printHex("Raw", It->Info);
+ W.printHex("Raw", It->Info.Info);
}
W.printHex(
("Repetitive block size" +
- Twine(It->getFDEType() == sframe::FDEType::PCMask ? "" : " (unused)"))
+ Twine(It->Info.getFDEType() == sframe::FDEType::PCMask ? ""
+ : " (unused)"))
.str(),
It->RepSize);
@@ -6534,10 +6536,11 @@ void ELFDumper<ELFT>::printSFrameFDEs(
for (const typename SFrameParser<ELFT::Endianness>::FrameRowEntry &FRE :
Parser.fres(*It, Err)) {
DictScope FREScope(W, "Frame Row Entry");
- W.printHex(
- "Start Address",
- (It->getFDEType() == sframe::FDEType::PCInc ? FDEStartAddress : 0) +
- FRE.StartAddress);
+ W.printHex("Start Address",
+ (It->Info.getFDEType() == sframe::FDEType::PCInc
+ ? FDEStartAddress
+ : 0) +
+ FRE.StartAddress);
W.printBoolean("Return Address Signed", FRE.Info.isReturnAddressSigned());
W.printEnum("Offset Size", FRE.Info.getOffsetSize(),
sframe::getFREOffsets());
diff --git a/llvm/unittests/BinaryFormat/SFrameTest.cpp b/llvm/unittests/BinaryFormat/SFrameTest.cpp
index 394e382e041e9..ab7b0fe20b750 100644
--- a/llvm/unittests/BinaryFormat/SFrameTest.cpp
+++ b/llvm/unittests/BinaryFormat/SFrameTest.cpp
@@ -54,28 +54,28 @@ TYPED_TEST_SUITE(SFrameTest, Types, NameGenerator);
TYPED_TEST(SFrameTest, FDEFlags) {
FuncDescEntry<TestFixture::Endian> FDE = {};
- EXPECT_EQ(FDE.Info, 0u);
- EXPECT_EQ(FDE.getPAuthKey(), 0);
- EXPECT_EQ(FDE.getFDEType(), FDEType::PCInc);
- EXPECT_EQ(FDE.getFREType(), FREType::Addr1);
-
- FDE.setPAuthKey(1);
- EXPECT_EQ(FDE.Info, 0x20u);
- EXPECT_EQ(FDE.getPAuthKey(), 1);
- EXPECT_EQ(FDE.getFDEType(), FDEType::PCInc);
- EXPECT_EQ(FDE.getFREType(), FREType::Addr1);
-
- FDE.setFDEType(FDEType::PCMask);
- EXPECT_EQ(FDE.Info, 0x30u);
- EXPECT_EQ(FDE.getPAuthKey(), 1);
- EXPECT_EQ(FDE.getFDEType(), FDEType::PCMask);
- EXPECT_EQ(FDE.getFREType(), FREType::Addr1);
-
- FDE.setFREType(FREType::Addr4);
- EXPECT_EQ(FDE.Info, 0x32u);
- EXPECT_EQ(FDE.getPAuthKey(), 1);
- EXPECT_EQ(FDE.getFDEType(), FDEType::PCMask);
- EXPECT_EQ(FDE.getFREType(), FREType::Addr4);
+ EXPECT_EQ(FDE.Info.Info, 0u);
+ EXPECT_EQ(FDE.Info.getPAuthKey(), 0);
+ EXPECT_EQ(FDE.Info.getFDEType(), FDEType::PCInc);
+ EXPECT_EQ(FDE.Info.getFREType(), FREType::Addr1);
+
+ FDE.Info.setPAuthKey(1);
+ EXPECT_EQ(FDE.Info.Info, 0x20u);
+ EXPECT_EQ(FDE.Info.getPAuthKey(), 1);
+ EXPECT_EQ(FDE.Info.getFDEType(), FDEType::PCInc);
+ EXPECT_EQ(FDE.Info.getFREType(), FREType::Addr1);
+
+ FDE.Info.setFDEType(FDEType::PCMask);
+ EXPECT_EQ(FDE.Info.Info, 0x30u);
+ EXPECT_EQ(FDE.Info.getPAuthKey(), 1);
+ EXPECT_EQ(FDE.Info.getFDEType(), FDEType::PCMask);
+ EXPECT_EQ(FDE.Info.getFREType(), FREType::Addr1);
+
+ FDE.Info.setFREType(FREType::Addr4);
+ EXPECT_EQ(FDE.Info.Info, 0x32u);
+ EXPECT_EQ(FDE.Info.getPAuthKey(), 1);
+ EXPECT_EQ(FDE.Info.getFDEType(), FDEType::PCMask);
+ EXPECT_EQ(FDE.Info.getFREType(), FREType::Addr4);
}
TYPED_TEST(SFrameTest, FREFlags) {
|
labath
approved these changes
Aug 14, 2025
| detail::packed<uint8_t, E> RepSize; | ||
| detail::packed<uint16_t, E> Padding2; | ||
| struct FDEInfo { | ||
| uint8_t Info; |
There was a problem hiding this comment.
FREInfo uses the endian types, even though they're kind of unnecessary. The only reason for that was consistency, so I don't really have a problem with using uint8_t directly, but if you do that here, please also change FREInfo to match.
There was a problem hiding this comment.
Switched it to be endian-based.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The sframe generator needs to construct this word separately from FDEs themselves, so split them into a separate data structure.