[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash
amccarth added inline comments.
Comment at:
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py:13
+self.build()
+ spec = lldb.SBModuleSpec()
+ spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact("a.out")))
Oops! Looks like this Python file got checked in with a mixture of tabs and
spaces.
Repository:
rL LLVM
https://reviews.llvm.org/D43596
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash
This revision was automatically updated to reflect the committed changes.
Closed by commit rL325927: Replace HashStringUsingDJB with llvm::djbHash
(authored by labath, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D43596?vs=135568=135658#toc
Repository:
rL LLVM
https://reviews.llvm.org/D43596
Files:
lldb/trunk/include/lldb/Core/MappedHash.h
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/Makefile
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/main.c
lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Target/ObjCLanguageRuntime.cpp
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/Makefile
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/Makefile
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/Makefile
@@ -0,0 +1,6 @@
+LEVEL = ../../../make
+
+C_SOURCES := main.c
+CFLAGS_EXTRAS += -finput-charset=UTF-8 -fextended-identifiers -std=c99
+
+include $(LEVEL)/Makefile.rules
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py
@@ -0,0 +1,19 @@
+# coding=utf8
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class TestUnicodeSymbols(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_union_members(self):
+self.build()
+ spec = lldb.SBModuleSpec()
+ spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact("a.out")))
+ module = lldb.SBModule(spec)
+ self.assertTrue(module.IsValid())
+ mytype = module.FindFirstType("foobár")
+ self.assertTrue(mytype.IsValid())
+ self.assertTrue(mytype.IsPointerType())
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/main.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/main.c
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/main.c
@@ -0,0 +1,5 @@
+typedef void *foob\u00E1r;
+foob\u00E1r X = 0;
+int main() {
+ return (long)X;
+}
Index: lldb/trunk/source/Target/ObjCLanguageRuntime.cpp
===
--- lldb/trunk/source/Target/ObjCLanguageRuntime.cpp
+++ lldb/trunk/source/Target/ObjCLanguageRuntime.cpp
@@ -23,6 +23,7 @@
#include "lldb/Utility/Timer.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/DJB.h"
using namespace lldb;
using namespace lldb_private;
@@ -45,8 +46,7 @@
if (isa != 0) {
m_isa_to_descriptor[isa] = descriptor_sp;
// class_name is assumed to be valid
-m_hash_to_isa_map.insert(
-std::make_pair(MappedHash::HashStringUsingDJB(class_name), isa));
+m_hash_to_isa_map.insert(std::make_pair(llvm::djbHash(class_name), isa));
return true;
}
return false;
@@ -180,8 +180,7 @@
} else {
// Name hashes were provided, so use them to efficiently lookup name to
// isa/descriptor
- const uint32_t name_hash =
- MappedHash::HashStringUsingDJB(name.GetCString());
+ const uint32_t name_hash = llvm::djbHash(name.GetStringRef());
std::pair range =
m_hash_to_isa_map.equal_range(name_hash);
for (HashToISAIterator range_pos = range.first; range_pos != range.second;
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1510,7 +1510,8 @@
method_die_offsets.clear();
if (m_using_apple_tables) {
if (m_apple_objc_ap.get())
- m_apple_objc_ap->FindByName(class_name.GetCString(), method_die_offsets);
+ m_apple_objc_ap->FindByName(class_name.GetStringRef(),
+ method_die_offsets);
} else {
if (!m_indexed)
Index();
@@ -2183,7 +2184,7 @@
basename))
basename = name_cstr;
- m_apple_names_ap->FindByName(basename.data(), die_offsets);
+ m_apple_names_ap->FindByName(basename, die_offsets);
}
} else {
// Index the DWARF if we haven't already
@@ -2489,8 +2490,6 @@
// Remember how many sc_list are in the list before we search in case
// we are appending the results to a variable list.
- const char *name_cstr = name.GetCString();
-
const
[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash
aprantl added a comment. Nice! https://reviews.llvm.org/D43596 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash
labath updated this revision to Diff 135568.
labath added a comment.
This adds a test which looks up a type with unicode characters. I have verified
this test does not pass when run against a pre-D42740 compiler.
https://reviews.llvm.org/D43596
Files:
include/lldb/Core/MappedHash.h
packages/Python/lldbsuite/test/lang/c/unicode/Makefile
packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py
packages/Python/lldbsuite/test/lang/c/unicode/main.c
source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Target/ObjCLanguageRuntime.cpp
Index: source/Target/ObjCLanguageRuntime.cpp
===
--- source/Target/ObjCLanguageRuntime.cpp
+++ source/Target/ObjCLanguageRuntime.cpp
@@ -23,6 +23,7 @@
#include "lldb/Utility/Timer.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/DJB.h"
using namespace lldb;
using namespace lldb_private;
@@ -45,8 +46,7 @@
if (isa != 0) {
m_isa_to_descriptor[isa] = descriptor_sp;
// class_name is assumed to be valid
-m_hash_to_isa_map.insert(
-std::make_pair(MappedHash::HashStringUsingDJB(class_name), isa));
+m_hash_to_isa_map.insert(std::make_pair(llvm::djbHash(class_name), isa));
return true;
}
return false;
@@ -180,8 +180,7 @@
} else {
// Name hashes were provided, so use them to efficiently lookup name to
// isa/descriptor
- const uint32_t name_hash =
- MappedHash::HashStringUsingDJB(name.GetCString());
+ const uint32_t name_hash = llvm::djbHash(name.GetStringRef());
std::pair range =
m_hash_to_isa_map.equal_range(name_hash);
for (HashToISAIterator range_pos = range.first; range_pos != range.second;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1510,7 +1510,8 @@
method_die_offsets.clear();
if (m_using_apple_tables) {
if (m_apple_objc_ap.get())
- m_apple_objc_ap->FindByName(class_name.GetCString(), method_die_offsets);
+ m_apple_objc_ap->FindByName(class_name.GetStringRef(),
+ method_die_offsets);
} else {
if (!m_indexed)
Index();
@@ -2183,7 +2184,7 @@
basename))
basename = name_cstr;
- m_apple_names_ap->FindByName(basename.data(), die_offsets);
+ m_apple_names_ap->FindByName(basename, die_offsets);
}
} else {
// Index the DWARF if we haven't already
@@ -2489,8 +2490,6 @@
// Remember how many sc_list are in the list before we search in case
// we are appending the results to a variable list.
- const char *name_cstr = name.GetCString();
-
const uint32_t original_size = sc_list.GetSize();
DWARFDebugInfo *info = DebugInfo();
@@ -2511,7 +2510,8 @@
// want to canonicalize this (strip double spaces, etc. For now, we
// just add all the
// dies that we find by exact match.
-num_matches = m_apple_names_ap->FindByName(name_cstr, die_offsets);
+num_matches =
+m_apple_names_ap->FindByName(name.GetStringRef(), die_offsets);
for (uint32_t i = 0; i < num_matches; i++) {
const DIERef _ref = die_offsets[i];
DWARFDIE die = info->GetDIE(die_ref);
@@ -2527,16 +2527,17 @@
GetObjectFile()->GetModule()->ReportErrorIfModifyDetected(
"the DWARF debug information has been modified (.apple_names "
"accelerator table had bad die 0x%8.8x for '%s')",
-die_ref.die_offset, name_cstr);
+die_ref.die_offset, name.GetCString());
}
}
}
if (name_type_mask & eFunctionNameTypeSelector) {
if (parent_decl_ctx && parent_decl_ctx->IsValid())
return 0; // no selectors in namespaces
-num_matches = m_apple_names_ap->FindByName(name_cstr, die_offsets);
+num_matches =
+m_apple_names_ap->FindByName(name.GetStringRef(), die_offsets);
// Now make sure these are actually ObjC methods. In this case we can
// simply look up the name,
// and if it is an ObjC method name, we're good.
@@ -2556,7 +2557,7 @@
GetObjectFile()->GetModule()->ReportError(
"the DWARF debug information has been modified (.apple_names "
"accelerator table had bad die 0x%8.8x for '%s')",
-die_ref.die_offset, name_cstr);
+die_ref.die_offset, name.GetCString());
}
}
die_offsets.clear();
@@ -2572,7 +2573,8 @@
// FIXME: Arrange the logic above so that we don't calculate
[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash
aprantl added a comment. In https://reviews.llvm.org/D43596#1015903, @clayborg wrote: > The only input we currently use this on is DWARF and I believe all DWARF > strings are UTF8 so this should all work correctly, no? The hashing algorithms produced different hashes for characters with bit 7 set. That would apply for all UTF-8 characters that are outside of the 7-bit ascii range. Jonas, do we have any tests in LLDB that try to lookup a variable with non-ASCII characters? https://reviews.llvm.org/D43596 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash
clayborg added a comment. The only input we currently use this on is DWARF and I believe all DWARF strings are UTF8 so this should all work correctly, no? https://reviews.llvm.org/D43596 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash
JDevlieghere added a comment. In https://reviews.llvm.org/D43596#1015848, @clayborg wrote: > No need to add additional dSYM tests, because all dSYM tests will fail if the > hashing isn't correct. Maybe I misunderstand, but https://reviews.llvm.org/D42740 suggested that this never worked (or maybe it did accidentally, because we ended up in the right bucket) for unicode characters? https://reviews.llvm.org/D43596 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash
clayborg accepted this revision. clayborg added a comment. No need to add additional dSYM tests, because all dSYM tests will fail if the hashing isn't correct. https://reviews.llvm.org/D43596 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash
davide accepted this revision. davide added a comment. Thanks. https://reviews.llvm.org/D43596 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash
aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.
Comment at: include/lldb/Core/MappedHash.h:156
- template
- class ExportTable {
- public:
Yeah this looks like it was a dead end dating back to 2011.
Comment at: source/Target/ObjCLanguageRuntime.cpp:49
// class_name is assumed to be valid
-m_hash_to_isa_map.insert(
-std::make_pair(MappedHash::HashStringUsingDJB(class_name), isa));
+m_hash_to_isa_map.insert(std::make_pair(llvm::djbHash(class_name), isa));
return true;
`m_hash_to_isa_map.insert({llvm::djbHash(class_name), isa});`
https://reviews.llvm.org/D43596
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash
labath created this revision.
labath added reviewers: aprantl, davide.
Herald added a subscriber: JDevlieghere.
The llvm function is equivalent to this one. Where possible I tried to
replace const char* with llvm::StringRef to avoid extra strlen
computations. In most places, I was able to track the c string back to
the ConstString it was created from.
This also removes the unused ExportTable class.
https://reviews.llvm.org/D43596
Files:
include/lldb/Core/MappedHash.h
source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Target/ObjCLanguageRuntime.cpp
Index: source/Target/ObjCLanguageRuntime.cpp
===
--- source/Target/ObjCLanguageRuntime.cpp
+++ source/Target/ObjCLanguageRuntime.cpp
@@ -23,6 +23,7 @@
#include "lldb/Utility/Timer.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/DJB.h"
using namespace lldb;
using namespace lldb_private;
@@ -45,8 +46,7 @@
if (isa != 0) {
m_isa_to_descriptor[isa] = descriptor_sp;
// class_name is assumed to be valid
-m_hash_to_isa_map.insert(
-std::make_pair(MappedHash::HashStringUsingDJB(class_name), isa));
+m_hash_to_isa_map.insert(std::make_pair(llvm::djbHash(class_name), isa));
return true;
}
return false;
@@ -180,8 +180,7 @@
} else {
// Name hashes were provided, so use them to efficiently lookup name to
// isa/descriptor
- const uint32_t name_hash =
- MappedHash::HashStringUsingDJB(name.GetCString());
+ const uint32_t name_hash = llvm::djbHash(name.GetStringRef());
std::pair range =
m_hash_to_isa_map.equal_range(name_hash);
for (HashToISAIterator range_pos = range.first; range_pos != range.second;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1510,7 +1510,8 @@
method_die_offsets.clear();
if (m_using_apple_tables) {
if (m_apple_objc_ap.get())
- m_apple_objc_ap->FindByName(class_name.GetCString(), method_die_offsets);
+ m_apple_objc_ap->FindByName(class_name.GetStringRef(),
+ method_die_offsets);
} else {
if (!m_indexed)
Index();
@@ -2183,7 +2184,7 @@
basename))
basename = name_cstr;
- m_apple_names_ap->FindByName(basename.data(), die_offsets);
+ m_apple_names_ap->FindByName(basename, die_offsets);
}
} else {
// Index the DWARF if we haven't already
@@ -2489,8 +2490,6 @@
// Remember how many sc_list are in the list before we search in case
// we are appending the results to a variable list.
- const char *name_cstr = name.GetCString();
-
const uint32_t original_size = sc_list.GetSize();
DWARFDebugInfo *info = DebugInfo();
@@ -2511,7 +2510,8 @@
// want to canonicalize this (strip double spaces, etc. For now, we
// just add all the
// dies that we find by exact match.
-num_matches = m_apple_names_ap->FindByName(name_cstr, die_offsets);
+num_matches =
+m_apple_names_ap->FindByName(name.GetStringRef(), die_offsets);
for (uint32_t i = 0; i < num_matches; i++) {
const DIERef _ref = die_offsets[i];
DWARFDIE die = info->GetDIE(die_ref);
@@ -2527,16 +2527,17 @@
GetObjectFile()->GetModule()->ReportErrorIfModifyDetected(
"the DWARF debug information has been modified (.apple_names "
"accelerator table had bad die 0x%8.8x for '%s')",
-die_ref.die_offset, name_cstr);
+die_ref.die_offset, name.GetCString());
}
}
}
if (name_type_mask & eFunctionNameTypeSelector) {
if (parent_decl_ctx && parent_decl_ctx->IsValid())
return 0; // no selectors in namespaces
-num_matches = m_apple_names_ap->FindByName(name_cstr, die_offsets);
+num_matches =
+m_apple_names_ap->FindByName(name.GetStringRef(), die_offsets);
// Now make sure these are actually ObjC methods. In this case we can
// simply look up the name,
// and if it is an ObjC method name, we're good.
@@ -2556,7 +2557,7 @@
GetObjectFile()->GetModule()->ReportError(
"the DWARF debug information has been modified (.apple_names "
"accelerator table had bad die 0x%8.8x for '%s')",
-die_ref.die_offset, name_cstr);
+die_ref.die_offset, name.GetCString());
}
}
die_offsets.clear();
@@ -2572,7 +2573,8 @@
// FIXME: Arrange the logic above so that we don't
