User Details
- User Since
- Mar 2 2013, 8:12 AM (524 w, 5 d)
Today
breaks basically anything that examines debugger output
Yesterday
If it's the latter, then, yeah, some "transparent to debugger" source attribute might be appropriate - that lowers to a bit on the DISubprogram and instructs LLVM to, if the function definition ends up lowering to a trampoline, mark that for the debugger so it's transparent - and if it lowers to a trampoline but doesn't have the source attribute, you can then still step into the function separately from stepping into the subsequent function?
So this attribute will lower into a DW_AT_trampoline("target_func_name") attribute on the DW_TAG_subprogram of the function definition?
Tue, Mar 21
Thanks for adding the check!
Mon, Mar 20
Thanks! It works.
Oh wait, I didn't realize you haven't landed it yet :-)
Thanks, but this does not seem to have worked: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/52610/consoleText
Fri, Mar 17
You could add a test similar to lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
Otherwise LGTM
Here is an example of how the Swift Runtime plugin detects both itself and whether ObjC interop is enabled. Basically all I'm asking is to not accidentally break this mechanism.
https://github.com/apple/llvm-project/blob/7750ffa35111df6d38a5c73ab7e78ccebc9b43c3/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp#L124
If you want to make 100% sure, you could checkout the Swift compiler on Linux (or I suppose Windows) from github.com/apple/swift run swift/utils/update-checkout --clone apply your patch to llvm-project and run swift/utils/build-script --lldb --test -- --skip-build-benchmarks --skip-test-cmark --skip-test-swift and make sure the lldb tests still pass after applying you patch. But if you check for the symbol that would be sufficient for me.
In module 'LLVM_Utils' imported from /Users/buildslave/jenkins/workspace/lldb-cmake@2/llvm-project/llvm/tools/llvm-pdbutil/StreamUtil.h:12: /Users/buildslave/jenkins/workspace/lldb-cmake@2/llvm-project/llvm/include/llvm/ADT/STLExtras.h:2389:11: error: no matching function for call to 'all_equal' all_equal({std::distance(adl_begin(First), adl_end(First)), ^~~~~~~~~ /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/assert.h:93:25: note: expanded from macro 'assert' (__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __FILE__, __LINE__, #e) : (void)0) ^ /Users/buildslave/jenkins/workspace/lldb-cmake@2/llvm-project/llvm/tools/llvm-pdbutil/ExplainOutputStyle.cpp:129:28: note: in instantiation of function template specialization 'llvm::enumerate<const std::__1::vector<llvm::ArrayRef<llvm::support::detail::packed_endian_specific_integral<unsigned int, llvm::support::little, 1, 1>>> &>' requested here for (const auto &Entry : enumerate(Layout.StreamMap)) { ^ /Users/buildslave/jenkins/workspace/lldb-cmake@2/llvm-project/llvm/include/llvm/ADT/STLExtras.h:2056:28: note: candidate template ignored: couldn't infer template argument 'R' template <typename R> bool all_equal(R &&Range) { ^ /Users/buildslave/jenkins/workspace/lldb-cmake@2/llvm-project/llvm/include/llvm/ADT/STLExtras.h:2064:28: note: candidate template ignored: couldn't infer template argument 'T' template <typename T> bool all_equal(std::initializer_list<T> Values) { ^ 1 error generated.
This somehow broke the -DLLVM_ENABLE_MODULES=1 build:
Wed, Mar 15
I.e., can you detect based on the presence of a symbol or shared object that an GNUstep runtime is present?
One thing I just realized — we need to make sure that we don't accidentally create a GNUstep ObjC runtime in a Swift process that was built without ObjC support on Linux. How can we ensure this works for both cases?
In general I don't think I have a problem with adding this, especially since Clang also supports the runtime as a compilation target, and LLDB is tightly integrated with Clang.
I don't /think/ that's the case - I know Apple used to have a pretty significant dependence on IR upgrading at least invalidating the IR metadata & dropping it (so maybe all you need to do is update the debug info IR verifier so that old IR gets deemed invalid and dropped - not necessarily upgraded).
Tue, Mar 14
Mechanically, this patch looks great to me. @JDevlieghere, do you have any high-level concerns?
I'll defer to @JDevlieghere
We should consider factoring this function out and writing a unit test for it. The current tests are very indirect.
Mon, Mar 13
Fri, Mar 10
Tue, Mar 7
This doesn't affect any tests?
Mon, Mar 6
We might consider using dwim-print here in the future?
That seems to be clearly better, since p could be aliased to anything.
From an end-user perspective, I like this.
Fri, Mar 3
Nice! I appreciate the detailed explanation of the algorithm. I have some suggestions for how to streamline it a bit and potentially make it easier to read inside.
Deleting specialized code while improving correctness? That's always welcome!
I'm fine with taking this, thanks!
I think this is fine. It's concise, adds useful plugin functionality and we're already hardcoding all sorts of special knowledge about libc++.
Nice.
Looking good!
Thu, Mar 2
So from my point of view, this is a good path forward, so assuming that the other reviewers agree, this LGTM.
So, one could say that (right now) this patch is NFC for end-users, because dwim-print supports all use-cases that the old p alias supported?
Wed, Mar 1
Looks pretty good, some nitpicks inside.
I originally was hoping we wouldn't have to introduce a new attribute for this, but it looks like there are legitimate concerns about the alternatives, so in that sense this looks good!
Tue, Feb 28
How is this related to https://reviews.llvm.org/D142283?
Thu, Feb 23
I think this SGTM.
Wed, Feb 22
Feb 20 2023
Feb 17 2023
Feb 16 2023
Did you forget to git-add the test?
Yeah, this is better.
So your build system guarantees that all bitcode that is being consumed was generated by compilers linked against the same version of LLVM that is used in the LTO plugin and if that version were to serialize malformed metadata, you wouldn't be any worse off than a non-LTO build, which also doesn't verify between passes.
Size-wise this looks like an acceptable increase. If we created a new DW_AT_LLVM_abi_tag, we could save an extra 4 bytes (assuming DW_FORM_strp) per DIE. That might be worth it?
Feb 15 2023
Conceptually, I think this looks good to me.
Feb 14 2023
Alternatively, I suppose, the DWARF emission could just look at the preferred name and use that as the DW_AT_type in all cases anyway? Avoids needing a new attribute, etc, though would be a bit quirky in its own way.
It seems reasonable to me to provide a way to skip this for situations where the build system guarantees that the bitcode is always generated by the same compiler, e.g. in a distributed ThinLTO build where any change in compiler will force a rebuild anyway, which is the case @aeubanks is looking at.
Test looks good!
Feb 13 2023
Should we put something like
Feb 10 2023
Why are only these two tests affected? Should this be something we set globally for all the tests? The API tests already have support for forwarding ASAN_OPTIONS and lit has a similar concept.
Generally LGTM, with one possible improvement inline!
Feb 9 2023
Nice!
Looks like this was fixed in
commit 2700d66a054327c2dbb90d8b44b1d5cf3b0043b8 Author: Kazu Hirata <kazu@google.com> Date: Wed Feb 8 21:00:45 2023 -0800