[Lldb-commits] [PATCH] D41086: [lldb] Stop searching for a symbol in a pdb by regex
labath added a comment. This is making the windows unit tests fail: http://lab.llvm.org:8011/builders/lldb-windows7-android/builds/7378/steps/run%20unit%20tests/logs/stdio. If I understand correctly, this changes FindTypes to **not** search by regex. If that's the case, then the `SymbolFilePDBTests::TestRegexNameMatch` needs to be updated to not expect that `.*` will find a match. Also, while technically not failing, the `SymbolFilePDBTests::TestMaxMatches` also looks weird in this new world, and probably needs updating. Could you look into those? Repository: rL LLVM https://reviews.llvm.org/D41086 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r321353 - Enable TestReadMemCString on non-darwin targets
Author: labath Date: Fri Dec 22 02:26:59 2017 New Revision: 321353 URL: http://llvm.org/viewvc/llvm-project?rev=321353&view=rev Log: Enable TestReadMemCString on non-darwin targets The test works fine on linux, and I believe other targets should not have an issue with as well. If they do, we can start blacklisting instead of whitelisting. The idea of using "-1" as the value of the pointer on non-apple targets backfired, as it fails the "address != LLDB_INVALID_ADDRESS" test (-1 is the value of LLDB_INVALID_ADDRESS). However, it should be safe to use 0x100 for other targets as well. The first page of memory is generally kept unreadable to catch null pointer dereferences. Modified: lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/TestReadMemCString.py lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/main.c Modified: lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/TestReadMemCString.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/TestReadMemCString.py?rev=321353&r1=321352&r2=321353&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/TestReadMemCString.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/TestReadMemCString.py Fri Dec 22 02:26:59 2017 @@ -12,13 +12,8 @@ from lldbsuite.test import lldbutil class TestReadMemCString(TestBase): mydir = TestBase.compute_mydir(__file__) +NO_DEBUG_INFO_TESTCASE = True -def setUp(self): -TestBase.setUp(self) - -# Need to have a char* pointer that points to unmapped memory to run -# this test on other platforms -- Darwin only for now. -@skipUnlessDarwin def test_read_memory_c_string(self): """Test corner case behavior of SBProcess::ReadCStringFromMemory""" self.build() Modified: lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/main.c URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/main.c?rev=321353&r1=321352&r2=321353&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/main.c (original) +++ lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/main.c Fri Dec 22 02:26:59 2017 @@ -3,11 +3,9 @@ int main () { const char *empty_string = ""; const char *one_letter_string = "1"; -#if defined (__APPLE__) - const char *invalid_memory_string = (char*)0x100; // lower 4k is always PAGEZERO & unreadable on darwin -#else - const char *invalid_memory_string = -1ULL; // maybe an invalid address on other platforms? -#endif + // This expects that lower 4k of memory will be mapped unreadable, which most + // OSs do (to catch null pointer dereferences). + const char *invalid_memory_string = (char*)0x100; return empty_string[0] + one_letter_string[0]; // breakpoint here } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r321355 - debugserver: Propagate environment in launch-mode (pr35671)
Author: labath Date: Fri Dec 22 03:09:21 2017 New Revision: 321355 URL: http://llvm.org/viewvc/llvm-project?rev=321355&view=rev Log: debugserver: Propagate environment in launch-mode (pr35671) Summary: Make sure we propagate environment when starting debugserver with a pre-loaded inferior. AFAIK, RNBRunLoopLaunchInferior is only called in pre-loaded inferior scenario, so we can just pick up the debugserver environment instead of trying to construct an envp from the (empty) context. This makes debugserver pass an test added for an equivalent lldb-server fix. Reviewers: jasonmolenda, clayborg Subscribers: JDevlieghere, lldb-commits Differential Revision: https://reviews.llvm.org/D41352 Modified: lldb/trunk/tools/debugserver/source/RNBContext.cpp lldb/trunk/tools/debugserver/source/RNBContext.h lldb/trunk/tools/debugserver/source/debugserver.cpp lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h Modified: lldb/trunk/tools/debugserver/source/RNBContext.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBContext.cpp?rev=321355&r1=321354&r2=321355&view=diff == --- lldb/trunk/tools/debugserver/source/RNBContext.cpp (original) +++ lldb/trunk/tools/debugserver/source/RNBContext.cpp Fri Dec 22 03:09:21 2017 @@ -42,6 +42,25 @@ const char *RNBContext::EnvironmentAtInd return NULL; } +static std::string GetEnvironmentKey(const std::string &env) { + std::string key = env.substr(0, env.find('=')); + if (!key.empty() && key.back() == '=') +key.pop_back(); + return key; +} + +void RNBContext::PushEnvironmentIfNeeded(const char *arg) { + if (!arg) +return; + std::string arg_key = GetEnvironmentKey(arg); + + for (const std::string &entry: m_env_vec) { +if (arg_key == GetEnvironmentKey(entry)) + return; + } + m_env_vec.push_back(arg); +} + const char *RNBContext::ArgumentAtIndex(size_t index) { if (index < m_arg_vec.size()) return m_arg_vec[index].c_str(); Modified: lldb/trunk/tools/debugserver/source/RNBContext.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBContext.h?rev=321355&r1=321354&r2=321355&view=diff == --- lldb/trunk/tools/debugserver/source/RNBContext.h (original) +++ lldb/trunk/tools/debugserver/source/RNBContext.h Fri Dec 22 03:09:21 2017 @@ -86,6 +86,7 @@ public: if (arg) m_env_vec.push_back(arg); } + void PushEnvironmentIfNeeded(const char *arg); void ClearEnvironment() { m_env_vec.erase(m_env_vec.begin(), m_env_vec.end()); } Modified: lldb/trunk/tools/debugserver/source/debugserver.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/debugserver.cpp?rev=321355&r1=321354&r2=321355&view=diff == --- lldb/trunk/tools/debugserver/source/debugserver.cpp (original) +++ lldb/trunk/tools/debugserver/source/debugserver.cpp Fri Dec 22 03:09:21 2017 @@ -1020,6 +1020,7 @@ int main(int argc, char *argv[]) { optind = 1; #endif + bool forward_env = false; while ((ch = getopt_long_only(argc, argv, short_options, g_long_options, &long_option_index)) != -1) { DNBLogDebug("option: ch == %c (0x%2.2x) --%s%c%s\n", ch, (uint8_t)ch, @@ -1251,14 +1252,7 @@ int main(int argc, char *argv[]) { break; case 'F': - // Pass the current environment down to the process that gets launched - { -char **host_env = *_NSGetEnviron(); -char *env_entry; -size_t i; -for (i = 0; (env_entry = host_env[i]) != NULL; ++i) - remote->Context().PushEnvironment(env_entry); - } + forward_env = true; break; case '2': @@ -1420,6 +1414,18 @@ int main(int argc, char *argv[]) { if (start_mode == eRNBRunLoopModeExit) return -1; + if (forward_env || start_mode == eRNBRunLoopModeInferiorLaunching) { +// Pass the current environment down to the process that gets launched +// This happens automatically in the "launching" mode. For the rest, we +// only do that if the user explicitly requested this via --forward-env +// argument. +char **host_env = *_NSGetEnviron(); +char *env_entry; +size_t i; +for (i = 0; (env_entry = host_env[i]) != NULL; ++i) + remote->Context().PushEnvironmentIfNeeded(env_entry); + } + RNBRunLoopMode mode = start_mode; char err_str[1024] = {'\0'}; Modified: lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt?rev=321355&r1=321354&r2=321355&view=diff
[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)
This revision was automatically updated to reflect the committed changes. Closed by commit rL321355: debugserver: Propagate environment in launch-mode (pr35671) (authored by labath, committed by ). Repository: rL LLVM https://reviews.llvm.org/D41352 Files: lldb/trunk/tools/debugserver/source/RNBContext.cpp lldb/trunk/tools/debugserver/source/RNBContext.h lldb/trunk/tools/debugserver/source/debugserver.cpp lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h Index: lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt === --- lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt +++ lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt @@ -13,9 +13,9 @@ add_lldb_test_executable(environment_check inferior/environment_check.cpp) if (CMAKE_SYSTEM_NAME MATCHES "Darwin") - add_definitions(-DLLDB_SERVER="$") + add_definitions(-DLLDB_SERVER="$" -DLLDB_SERVER_IS_DEBUGSERVER=1) else() - add_definitions(-DLLDB_SERVER="$") + add_definitions(-DLLDB_SERVER="$" -DLLDB_SERVER_IS_DEBUGSERVER=0) endif() add_definitions( Index: lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h === --- lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h +++ lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h @@ -21,12 +21,20 @@ #include #include +#if LLDB_SERVER_IS_DEBUGSERVER +#define LLGS_TEST(x) DISABLED_ ## x +#define DS_TEST(x) x +#else +#define LLGS_TEST(x) x +#define DS_TEST(x) DISABLED_ ## x +#endif + namespace llgs_tests { class TestClient : public lldb_private::process_gdb_remote::GDBRemoteCommunicationClient { public: - static bool IsDebugServer(); - static bool IsLldbServer(); + static bool IsDebugServer() { return LLDB_SERVER_IS_DEBUGSERVER; } + static bool IsLldbServer() { return !IsDebugServer(); } /// Launches the server, connects it to the client and returns the client. If /// Log is non-empty, the server will write it's log to this file. @@ -37,6 +45,13 @@ static llvm::Expected> launch(llvm::StringRef Log, llvm::ArrayRef InferiorArgs); + /// Allows user to pass additional arguments to the server. Be careful when + /// using this for generic tests, as the two stubs have different + /// command-line interfaces. + static llvm::Expected> + launchCustom(llvm::StringRef Log, llvm::ArrayRef ServerArgs, llvm::ArrayRef InferiorArgs); + + ~TestClient() override; llvm::Error SetInferior(llvm::ArrayRef inferior_args); llvm::Error ListThreadsInStopReply(); Index: lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp === --- lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp +++ lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp @@ -16,11 +16,6 @@ using namespace llvm; TEST_F(TestBase, LaunchModePreservesEnvironment) { - if (TestClient::IsDebugServer()) { -GTEST_LOG_(WARNING) << "Test fails with debugserver: llvm.org/pr35671"; -return; - } - putenv(const_cast("LLDB_TEST_MAGIC_VARIABLE=LLDB_TEST_MAGIC_VALUE")); auto ClientOr = TestClient::launch(getLogFileName(), @@ -34,3 +29,20 @@ HasValue(testing::Property(&StopReply::getKind, WaitStatus{WaitStatus::Exit, 0}))); } + +TEST_F(TestBase, DS_TEST(DebugserverEnv)) { + // Test that --env takes precedence over inherited environment variables. + putenv(const_cast("LLDB_TEST_MAGIC_VARIABLE=foobar")); + + auto ClientOr = TestClient::launchCustom(getLogFileName(), + { "--env", "LLDB_TEST_MAGIC_VARIABLE=LLDB_TEST_MAGIC_VALUE" }, + {getInferiorPath("environment_check")}); + ASSERT_THAT_EXPECTED(ClientOr, Succeeded()); + auto &Client = **ClientOr; + + ASSERT_THAT_ERROR(Client.ContinueAll(), Succeeded()); + ASSERT_THAT_EXPECTED( + Client.GetLatestStopReplyAs(), + HasValue(testing::Property(&StopReply::getKind, + WaitStatus{WaitStatus::Exit, 0}))); +} Index: lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp === --- lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp +++ lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp @@ -27,11 +27,6 @@ using namespace llvm; namespace llgs_tests { -bool TestClient::IsDebugServer() { - return sys::path::filename(LLDB_SERVER).contains("debugserver"); -} - -bool TestClient::IsLldbServer() { return !IsDebugServer(); } TestClient::TestClient(std::unique_ptr Conn) { SetConnection(Conn.release()); @@ -56,6 +51,10 @@ } Expected> TestClient::launch(StringRef Log, ArrayRef InferiorArgs) { + return launchCustom(Log, {}, InferiorArgs); +} + +Exp
Re: [Lldb-commits] [PATCH] D41086: [lldb] Stop searching for a symbol in a pdb by regex
We should fix the test case to not require regex, or add SymbolFile::FindTypesByRegex(...) and pass that through to public API if needed. Not sure where the test is failing (gtest, or public API test). > On Dec 22, 2017, at 2:01 AM, Pavel Labath via Phabricator > wrote: > > labath added a comment. > > This is making the windows unit tests fail: > http://lab.llvm.org:8011/builders/lldb-windows7-android/builds/7378/steps/run%20unit%20tests/logs/stdio. > If I understand correctly, this changes FindTypes to **not** search by regex. > If that's the case, then the `SymbolFilePDBTests::TestRegexNameMatch` needs > to be updated to not expect that `.*` will find a match. > Also, while technically not failing, the `SymbolFilePDBTests::TestMaxMatches` > also looks weird in this new world, and probably needs updating. > > Could you look into those? > > > Repository: > rL LLVM > > https://reviews.llvm.org/D41086 > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D41086: [lldb] Stop searching for a symbol in a pdb by regex
The url provided times out but my guess is these two unit tests are what are failing. lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp - TEST_F(SymbolFilePDBTests, TestRegexNameMatch) - TEST_F(SymbolFilePDBTests, TestMaxMatches) The first one is testing regex name matching which was decided in this code review to not support right now. The second one is using a regex to return all symbol names and checking that the number of results returned by FindTypes() is consistent. The first one can be updated to use FindTypesByRegex() if the API is made public as Greg recommends. The second needs more thought. On Fri, Dec 22, 2017 at 7:54 AM, Greg Clayton wrote: > We should fix the test case to not require regex, or add > SymbolFile::FindTypesByRegex(...) and pass that through to public API if > needed. Not sure where the test is failing (gtest, or public API test). > >> On Dec 22, 2017, at 2:01 AM, Pavel Labath via Phabricator >> wrote: >> >> labath added a comment. >> >> This is making the windows unit tests fail: >> http://lab.llvm.org:8011/builders/lldb-windows7-android/builds/7378/steps/run%20unit%20tests/logs/stdio. >> If I understand correctly, this changes FindTypes to **not** search by >> regex. >> If that's the case, then the `SymbolFilePDBTests::TestRegexNameMatch` needs >> to be updated to not expect that `.*` will find a match. >> Also, while technically not failing, the >> `SymbolFilePDBTests::TestMaxMatches` also looks weird in this new world, >> and probably needs updating. >> >> Could you look into those? >> >> >> Repository: >> rL LLVM >> >> https://reviews.llvm.org/D41086 >> >> >> > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41550: Update failing PDB unit tests that are searching for symbols by regex to use FindTypesByRegex instead of FindTypes
asmith created this revision. asmith added reviewers: zturner, lldb-commits, labath, clayborg. https://reviews.llvm.org/D41086 fixed an exception in FindTypes()/FindTypesByRegex() and caused two lldb unit test to fail. This change updates the unit tests to pass again. Related review: https://reviews.llvm.org/D41086 Repository: rL LLVM https://reviews.llvm.org/D41550 Files: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp Index: unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp === --- unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp +++ unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp @@ -512,13 +512,14 @@ SymbolVendor *plugin = module->GetSymbolVendor(); SymbolFilePDB *symfile = static_cast(plugin->GetSymbolFile()); - SymbolContext sc; - llvm::DenseSet searched_files; TypeMap results; - uint32_t num_results = symfile->FindTypes(sc, ConstString(".*"), nullptr, -false, 0, searched_files, results); - EXPECT_GT(num_results, 1u); - EXPECT_EQ(num_results, results.GetSize()); + symfile->FindTypesByRegex(RegularExpression(".*"), 0, results); + EXPECT_GT(results.GetSize(), 1u); + + // We expect no exception thrown if the given regex can't be compiled + results.Clear(); + symfile->FindTypesByRegex(RegularExpression("**"), 0, results); + EXPECT_EQ(0u, results.GetSize()); } TEST_F(SymbolFilePDBTests, TestMaxMatches) { @@ -532,7 +533,8 @@ SymbolContext sc; llvm::DenseSet searched_files; TypeMap results; - uint32_t num_results = symfile->FindTypes(sc, ConstString(".*"), nullptr, + const ConstString name("ClassTypedef"); + uint32_t num_results = symfile->FindTypes(sc, name, nullptr, false, 0, searched_files, results); // Try to limit ourselves from 1 to 10 results, otherwise we could be doing // this thousands of times. @@ -542,7 +544,7 @@ uint32_t iterations = std::min(num_results, 10u); for (uint32_t i = 1; i <= iterations; ++i) { uint32_t num_limited_results = symfile->FindTypes( -sc, ConstString(".*"), nullptr, false, i, searched_files, results); +sc, name, nullptr, false, i, searched_files, results); EXPECT_EQ(i, num_limited_results); EXPECT_EQ(num_limited_results, results.GetSize()); } Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h === --- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h +++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h @@ -140,6 +140,10 @@ size_t FindTypes(const std::vector &context, bool append, lldb_private::TypeMap &types) override; + void FindTypesByRegex(const lldb_private::RegularExpression ®ex, +uint32_t max_matches, +lldb_private::TypeMap &types); + lldb_private::TypeList *GetTypeList() override; size_t GetTypes(lldb_private::SymbolContextScope *sc_scope, @@ -172,9 +176,6 @@ const llvm::pdb::PDBSymbolCompiland &cu, llvm::DenseMap &index_map) const; - void FindTypesByRegex(const std::string ®ex, uint32_t max_matches, -lldb_private::TypeMap &types); - void FindTypesByName(const std::string &name, uint32_t max_matches, lldb_private::TypeMap &types); Index: unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp === --- unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp +++ unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp @@ -512,13 +512,14 @@ SymbolVendor *plugin = module->GetSymbolVendor(); SymbolFilePDB *symfile = static_cast(plugin->GetSymbolFile()); - SymbolContext sc; - llvm::DenseSet searched_files; TypeMap results; - uint32_t num_results = symfile->FindTypes(sc, ConstString(".*"), nullptr, -false, 0, searched_files, results); - EXPECT_GT(num_results, 1u); - EXPECT_EQ(num_results, results.GetSize()); + symfile->FindTypesByRegex(RegularExpression(".*"), 0, results); + EXPECT_GT(results.GetSize(), 1u); + + // We expect no exception thrown if the given regex can't be compiled + results.Clear(); + symfile->FindTypesByRegex(RegularExpression("**"), 0, results); + EXPECT_EQ(0u, results.GetSize()); } TEST_F(SymbolFilePDBTests, TestMaxMatches) { @@ -532,7 +533,8 @@ SymbolContext sc; llvm::DenseSet searched_files; TypeMap results; - uint32_t num_results = symfile->FindTypes(sc, ConstString(".*"), nullptr, + const ConstString name("ClassTypedef"); + uint32_t num_results = symfile->FindTypes(sc, name, nullptr, false, 0, searched_files, results); // Try to limit ourselves from 1 to 10 results, otherwise we could be doing // this thousands of times. @@ -542,7 +544,7 @@ uint32_t iterations = std::min
[Lldb-commits] [PATCH] D41427: Fix crash when parsing the type of a function without any arguments
asmith updated this revision to Diff 128069. asmith retitled this revision from "[lldb] Fix crash when parsing the type of a function without any arguments" to "Fix crash when parsing the type of a function without any arguments". asmith edited the summary of this revision. Repository: rL LLVM https://reviews.llvm.org/D41427 Files: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp unittests/SymbolFile/PDB/Inputs/test-pdb-types.exe unittests/SymbolFile/PDB/Inputs/test-pdb-types.pdb unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp Index: unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp === --- unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp +++ unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp @@ -484,7 +484,10 @@ llvm::DenseSet searched_files; TypeMap results; - const char *TypedefsToCheck[] = {"ClassTypedef", "NSClassTypedef"}; + const char *TypedefsToCheck[] = { + "ClassTypedef", "NSClassTypedef", + "FuncPointerTypedef", "VariadicFuncPointerTypedef" + }; for (auto Typedef : TypedefsToCheck) { TypeMap results; EXPECT_EQ(1u, symfile->FindTypes(sc, ConstString(Typedef), nullptr, false, Index: unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp === --- unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp +++ unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp @@ -48,6 +48,10 @@ typedef Class ClassTypedef; typedef NS::NSClass NSClassTypedef; +typedef int(*FuncPointerTypedef)(); +typedef int(*VariadicFuncPointerTypedef)(char,...); +FuncPointerTypedef GlobalFunc; +VariadicFuncPointerTypedef GlobalVariadicFunc; int GlobalArray[10]; static const int sizeof_NSClass = sizeof(NS::NSClass); @@ -57,6 +61,8 @@ static const int sizeof_ShortEnum = sizeof(ShortEnum); static const int sizeof_ClassTypedef = sizeof(ClassTypedef); static const int sizeof_NSClassTypedef = sizeof(NSClassTypedef); +static const int sizeof_FuncPointerTypedef = sizeof(FuncPointerTypedef); +static const int sizeof_VariadicFuncPointerTypedef = sizeof(VariadicFuncPointerTypedef); static const int sizeof_GlobalArray = sizeof(GlobalArray); int main(int argc, char **argv) { Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp === --- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp +++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp @@ -26,12 +26,12 @@ #include "llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h" #include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionArg.h" #include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h" +#include "llvm/DebugInfo/PDB/PDBSymbolTypePointer.h" #include "llvm/DebugInfo/PDB/PDBSymbolTypeTypedef.h" #include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h" using namespace lldb; using namespace lldb_private; -using namespace llvm; using namespace llvm::pdb; namespace { @@ -141,8 +141,14 @@ } else if (auto func_sig = llvm::dyn_cast(&type)) { auto arg_enum = func_sig->getArguments(); uint32_t num_args = arg_enum->getChildCount(); +bool is_variadic = false; std::vector arg_list; while (auto arg = arg_enum->getNext()) { + if (arg->getSymTag() == PDB_SymType::BuiltinType && + arg->getRawSymbol().getLength() == 0) { +is_variadic = true; +continue; + } lldb_private::Type *arg_type = m_ast.GetSymbolFile()->ResolveTypeUID(arg->getSymIndexId()); // If there's some error looking up one of the dependent types of this @@ -168,7 +174,8 @@ if (func_sig->isVolatileType()) type_quals |= clang::Qualifiers::Volatile; CompilerType func_sig_ast_type = m_ast.CreateFunctionType( -return_ast_type, arg_list.data(), arg_list.size(), false, type_quals); +return_ast_type, arg_list.data(), arg_list.size(), is_variadic, +type_quals); return std::make_shared( func_sig->getSymIndexId(), m_ast.GetSymbolFile(), ConstString(), 0, @@ -188,6 +195,35 @@ array_type->getSymIndexId(), m_ast.GetSymbolFile(), ConstString(), bytes, nullptr, LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID, decl, array_ast_type, lldb_private::Type::eResolveStateFull); + } else if (auto *builtin_type = llvm::dyn_cast(&type)) { +uint64_t bytes = builtin_type->getLength(); +PDB_BuiltinType pdb_builtin = builtin_type->getBuiltinType(); +Encoding encoding = TranslateBuiltinEncoding(pdb_builtin); + +// A 'void' builtin type has zero byte and invalid encoding type. +if (bytes || encoding != eEncodingInvalid || +pdb_builtin == PDB_BuiltinType::Void) { + CompilerType builtin_ast_type = + m_ast.GetBuiltinTypeForEncodingAndBitSize(encoding, bytes * 8); + + return std::make_shared( + builtin_type->getSymIndexId(), m_ast.GetSymbolFile(), ConstString(), + bytes, nullptr, LLDB_INV
[Lldb-commits] [PATCH] D41427: Fix crash when parsing the type of a function without any arguments
zturner added a comment. This was originally written as a unit test because at the time we didn't have `lldb-test`. To be honest I think it's time to remove these checked in binaries and convert everything to FileCheck tests. There's a couple of reasons this is more practical. For starters, the binaries can be really large and having lots of large binaries in the repo is a real drag for developers. Secondly, we've recently had some bug reports where some virus scan software is flagging our executables as malicious due to some heuristics. So while you're just modifying an existing binary / test, if it's not a ton of work I would really prefer standardizing on `lldb-test` + `FileCheck` for this sort of thing. Did you explore that at all? While there's a little higher up front cost because you have to go update the `lldb-test` tool to support this, after that work is done it will make writing future tests very easy. All you would need is something like this: REQUIRES: system-windows RUN: clang-cl /Z7 /c %S/Inputs/foo.cpp /o %T/foo.cpp.obj RUN: link /DEBUG /PDB:%T/foo.cpp.pdb /Fo:%T/foo.cpp.exe RUN: lldb-test symbols -types foo.cpp.exe | FileCheck %s CHECK: int(*FuncPointerTypedef)(); CHECK: int(*VariadicFuncPointerTypedef)(char,...); Repository: rL LLVM https://reviews.llvm.org/D41427 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits