Page MenuHomePhabricator

sgraenitz (Stefan Gränitz)
dev

Projects

User does not belong to any projects.

User Details

User Since
Jul 10 2018, 11:23 AM (32 w, 3 d)

weliveindetail

Recent Activity

Wed, Feb 20

sgraenitz accepted D58193: Do not explicitly depend on llvm tools during standalone build.

@serge-sans-paille After all, it appears to me that your issue is caused by something else, but if this change fixes it, I am not against taking it for now. It shouldn't do any harm. Please get in touch with @hans if this is really relevant for release_80.

Wed, Feb 20, 10:29 AM · Restricted Project

Tue, Feb 19

sgraenitz added a comment to D55653: [lldb-mi] Check raw pointers before passing them to std::string ctor/assignment.

Good initiative to fix potential nullptr assignments to std::string.

Tue, Feb 19, 10:29 AM

Mon, Feb 18

sgraenitz added a comment to D58193: Do not explicitly depend on llvm tools during standalone build.

@sgraenitz I currently have this patch applied to LLVM 8rc1 source tree for fedora, because it wasn't working automagically otherwise. Reading the discussion, I don't think I missed some configuration stuff, or what did I miss when configuring the build of lldb in standalone mode?

Mon, Feb 18, 12:03 PM · Restricted Project
sgraenitz added a reviewer for D58193: Do not explicitly depend on llvm tools during standalone build: mgorny.
Mon, Feb 18, 11:45 AM · Restricted Project
sgraenitz added a comment to D58193: Do not explicitly depend on llvm tools during standalone build.

Yap, will try and come up with a proposal soon.

Mon, Feb 18, 4:37 AM · Restricted Project

Fri, Feb 15

sgraenitz committed rG42a9da7b3550: Fix potential UB when target_file directory is null (authored by sgraenitz).
Fix potential UB when target_file directory is null
Fri, Feb 15, 8:42 AM
sgraenitz added a comment to D58193: Do not explicitly depend on llvm tools during standalone build.

BTW, what's the reason for having super-high-fidelity list of dependencies in a standalone build?

Fri, Feb 15, 7:14 AM · Restricted Project

Thu, Feb 14

sgraenitz added a comment to D57402: build: remove custom variables.

I don't want to play the nitpicker here, but did you test this? If so, please provide a sample configuration command line.

Thu, Feb 14, 10:40 AM · Restricted Project
sgraenitz added a comment to D58193: Do not explicitly depend on llvm tools during standalone build.

@xiaobai && @compnerd Do you know if find_package can tell us anything about it?

Thu, Feb 14, 10:17 AM · Restricted Project
sgraenitz added a comment to D58193: Do not explicitly depend on llvm tools during standalone build.

All these targets are exported from the LLVM build-tree since D56606 and from the install-tree since D57383.

Thu, Feb 14, 10:16 AM · Restricted Project
sgraenitz updated subscribers of D58193: Do not explicitly depend on llvm tools during standalone build.
Thu, Feb 14, 10:16 AM · Restricted Project
sgraenitz added a comment to D57964: Fix potential UB when target_file directory is null.

Ping

Thu, Feb 14, 10:05 AM · Restricted Project
sgraenitz committed rGdb85fdd11595: [CMake] Fix RPATH handling for LLDB.framework (authored by sgraenitz).
[CMake] Fix RPATH handling for LLDB.framework
Thu, Feb 14, 9:36 AM
sgraenitz closed D57989: [CMake] Fix RPATH handling for LLDB.framework.
Thu, Feb 14, 9:36 AM · Restricted Project

Wed, Feb 13

sgraenitz added a comment to D57989: [CMake] Fix RPATH handling for LLDB.framework.

we are also doing a CMake version check in LLDBConfig for the same purpose. Seems like you could consolidate those?

Wed, Feb 13, 5:53 AM · Restricted Project
sgraenitz updated the diff for D57989: [CMake] Fix RPATH handling for LLDB.framework.

Submit proposal from Diff 186517

Wed, Feb 13, 5:44 AM · Restricted Project
sgraenitz retitled D57989: [CMake] Fix RPATH handling for LLDB.framework from Convert genexp that used an actual string instead of the genexp to [CMake] Fix RPATH handling for LLDB.framework.
Wed, Feb 13, 5:44 AM · Restricted Project
sgraenitz commandeered D57989: [CMake] Fix RPATH handling for LLDB.framework.
Wed, Feb 13, 5:44 AM · Restricted Project

Tue, Feb 12

sgraenitz added a comment to D57989: [CMake] Fix RPATH handling for LLDB.framework.

Looking at the code more closely, it turns out this is not the only issue:

Tue, Feb 12, 11:07 AM · Restricted Project
sgraenitz added a comment to D57989: [CMake] Fix RPATH handling for LLDB.framework.

Tested it for the lldb driver and you are right, that the genex ends up in the binary -- interesting. I then also tested with your change, but it gave me the same:

Load command 17
          cmd LC_RPATH
      cmdsize 40
         path $<TARGET_FILE:liblldb> (offset 12)
Tue, Feb 12, 6:57 AM · Restricted Project
sgraenitz added a comment to D57989: [CMake] Fix RPATH handling for LLDB.framework.

A chunk of code referred to a generator expression wrapped in quotations and then referred to later again in quotations which caused cmake to think it was something to be referred to as a string.

Tue, Feb 12, 5:28 AM · Restricted Project
sgraenitz requested changes to D57402: build: remove custom variables.

Actually, both use cases already work without this change: passing LLVM_DIR && Clang_DIR or passing LLDB_PATH_TO_LLVM_BUILD. IMHO this patch needs good reason to land.

Tue, Feb 12, 5:13 AM · Restricted Project
sgraenitz added a comment to D58109: [llvm] [cmake] Provide split include paths in LLVMConfig.

Can't speak for all the subproject, but with D57995 this should work for LLDB.

Tue, Feb 12, 3:51 AM · Restricted Project
sgraenitz added a comment to D57995: [lldb] [cmake] Use install directories for LLVM_* variables.

Indeed it is a problem. However, it seems to be a bug that LLVM_INCLUDE_DIR contains two directories.

Tue, Feb 12, 3:28 AM · Restricted Project, Restricted Project

Mon, Feb 11

sgraenitz added inline comments to D56531: [CMake] Replace use of llvm-config with LLVM and Clang CMake packages.
Mon, Feb 11, 12:25 PM · Restricted Project
sgraenitz added a comment to D57995: [lldb] [cmake] Use install directories for LLVM_* variables.

Indeed, these variables do not exist when running against a LLVM/Clang install-tree and this should be fixed. Did you give this a try running against a LLVM/Clang build-tree? Dumping the values here locally, gives me:

LLVM_BUILD_MAIN_INCLUDE_DIR /path/to/llvm/include
LLVM_BUILD_LIBRARY_DIR      /path/to/llvm-build/./lib
LLVM_BUILD_BINARY_DIR       /path/to/llvm-build
Mon, Feb 11, 12:19 PM · Restricted Project, Restricted Project
sgraenitz updated the diff for D57964: Fix potential UB when target_file directory is null.

Polish && error out also on empty string

Mon, Feb 11, 3:44 AM · Restricted Project

Fri, Feb 8

sgraenitz updated the diff for D57964: Fix potential UB when target_file directory is null.

Remove useless newline

Fri, Feb 8, 11:00 AM · Restricted Project
sgraenitz created D57964: Fix potential UB when target_file directory is null.
Fri, Feb 8, 10:56 AM · Restricted Project

Thu, Feb 7

sgraenitz added a comment to D57402: build: remove custom variables.

it is pretty reasonable to ask that the user tell us where LLVM and Clang are built

Thu, Feb 7, 4:52 AM · Restricted Project

Wed, Feb 6

sgraenitz added a comment to D57402: build: remove custom variables.

I don't think it's too difficult to change how it works downstream in lldb-swift. I actually already have an open PR trying to make swift-lldb closer to upstream in this particular file: https://github.com/apple/swift-lldb/pull/1252
This change would be a cherry-pick on top of that, I think.

Wed, Feb 6, 2:24 AM · Restricted Project

Tue, Feb 5

Herald added a project to D57402: build: remove custom variables: Restricted Project.

Hey sorry for the late reply, didn't see this earlier. Personally, I think the move away from the llvm-config approach is good, but I have no strong opinion about the solution.

Tue, Feb 5, 9:29 AM · Restricted Project
sgraenitz added a comment to D57750: [CMake] Don't add `cxx` to `LLDB_TEST_DEPS` if it doesn't exist..

Not sure if we want to change the behavior this way. The purpose of this code was to make the dependency to libc++ explicit because it's specific to macOS and it's missed quite often. In my experience warnings like the proposed one are hard to recognize in the load of CMake output.

Tue, Feb 5, 9:03 AM · Restricted Project
sgraenitz committed rG26693b909c12: Update Xcode project after r353047 (authored by sgraenitz).
Update Xcode project after r353047
Tue, Feb 5, 6:42 AM

Fri, Feb 1

sgraenitz added a comment to D57521: [CMake] External compiler-rt-configure requires LLVMTestingSupport when including tests.

@phosek, does LLVM’s runtime directory do this correctly?

At some point we should try and deprecate and remove this option in favor of the LLVM directory because the LLVM approach is much more complete and robust.

Fri, Feb 1, 4:25 AM · Restricted Project, Restricted Project

Thu, Jan 31

sgraenitz added a comment to D55891: [compiler-rt] [xray] [tests] Detect and handle missing LLVMTestingSupport gracefully.

In case compiler-rt abandons llvm-config in favor of find_package(LLVM) one day, we could drop the COMPILER_RT_HAS_LLVMTESTINGSUPPORT logic here and use imported target properties instead. It seems a cleaner solution to me and avoids issues like D57521.

Thu, Jan 31, 10:11 AM
sgraenitz added a comment to D57521: [CMake] External compiler-rt-configure requires LLVMTestingSupport when including tests.

FYI: Seems to be happening since D55891

Thu, Jan 31, 9:40 AM · Restricted Project, Restricted Project
sgraenitz added a reviewer for D57521: [CMake] External compiler-rt-configure requires LLVMTestingSupport when including tests: dberris.
Thu, Jan 31, 9:39 AM · Restricted Project, Restricted Project
sgraenitz added a reviewer for D57521: [CMake] External compiler-rt-configure requires LLVMTestingSupport when including tests: mgorny.
Thu, Jan 31, 9:38 AM · Restricted Project, Restricted Project
sgraenitz updated the diff for D57521: [CMake] External compiler-rt-configure requires LLVMTestingSupport when including tests.

Polishing

Thu, Jan 31, 9:38 AM · Restricted Project, Restricted Project
sgraenitz created D57521: [CMake] External compiler-rt-configure requires LLVMTestingSupport when including tests.
Thu, Jan 31, 9:21 AM · Restricted Project, Restricted Project

Tue, Jan 29

sgraenitz created D57383: [CMake] Add install targets for utilities to LLVM exports if LLVM_INSTALL_UTILS=ON.
Tue, Jan 29, 7:01 AM · Restricted Project
sgraenitz added a comment to D57334: [CMake] Accept ENTITLEMENTS in llvm_add_library().

Tested this with LLDB library target.

Tue, Jan 29, 3:49 AM
sgraenitz added a reviewer for D57334: [CMake] Accept ENTITLEMENTS in llvm_add_library(): bogner.
Tue, Jan 29, 3:46 AM
sgraenitz created D57378: [CMake] Accept entitlements for code signing in add_lldb_library().
Tue, Jan 29, 3:35 AM

Mon, Jan 28

sgraenitz created D57334: [CMake] Accept ENTITLEMENTS in llvm_add_library().
Mon, Jan 28, 8:41 AM
sgraenitz accepted D57282: [cmake] Fix get_llvm_lit_path() to respect LLVM_EXTERNAL_LIT always.

LGTM

Mon, Jan 28, 3:31 AM

Fri, Jan 25

sgraenitz created D57233: [CMake] Quick-Fix targets don't exist when building against LLVM install-tree with LLDB_INCLUDE_TESTS=ON.
Fri, Jan 25, 4:55 AM
sgraenitz retitled D57224: [CMake] Quick-Fix FileCheck target does not exist when building against LLVM install-tree with COMPILER_RT_INCLUDE_TESTS=ON from [CMake] Fix FileCheck target does not exist when building against LLVM install-tree with COMPILER_RT_INCLUDE_TESTS=ON to [CMake] Quick-Fix FileCheck target does not exist when building against LLVM install-tree with COMPILER_RT_INCLUDE_TESTS=ON.
Fri, Jan 25, 4:38 AM
sgraenitz created D57224: [CMake] Quick-Fix FileCheck target does not exist when building against LLVM install-tree with COMPILER_RT_INCLUDE_TESTS=ON.
Fri, Jan 25, 3:02 AM

Thu, Jan 24

sgraenitz added a comment to D56531: [CMake] Replace use of llvm-config with LLVM and Clang CMake packages.

Thanks -- and one more :) https://reviews.llvm.org/rL352058

Thu, Jan 24, 8:24 AM · Restricted Project

Jan 22 2019

sgraenitz added a comment to D56531: [CMake] Replace use of llvm-config with LLVM and Clang CMake packages.

FYI Fixed two small details with https://reviews.llvm.org/rL351879, hope that's ok:

  • LLDB_PATH_TO_CLANG_BUILD defaults to LLDB_PATH_TO_LLVM_BUILD
  • switch args for file(TO_CMAKE_PATH "<path>" <variable>)
Jan 22 2019, 1:18 PM · Restricted Project

Jan 18 2019

sgraenitz accepted D56531: [CMake] Replace use of llvm-config with LLVM and Clang CMake packages.
Jan 18 2019, 8:51 AM · Restricted Project

Jan 16 2019

sgraenitz added a comment to D56763: [CMake] Prevent lldbDebugserverCommon from building if you disable debugserver builds.

Yes this sounds very reasonable.

Jan 16 2019, 6:34 AM

Jan 14 2019

sgraenitz added a comment to D56531: [CMake] Replace use of llvm-config with LLVM and Clang CMake packages.

it should be possible to build lldb against an already-installed llvm

Jan 14 2019, 2:10 AM · Restricted Project

Jan 13 2019

sgraenitz added a comment to D56531: [CMake] Replace use of llvm-config with LLVM and Clang CMake packages.

Please don't risk merging this before the branching.

Jan 13 2019, 11:57 PM · Restricted Project

Jan 11 2019

sgraenitz added a comment to D56531: [CMake] Replace use of llvm-config with LLVM and Clang CMake packages.

Thanks for the initiative! Would be great to have this part cleaned up as well.

Jan 11 2019, 12:56 PM · Restricted Project
sgraenitz abandoned D56440: [CMake] Phase out LLDB_TEST_C/CXX_COMPILER in favor of single LLDB_TEST_COMPILER.

Closing this in favor of a more comprehensive fix.

Jan 11 2019, 10:34 AM
sgraenitz added a comment to D56609: [CMake] Remove dead code and outdated comments.

Simple cleanup parts of the previous reviews D56400 and D56440

Jan 11 2019, 9:50 AM
sgraenitz created D56609: [CMake] Remove dead code and outdated comments.
Jan 11 2019, 9:35 AM
sgraenitz abandoned D56400: [CMake] Some cleanup around test preparations.

I will split off the "dead code elimination" part and close both reviews in favor of a more comprehensive fix.

Jan 11 2019, 9:31 AM
sgraenitz created D56606: [CMake] Export utility targets to the build/install tree depending on LLVM_BUILD/INSTALL_UTILS.
Jan 11 2019, 9:16 AM

Jan 10 2019

sgraenitz added a comment to D56440: [CMake] Phase out LLDB_TEST_C/CXX_COMPILER in favor of single LLDB_TEST_COMPILER.

It looks like LLDB_TEST_COMPILER_IS_DEFAULT is set but never read. Why do we need it exactly?

[...] I will check why dotest doesn't need them and either fix it (which adds a use case) or remove it.

Jan 10 2019, 8:55 AM
sgraenitz added a reviewer for D56400: [CMake] Some cleanup around test preparations: xiaobai.
Jan 10 2019, 4:39 AM

Jan 9 2019

sgraenitz added inline comments to D56440: [CMake] Phase out LLDB_TEST_C/CXX_COMPILER in favor of single LLDB_TEST_COMPILER.
Jan 9 2019, 11:27 AM
sgraenitz updated the diff for D56440: [CMake] Phase out LLDB_TEST_C/CXX_COMPILER in favor of single LLDB_TEST_COMPILER.

Move BUILD_SHARED_LIBS and LLDB_HAVE_LLD to a separate commit

Jan 9 2019, 11:26 AM
sgraenitz updated the diff for D56443: [CMake] In standalone builds, LLVM_BINARY_DIR should point to LLVM's binary directory.

Fix typo

Jan 9 2019, 6:16 AM
sgraenitz added inline comments to D56443: [CMake] In standalone builds, LLVM_BINARY_DIR should point to LLVM's binary directory.
Jan 9 2019, 6:16 AM
sgraenitz added a comment to D56440: [CMake] Phase out LLDB_TEST_C/CXX_COMPILER in favor of single LLDB_TEST_COMPILER.

It looks like LLDB_TEST_COMPILER_IS_DEFAULT is set but never read. Why do we need it exactly?

Jan 9 2019, 6:13 AM
sgraenitz added a comment to D55376: Generate LLDB website/documentation from rst with Sphinx.

I think this is going to be really nice! Here's a few details I found.

Jan 9 2019, 4:10 AM

Jan 8 2019

sgraenitz updated the diff for D56443: [CMake] In standalone builds, LLVM_BINARY_DIR should point to LLVM's binary directory.

Use LLVM_BINARY_DIR when checking for existance of libc++ include directory

Jan 8 2019, 8:44 AM
sgraenitz updated the diff for D56399: [CMake] Fix standalone builds: workaround the cxx target not getting imported yet (unlike clang target).

Update documentation: libc++ should be checked out when building for macOS

Jan 8 2019, 8:34 AM
sgraenitz added inline comments to D56400: [CMake] Some cleanup around test preparations.
Jan 8 2019, 8:32 AM
sgraenitz updated the summary of D56440: [CMake] Phase out LLDB_TEST_C/CXX_COMPILER in favor of single LLDB_TEST_COMPILER.
Jan 8 2019, 8:23 AM
sgraenitz created D56443: [CMake] In standalone builds, LLVM_BINARY_DIR should point to LLVM's binary directory.
Jan 8 2019, 7:55 AM
sgraenitz updated the diff for D56440: [CMake] Phase out LLDB_TEST_C/CXX_COMPILER in favor of single LLDB_TEST_COMPILER.

Remove remove unused LLDB_HAVE_LLD and ENABLE_SHARED in lit/CMakeLists.txt

Jan 8 2019, 7:27 AM
sgraenitz updated the summary of D56440: [CMake] Phase out LLDB_TEST_C/CXX_COMPILER in favor of single LLDB_TEST_COMPILER.
Jan 8 2019, 7:15 AM
sgraenitz updated the diff for D56440: [CMake] Phase out LLDB_TEST_C/CXX_COMPILER in favor of single LLDB_TEST_COMPILER.

In test/CMakeLists.txt pass on LLDB_TEST_COMPILER_USED instead of LLDB_TEST_COMPILER

Jan 8 2019, 7:06 AM
sgraenitz created D56440: [CMake] Phase out LLDB_TEST_C/CXX_COMPILER in favor of single LLDB_TEST_COMPILER.
Jan 8 2019, 7:02 AM
sgraenitz updated the diff for D56400: [CMake] Some cleanup around test preparations.

Act on FIXME's in subsequent commits and remove comments here

Jan 8 2019, 5:19 AM
sgraenitz added inline comments to D56400: [CMake] Some cleanup around test preparations.
Jan 8 2019, 2:48 AM

Jan 7 2019

sgraenitz added inline comments to D56400: [CMake] Some cleanup around test preparations.
Jan 7 2019, 11:31 AM
sgraenitz added inline comments to D56400: [CMake] Some cleanup around test preparations.
Jan 7 2019, 11:09 AM
sgraenitz created D56400: [CMake] Some cleanup around test preparations.
Jan 7 2019, 11:02 AM
sgraenitz created D56399: [CMake] Fix standalone builds: workaround the cxx target not getting imported yet (unlike clang target).
Jan 7 2019, 10:53 AM
sgraenitz created D56389: [CMake] Fix standalone builds: make dependency to LLVM's `count` conditional.
Jan 7 2019, 7:23 AM

Jan 4 2019

sgraenitz updated subscribers of D56084: Resubmit of rL345008 "Split MachinePipeliner code into header and cpp files".

@aprantl Found a workaround for the lldb-cmake bot: https://reviews.llvm.org/rL350346

Jan 4 2019, 1:31 AM

Jan 3 2019

sgraenitz added a comment to D56084: Resubmit of rL345008 "Split MachinePipeliner code into header and cpp files".

Hello lsaba, sorry for having to revert this after it broke http://green.lab.llvm.org/green/job/lldb-cmake/ again.
I tried to find the root cause, but so far didn't succeed. I will continue tomorrow and keep you in the loop so you can land this as soon as possible.

Jan 3 2019, 11:18 AM
sgraenitz added a comment to rL350094: Reduce indentation in ObjectFilePECOFF::CreateSections via an early return.

The indentations of section_type and other lines look like you accidentally committed tabs instead of spaces. Can we fix that? Thanks

Jan 3 2019, 4:04 AM

Dec 18 2018

sgraenitz added a comment to D55328: [CMake] Revised LLDB.framework builds.

Thanks everyone for your patience with the review. I will spare us the potential merge conflicts before Christmas and land this after the holidays. Cheers

Dec 18 2018, 9:33 AM
sgraenitz created D55816: [CMake] Use XCODE_ATTRIBUTE properties for code signing and entitlements in Xcode.
Dec 18 2018, 2:57 AM

Dec 13 2018

sgraenitz added a comment to D55116: [CMake] llvm_codesign workaround for Xcode double-signing errors.

Fair enough. Will create a ticket for it. At least this works for now.

Dec 13 2018, 11:24 PM

Dec 12 2018

sgraenitz added a comment to D55116: [CMake] llvm_codesign workaround for Xcode double-signing errors.

Ninja builds work well without it. In order to catch actual double-signing problems here, I would prefer to pass it only in case of Xcode generator.

Dec 12 2018, 11:49 AM
sgraenitz added inline comments to D55332: [CMake] Python bindings generation polishing.
Dec 12 2018, 11:01 AM · Restricted Project
sgraenitz retitled D55116: [CMake] llvm_codesign workaround for Xcode double-signing errors from [CMake] llvm_codesign polishing to [CMake] llvm_codesign workaround for Xcode double-signing errors.
Dec 12 2018, 10:07 AM
sgraenitz updated the diff for D55116: [CMake] llvm_codesign workaround for Xcode double-signing errors.

Add --force flag when using the Xcode generator to avoid double-signing errors

Dec 12 2018, 9:52 AM

Dec 10 2018

sgraenitz updated the summary of D55332: [CMake] Python bindings generation polishing.
Dec 10 2018, 12:46 PM · Restricted Project
sgraenitz added a parent revision for D55317: [CMake] Aggregate options for LLDB in LLDBConfig.cmake: D55013: [CMake] Streamline code signing for debugserver #2.
Dec 10 2018, 12:15 PM
sgraenitz added a child revision for D55013: [CMake] Streamline code signing for debugserver #2: D55317: [CMake] Aggregate options for LLDB in LLDBConfig.cmake.
Dec 10 2018, 12:15 PM
sgraenitz updated the diff for D55013: [CMake] Streamline code signing for debugserver #2.

Turn LLDB_CODESIGN_IDENTITY_USED variable from STRING into INTERNAL

Dec 10 2018, 11:35 AM