Skip to content

Conversation

kazutakahirata
Copy link
Contributor

Without this patch, we pass Endian as one of the parameters to the
constructor of DataExtractor. The problem is that Endian is of:

enum endianness {big, little, native};

whereas the constructor is expecting "bool IsLittleEndian". That is,
we are relying on an implicit conversion to convert big and little to
false and true, respectively.

When we migrate llvm::support::endianness to std::endian in future, we
can no longer rely on an implicit conversion because std::endian is
declared with "enum class". Even if we could, the conversion would
not be guaranteed to work because, for example, libcxx defines:

enum class endian {
little = 0xDEAD,
big = 0xFACE,
:

where big and little are not boolean values.

This patch fixes the problem by properly converting Endian to a
boolean value.

Without this patch, we pass Endian as one of the parameters to the
constructor of DataExtractor.  The problem is that Endian is of:

  enum endianness {big, little, native};

whereas the constructor is expecting "bool IsLittleEndian".  That is,
we are relying on an implicit conversion to convert big and little to
false and true, respectively.

When we migrate llvm::support::endianness to std::endian in future, we
can no longer rely on an implicit conversion because std::endian is
declared with "enum class".  Even if we could, the conversion would
not be guaranteed to work because, for example, libcxx defines:

  enum class endian {
    little = 0xDEAD,
    big = 0xFACE,
    :

where big and little are not boolean values.

This patch fixes the problem by properly converting Endian to a
boolean value.
@llvmbot
Copy link
Member

llvmbot commented Oct 1, 2023

@llvm/pr-subscribers-debuginfo

Changes

Without this patch, we pass Endian as one of the parameters to the
constructor of DataExtractor. The problem is that Endian is of:

enum endianness {big, little, native};

whereas the constructor is expecting "bool IsLittleEndian". That is,
we are relying on an implicit conversion to convert big and little to
false and true, respectively.

When we migrate llvm::support::endianness to std::endian in future, we
can no longer rely on an implicit conversion because std::endian is
declared with "enum class". Even if we could, the conversion would
not be guaranteed to work because, for example, libcxx defines:

enum class endian {
little = 0xDEAD,
big = 0xFACE,
:

where big and little are not boolean values.

This patch fixes the problem by properly converting Endian to a
boolean value.


Full diff: https://github.com/llvm/llvm-project/pull/67904.diff

1 Files Affected:

  • (modified) llvm/lib/DebugInfo/GSYM/GsymReader.cpp (+8-2)
diff --git a/llvm/lib/DebugInfo/GSYM/GsymReader.cpp b/llvm/lib/DebugInfo/GSYM/GsymReader.cpp
index 6afaeea8f598e12..689d8f83f4b2204 100644
--- a/llvm/lib/DebugInfo/GSYM/GsymReader.cpp
+++ b/llvm/lib/DebugInfo/GSYM/GsymReader.cpp
@@ -261,7 +261,10 @@ llvm::Expected<FunctionInfo> GsymReader::getFunctionInfo(uint64_t Addr) const {
   // Address info offsets size should have been checked in parse().
   assert(*AddressIndex < AddrInfoOffsets.size());
   auto AddrInfoOffset = AddrInfoOffsets[*AddressIndex];
-  DataExtractor Data(MemBuffer->getBuffer().substr(AddrInfoOffset), Endian, 4);
+  assert((Endian == support::big || Endian == support::little) &&
+         "Endian must be either big or little");
+  DataExtractor Data(MemBuffer->getBuffer().substr(AddrInfoOffset),
+                     Endian == support::little, 4);
   if (std::optional<uint64_t> OptAddr = getAddress(*AddressIndex)) {
     auto ExpectedFI = FunctionInfo::decode(Data, *OptAddr);
     if (ExpectedFI) {
@@ -283,7 +286,10 @@ llvm::Expected<LookupResult> GsymReader::lookup(uint64_t Addr) const {
   // Address info offsets size should have been checked in parse().
   assert(*AddressIndex < AddrInfoOffsets.size());
   auto AddrInfoOffset = AddrInfoOffsets[*AddressIndex];
-  DataExtractor Data(MemBuffer->getBuffer().substr(AddrInfoOffset), Endian, 4);
+  assert((Endian == support::big || Endian == support::little) &&
+         "Endian must be either big or little");
+  DataExtractor Data(MemBuffer->getBuffer().substr(AddrInfoOffset),
+                     Endian == support::little, 4);
   if (std::optional<uint64_t> OptAddr = getAddress(*AddressIndex))
     return FunctionInfo::lookup(Data, *this, *OptAddr, Addr);
   return createStringError(std::errc::invalid_argument,

@kazutakahirata kazutakahirata requested a review from kuhar October 1, 2023 07:22
@kazutakahirata kazutakahirata merged commit 95f4b2a into llvm:main Oct 1, 2023
@kazutakahirata kazutakahirata deleted the pr_endian_gsym branch October 1, 2023 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants