[Lldb-commits] [lldb] r314856 - updating svn:ignore
Author: lemo Date: Tue Oct 3 15:30:02 2017 New Revision: 314856 URL: http://llvm.org/viewvc/llvm-project?rev=314856&view=rev Log: updating svn:ignore Modified: lldb/trunk/ (props changed) Propchange: lldb/trunk/ -- --- svn:ignore (original) +++ svn:ignore Tue Oct 3 15:30:02 2017 @@ -2,3 +2,5 @@ build intermediates llvm llvm-build +.vscode + ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D36598: cmake + xcode: prevent gtests from using includes from project root
This revision was automatically updated to reflect the committed changes.
Closed by commit rL314849: cmake + xcode: prevent gtests from using includes
from project root (authored by penryu).
Changed prior to commit:
https://reviews.llvm.org/D36598?vs=112052&id=117587#toc
Repository:
rL LLVM
https://reviews.llvm.org/D36598
Files:
lldb/trunk/lldb.xcodeproj/project.pbxproj
lldb/trunk/unittests/CMakeLists.txt
lldb/trunk/unittests/Interpreter/TestCompletion.cpp
lldb/trunk/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
lldb/trunk/unittests/Symbol/TestDWARFCallFrameInfo.cpp
lldb/trunk/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
lldb/trunk/unittests/Target/ModuleCacheTest.cpp
lldb/trunk/unittests/TestingSupport/CMakeLists.txt
lldb/trunk/unittests/TestingSupport/MockTildeExpressionResolver.cpp
lldb/trunk/unittests/TestingSupport/MockTildeExpressionResolver.h
lldb/trunk/unittests/TestingSupport/TestUtilities.cpp
lldb/trunk/unittests/TestingSupport/TestUtilities.h
lldb/trunk/unittests/Utility/CMakeLists.txt
lldb/trunk/unittests/Utility/Helpers/CMakeLists.txt
lldb/trunk/unittests/Utility/Helpers/MockTildeExpressionResolver.cpp
lldb/trunk/unittests/Utility/Helpers/MockTildeExpressionResolver.h
lldb/trunk/unittests/Utility/Helpers/TestUtilities.cpp
lldb/trunk/unittests/Utility/Helpers/TestUtilities.h
lldb/trunk/unittests/Utility/StructuredDataTest.cpp
lldb/trunk/unittests/Utility/TildeExpressionResolverTest.cpp
Index: lldb/trunk/lldb.xcodeproj/project.pbxproj
===
--- lldb/trunk/lldb.xcodeproj/project.pbxproj
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj
@@ -876,8 +876,8 @@
966C6B7C18E6A56A0093F5EC /* libz.dylib in Frameworks */ = {isa = PBXBuildFile; fileRef = 966C6B7818E6A56A0093F5EC /* libz.dylib */; };
9694FA711B32AA64005EBB16 /* ABISysV_mips.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9694FA6F1B32AA64005EBB16 /* ABISysV_mips.cpp */; };
9A0FDEA71E8EF5110086B2F5 /* RegisterContextLinux_mips.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9A0FDE971E8EF5010086B2F5 /* RegisterContextLinux_mips.cpp */; };
- 9A1542F91F0EE48600DEA1D8 /* MockTildeExpressionResolver.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9A1542F51F0EE44000DEA1D8 /* MockTildeExpressionResolver.cpp */; };
- 9A1542FA1F0EE48600DEA1D8 /* TestUtilities.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9A1542F71F0EE44000DEA1D8 /* TestUtilities.cpp */; };
+ 9A18903B1F47D5E600394BCA /* MockTildeExpressionResolver.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9A1890321F47D5D400394BCA /* MockTildeExpressionResolver.cpp */; };
+ 9A18903C1F47D5E600394BCA /* TestUtilities.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9A1890341F47D5D400394BCA /* TestUtilities.cpp */; };
9A19A6AF1163BBB200E0D453 /* SBValue.h in Headers */ = {isa = PBXBuildFile; fileRef = 9A19A6A51163BB7E00E0D453 /* SBValue.h */; settings = {ATTRIBUTES = (Public, ); }; };
9A19A6B01163BBB300E0D453 /* SBValue.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9A19A6AD1163BB9800E0D453 /* SBValue.cpp */; };
9A1E595C1EB2B141002206A5 /* SBTrace.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9A1E59521EB2B0B9002206A5 /* SBTrace.cpp */; };
@@ -2864,11 +2864,11 @@
9A0FDE991E8EF5010086B2F5 /* RegisterInfos_arm.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = RegisterInfos_arm.h; path = Utility/RegisterInfos_arm.h; sourceTree = ""; };
9A0FDE9A1E8EF5010086B2F5 /* RegisterInfos_arm64.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = RegisterInfos_arm64.h; path = Utility/RegisterInfos_arm64.h; sourceTree = ""; };
9A0FDE9B1E8EF5010086B2F5 /* RegisterInfos_mips.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = RegisterInfos_mips.h; path = Utility/RegisterInfos_mips.h; sourceTree = ""; };
- 9A1542F41F0EE44000DEA1D8 /* CMakeLists.txt */ = {isa = PBXFileReference; lastKnownFileType = text; path = CMakeLists.txt; sourceTree = ""; };
- 9A1542F51F0EE44000DEA1D8 /* MockTildeExpressionResolver.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = MockTildeExpressionResolver.cpp; sourceTree = ""; };
- 9A1542F61F0EE44000DEA1D8 /* MockTildeExpressionResolver.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MockTildeExpressionResolver.h; sourceTree = ""; };
- 9A1542F71F0EE44000DEA1D8 /* TestUtilities.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = TestUtilities.cpp; sourceTree = ""; };
- 9A1542F81F0EE44000DEA1D8 /* TestUtilities.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = TestUtilities.h; sourceTree = ""; };
+ 9A1890311F47D5D400394BCA /* CMakeLists.txt */ = {isa = PBXFileReference;
[Lldb-commits] [lldb] r314849 - cmake + xcode: prevent gtests from using includes from project root
Author: penryu
Date: Tue Oct 3 14:20:18 2017
New Revision: 314849
URL: http://llvm.org/viewvc/llvm-project?rev=314849&view=rev
Log:
cmake + xcode: prevent gtests from using includes from project root
Summary:
At present, several gtests in the lldb open source codebase are using
#include statements rooted at $(SOURCE_ROOT)/${LLDB_PROJECT_ROOT}.
This patch cleans up this directory/include structure for both CMake and
Xcode build systems.
rdar://problem/33835795
Reviewers: zturner, jingham, beanz
Reviewed By: beanz
Subscribers: emaste, lldb-commits, mgorny
Differential Revision: https://reviews.llvm.org/D36598
Added:
lldb/trunk/unittests/TestingSupport/
lldb/trunk/unittests/TestingSupport/CMakeLists.txt
- copied, changed from r314832,
lldb/trunk/unittests/Utility/Helpers/CMakeLists.txt
lldb/trunk/unittests/TestingSupport/MockTildeExpressionResolver.cpp
- copied, changed from r314832,
lldb/trunk/unittests/Utility/Helpers/MockTildeExpressionResolver.cpp
lldb/trunk/unittests/TestingSupport/MockTildeExpressionResolver.h
- copied, changed from r314832,
lldb/trunk/unittests/Utility/Helpers/MockTildeExpressionResolver.h
lldb/trunk/unittests/TestingSupport/TestUtilities.cpp
- copied, changed from r314832,
lldb/trunk/unittests/Utility/Helpers/TestUtilities.cpp
lldb/trunk/unittests/TestingSupport/TestUtilities.h
- copied, changed from r314832,
lldb/trunk/unittests/Utility/Helpers/TestUtilities.h
Removed:
lldb/trunk/unittests/Utility/Helpers/CMakeLists.txt
lldb/trunk/unittests/Utility/Helpers/MockTildeExpressionResolver.cpp
lldb/trunk/unittests/Utility/Helpers/MockTildeExpressionResolver.h
lldb/trunk/unittests/Utility/Helpers/TestUtilities.cpp
lldb/trunk/unittests/Utility/Helpers/TestUtilities.h
Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj
lldb/trunk/unittests/CMakeLists.txt
lldb/trunk/unittests/Interpreter/TestCompletion.cpp
lldb/trunk/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
lldb/trunk/unittests/Symbol/TestDWARFCallFrameInfo.cpp
lldb/trunk/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
lldb/trunk/unittests/Target/ModuleCacheTest.cpp
lldb/trunk/unittests/Utility/CMakeLists.txt
lldb/trunk/unittests/Utility/StructuredDataTest.cpp
lldb/trunk/unittests/Utility/TildeExpressionResolverTest.cpp
Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=314849&r1=314848&r2=314849&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Tue Oct 3 14:20:18 2017
@@ -876,8 +876,8 @@
966C6B7C18E6A56A0093F5EC /* libz.dylib in Frameworks */ = {isa
= PBXBuildFile; fileRef = 966C6B7818E6A56A0093F5EC /* libz.dylib */; };
9694FA711B32AA64005EBB16 /* ABISysV_mips.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 9694FA6F1B32AA64005EBB16 /* ABISysV_mips.cpp */;
};
9A0FDEA71E8EF5110086B2F5 /* RegisterContextLinux_mips.cpp in
Sources */ = {isa = PBXBuildFile; fileRef = 9A0FDE971E8EF5010086B2F5 /*
RegisterContextLinux_mips.cpp */; };
- 9A1542F91F0EE48600DEA1D8 /* MockTildeExpressionResolver.cpp in
Sources */ = {isa = PBXBuildFile; fileRef = 9A1542F51F0EE44000DEA1D8 /*
MockTildeExpressionResolver.cpp */; };
- 9A1542FA1F0EE48600DEA1D8 /* TestUtilities.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 9A1542F71F0EE44000DEA1D8 /* TestUtilities.cpp
*/; };
+ 9A18903B1F47D5E600394BCA /* MockTildeExpressionResolver.cpp in
Sources */ = {isa = PBXBuildFile; fileRef = 9A1890321F47D5D400394BCA /*
MockTildeExpressionResolver.cpp */; };
+ 9A18903C1F47D5E600394BCA /* TestUtilities.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 9A1890341F47D5D400394BCA /* TestUtilities.cpp
*/; };
9A19A6AF1163BBB200E0D453 /* SBValue.h in Headers */ = {isa =
PBXBuildFile; fileRef = 9A19A6A51163BB7E00E0D453 /* SBValue.h */; settings =
{ATTRIBUTES = (Public, ); }; };
9A19A6B01163BBB300E0D453 /* SBValue.cpp in Sources */ = {isa =
PBXBuildFile; fileRef = 9A19A6AD1163BB9800E0D453 /* SBValue.cpp */; };
9A1E595C1EB2B141002206A5 /* SBTrace.cpp in Sources */ = {isa =
PBXBuildFile; fileRef = 9A1E59521EB2B0B9002206A5 /* SBTrace.cpp */; };
@@ -2864,11 +2864,11 @@
9A0FDE991E8EF5010086B2F5 /* RegisterInfos_arm.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name =
RegisterInfos_arm.h; path = Utility/RegisterInfos_arm.h; sourceTree =
""; };
9A0FDE9A1E8EF5010086B2F5 /* RegisterInfos_arm64.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcec
[Lldb-commits] [PATCH] D38492: [lldb] Fix initialization of m_debug_cu_index_map
This revision was automatically updated to reflect the committed changes.
Closed by commit rL314832: [lldb] Fix initialization of m_debug_cu_index_map
(authored by alexshap).
Changed prior to commit:
https://reviews.llvm.org/D38492?vs=117470&id=117570#toc
Repository:
rL LLVM
https://reviews.llvm.org/D38492
Files:
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
@@ -69,17 +69,21 @@
debug_cu_index.GetAddressByteSize());
if (!dwp_symfile->m_debug_cu_index.parse(llvm_debug_cu_index))
return nullptr;
+ dwp_symfile->InitDebugCUIndexMap();
return dwp_symfile;
}
-SymbolFileDWARFDwp::SymbolFileDWARFDwp(lldb::ModuleSP module_sp,
- lldb::ObjectFileSP obj_file)
-: m_obj_file(std::move(obj_file)), m_debug_cu_index(llvm::DW_SECT_INFO) {
- for (const auto &entry : m_debug_cu_index.getRows()) {
+void SymbolFileDWARFDwp::InitDebugCUIndexMap() {
+ m_debug_cu_index_map.clear();
+ for (const auto &entry : m_debug_cu_index.getRows())
m_debug_cu_index_map.emplace(entry.getSignature(), &entry);
- }
}
+SymbolFileDWARFDwp::SymbolFileDWARFDwp(lldb::ModuleSP module_sp,
+ lldb::ObjectFileSP obj_file)
+: m_obj_file(std::move(obj_file)), m_debug_cu_index(llvm::DW_SECT_INFO)
+{}
+
std::unique_ptr
SymbolFileDWARFDwp::GetSymbolFileForDwoId(DWARFCompileUnit *dwarf_cu,
uint64_t dwo_id) {
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h
@@ -40,6 +40,8 @@
bool LoadRawSectionData(lldb::SectionType sect_type,
lldb_private::DWARFDataExtractor &data);
+
+ void InitDebugCUIndexMap();
lldb::ObjectFileSP m_obj_file;
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
@@ -69,17 +69,21 @@
debug_cu_index.GetAddressByteSize());
if (!dwp_symfile->m_debug_cu_index.parse(llvm_debug_cu_index))
return nullptr;
+ dwp_symfile->InitDebugCUIndexMap();
return dwp_symfile;
}
-SymbolFileDWARFDwp::SymbolFileDWARFDwp(lldb::ModuleSP module_sp,
- lldb::ObjectFileSP obj_file)
-: m_obj_file(std::move(obj_file)), m_debug_cu_index(llvm::DW_SECT_INFO) {
- for (const auto &entry : m_debug_cu_index.getRows()) {
+void SymbolFileDWARFDwp::InitDebugCUIndexMap() {
+ m_debug_cu_index_map.clear();
+ for (const auto &entry : m_debug_cu_index.getRows())
m_debug_cu_index_map.emplace(entry.getSignature(), &entry);
- }
}
+SymbolFileDWARFDwp::SymbolFileDWARFDwp(lldb::ModuleSP module_sp,
+ lldb::ObjectFileSP obj_file)
+: m_obj_file(std::move(obj_file)), m_debug_cu_index(llvm::DW_SECT_INFO)
+{}
+
std::unique_ptr
SymbolFileDWARFDwp::GetSymbolFileForDwoId(DWARFCompileUnit *dwarf_cu,
uint64_t dwo_id) {
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h
@@ -40,6 +40,8 @@
bool LoadRawSectionData(lldb::SectionType sect_type,
lldb_private::DWARFDataExtractor &data);
+
+ void InitDebugCUIndexMap();
lldb::ObjectFileSP m_obj_file;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r314832 - [lldb] Fix initialization of m_debug_cu_index_map
Author: alexshap
Date: Tue Oct 3 12:56:21 2017
New Revision: 314832
URL: http://llvm.org/viewvc/llvm-project?rev=314832&view=rev
Log:
[lldb] Fix initialization of m_debug_cu_index_map
SymbolFileDWARFDwp contains m_debug_cu_index_map which was previously
initialized incorrectly: before m_debug_cu_index.parse
is called m_debug_cu_index is empty, thus
the map was not actually getting populated properly.
This diff moves this step into a private helper method
and calls it after m_debug_cu_index.parse inside SymbolFileDWARFDwp::Create.
Test plan:
Build a toy test example
main.cpp
clang -gsplit-dwarf -g -O0 main.cpp -o main.exe
llvm-dwp -e main.exe -o main.exe.dwp
Build LLDB with ENABLE_DEBUG_PRINTF set.
Run: lldb -- ./main.exe
Check that the indexes are now correct
(before this change they were empty)
Check that debugging works
(setting breakpoints, printing local variables (this was not working before))
Differential revision: http://reviews.llvm.org/D38492
Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h
Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp?rev=314832&r1=314831&r2=314832&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp Tue Oct
3 12:56:21 2017
@@ -69,17 +69,21 @@ SymbolFileDWARFDwp::Create(lldb::ModuleS
debug_cu_index.GetAddressByteSize());
if (!dwp_symfile->m_debug_cu_index.parse(llvm_debug_cu_index))
return nullptr;
+ dwp_symfile->InitDebugCUIndexMap();
return dwp_symfile;
}
-SymbolFileDWARFDwp::SymbolFileDWARFDwp(lldb::ModuleSP module_sp,
- lldb::ObjectFileSP obj_file)
-: m_obj_file(std::move(obj_file)), m_debug_cu_index(llvm::DW_SECT_INFO) {
- for (const auto &entry : m_debug_cu_index.getRows()) {
+void SymbolFileDWARFDwp::InitDebugCUIndexMap() {
+ m_debug_cu_index_map.clear();
+ for (const auto &entry : m_debug_cu_index.getRows())
m_debug_cu_index_map.emplace(entry.getSignature(), &entry);
- }
}
+SymbolFileDWARFDwp::SymbolFileDWARFDwp(lldb::ModuleSP module_sp,
+ lldb::ObjectFileSP obj_file)
+: m_obj_file(std::move(obj_file)), m_debug_cu_index(llvm::DW_SECT_INFO)
+{}
+
std::unique_ptr
SymbolFileDWARFDwp::GetSymbolFileForDwoId(DWARFCompileUnit *dwarf_cu,
uint64_t dwo_id) {
Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h?rev=314832&r1=314831&r2=314832&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h Tue Oct 3
12:56:21 2017
@@ -40,6 +40,8 @@ private:
bool LoadRawSectionData(lldb::SectionType sect_type,
lldb_private::DWARFDataExtractor &data);
+
+ void InitDebugCUIndexMap();
lldb::ObjectFileSP m_obj_file;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38394: Fix dumping of characters with non-standard sizes
zturner added inline comments.
Comment at: source/Utility/DataExtractor.cpp:566
size_t byte_size) const {
- switch (byte_size) {
- case 1:
-return GetU8(offset_ptr);
-break;
- case 2:
-return GetU16(offset_ptr);
-break;
- case 4:
-return GetU32(offset_ptr);
-break;
- default:
-assert(false && "GetMaxU32 unhandled case!");
-break;
- }
- return 0;
+ assert(byte_size <= 4 && "GetMaxU32 unhandled case!");
+ return GetMaxU64(offset_ptr, byte_size);
jingham wrote:
> petpav01 wrote:
> > jingham wrote:
> > > This is trivial, and you didn't change what was there, but this message
> > > makes it sound like this is just something we haven't gotten to yet.
> > > It's really "You passed in an illegal byte size"... Might be clearer if
> > > the message said that.
> > I was not sure what is the expected behaviour when the input `byte_size`
> > exceeds the size of the return type of each of these `GetMax...()` methods.
> > The current behaviour is to assert this situation but comments describing
> > the methods (in both `DataExtractor.cpp` and `DataExtractor.h`) say that
> > nothing should get extracted in these cases and zero is returned.
> >
> > Maybe the patch should go a bit further and clean this up as follows:
> > * Remove duplicated comments in `DataExtractor.cpp` for
> > `DataExtractor::GetMaxU32()` and `GetMaxU64()` and keep only their Doxygen
> > versions in `DataExtractor.h`.
> > * Update comments in `DataExtractor.h` for `DataExtractor::GetMaxU32()`,
> > `GetMaxU64()`, `GetMaxS64()`, `GetMaxU64Bitfield()` and
> > `GetMaxS64Bitfield()` to match the current implementation.
> > * Change assertion text in `DataExtractor::GetMaxU32()` and `GetMaxU64()`
> > from "unhandled case" to "invalid byte size".
> >
> > Does this sound reasonable?
> The released versions of lldb - at least the ones Apple releases - have
> asserts disabled. This isn't unique to lldb, clang does the same thing.
>
> I do my day-to-day debugging using a TOT build with asserts enabled, and we
> run the testsuite that way so the asserts catch errors at this stage. But
> for the general public, the function will behave as described. It would be
> great to remove the duplicated docs - that's just begging for one or the
> other to get out of date. But the descriptions are functionally correct.
> And then changing the text to "invalid byte size" also seems good to me.
Being pedantic, this *is* a functionality change. Previously, we would assert
on a size of 3 or 0, with this change we will allow those cases through.
https://reviews.llvm.org/D38394
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38394: Fix dumping of characters with non-standard sizes
jingham added a comment.
See inlined comments.
Comment at: source/Core/DumpDataExtractor.cpp:275-281
+ // Reject invalid item_byte_size.
+ if (item_byte_size > 8) {
+s->Printf("error: unsupported byte size (%" PRIu64 ") for char format",
+ (uint64_t)item_byte_size);
+return offset;
+ }
+
petpav01 wrote:
> jingham wrote:
> > Should this consume the weird input we couldn't print? I actually don't
> > have a good feel for which would be better.
> The behaviour implemented in `DumpDataExtractor()` for other formats, such as
> `eFormatBoolean` or `eFormatComplexInteger`, is to report an error and do not
> advance the offset. The approach that the patch takes is to make
> `eFormatChar` (and its variants) consistent with this behaviour.
The doc's say an error in the size "will result in nothing being extracted".
That seems like a roundabout way of saying that offset won't be advanced, but I
probably should have figured it out from there. Thanks for checking on the
error behavior of the other dumpers! You are doing the right thing.
Comment at: source/Utility/DataExtractor.cpp:566
size_t byte_size) const {
- switch (byte_size) {
- case 1:
-return GetU8(offset_ptr);
-break;
- case 2:
-return GetU16(offset_ptr);
-break;
- case 4:
-return GetU32(offset_ptr);
-break;
- default:
-assert(false && "GetMaxU32 unhandled case!");
-break;
- }
- return 0;
+ assert(byte_size <= 4 && "GetMaxU32 unhandled case!");
+ return GetMaxU64(offset_ptr, byte_size);
petpav01 wrote:
> jingham wrote:
> > This is trivial, and you didn't change what was there, but this message
> > makes it sound like this is just something we haven't gotten to yet. It's
> > really "You passed in an illegal byte size"... Might be clearer if the
> > message said that.
> I was not sure what is the expected behaviour when the input `byte_size`
> exceeds the size of the return type of each of these `GetMax...()` methods.
> The current behaviour is to assert this situation but comments describing the
> methods (in both `DataExtractor.cpp` and `DataExtractor.h`) say that nothing
> should get extracted in these cases and zero is returned.
>
> Maybe the patch should go a bit further and clean this up as follows:
> * Remove duplicated comments in `DataExtractor.cpp` for
> `DataExtractor::GetMaxU32()` and `GetMaxU64()` and keep only their Doxygen
> versions in `DataExtractor.h`.
> * Update comments in `DataExtractor.h` for `DataExtractor::GetMaxU32()`,
> `GetMaxU64()`, `GetMaxS64()`, `GetMaxU64Bitfield()` and `GetMaxS64Bitfield()`
> to match the current implementation.
> * Change assertion text in `DataExtractor::GetMaxU32()` and `GetMaxU64()`
> from "unhandled case" to "invalid byte size".
>
> Does this sound reasonable?
The released versions of lldb - at least the ones Apple releases - have asserts
disabled. This isn't unique to lldb, clang does the same thing.
I do my day-to-day debugging using a TOT build with asserts enabled, and we run
the testsuite that way so the asserts catch errors at this stage. But for the
general public, the function will behave as described. It would be great to
remove the duplicated docs - that's just begging for one or the other to get
out of date. But the descriptions are functionally correct. And then
changing the text to "invalid byte size" also seems good to me.
https://reviews.llvm.org/D38394
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38394: Fix dumping of characters with non-standard sizes
petpav01 added a comment.
Thank you for the initial review.
Comment at: source/Core/DumpDataExtractor.cpp:275-281
+ // Reject invalid item_byte_size.
+ if (item_byte_size > 8) {
+s->Printf("error: unsupported byte size (%" PRIu64 ") for char format",
+ (uint64_t)item_byte_size);
+return offset;
+ }
+
jingham wrote:
> Should this consume the weird input we couldn't print? I actually don't have
> a good feel for which would be better.
The behaviour implemented in `DumpDataExtractor()` for other formats, such as
`eFormatBoolean` or `eFormatComplexInteger`, is to report an error and do not
advance the offset. The approach that the patch takes is to make `eFormatChar`
(and its variants) consistent with this behaviour.
Comment at: source/Utility/DataExtractor.cpp:566
size_t byte_size) const {
- switch (byte_size) {
- case 1:
-return GetU8(offset_ptr);
-break;
- case 2:
-return GetU16(offset_ptr);
-break;
- case 4:
-return GetU32(offset_ptr);
-break;
- default:
-assert(false && "GetMaxU32 unhandled case!");
-break;
- }
- return 0;
+ assert(byte_size <= 4 && "GetMaxU32 unhandled case!");
+ return GetMaxU64(offset_ptr, byte_size);
jingham wrote:
> This is trivial, and you didn't change what was there, but this message makes
> it sound like this is just something we haven't gotten to yet. It's really
> "You passed in an illegal byte size"... Might be clearer if the message said
> that.
I was not sure what is the expected behaviour when the input `byte_size`
exceeds the size of the return type of each of these `GetMax...()` methods. The
current behaviour is to assert this situation but comments describing the
methods (in both `DataExtractor.cpp` and `DataExtractor.h`) say that nothing
should get extracted in these cases and zero is returned.
Maybe the patch should go a bit further and clean this up as follows:
* Remove duplicated comments in `DataExtractor.cpp` for
`DataExtractor::GetMaxU32()` and `GetMaxU64()` and keep only their Doxygen
versions in `DataExtractor.h`.
* Update comments in `DataExtractor.h` for `DataExtractor::GetMaxU32()`,
`GetMaxU64()`, `GetMaxS64()`, `GetMaxU64Bitfield()` and `GetMaxS64Bitfield()`
to match the current implementation.
* Change assertion text in `DataExtractor::GetMaxU32()` and `GetMaxU64()` from
"unhandled case" to "invalid byte size".
Does this sound reasonable?
https://reviews.llvm.org/D38394
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38323: Enable breakpoints and read/write GPRs for ppc64le
alexandreyy added a comment.
Are these changes ok?
I am implementing the read/write functions for the other registers.
And I will add it later.
In https://reviews.llvm.org/D38323#883429, @clayborg wrote:
> Looks fine. One main questions for new linux archs in particular: is linux
> using the lldb-server to debug these days even when debugging locally? If so,
> then this patch only needs to implement both a native register content and
> not the lldb_private::RegisterContext subclass.
When I debug locally, the lldb launches the lldb-server.
However, the remote debug is not working yet. The server crashes when I try to
launch a process.
It shows that error:
lldb-server:
/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp:134:
lldb_private::Status
lldb_private::process_gdb_remote::GDBRemoteCommunicationServerPlatform::LaunchGDBServer(const
lldb_private::Args&, std::__cxx11::string, lldb::pid_t&, uint16_t&,
std::__cxx11::string&): Assertion `ok' failed.
I will investigate that later.
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h:66-68
+ struct Reg {
+uint8_t bytes[8];
+ };
alexandreyy wrote:
> clayborg wrote:
> > Any reason to not use "uint64_t" instead of "struct Reg"?
> Changed to uint64_t.
Changed register set type to GPR.
https://reviews.llvm.org/D38323
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38492: [lldb] Fix initialization of m_debug_cu_index_map
tberghammer accepted this revision. tberghammer added a comment. This revision is now accepted and ready to land. Looks good, thanks for fixing it. I am slightly confused why it was working for me when I tested it before submission but I must have messed up something during testing. Repository: rL LLVM https://reviews.llvm.org/D38492 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
