[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
labath added a comment. Sorry, I have been a bit busy. I've committed this in now. Thank you for the patch. Repository: rL LLVM https://reviews.llvm.org/D29095 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
This revision was automatically updated to reflect the committed changes.
Closed by commit rL293714: Open ELF core dumps with more than 64K sections
(authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D29095?vs=86148=86507#toc
Repository:
rL LLVM
https://reviews.llvm.org/D29095
Files:
lldb/trunk/source/Plugins/ObjectFile/ELF/ELFHeader.cpp
lldb/trunk/source/Plugins/ObjectFile/ELF/ELFHeader.h
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/trunk/unittests/CMakeLists.txt
lldb/trunk/unittests/ObjectFile/CMakeLists.txt
lldb/trunk/unittests/ObjectFile/ELF/CMakeLists.txt
lldb/trunk/unittests/ObjectFile/ELF/TestELFHeader.cpp
Index: lldb/trunk/unittests/CMakeLists.txt
===
--- lldb/trunk/unittests/CMakeLists.txt
+++ lldb/trunk/unittests/CMakeLists.txt
@@ -53,6 +53,7 @@
add_subdirectory(Host)
add_subdirectory(Interpreter)
add_subdirectory(Language)
+add_subdirectory(ObjectFile)
add_subdirectory(Platform)
add_subdirectory(Process)
add_subdirectory(ScriptInterpreter)
Index: lldb/trunk/unittests/ObjectFile/ELF/TestELFHeader.cpp
===
--- lldb/trunk/unittests/ObjectFile/ELF/TestELFHeader.cpp
+++ lldb/trunk/unittests/ObjectFile/ELF/TestELFHeader.cpp
@@ -0,0 +1,62 @@
+//===-- TestELFHeader.cpp ---*- C++ -*-===//
+//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Plugins/ObjectFile/ELF/ELFHeader.h"
+#include "lldb/Core/DataExtractor.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+
+TEST(ELFHeader, ParseHeaderExtension) {
+ uint8_t data[] = {
+ // e_ident
+ 0x7f, 0x45, 0x4c, 0x46, 0x02, 0x01, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ // e_type, e_machine, e_version, e_entry
+ 0x03, 0x00, 0x3e, 0x00, 0x01, 0x00, 0x00, 0x00, 0x90, 0x48, 0x40, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ // e_phoff, e_shoff
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ // e_flags, e_ehsize, e_phentsize, e_phnum, e_shentsize, e_shnum,
+ // e_shstrndx
+ 0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x38, 0x00, 0xff, 0xff, 0x40, 0x00,
+ 0x00, 0x00, 0xff, 0xff,
+
+ // sh_name, sh_type, sh_flags
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ // sh_addr, sh_offset
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ // sh_size, sh_link, sh_info
+ 0x23, 0x45, 0x67, 0x00, 0x00, 0x00, 0x00, 0x00, 0x34, 0x56, 0x78, 0x00,
+ 0x12, 0x34, 0x56, 0x00,
+
+ // sh_addralign, sh_entsize
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ };
+
+ DataExtractor extractor(data, sizeof data, eByteOrderLittle, 8);
+ elf::ELFHeader header;
+ offset_t offset = 0;
+ ASSERT_TRUE(header.Parse(extractor, ));
+ EXPECT_EQ(0x563412u, header.e_phnum);
+ EXPECT_EQ(0x785634u, header.e_shstrndx);
+ EXPECT_EQ(0x674523u, header.e_shnum);
+}
Index: lldb/trunk/unittests/ObjectFile/ELF/CMakeLists.txt
===
--- lldb/trunk/unittests/ObjectFile/ELF/CMakeLists.txt
+++ lldb/trunk/unittests/ObjectFile/ELF/CMakeLists.txt
@@ -0,0 +1,3 @@
+add_lldb_unittest(ObjectFileELFTests
+ TestELFHeader.cpp
+ )
Index: lldb/trunk/unittests/ObjectFile/CMakeLists.txt
===
--- lldb/trunk/unittests/ObjectFile/CMakeLists.txt
+++ lldb/trunk/unittests/ObjectFile/CMakeLists.txt
@@ -0,0 +1 @@
+add_subdirectory(ELF)
Index: lldb/trunk/source/Plugins/ObjectFile/ELF/ELFHeader.h
===
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ELFHeader.h
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ELFHeader.h
@@ -24,6 +24,7 @@
#include "llvm/Support/ELF.h"
#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-types.h"
namespace lldb_private {
class DataExtractor;
@@ -65,10 +66,17 @@
elf_half e_machine; ///< Target architecture.
elf_half e_ehsize;///< Byte size of the ELF header.
elf_half e_phentsize; ///< Size of a program header table entry.
- elf_half e_phnum; ///< Number of program header entries.
+ elf_half e_phnum_hdr; ///< Number of program header entries.
elf_half e_shentsize; ///< Size of a section header table entry.
- elf_half e_shnum; ///< Number of section header entries.
- elf_half e_shstrndx;
[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
EugeneBi added a comment. So, what's now? Can somebody commit it, please? I do not see any option to do it myself. https://reviews.llvm.org/D29095 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
EugeneBi updated this revision to Diff 86148.
EugeneBi added a comment.
Used named constants for SHN_UNDEF and SHN_XINDEX sentinels.
Unfortunately ELF.h lacks definition of PN_XNUM, so left it as a comment.
https://reviews.llvm.org/D29095
Files:
source/Plugins/ObjectFile/ELF/ELFHeader.cpp
source/Plugins/ObjectFile/ELF/ELFHeader.h
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
source/Plugins/Process/elf-core/ProcessElfCore.cpp
unittests/CMakeLists.txt
unittests/ObjectFile/CMakeLists.txt
unittests/ObjectFile/ELF/CMakeLists.txt
unittests/ObjectFile/ELF/TestELFHeader.cpp
Index: unittests/ObjectFile/ELF/TestELFHeader.cpp
===
--- /dev/null
+++ unittests/ObjectFile/ELF/TestELFHeader.cpp
@@ -0,0 +1,62 @@
+//===-- TestELFHeader.cpp ---*- C++ -*-===//
+//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Plugins/ObjectFile/ELF/ELFHeader.h"
+#include "lldb/Core/DataExtractor.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+
+TEST(ELFHeader, ParseHeaderExtension) {
+ uint8_t data[] = {
+ // e_ident
+ 0x7f, 0x45, 0x4c, 0x46, 0x02, 0x01, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ // e_type, e_machine, e_version, e_entry
+ 0x03, 0x00, 0x3e, 0x00, 0x01, 0x00, 0x00, 0x00, 0x90, 0x48, 0x40, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ // e_phoff, e_shoff
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ // e_flags, e_ehsize, e_phentsize, e_phnum, e_shentsize, e_shnum,
+ // e_shstrndx
+ 0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x38, 0x00, 0xff, 0xff, 0x40, 0x00,
+ 0x00, 0x00, 0xff, 0xff,
+
+ // sh_name, sh_type, sh_flags
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ // sh_addr, sh_offset
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+
+ // sh_size, sh_link, sh_info
+ 0x23, 0x45, 0x67, 0x00, 0x00, 0x00, 0x00, 0x00, 0x34, 0x56, 0x78, 0x00,
+ 0x12, 0x34, 0x56, 0x00,
+
+ // sh_addralign, sh_entsize
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ };
+
+ DataExtractor extractor(data, sizeof data, eByteOrderLittle, 8);
+ elf::ELFHeader header;
+ offset_t offset = 0;
+ ASSERT_TRUE(header.Parse(extractor, ));
+ EXPECT_EQ(0x563412u, header.e_phnum);
+ EXPECT_EQ(0x785634u, header.e_shstrndx);
+ EXPECT_EQ(0x674523u, header.e_shnum);
+}
Index: unittests/ObjectFile/ELF/CMakeLists.txt
===
--- /dev/null
+++ unittests/ObjectFile/ELF/CMakeLists.txt
@@ -0,0 +1,3 @@
+add_lldb_unittest(ObjectFileELFTests
+ TestELFHeader.cpp
+ )
Index: unittests/ObjectFile/CMakeLists.txt
===
--- /dev/null
+++ unittests/ObjectFile/CMakeLists.txt
@@ -0,0 +1 @@
+add_subdirectory(ELF)
Index: unittests/CMakeLists.txt
===
--- unittests/CMakeLists.txt
+++ unittests/CMakeLists.txt
@@ -53,6 +53,7 @@
add_subdirectory(Host)
add_subdirectory(Interpreter)
add_subdirectory(Language)
+add_subdirectory(ObjectFile)
add_subdirectory(Platform)
add_subdirectory(Process)
add_subdirectory(ScriptInterpreter)
Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -56,6 +56,8 @@
lldb::ProcessSP process_sp;
if (crash_file) {
// Read enough data for a ELF32 header or ELF64 header
+// Note: Here we care about e_type field only, so it is safe
+// to ignore possible presence of the header extension.
const size_t header_size = sizeof(llvm::ELF::Elf64_Ehdr);
lldb::DataBufferSP data_sp(crash_file->ReadFileContents(0, header_size));
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -610,7 +610,8 @@
DataExtractor data;
data.SetData(data_sp);
elf::ELFHeader header;
-if (header.Parse(data, _offset)) {
+lldb::offset_t header_offset = data_offset;
+if (header.Parse(data, _offset)) {
if (data_sp) {
ModuleSpec spec(file);
@@ -645,10 +646,24 @@
__FUNCTION__, file.GetPath().c_str());
}
+
[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
EugeneBi added inline comments.
Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.cpp:108
+ if (ok) {
+if (e_phnum_hdr == 0x)
+ e_phnum = section_zero.sh_info;
EugeneBi wrote:
> EugeneBi wrote:
> > davidb wrote:
> > > Would this make more sense to compare against a named constant
> > > ELF::PN_XNUM?
> > Would be nice, but - where is it defined? I tried ELF::PN_XNUM,
> > elf::PN_XNUM, PN_XNUM - compiler barks anyway.
> >
> OK, I see the other two are defined in Support/ELF.h, I'll put PN_XNUM there
> then.
Oh, this ELF.h is a part of a different git repo - it lives in llvm. I am not
sure what to do about it.
https://reviews.llvm.org/D29095
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
EugeneBi added inline comments.
Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.cpp:108
+ if (ok) {
+if (e_phnum_hdr == 0x)
+ e_phnum = section_zero.sh_info;
EugeneBi wrote:
> davidb wrote:
> > Would this make more sense to compare against a named constant ELF::PN_XNUM?
> Would be nice, but - where is it defined? I tried ELF::PN_XNUM, elf::PN_XNUM,
> PN_XNUM - compiler barks anyway.
>
OK, I see the other two are defined in Support/ELF.h, I'll put PN_XNUM there
then.
https://reviews.llvm.org/D29095
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
EugeneBi added inline comments.
Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.cpp:108
+ if (ok) {
+if (e_phnum_hdr == 0x)
+ e_phnum = section_zero.sh_info;
davidb wrote:
> Would this make more sense to compare against a named constant ELF::PN_XNUM?
Would be nice, but - where is it defined? I tried ELF::PN_XNUM, elf::PN_XNUM,
PN_XNUM - compiler barks anyway.
https://reviews.llvm.org/D29095
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
davidb added inline comments.
Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.cpp:108
+ if (ok) {
+if (e_phnum_hdr == 0x)
+ e_phnum = section_zero.sh_info;
Would this make more sense to compare against a named constant ELF::PN_XNUM?
Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.cpp:110
+ e_phnum = section_zero.sh_info;
+if (e_shnum_hdr == 0)
+ e_shnum = section_zero.sh_size;
And this with SHN_UNDEF?
Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.cpp:112
+ e_shnum = section_zero.sh_size;
+if (e_shstrndx_hdr == 0x)
+ e_shstrndx = section_zero.sh_link;
and this with ELF::SHN_XINDEX.
https://reviews.llvm.org/D29095
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
labath added a subscriber: lldb-commits. labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. I've added a quick test to demonstrate what I had in mind. > Unfortunately, unlike release_39 branch, I cannot open my core dump, but the > problem seems unrelated. So I think I want to push it through and continue > investigation what is not working this time. That's kinda the reason I wanted to add tests. :) This test would not have actually caught your new problem, but it could add at least some protection for the future, as I doubt many people will run into real files with this many sections. I think this should be ok to put in if there are no objections from anyone else. Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.h:71 elf_half e_phentsize;///< Size of a program header table entry. -elf_half e_phnum;///< Number of program header entries. +elf_half e_phnum_hdr;///< Number of program header entries. elf_half e_shentsize;///< Size of a section header table entry. EugeneBi wrote: > labath wrote: > > I am wondering whether these is any use in keeping these old values. It > > sounds like it's a recipe for someone getting confused and using the wrong > > ones. What do you think about just deleting these? > I will leave it to you - just tell me what you prefer. I see both pros and > cons of my current code. Ok, let's leave it this way then. https://reviews.llvm.org/D29095 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
