[libunwind] [libunwind] Consistently pass start+length to decodeEHHdr() (PR #68813)
https://github.com/arichardson edited https://github.com/llvm/llvm-project/pull/68813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libunwind] Consistently pass start+length to decodeEHHdr() (PR #68813)
https://github.com/arichardson updated
https://github.com/llvm/llvm-project/pull/68813
>From ff360ee7f304424dd0d12d00b8c0ba6801241410 Mon Sep 17 00:00:00 2001
From: Alex Richardson
Date: Wed, 11 Oct 2023 08:34:55 -0700
Subject: [PATCH] [libunwind] Fix wrong end argument passed to decodeEHHdr()
All but one callsite were actually passing start+length arguments.
This should not have any functional change since the end argument is
almost always ignored.
I noticed this while debugging some incorrect error messages being
printed while running the testsuite baremetal (using binaries that did
not have a valid eh_frame_hdr section): the tests print
`libunwind: unsupported .eh_frame_hdr version: 20 at 8000d308` because
libunwind is reading nonsense data for .eh_frame_hdr.
---
libunwind/src/AddressSpace.hpp | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/libunwind/src/AddressSpace.hpp b/libunwind/src/AddressSpace.hpp
index 1abbc822546878d..5551c7d4bef1c56 100644
--- a/libunwind/src/AddressSpace.hpp
+++ b/libunwind/src/AddressSpace.hpp
@@ -414,8 +414,8 @@ static bool checkForUnwindInfoSegment(const Elf_Phdr *phdr,
size_t image_base,
cbdata->sects->dwarf_index_section = eh_frame_hdr_start;
cbdata->sects->dwarf_index_section_length = phdr->p_memsz;
if (EHHeaderParser::decodeEHHdr(
-*cbdata->addressSpace, eh_frame_hdr_start, phdr->p_memsz,
-hdrInfo)) {
+*cbdata->addressSpace, eh_frame_hdr_start,
+eh_frame_hdr_start + phdr->p_memsz, hdrInfo)) {
// .eh_frame_hdr records the start of .eh_frame, but not its size.
// Rely on a zero terminator to find the end of the section.
cbdata->sects->dwarf_section = hdrInfo.eh_frame_ptr;
@@ -638,7 +638,8 @@ inline bool LocalAddressSpace::findUnwindSections(pint_t
targetAddr,
info.dwarf_index_section_length = SIZE_MAX;
EHHeaderParser::EHHeaderInfo hdrInfo;
if (!EHHeaderParser::decodeEHHdr(
-*this, info.dwarf_index_section, info.dwarf_index_section_length,
+*this, info.dwarf_index_section,
+info.dwarf_index_section + info.dwarf_index_section_length,
hdrInfo)) {
return false;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libunwind] Consistently pass start+length to decodeEHHdr() (PR #68813)
https://github.com/compnerd requested changes to this pull request. Can we merge this into #68815 please? https://github.com/llvm/llvm-project/pull/68813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libunwind] Consistently pass start+length to decodeEHHdr() (PR #68813)
llvmbot wrote:
@llvm/pr-subscribers-libunwind
Author: Alexander Richardson (arichardson)
Changes
It previously took a start+end pint_t, but all but one callsite were actually
passing start+length arguments. This should not have any functional change
since the end argument is almost always ignored. I noticed this while debugging
some incorrect error messages being printed while running the testsuite
baremetal (using binaries that did not have a valid eh_frame_hdr section): the
tests print `libunwind: unsupported .eh_frame_hdr version: 20 at 8000d308`
because libunwind is reading nonsense data for .eh_frame_hdr.
---
Full diff: https://github.com/llvm/llvm-project/pull/68813.diff
1 Files Affected:
- (modified) libunwind/src/EHHeaderParser.hpp (+4-3)
``diff
diff --git a/libunwind/src/EHHeaderParser.hpp b/libunwind/src/EHHeaderParser.hpp
index ed4317c89055c9e..2162178e10bb2fb 100644
--- a/libunwind/src/EHHeaderParser.hpp
+++ b/libunwind/src/EHHeaderParser.hpp
@@ -35,7 +35,7 @@ template class EHHeaderParser {
uint8_t table_enc;
};
- static bool decodeEHHdr(A &addressSpace, pint_t ehHdrStart, pint_t ehHdrEnd,
+ static bool decodeEHHdr(A &addressSpace, pint_t ehHdrStart, size_t ehHdrSize,
EHHeaderInfo &ehHdrInfo);
static bool findFDE(A &addressSpace, pint_t pc, pint_t ehHdrStart,
uint32_t sectionLength,
@@ -53,8 +53,9 @@ template class EHHeaderParser {
template
bool EHHeaderParser::decodeEHHdr(A &addressSpace, pint_t ehHdrStart,
-pint_t ehHdrEnd, EHHeaderInfo &ehHdrInfo) {
+size_t ehHdrSize, EHHeaderInfo &ehHdrInfo)
{
pint_t p = ehHdrStart;
+ pint_t ehHdrEnd = ehHdrStart + ehHdrSize;
uint8_t version = addressSpace.get8(p++);
if (version != 1) {
_LIBUNWIND_LOG("unsupported .eh_frame_hdr version: %" PRIu8 " at %" PRIx64,
@@ -106,7 +107,7 @@ bool EHHeaderParser::findFDE(A &addressSpace, pint_t pc,
pint_t ehHdrStart,
pint_t ehHdrEnd = ehHdrStart + sectionLength;
EHHeaderParser::EHHeaderInfo hdrInfo;
- if (!EHHeaderParser::decodeEHHdr(addressSpace, ehHdrStart, ehHdrEnd,
+ if (!EHHeaderParser::decodeEHHdr(addressSpace, ehHdrStart, sectionLength,
hdrInfo))
return false;
``
https://github.com/llvm/llvm-project/pull/68813
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libunwind] Consistently pass start+length to decodeEHHdr() (PR #68813)
https://github.com/arichardson created
https://github.com/llvm/llvm-project/pull/68813
It previously took a start+end pint_t, but all but one callsite were actually
passing start+length arguments. This should not have any functional change
since the end argument is almost always ignored. I noticed this while debugging
some incorrect error messages being printed while running the testsuite
baremetal (using binaries that did not have a valid eh_frame_hdr section): the
tests print `libunwind: unsupported .eh_frame_hdr version: 20 at 8000d308`
because libunwind is reading nonsense data for .eh_frame_hdr.
>From f5b7f1fd65135412f24dc4dd7887911fb0e4c2ed Mon Sep 17 00:00:00 2001
From: Alex Richardson
Date: Wed, 11 Oct 2023 08:34:55 -0700
Subject: [PATCH] [libunwind] Consistently pass start+length to decodeEHHdr()
It previously took a start+end pint_t, but all but one callsite were
actually passing start+length arguments. This should not have any
functional change since the end argument is almost always ignored.
I noticed this while debugging some incorrect error messages being
printed while running the testsuite baremetal (using binaries that did
not have a valid eh_frame_hdr section): the tests print
`libunwind: unsupported .eh_frame_hdr version: 20 at 8000d308` because
libunwind is reading nonsense data for .eh_frame_hdr.
---
libunwind/src/EHHeaderParser.hpp | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/libunwind/src/EHHeaderParser.hpp b/libunwind/src/EHHeaderParser.hpp
index ed4317c89055c9e..2162178e10bb2fb 100644
--- a/libunwind/src/EHHeaderParser.hpp
+++ b/libunwind/src/EHHeaderParser.hpp
@@ -35,7 +35,7 @@ template class EHHeaderParser {
uint8_t table_enc;
};
- static bool decodeEHHdr(A &addressSpace, pint_t ehHdrStart, pint_t ehHdrEnd,
+ static bool decodeEHHdr(A &addressSpace, pint_t ehHdrStart, size_t ehHdrSize,
EHHeaderInfo &ehHdrInfo);
static bool findFDE(A &addressSpace, pint_t pc, pint_t ehHdrStart,
uint32_t sectionLength,
@@ -53,8 +53,9 @@ template class EHHeaderParser {
template
bool EHHeaderParser::decodeEHHdr(A &addressSpace, pint_t ehHdrStart,
-pint_t ehHdrEnd, EHHeaderInfo &ehHdrInfo) {
+size_t ehHdrSize, EHHeaderInfo &ehHdrInfo)
{
pint_t p = ehHdrStart;
+ pint_t ehHdrEnd = ehHdrStart + ehHdrSize;
uint8_t version = addressSpace.get8(p++);
if (version != 1) {
_LIBUNWIND_LOG("unsupported .eh_frame_hdr version: %" PRIu8 " at %" PRIx64,
@@ -106,7 +107,7 @@ bool EHHeaderParser::findFDE(A &addressSpace, pint_t pc,
pint_t ehHdrStart,
pint_t ehHdrEnd = ehHdrStart + sectionLength;
EHHeaderParser::EHHeaderInfo hdrInfo;
- if (!EHHeaderParser::decodeEHHdr(addressSpace, ehHdrStart, ehHdrEnd,
+ if (!EHHeaderParser::decodeEHHdr(addressSpace, ehHdrStart, sectionLength,
hdrInfo))
return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
