[Lldb-commits] [PATCH] D32441: Remove the home-grown android toolchain file and all references to it
labath marked 2 inline comments as done. labath added inline comments. Comment at: www/build.html:488-493 + -DCMAKE_TOOLCHAIN_FILE=$ANDROID_NDK_HOME/build/cmake/android.toolchain.cmake \ + -DANDROID_ABI=arm64-v8a \ + -DANDROID_PLATFORM=android-21 \ + -DANDROID_ALLOW_UNDEFINED_SYMBOLS=On \ + -DLLVM_HOST_TRIPLE=aarch64-unknown-linux-android \ + -DCROSS_TOOLCHAIN_FLAGS_NATIVE='-DCMAKE_C_COMPILER=cc;-DCMAKE_CXX_COMPILER=c++' tberghammer wrote: > tberghammer wrote: > > tberghammer wrote: > > > Can you add some instructions about the supported ANDROID_ABI and > > > LLVM_HOST_TRIPLE pairs? I would find it a bit hard to figure them out on > > > my own (especially the correct triple) > > Shouldn't we specify LLVM_TARGETS_TO_BUILD as well to reduce the size of > > the executable? > Previously we specified LLVM_TARGET_ARCH as well. Is it not needed anymore? The `LLVM_HOST_TRIPLE` thingy is described in the generic section above as ```The triple of the system that lldb (or lldb-server) will run on. Not setting this (or setting it incorrectly) can cause a lot of issues with remote debugging as a lot of the choices lldb makes depend on the triple reported by the remote platform. ``` LLVM_TARGET_ARCH is computed from the host triple, so it's not necessary if you set the latter properly. As for TARGETS_TO_BUILD, I chose to omit that as I just want to present a minimal working configuration. If we go the optimization route, then there are many other variables you might want to set (LLVM_BUILD_STATIC, LLVM_USE_LINKER=gold to enable dead stripping, ...), but I don't think that's the purpose of this example. https://reviews.llvm.org/D32441 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32441: Remove the home-grown android toolchain file and all references to it
labath updated this revision to Diff 96514. labath added a comment. Address Tamas's comments. https://reviews.llvm.org/D32441 Files: cmake/platforms/Android.cmake www/build.html Index: www/build.html === --- www/build.html +++ www/build.html @@ -354,9 +354,8 @@ the target architecture. Since you already have a checkout of clang and lldb, you can compile a host version of clang in a separate folder and use that. Alternatively you can use system clang or even cross-gcc if your distribution - provides such packages (e.g., g++-aarch64-linux-gnu on Ubuntu). On - Android, a working toolchain can be produced by downloading the Android NDK and - running the contained make-standalone-toolchain.sh script. + provides such packages (e.g., g++-aarch64-linux-gnu + on Ubuntu). @@ -381,11 +380,6 @@ - In the case of Android, all required headers and libraries are provided by the - aforementioned make-standalone-toolchain.sh script. - - - Once all of the dependencies are in place, it's just a matter of configuring the build system with the locations and arguments of all the necessary tools. The most important cmake options here are: @@ -472,38 +466,37 @@ Example 2: Cross-compiling for Android on Linux - All tools needed to build LLDB for android are available in the Android NDK. For - example, we can produce an x86 toolchain along with all the libraries and headers - by running + In the case of Android, the toolchain and all required headers and + libraries are available in the Android NDK. - - ./build/tools/make-standalone-toolchain.sh \ - --platform=android-21 \ - --toolchain=x86-4.9 \ - --install-dir=$HOME/Toolchains/x86-android-toolchain - + - from inside the unzipped NDK. Toolchains for other architectures can be produced in - a similar manner. + The NDK also contains a cmake toolchain file, which makes + configuring the build much simpler. The compiler, include and + library paths will be configured by the toolchain file and all you + need to do is to select the architecture (ANDROID_ABI) and + platform level (ANDROID_PLATFORM, should be at least 21). You will + also need to set ANDROID_ALLOW_UNDEFINED_SYMBOLS=On, as the + toolchain file defaults to "no undefined symbols in shared + libraries", which is not compatible with some llvm libraries. The + first version of NDK which supports this approach is r14. - - For Android we provide a Android.cmake script which sets a lot of the required - options automatically. A cmake build can therefore be prepared with the following parameters: + For example, the following arguments are sufficient to configure + an android arm64 build: - -DCMAKE_TOOLCHAIN_FILE=cmake/platforms/Android.cmake \ - -DANDROID_TOOLCHAIN_DIR=$HOME/Toolchains/x86-android-toolchain \ - -DANDROID_ABI=x86 \ - -DLLVM_HOST_TRIPLE=i386-unknown-linux-android \ - -DLLVM_TABLEGEN=/bin/llvm-tblgen \ - -DCLANG_TABLEGEN= /bin/clang-tblgen + -DCMAKE_TOOLCHAIN_FILE=$ANDROID_NDK_HOME/build/cmake/android.toolchain.cmake \ + -DANDROID_ABI=arm64-v8a \ + -DANDROID_PLATFORM=android-21 \ + -DANDROID_ALLOW_UNDEFINED_SYMBOLS=On \ + -DLLVM_HOST_TRIPLE=aarch64-unknown-linux-android \ + -DCROSS_TOOLCHAIN_FLAGS_NATIVE='-DCMAKE_C_COMPILER=cc;-DCMAKE_CXX_COMPILER=c++' - Note that the full LLVM build is not functional on android yet, so simply running - ninja will not work. You will need to manually specify the target you - want to build: lldb, lldb-server, etc. + Note that currently only lldb-server is functional on android. The + lldb client is not supported and unlikely to work. Index: cmake/platforms/Android.cmake === --- cmake/platforms/Android.cmake +++ /dev/null @@ -1,155 +0,0 @@ -# Toolchain config for Android standalone NDK. -# -# Usage: -# build host llvm and clang first -# cmake -DCMAKE_TOOLCHAIN_FILE=../lldb/cmake/platforms/Android.cmake \ -# -DANDROID_TOOLCHAIN_DIR= \ -#
Re: [Lldb-commits] [PATCH] D32434: ObjectFileELF: Fix symbol lookup in bss section
On 24 April 2017 at 23:16, Zachary Turner wrote: > I suppose that's a reasonable concern. I also didn't notice that this was > a unittest, I assumed it was a lit test. > > If that's the case, is there any way to use llvm/Object to construct a > minimal ELF file in memory with a hardcoded symbol in the BSS section. It > can even be missing certain critical things that would be normally required > by an ELF file, and contain nothing but a single BSS section. Write it to > some memory buffer, and then use ObjectFileELF to try to do a lookup on it. > > This way there's no input files of *any* kind. Is this possible? > That's sort of that yaml2obj is, except that you use yaml instead of c++ to describe the file you want to construct. That makes it a lot easier to write the test, as you can just use obj2yaml to get the yaml (and prune it if necessary), so this is the direction I'd like to move to. It does mean that there will be an input file, but I don't think that's such a bad thing. (well, I suppose I could pipe the yaml into yaml2obj, but that is not going to help anything). I'll still need to create a temporary elf file, as the api's I am using accept a file name. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32125: [LLVM][MIPS] Fix different definition of off_t in LLDB and LLVM
sdardis added a comment. Hi Nitesh, this commit broke clang-cmake-mips. Can you investigate? http://lab.llvm.org:8011/builders/clang-cmake-mips/builds/3189 Thanks, Simon Repository: rL LLVM https://reviews.llvm.org/D32125 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32421: Fix segfault resulting from empty print prompt
krytarowski added a comment. I cannot reproduce it locally. $ lldb (lldb) print Enter expressions, then terminate with an empty line to evaluate: (lldb) Enter expressions, then terminate with an empty line to evaluate: (lldb) Steps: 1. start lldb 2. "print" 3. 4. `NetBSD 7.99.70 amd64` with our basesystem editline(3). https://reviews.llvm.org/D32421 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32441: Remove the home-grown android toolchain file and all references to it
tberghammer accepted this revision. tberghammer added a comment. Looks good https://reviews.llvm.org/D32441 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32434: ObjectFileELF: Fix symbol lookup in bss section
labath updated this revision to Diff 96540.
labath added a comment.
Use yaml2obj to avoid checking in a binary.
https://reviews.llvm.org/D32434
Files:
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
unittests/ObjectFile/ELF/CMakeLists.txt
unittests/ObjectFile/ELF/Inputs/sections-resolve-consistently.obj
unittests/ObjectFile/ELF/Inputs/sections-resolve-consistently.yaml
unittests/ObjectFile/ELF/TestObjectFileELF.cpp
Index: unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- /dev/null
+++ unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -0,0 +1,104 @@
+//===-- TestObjectFileELF.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/ObjectFileELF.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/HostInfo.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/SymbolVendor/ELF/SymbolVendorELF.h"
+
+extern const char *TestMainArgv0;
+
+using namespace lldb_private;
+using namespace lldb;
+
+class ObjectFileELFTest : public testing::Test {
+public:
+ void SetUp() override {
+HostInfo::Initialize();
+ObjectFileELF::Initialize();
+SymbolVendorELF::Initialize();
+
+m_inputs_folder = llvm::sys::path::parent_path(TestMainArgv0);
+llvm::sys::path::append(m_inputs_folder, "Inputs");
+llvm::sys::fs::make_absolute(m_inputs_folder);
+ }
+
+ void TearDown() override {
+SymbolVendorELF::Terminate();
+ObjectFileELF::Terminate();
+HostInfo::Terminate();
+ }
+
+protected:
+ llvm::SmallString<128> m_inputs_folder;
+};
+
+#define ASSERT_NO_ERROR(x) \
+ if (std::error_code ASSERT_NO_ERROR_ec = x) {\
+llvm::SmallString<128> MessageStorage; \
+llvm::raw_svector_ostream Message(MessageStorage); \
+Message << #x ": did not return errc::success.\n" \
+<< "error number: " << ASSERT_NO_ERROR_ec.value() << "\n" \
+<< "error message: " << ASSERT_NO_ERROR_ec.message() << "\n"; \
+GTEST_FATAL_FAILURE_(MessageStorage.c_str()); \
+ } else { \
+ }
+
+TEST_F(ObjectFileELFTest, SectionsResolveConsistently) {
+ llvm::SmallString<128> yaml = m_inputs_folder;
+ llvm::sys::path::append(yaml, "sections-resolve-consistently.yaml");
+ llvm::SmallString<128> obj = m_inputs_folder;
+ ASSERT_NO_ERROR(llvm::sys::fs::createTemporaryFile(
+ "sections-resolve-consistently-%%", "obj", obj));
+
+ std::vector args{YAML2OBJ, yaml.c_str(), nullptr};
+ llvm::StringRef obj_ref = obj;
+ std::vector redirects{nullptr, &obj_ref, nullptr};
+ ASSERT_EQ(0, llvm::sys::ExecuteAndWait(YAML2OBJ, args.data(), nullptr,
+ redirects.data()));
+ uint64_t size;
+ ASSERT_NO_ERROR(llvm::sys::fs::file_size(obj, size));
+ ASSERT_GT(size, 0u);
+
+ ModuleSpec spec{FileSpec(obj, false)};
+ spec.GetSymbolFileSpec().SetFile(obj, false);
+ auto module_sp = std::make_shared(spec);
+ SectionList *list = module_sp->GetSectionList();
+ ASSERT_NE(nullptr, list);
+
+ auto bss_sp = list->FindSectionByName(ConstString(".bss"));
+ ASSERT_NE(nullptr, bss_sp);
+ auto data_sp = list->FindSectionByName(ConstString(".data"));
+ ASSERT_NE(nullptr, data_sp);
+ auto text_sp = list->FindSectionByName(ConstString(".text"));
+ ASSERT_NE(nullptr, text_sp);
+
+ const Symbol *X = module_sp->FindFirstSymbolWithNameAndType(ConstString("X"),
+ eSymbolTypeAny);
+ ASSERT_NE(nullptr, X);
+ EXPECT_EQ(bss_sp, X->GetAddress().GetSection());
+
+ const Symbol *Y = module_sp->FindFirstSymbolWithNameAndType(ConstString("Y"),
+ eSymbolTypeAny);
+ ASSERT_NE(nullptr, Y);
+ EXPECT_EQ(data_sp, Y->GetAddress().GetSection());
+
+ const Symbol *start = module_sp->FindFirstSymbolWithNameAndType(
+ ConstString("_start"), eSymbolTypeAny);
+ ASSERT_NE(nullptr, start);
+ EXPECT_EQ(text_sp, start->GetAddress().GetSection());
+}
Index: unittests/ObjectFile/ELF/Inputs/sections-resolve-consistently.yaml
===
--- /dev/null
+++ unittests/ObjectFile/ELF/Inputs/sections-resolve-consistently.yaml
@@ -0,0 +1,50 @@
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data:
[Lldb-commits] [PATCH] D32434: ObjectFileELF: Fix symbol lookup in bss section
labath updated this revision to Diff 96541.
labath added a comment.
Use yaml2obj to avoid checking in a binary.
https://reviews.llvm.org/D32434
Files:
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
unittests/ObjectFile/ELF/CMakeLists.txt
unittests/ObjectFile/ELF/Inputs/sections-resolve-consistently.yaml
unittests/ObjectFile/ELF/TestObjectFileELF.cpp
Index: unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- /dev/null
+++ unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -0,0 +1,104 @@
+//===-- TestObjectFileELF.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/ObjectFileELF.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/HostInfo.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/SymbolVendor/ELF/SymbolVendorELF.h"
+
+extern const char *TestMainArgv0;
+
+using namespace lldb_private;
+using namespace lldb;
+
+class ObjectFileELFTest : public testing::Test {
+public:
+ void SetUp() override {
+HostInfo::Initialize();
+ObjectFileELF::Initialize();
+SymbolVendorELF::Initialize();
+
+m_inputs_folder = llvm::sys::path::parent_path(TestMainArgv0);
+llvm::sys::path::append(m_inputs_folder, "Inputs");
+llvm::sys::fs::make_absolute(m_inputs_folder);
+ }
+
+ void TearDown() override {
+SymbolVendorELF::Terminate();
+ObjectFileELF::Terminate();
+HostInfo::Terminate();
+ }
+
+protected:
+ llvm::SmallString<128> m_inputs_folder;
+};
+
+#define ASSERT_NO_ERROR(x) \
+ if (std::error_code ASSERT_NO_ERROR_ec = x) {\
+llvm::SmallString<128> MessageStorage; \
+llvm::raw_svector_ostream Message(MessageStorage); \
+Message << #x ": did not return errc::success.\n" \
+<< "error number: " << ASSERT_NO_ERROR_ec.value() << "\n" \
+<< "error message: " << ASSERT_NO_ERROR_ec.message() << "\n"; \
+GTEST_FATAL_FAILURE_(MessageStorage.c_str()); \
+ } else { \
+ }
+
+TEST_F(ObjectFileELFTest, SectionsResolveConsistently) {
+ llvm::SmallString<128> yaml = m_inputs_folder;
+ llvm::sys::path::append(yaml, "sections-resolve-consistently.yaml");
+ llvm::SmallString<128> obj = m_inputs_folder;
+ ASSERT_NO_ERROR(llvm::sys::fs::createTemporaryFile(
+ "sections-resolve-consistently-%%", "obj", obj));
+
+ std::vector args{YAML2OBJ, yaml.c_str(), nullptr};
+ llvm::StringRef obj_ref = obj;
+ std::vector redirects{nullptr, &obj_ref, nullptr};
+ ASSERT_EQ(0, llvm::sys::ExecuteAndWait(YAML2OBJ, args.data(), nullptr,
+ redirects.data()));
+ uint64_t size;
+ ASSERT_NO_ERROR(llvm::sys::fs::file_size(obj, size));
+ ASSERT_GT(size, 0u);
+
+ ModuleSpec spec{FileSpec(obj, false)};
+ spec.GetSymbolFileSpec().SetFile(obj, false);
+ auto module_sp = std::make_shared(spec);
+ SectionList *list = module_sp->GetSectionList();
+ ASSERT_NE(nullptr, list);
+
+ auto bss_sp = list->FindSectionByName(ConstString(".bss"));
+ ASSERT_NE(nullptr, bss_sp);
+ auto data_sp = list->FindSectionByName(ConstString(".data"));
+ ASSERT_NE(nullptr, data_sp);
+ auto text_sp = list->FindSectionByName(ConstString(".text"));
+ ASSERT_NE(nullptr, text_sp);
+
+ const Symbol *X = module_sp->FindFirstSymbolWithNameAndType(ConstString("X"),
+ eSymbolTypeAny);
+ ASSERT_NE(nullptr, X);
+ EXPECT_EQ(bss_sp, X->GetAddress().GetSection());
+
+ const Symbol *Y = module_sp->FindFirstSymbolWithNameAndType(ConstString("Y"),
+ eSymbolTypeAny);
+ ASSERT_NE(nullptr, Y);
+ EXPECT_EQ(data_sp, Y->GetAddress().GetSection());
+
+ const Symbol *start = module_sp->FindFirstSymbolWithNameAndType(
+ ConstString("_start"), eSymbolTypeAny);
+ ASSERT_NE(nullptr, start);
+ EXPECT_EQ(text_sp, start->GetAddress().GetSection());
+}
Index: unittests/ObjectFile/ELF/Inputs/sections-resolve-consistently.yaml
===
--- /dev/null
+++ unittests/ObjectFile/ELF/Inputs/sections-resolve-consistently.yaml
@@ -0,0 +1,50 @@
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data:ELFDATA2LSB
+ Type:ET_EXEC
+ Machine:
[Lldb-commits] [PATCH] D32434: ObjectFileELF: Fix symbol lookup in bss section
labath added a reviewer: beanz. labath added a subscriber: beanz. labath added a comment. Ok, wiring yaml2obj up was easier than I expected (for cmake, we'll still need to figure out what to do with the xcode build). Let me know what you make of this. Also adding @beanz, in case he has any thoughts on the subject. https://reviews.llvm.org/D32434 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r301306 - Remove the home-grown android toolchain file and all references to it
Author: labath
Date: Tue Apr 25 07:58:49 2017
New Revision: 301306
URL: http://llvm.org/viewvc/llvm-project?rev=301306&view=rev
Log:
Remove the home-grown android toolchain file and all references to it
Summary:
The toolchain file has been deprecated in favor of the "official"
toolchain file present in the Android NDK. Also update the web
build instructions to reflect this.
Reviewers: eugene
Subscribers: srhines, mgorny, dgross, tberghammer, lldb-commits
Differential Revision: https://reviews.llvm.org/D32441
Removed:
lldb/trunk/cmake/platforms/Android.cmake
Modified:
lldb/trunk/www/build.html
Removed: lldb/trunk/cmake/platforms/Android.cmake
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/platforms/Android.cmake?rev=301305&view=auto
==
--- lldb/trunk/cmake/platforms/Android.cmake (original)
+++ lldb/trunk/cmake/platforms/Android.cmake (removed)
@@ -1,155 +0,0 @@
-# Toolchain config for Android standalone NDK.
-#
-# Usage:
-# build host llvm and clang first
-# cmake -DCMAKE_TOOLCHAIN_FILE=../lldb/cmake/platforms/Android.cmake \
-# -DANDROID_TOOLCHAIN_DIR= \
-# -DANDROID_ABI= \
-# -DCMAKE_CXX_COMPILER_VERSION= \
-# -DLLVM_TARGET_ARCH= \
-# -DLLVM_TARGETS_TO_BUILD= \
-# -DLLVM_TABLEGEN= \
-# -DCLANG_TABLEGEN=
-#
-# Current Support:
-# ANDROID_ABI = x86, x86_64
-# CMAKE_CXX_COMPILER_VERSION = 4.9
-# LLVM_TARGET_ARCH = X86
-# LLVM_TARGETS_TO_BUILD = X86
-# LLVM_TABLEGEN = path to host llvm-tblgen
-# CLANG_TABLEGEN = path to host clang-tblgen
-
-if( DEFINED CMAKE_CROSSCOMPILING )
- return()
-endif()
-
-get_property( IS_IN_TRY_COMPILE GLOBAL PROPERTY IN_TRY_COMPILE )
-if( IS_IN_TRY_COMPILE )
- # this seems necessary and works fine but I'm unsure if it breaks anything
- return()
-endif()
-
-set( CMAKE_SYSTEM_NAME Android )
-include( CMakeForceCompiler )
-
-# flags and definitions
-add_definitions( -DANDROID -DLLDB_DISABLE_LIBEDIT )
-set( ANDROID True )
-set( LLDB_DEFAULT_DISABLE_LIBEDIT True )
-
-# linking lldb-server statically for Android avoids the need to ship two
-# binaries (pie for API 21+ and non-pie for API 16-). It's possible to use
-# a non-pie shim on API 16-, but that requires lldb-server to dynamically
export
-# its symbols, which significantly increases the binary size. Static linking,
on
-# the other hand, has little to no effect on the binary size.
-if( NOT DEFINED LLVM_BUILD_STATIC )
- set( LLVM_BUILD_STATIC True CACHE INTERNAL "" FORCE )
- set( LLVM_ENABLE_PIC FALSE CACHE INTERNAL "" FORCE )
- set( BUILD_SHARED_LIBS FALSE CACHE INTERNAL "" FORCE )
-endif()
-
-set( ANDROID_ABI "${ANDROID_ABI}" CACHE INTERNAL "Android Abi" FORCE )
-if( ANDROID_ABI STREQUAL "x86" )
- set( CMAKE_SYSTEM_PROCESSOR "i686" )
- set( ANDROID_TOOLCHAIN_NAME "i686-linux-android" )
-elseif( ANDROID_ABI STREQUAL "x86_64" )
- set( CMAKE_SYSTEM_PROCESSOR "x86_64" )
- set( ANDROID_TOOLCHAIN_NAME "x86_64-linux-android" )
-elseif( ANDROID_ABI STREQUAL "armeabi" )
- set( CMAKE_SYSTEM_PROCESSOR "armv5te" )
- set( ANDROID_TOOLCHAIN_NAME "arm-linux-androideabi" )
-elseif( ANDROID_ABI STREQUAL "aarch64" )
- set( CMAKE_SYSTEM_PROCESSOR "aarch64" )
- set( ANDROID_TOOLCHAIN_NAME "aarch64-linux-android" )
-elseif( ANDROID_ABI STREQUAL "mips" )
- set( CMAKE_SYSTEM_PROCESSOR "mips" )
- set( ANDROID_TOOLCHAIN_NAME "mipsel-linux-android" )
-elseif( ANDROID_ABI STREQUAL "mips64" )
- set( CMAKE_SYSTEM_PROCESSOR "mips64" )
- set( ANDROID_TOOLCHAIN_NAME "mips64el-linux-android" )
-else()
- message( SEND_ERROR "Unknown ANDROID_ABI = \"${ANDROID_ABI}\"." )
-endif()
-
-set( ANDROID_TOOLCHAIN_DIR "${ANDROID_TOOLCHAIN_DIR}" CACHE PATH "Android
standalone toolchain directory" )
-set( ANDROID_SYSROOT "${ANDROID_TOOLCHAIN_DIR}/sysroot" CACHE PATH "Android
Sysroot" )
-
-# CMAKE_EXECUTABLE_SUFFIX is undefined in CMAKE_TOOLCHAIN_FILE
-if( WIN32 )
- set( EXECUTABLE_SUFFIX ".exe" )
-endif()
-
-set( PYTHON_EXECUTABLE
"${ANDROID_TOOLCHAIN_DIR}/bin/python${EXECUTABLE_SUFFIX}" CACHE PATH "Python
exec path" )
-
-if( NOT CMAKE_C_COMPILER )
- set( CMAKE_C_COMPILER
"${ANDROID_TOOLCHAIN_DIR}/bin/${ANDROID_TOOLCHAIN_NAME}-gcc${EXECUTABLE_SUFFIX}"
CACHE PATH "C compiler" )
- set( CMAKE_CXX_COMPILER
"${ANDROID_TOOLCHAIN_DIR}/bin/${ANDROID_TOOLCHAIN_NAME}-g++${EXECUTABLE_SUFFIX}"
CACHE PATH "C++ compiler" )
- set( CMAKE_ASM_COMPILER
"${ANDROID_TOOLCHAIN_DIR}/bin/${ANDROID_TOOLCHAIN_NAME}-gcc${EXECUTABLE_SUFFIX}"
CACHE PATH "assembler" )
- set( CMAKE_STRIP
"${ANDROID_TOOLCHAIN_DIR}/bin/${ANDROID_TOOLCHAIN_NAME}-strip${EXECUTABLE_SUFFIX}"
CACHE PATH "strip" )
- set( CMAKE_AR
"${ANDROID_TOOLCHAIN_DIR}/bin/${ANDROID_TOOLCHAIN_NAME}-ar${EXECUTABLE_SUFFIX}"
CACHE PATH "archive" )
- set( CMAKE_LINKER
"${ANDROID_TOOLCHAIN_DIR}/bin/${ANDROID_TOOLCHAIN_NAME}-ld${EXECUTABLE_SUFFIX}"
CACHE PATH "linker" )
- set( CMAKE_NM
"${ANDROID_TOOLCHAIN_DIR}/bin/$
[Lldb-commits] [PATCH] D32441: Remove the home-grown android toolchain file and all references to it
This revision was automatically updated to reflect the committed changes. Closed by commit rL301306: Remove the home-grown android toolchain file and all references to it (authored by labath). Changed prior to commit: https://reviews.llvm.org/D32441?vs=96514&id=96542#toc Repository: rL LLVM https://reviews.llvm.org/D32441 Files: lldb/trunk/cmake/platforms/Android.cmake lldb/trunk/www/build.html Index: lldb/trunk/www/build.html === --- lldb/trunk/www/build.html +++ lldb/trunk/www/build.html @@ -354,9 +354,8 @@ the target architecture. Since you already have a checkout of clang and lldb, you can compile a host version of clang in a separate folder and use that. Alternatively you can use system clang or even cross-gcc if your distribution - provides such packages (e.g., g++-aarch64-linux-gnu on Ubuntu). On - Android, a working toolchain can be produced by downloading the Android NDK and - running the contained make-standalone-toolchain.sh script. + provides such packages (e.g., g++-aarch64-linux-gnu + on Ubuntu). @@ -381,11 +380,6 @@ - In the case of Android, all required headers and libraries are provided by the - aforementioned make-standalone-toolchain.sh script. - - - Once all of the dependencies are in place, it's just a matter of configuring the build system with the locations and arguments of all the necessary tools. The most important cmake options here are: @@ -472,38 +466,37 @@ Example 2: Cross-compiling for Android on Linux - All tools needed to build LLDB for android are available in the Android NDK. For - example, we can produce an x86 toolchain along with all the libraries and headers - by running + In the case of Android, the toolchain and all required headers and + libraries are available in the Android NDK. - - ./build/tools/make-standalone-toolchain.sh \ - --platform=android-21 \ - --toolchain=x86-4.9 \ - --install-dir=$HOME/Toolchains/x86-android-toolchain - + - from inside the unzipped NDK. Toolchains for other architectures can be produced in - a similar manner. + The NDK also contains a cmake toolchain file, which makes + configuring the build much simpler. The compiler, include and + library paths will be configured by the toolchain file and all you + need to do is to select the architecture (ANDROID_ABI) and + platform level (ANDROID_PLATFORM, should be at least 21). You will + also need to set ANDROID_ALLOW_UNDEFINED_SYMBOLS=On, as the + toolchain file defaults to "no undefined symbols in shared + libraries", which is not compatible with some llvm libraries. The + first version of NDK which supports this approach is r14. - - For Android we provide a Android.cmake script which sets a lot of the required - options automatically. A cmake build can therefore be prepared with the following parameters: + For example, the following arguments are sufficient to configure + an android arm64 build: - -DCMAKE_TOOLCHAIN_FILE=cmake/platforms/Android.cmake \ - -DANDROID_TOOLCHAIN_DIR=$HOME/Toolchains/x86-android-toolchain \ - -DANDROID_ABI=x86 \ - -DLLVM_HOST_TRIPLE=i386-unknown-linux-android \ - -DLLVM_TABLEGEN=/bin/llvm-tblgen \ - -DCLANG_TABLEGEN= /bin/clang-tblgen + -DCMAKE_TOOLCHAIN_FILE=$ANDROID_NDK_HOME/build/cmake/android.toolchain.cmake \ + -DANDROID_ABI=arm64-v8a \ + -DANDROID_PLATFORM=android-21 \ + -DANDROID_ALLOW_UNDEFINED_SYMBOLS=On \ + -DLLVM_HOST_TRIPLE=aarch64-unknown-linux-android \ + -DCROSS_TOOLCHAIN_FLAGS_NATIVE='-DCMAKE_C_COMPILER=cc;-DCMAKE_CXX_COMPILER=c++' - Note that the full LLVM build is not functional on android yet, so simply running - ninja will not work. You will need to manually specify the target you - want to build: lldb, lldb-server, etc. + Note that currently only lldb-server is functional on android. The + lldb client is not supported and unlikely to work. Index: lldb/trunk/cmake/platforms/Android.cmake === ---
[Lldb-commits] [PATCH] D32306: Remove lock from ConstString::GetLength
labath added inline comments. Comment at: source/Utility/ConstString.cpp:49 + // pointer, we don't need the lock. const StringPoolEntryType &entry = GetStringMapEntryFromKeyData(ccstr); return entry.getKey().size(); zturner wrote: > Why do we even have this function which digs into the `StringMap` internals > rather than just calling existing `StringMap` member functions? Can Can we > just delete `GetStringMapEntryFromKeyData` entirely and use `StringMap::find`? > Can we just delete GetStringMapEntryFromKeyData entirely and use > StringMap::find? Unfortunately, I don't think that's possible. `StringMap::find` expects a StringRef. In order to construct that, we need to know the length of the string, and we're back where we started :( In reality, this is doing a very different operation than find (which takes a random string and checks whether it's in the map) -- this takes a string which we know to be in the map and get its size. It will take some rearchitecting of the ConstString class to get rid of this hack. Probably it could be fixed by ConstString storing a StringMap::iterator instead of the raw pointer. In any case, that seems out of scope of this change. Repository: rL LLVM https://reviews.llvm.org/D32306 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32306: Remove lock from ConstString::GetLength
labath added a comment. In https://reviews.llvm.org/D32306#736115, @scott.smith wrote: > 10.2% reduction in # of instructions executed, 9.1% reduction in # of cycles, > as measured by 'perf stat' in single threaded mode (I disabled TaskPool in > order to get more repeatable results). That is awesome, thanks. > Can you commit this for me? I don't have commit access. I will, let's just wait what Zachary says. Repository: rL LLVM https://reviews.llvm.org/D32306 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32125: [LLVM][MIPS] Fix different definition of off_t in LLDB and LLVM
nitesh.jain added a comment. In https://reviews.llvm.org/D32125#736543, @sdardis wrote: > Hi Nitesh, > > this commit broke clang-cmake-mips. Can you investigate? > > http://lab.llvm.org:8011/builders/clang-cmake-mips/builds/3189 > > Thanks, > Simon Hi Simon, The assertion has been fixed. Thanks Repository: rL LLVM https://reviews.llvm.org/D32125 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32421: Fix segfault resulting from empty print prompt
labath added a comment. Thanks for the patch Alex. After looking around the code a bit (I'm quite new to that area as well), I think a better approach would be to fix MoveCursor to handle this situation gracefully. If you look at what this code does in the "normal" case, you'll see that it deletes the "n: " prompt on the last empty line. Your fix would bypass that and leave a dangling "1:" on the screen (which is not that bad really, but let's be consistent). If you look closely you'll see that the exact column we move the cursor to does not matter here (we immediately print \n), but we should still move the cursor one line up. Probably moving it to column 1 would be sufficient in this case. You don't have worry about performance of this part of the code too much, as nothing it does will be slower than your hands. Readability is more important here. A good way to "enhance" :) your learning experience would be to also create a test for this bug. I think that you will have to go for a pexpect-style test to reproduce this. TestConvenienceVariables.py would be a good candidate to model the test on. Let me know if you have any questions. > I cannot reproduce it locally. Well.. I guess that's how undefined behavior works in practice. :) https://reviews.llvm.org/D32421 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32316: Change UniqueCStringMap to use ConstString as the key
labath added inline comments.
Comment at: include/lldb/Interpreter/Property.h:43
+ ConstString GetName() const { return m_name; }
+ ConstString GetDescription() const { return m_description; }
scott.smith wrote:
> clayborg wrote:
> > This shouldn't be const-ified, Only names of things like variables,
> > enumerators, typenames, paths, and other strings that are going to be
> > uniqued should go into the ConstStringPool
> ok but note the original code - it's already storing m_name and m_description
> as ConstString. All I'm doing is exposing the underlying type. Would you
> prefer I change m_name and m_description to be std::string instead?
> Otherwise it won't actually save anything.
>
> Also note that later uses take the names are put into m_name_to_index in the
> option parser, which is a UniqueCString, which requires ConstString. I don't
> know the code well enough to know whether all options or only some options go
> through this, so I could see it being worth it to only convert to ConstString
> at that point (for example, see source/Interpreter/OptionValueProperties.cpp
> in this review).
I'd suggest keeping these as StringRef as well. The option parser does not do
anything performance critical (which probably means it shouldn't be using
UniqueCStringMap in the first place, but that is a different discussion...).
Comment at: include/lldb/Utility/ConstString.h:481
+ char operator[](size_t Index) const {
+assert(Index < GetLength() && "Invalid index!");
+return m_string[Index];
scott.smith wrote:
> clayborg wrote:
> > I really don't like the prospect of crashing the debugger if someone calls
> > this with an invalid index. Many people ship with asserts on. I would
> > either make it safe if it is indeed just for reporting or remove the assert.
> This code is copied from StringRef, which is what lldb used before this
> change. Do you still want me to revert the assert?
>
> Though IMO it should be changed to <=, not <, so that the null terminator can
> be read safely.
I think that asserting here is fine... There's no way you can make code that
likes to tread off the end of an array safe by changing this function.
However, I am not really fond of the idea of ConstString taking over StringRef
functionality. I think we should stick to requiring stringref conversions when
doing string manipulation. Maybe if you reduce the number of ConstString
occurences according to other comments, this will not be that necessary anymore
(?)
Repository:
rL LLVM
https://reviews.llvm.org/D32316
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32421: Fix segfault resulting from empty print prompt
xiaobai added a comment. @krytarowski: Thanks for checking! I can set up a NetBSD environment sometime in the next few days to see what's going on. While it might not be an issue on this platform, I think it's an issue that MoveCursor() accesses `m_input_lines[m_input_lines.size() - 1]` without verifying that `m_input_lines.size() > 0` explicitly. @labath: After reading over what you said, I believe I understand what you're saying. An older version of lldb (I have checked with the default version provided on MacOS Sierra 10.12.3) will leave the dangling "1: ", but I agree that consistency will be better than going back to the old behavior. I'll change my patch and add a test in the next day or two. Thank you so much for your feedback! :) https://reviews.llvm.org/D32421 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32434: ObjectFileELF: Fix symbol lookup in bss section
eugene accepted this revision. eugene added a comment. This revision is now accepted and ready to land. Neatly done! https://reviews.llvm.org/D32434 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32434: ObjectFileELF: Fix symbol lookup in bss section
zturner added a comment. If you look at the source code of yaml2obj, all this boils down to a single call to `ELFState::writeELF`. If you just link against that, we could avoid even shelling out to an external tool, and the YAML could be a string literal defined inside the unit test cpp file. This would also greatly simplify the xcode build. Thoughts? https://reviews.llvm.org/D32434 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32434: ObjectFileELF: Fix symbol lookup in bss section
labath planned changes to this revision. labath added a comment. In https://reviews.llvm.org/D32434#737179, @zturner wrote: > If you look at the source code of yaml2obj, all this boils down to a single > call to `ELFState::writeELF`. If you just link against that, we could > avoid even shelling out to an external tool, and the YAML could be a string > literal defined inside the unit test cpp file. This would also greatly > simplify the xcode build. > > Thoughts? I like that. Let me see how that goes.. https://reviews.llvm.org/D32434 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32503: Remove unused code related to CPlusPlusLanguage::FindEquivalentNames
scott.smith created this revision.
It is simply unused, and the header for it is private, so there should be no
external dependencies.
Repository:
rL LLVM
https://reviews.llvm.org/D32503
Files:
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
Index: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -119,18 +119,6 @@
llvm::StringRef &context,
llvm::StringRef &identifier);
- // in some cases, compilers will output different names for one same type.
- // when that happens, it might be impossible
- // to construct SBType objects for a valid type, because the name that is
- // available is not the same as the name that
- // can be used as a search key in FindTypes(). the equivalents map here is
- // meant to return possible alternative names
- // for a type through which a search can be conducted. Currently, this is only
- // enabled for C++ but can be extended
- // to ObjC or other languages if necessary
- static uint32_t FindEquivalentNames(ConstString type_name,
- std::vector &equivalents);
-
// Given a mangled function name, calculates some alternative manglings since
// the compiler mangling may not line up with the symbol we are expecting
static uint32_t
Index: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -271,144 +271,6 @@
return false;
}
-class CPPRuntimeEquivalents {
-public:
- CPPRuntimeEquivalents() {
-m_impl.Append(ConstString("std::basic_string, "
- "std::allocator >")
- .GetStringRef(),
- ConstString("basic_string"));
-
-// these two (with a prefixed std::) occur when c++stdlib string class
-// occurs as a template argument in some STL container
-m_impl.Append(ConstString("std::basic_string, "
- "std::allocator >")
- .GetStringRef(),
- ConstString("std::basic_string"));
-
-m_impl.Sort();
- }
-
- void Add(ConstString &type_name, ConstString &type_equivalent) {
-m_impl.Insert(type_name.GetStringRef(), type_equivalent);
- }
-
- uint32_t FindExactMatches(ConstString &type_name,
-std::vector &equivalents) {
-uint32_t count = 0;
-
-for (ImplData match =
- m_impl.FindFirstValueForName(type_name.GetStringRef());
- match != nullptr; match = m_impl.FindNextValueForName(match)) {
- equivalents.push_back(match->value);
- count++;
-}
-
-return count;
- }
-
- // partial matches can occur when a name with equivalents is a template
- // argument.
- // e.g. we may have "class Foo" be a match for "struct Bar". if we have a
- // typename
- // such as "class Templatized" we want this to be
- // replaced with
- // "class Templatized". Since partial matching is time
- // consuming
- // once we get a partial match, we add it to the exact matches list for faster
- // retrieval
- uint32_t FindPartialMatches(ConstString &type_name,
- std::vector &equivalents) {
-uint32_t count = 0;
-
-llvm::StringRef type_name_cstr = type_name.GetStringRef();
-
-size_t items_count = m_impl.GetSize();
-
-for (size_t item = 0; item < items_count; item++) {
- llvm::StringRef key_cstr = m_impl.GetCStringAtIndex(item);
- if (type_name_cstr.contains(key_cstr)) {
-count += AppendReplacements(type_name_cstr, key_cstr, equivalents);
- }
-}
-
-return count;
- }
-
-private:
- std::string &replace(std::string &target, std::string &pattern,
- std::string &with) {
-size_t pos;
-size_t pattern_len = pattern.size();
-
-while ((pos = target.find(pattern)) != std::string::npos)
- target.replace(pos, pattern_len, with);
-
-return target;
- }
-
- uint32_t AppendReplacements(llvm::StringRef original,
- llvm::StringRef matching_key,
- std::vector &equivalents) {
-std::string matching_key_str(matching_key);
-ConstString original_const(original);
-
-uint32_t count = 0;
-
-for (ImplData match = m_impl.FindFirstValueForName(matching_key);
- match != nullptr; match = m_impl.FindNextValueForName(match)) {
- std::string target(original);
- std::string equiv_class(match->value.AsCString());
-
- replace(target, matching_key_str, equiv_class);
-
- ConstString target_const(target.c_str());
-
-// you wi
[Lldb-commits] [PATCH] D29581: Initial implementation of SB APIs for Tracing support.
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good, lets start with this and iterate. https://reviews.llvm.org/D29581 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32316: Change UniqueCStringMap to use ConstString as the key
scott.smith updated this revision to Diff 96629.
scott.smith marked 21 inline comments as done.
scott.smith added a comment.
address review comments
Repository:
rL LLVM
https://reviews.llvm.org/D32316
Files:
include/lldb/Core/UniqueCStringMap.h
include/lldb/Symbol/ObjectFile.h
source/Interpreter/OptionValueEnumeration.cpp
source/Interpreter/OptionValueProperties.cpp
source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
source/Plugins/ObjectFile/ELF/ObjectFileELF.h
source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
source/Plugins/SymbolFile/DWARF/NameToDIE.h
source/Symbol/ClangASTContext.cpp
source/Symbol/GoASTContext.cpp
source/Symbol/Symtab.cpp
Index: source/Symbol/Symtab.cpp
===
--- source/Symbol/Symtab.cpp
+++ source/Symbol/Symtab.cpp
@@ -263,36 +263,35 @@
continue;
const Mangled &mangled = symbol->GetMangled();
- entry.cstring = mangled.GetMangledName().GetStringRef();
- if (!entry.cstring.empty()) {
+ entry.cstring = mangled.GetMangledName();
+ if (entry.cstring) {
m_name_to_index.Append(entry);
if (symbol->ContainsLinkerAnnotations()) {
// If the symbol has linker annotations, also add the version without
// the annotations.
entry.cstring = ConstString(m_objfile->StripLinkerSymbolAnnotations(
- entry.cstring))
- .GetStringRef();
+entry.cstring.GetStringRef()));
m_name_to_index.Append(entry);
}
const SymbolType symbol_type = symbol->GetType();
if (symbol_type == eSymbolTypeCode ||
symbol_type == eSymbolTypeResolver) {
- if (entry.cstring[0] == '_' && entry.cstring[1] == 'Z' &&
- (entry.cstring[2] != 'T' && // avoid virtual table, VTT structure,
- // typeinfo structure, and typeinfo
- // name
- entry.cstring[2] != 'G' && // avoid guard variables
- entry.cstring[2] != 'Z')) // named local entities (if we
+ llvm::StringRef entry_ref(entry.cstring.GetStringRef());
+ if (entry_ref[0] == '_' && entry_ref[1] == 'Z' &&
+ (entry_ref[2] != 'T' && // avoid virtual table, VTT structure,
+ // typeinfo structure, and typeinfo
+ // name
+ entry_ref[2] != 'G' && // avoid guard variables
+ entry_ref[2] != 'Z')) // named local entities (if we
// eventually handle eSymbolTypeData,
// we will want this back)
{
CPlusPlusLanguage::MethodName cxx_method(
mangled.GetDemangledName(lldb::eLanguageTypeC_plus_plus));
-entry.cstring =
-ConstString(cxx_method.GetBasename()).GetStringRef();
-if (!entry.cstring.empty()) {
+entry.cstring = ConstString(cxx_method.GetBasename());
+if (entry.cstring) {
// ConstString objects permanently store the string in the pool so
// calling
// GetCString() on the value gets us a const char * that will
@@ -300,7 +299,8 @@
const char *const_context =
ConstString(cxx_method.GetContext()).GetCString();
- if (entry.cstring[0] == '~' ||
+ entry_ref = entry.cstring.GetStringRef();
+ if (entry_ref[0] == '~' ||
!cxx_method.GetQualifiers().empty()) {
// The first character of the demangled basename is '~' which
// means we have a class destructor. We can use this information
@@ -341,17 +341,15 @@
}
}
- entry.cstring =
- mangled.GetDemangledName(symbol->GetLanguage()).GetStringRef();
- if (!entry.cstring.empty()) {
+ entry.cstring = mangled.GetDemangledName(symbol->GetLanguage());
+ if (entry.cstring) {
m_name_to_index.Append(entry);
if (symbol->ContainsLinkerAnnotations()) {
// If the symbol has linker annotations, also add the version without
// the annotations.
entry.cstring = ConstString(m_objfile->StripLinkerSymbolAnnotations(
- entry.cstring))
- .GetStringRef();
+entry.cstring.GetStringRef()));
m_name_to_index.Append(entry);
}
}
@@ -359,15 +357,15 @@
// If the demangled name turns out to be an ObjC name, and
// is a category name, add the version without categories to the index
[Lldb-commits] [PATCH] D32316: Change UniqueCStringMap to use ConstString as the key
scott.smith added inline comments.
Comment at: include/lldb/Symbol/ObjectFile.h:808-811
+ virtual ConstString
+ StripLinkerSymbolAnnotations(ConstString symbol_name) const {
+return symbol_name;
}
scott.smith wrote:
> clayborg wrote:
> > This actually doesn't need to change. Since it is merely stripping off
> > parts of the string, this should really stay as a StringRef and it should
> > return a StringRef. Change to use StringRef for return and for argument, or
> > revert.
> >
> The only user of StripLinkerSymbolAnnotations immediately converts it to a
> ConstString. Changing this back to using StringRef would mean an extra
> lookup for architectures that do not override this function.
reverting this has no measurable performance impact on my test, so even though
the caller has a ConstString, and will convert the result to a ConstString, I
will revert this.
Comment at: include/lldb/Utility/ConstString.h:481
+ char operator[](size_t Index) const {
+assert(Index < GetLength() && "Invalid index!");
+return m_string[Index];
labath wrote:
> scott.smith wrote:
> > clayborg wrote:
> > > I really don't like the prospect of crashing the debugger if someone
> > > calls this with an invalid index. Many people ship with asserts on. I
> > > would either make it safe if it is indeed just for reporting or remove
> > > the assert.
> > This code is copied from StringRef, which is what lldb used before this
> > change. Do you still want me to revert the assert?
> >
> > Though IMO it should be changed to <=, not <, so that the null terminator
> > can be read safely.
> I think that asserting here is fine... There's no way you can make code that
> likes to tread off the end of an array safe by changing this function.
>
> However, I am not really fond of the idea of ConstString taking over
> StringRef functionality. I think we should stick to requiring stringref
> conversions when doing string manipulation. Maybe if you reduce the number of
> ConstString occurences according to other comments, this will not be that
> necessary anymore (?)
The indexing is used in Symtab::InitNameIndexes, where is dealing with
ConstString. IMO converting to a StringRef there is a bad idea, as it means
there will be two variables representing the same thing (one ConstString, one
StringRef), which makes it easy to update one and not the other.
But there is no performance difference, at least if review
https://reviews.llvm.org/D32306 is committed (which removes hashing and locking
when getting the length). Otherwise it's a >1% penalty to convert to StringRef
in InitNameIndexes.
Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:324-326
+ ConstString key_cstr = m_impl.GetCStringAtIndex(item);
+ if (strstr(type_name.GetCString(), key_cstr.GetCString())) {
+count += AppendReplacements(type_name, key_cstr, equivalents);
scott.smith wrote:
> clayborg wrote:
> > StringRef is better to use for modifying simple strings and looking for
> > things. Actually looking at this class it is using ConstString all over the
> > place for putting strings together. For creating strings we should use
> > lldb_private::StreamString. For stripping stuff off and grabbing just part
> > of a string, seeing if a string starts with, ends with, contains, etc,
> > llvm::StringRef. So I would vote to just change it back, or fix the entire
> > class to use lldb_private::StreamString
> I can revert this fragment, but just note this won't affect the number of
> ConstStrings that are generated. m_impl has ConstStrings in it, and
> type_name is a ConstString, so yes they can be converted to StringRef just to
> call StringRef::contains, or we can leave them as ConstString and utilize
> strstr. Or, I can add ConstString::contains so the interface to ConstString
> and StringRef is more similar.
>
turns out this code is unused, filing a separate review to remove it.
Repository:
rL LLVM
https://reviews.llvm.org/D32316
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32503: Remove unused code related to CPlusPlusLanguage::FindEquivalentNames
jingham added a comment. This was used at some point. I'd be happier deleting if if I understood the reason why it is no longer needed. Repository: rL LLVM https://reviews.llvm.org/D32503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
