[Lldb-commits] [PATCH] D149213: [lldb] Add basic support to Rust enums in TypeSystemClang

[Vladimir Makaev via Phabricator via lldb-commits]

VladimirMakaev added a comment.

In D149213#4491594 , @bulbazord wrote:

> I'm curious to know why you don't try and implement some kind of 
> `TypeSystemRust`? I assume it's going to be a lengthy process, but eventually 
> you (or somebody else) is going to want to implement a `TypeSystemRust` in 
> some form and they'll have to revisit all of rust-specific things we've 
> tacked onto various parts of LLDB (e.g. this functionality with 
> DWARFASTParserClang). I suppose the question I'm trying to answer is "How 
> much do we add to LLDB before actually adding proper rust support?"

I think there was an attempt in the past to build TypeSystemRust. Rust Project 
had a fork of LLDB with this implemented by Tom Tromey and CodeLLDB 
maintainer(vadimcn) has one. Both of them were not able to merge that into LLDB 
codebase. A relevant link to a discussion 
https://rust-lang.zulipchat.com/#narrow/stream/317568-t-compiler.2Fwg-debugging/topic/upstreaming.20rust.20support.20into.20lldb

Apart from just implementing type system itself (which is much bigger scope 
than this change) there are other non-trivial issues:

1. There is no "compiler-as-a-service" in Rust so getting expressions to work 
is non-trivial. An interpreter of some sort needs to be built with subset of 
Rust support
2. Infra changes (DEVOPS) changes are required to start building Rust inferiors 
to run tests on CI
3. LLVM / rustc cross repository logistics. Rustc probably needs to live in 
LLVM to make this work well.

Given the fact that the majority of the things "just work" when debugging Rust 
with LLDB (using Clang declarations effectively as of today) I kinda believe 
this is not a bad solution to try to do a bit of extra care for Rust types in 
TypeSystemClang and the debugging experience will be great for Rust users. As 
long as LLDB emits enough information a conversion can be made in synthetic 
provider to display it nicely in debugger. Tests can be written to make sure it 
doesn't rot. This is definitely something I want to improve in this PR too.

I think if Rust Project Debugging WG decides to invest into native 
TypeSystemRust I think having these fixes around isn't going to slow down the 
progress but quite the contrary (in my opinion). And cleaning them up will also 
be trivial since they're specifically guarded by CU check.

> I was also curious about the test. I'm not sure how the test is supposed to 
> work here, is the rust source file actually built or is that just to show how 
> the yaml file was generated? There's no rust support in our testing 
> infrastructure AFAICT so I assume this just makes sure that something already 
> generated doesn't get broken in the future.

I'm gonna work on the test more and try to make assertions against global 
SBValue. Greg suggested this might work. So since we don't have access to rustc 
in the testing setup I've generated object file from main.rs manually and 
converted it to yaml. This makes the test portable. (main.rs is kept for the 
reference)




Comment at: lldb/test/API/lang/rust/enum-structs/TestEnumStructsGenerated.py:14
+src_dir = self.getSourceDir()
+yaml_path = os.path.join(src_dir, "main.yaml")
+obj_path = self.getBuildArtifact("main.o")

clayborg wrote:
> the main.yaml below is empty? Will this test work?
It't not empty it's just big and not opened automatically



Comment at: lldb/test/API/lang/rust/enum-structs/main.rs:1-4
+#![no_std]
+#![no_main]
+use core::ptr::NonNull;
+

clayborg wrote:
> Is this file used at all? I don't see any references to it. I am guessing 
> that the main.yaml is expected to reference this file?
It's not used I've just put it as a reference of the program that the object 
file was taken from. I used rustc to produce object file from this program


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149213/new/

https://reviews.llvm.org/D149213

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149213: [lldb] Add basic support to Rust enums in TypeSystemClang

[Vladimir Makaev via Phabricator via lldb-commits]

VladimirMakaev updated this revision to Diff 540251.
VladimirMakaev added a comment.
Herald added a reviewer: sscalpone.

Rebased on master (this diff was quite a bit old)
Addressed review comments

- renamed method to ParseRustVariantPart
- added CU check
- added a good bunch of test covering cases I could think of using global 
variabes.
- 2 tests for Option> couldn't be done with globals due to Rust 
compiler strictness on UBs. Was able to create SBValue based on raw data
- addressed VariantMember::GetName()




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149213/new/

https://reviews.llvm.org/D149213

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/test/API/lang/rust/enum-structs/RustEnumValue.py
  lldb/test/API/lang/rust/enum-structs/TestRustEnumStructs.py
  lldb/test/API/lang/rust/enum-structs/main.rs

Index: lldb/test/API/lang/rust/enum-structs/main.rs
===
--- /dev/null
+++ lldb/test/API/lang/rust/enum-structs/main.rs
@@ -0,0 +1,118 @@
+#![no_std]
+#![no_main]
+
+/// This file was manually compiled with rustc as object file
+/// obj2yaml tool was used to convert this to main.yaml
+/// This is done in order to make the test portable since LLVM codebase tests don't have setup to compile Rust programs
+/// no_std , no_main is used in order to make the object file as small as possible eliminating extra symbols from standard library
+/// static global variables are used because they can be inspected on object file without starting the process
+
+/// Command:
+/// rustc -g --emit=obj --crate-type=bin -C panic=abort -C link-arg=-nostdlib main.rs && obj2yaml main.o -o main.yaml
+use core::ptr::NonNull;
+
+enum CLikeEnumDefault {
+A = 2,
+B = 10,
+}
+
+#[repr(u8)]
+enum CLikeEnumReprU8 {
+VariantA,
+VariantB,
+VariantC,
+}
+
+#[repr(u32)]
+enum CLikeEnumReprU32 {
+VariantA = 1,
+VariantB = 2,
+VariantC = 3,
+}
+
+enum EnumWithTuples {
+A(u8),
+B(u16),
+C(u32),
+D(usize),
+AA(u8, u8),
+BB(u16, u16),
+BC(u16, u32),
+CC(u32, u32),
+// no DD on purpose to have D = CC in size
+}
+
+enum EnumWithStructs {
+A(Struct1),
+B(Struct2),
+}
+
+#[repr(usize)]
+enum MixedEnum {
+A,
+B(i32),
+C(u8, i32),
+D(Option),
+E(EnumWithStructs),
+}
+
+pub struct Struct1 {
+field: i32,
+}
+
+pub struct Struct2 {
+field: u32,
+inner: Struct1,
+}
+
+pub struct NonNullHolder {
+inner: Option>,
+}
+
+static CLIKE_DEFAULT_A: CLikeEnumDefault = CLikeEnumDefault::A;
+static CLIKE_DEFAULT_B: CLikeEnumDefault = CLikeEnumDefault::B;
+
+static CLIKE_U8_A: CLikeEnumReprU8 = CLikeEnumReprU8::VariantA;
+static CLIKE_U8_B: CLikeEnumReprU8 = CLikeEnumReprU8::VariantB;
+static CLIKE_U8_C: CLikeEnumReprU8 = CLikeEnumReprU8::VariantC;
+
+static CLIKE_U32_A: CLikeEnumReprU32 = CLikeEnumReprU32::VariantA;
+static CLIKE_U32_B: CLikeEnumReprU32 = CLikeEnumReprU32::VariantB;
+static CLIKE_U32_C: CLikeEnumReprU32 = CLikeEnumReprU32::VariantC;
+
+static ENUM_WITH_TUPLES_A: EnumWithTuples = EnumWithTuples::A(13);
+static ENUM_WITH_TUPLES_AA: EnumWithTuples = EnumWithTuples::AA(13, 37);
+static ENUM_WITH_TUPLES_B: EnumWithTuples = EnumWithTuples::B(37);
+static ENUM_WITH_TUPLES_BB: EnumWithTuples = EnumWithTuples::BB(37, 5535);
+static ENUM_WITH_TUPLES_BC: EnumWithTuples = EnumWithTuples::BC(65000, 165000);
+static ENUM_WITH_TUPLES_C: EnumWithTuples = EnumWithTuples::C(31337);
+static ENUM_WITH_TUPLES_CC: EnumWithTuples = EnumWithTuples::CC(31337, 87236);
+static ENUM_WITH_TUPLES_D: EnumWithTuples = EnumWithTuples::D(123456789012345678);
+
+static MIXED_ENUM_A: MixedEnum = MixedEnum::A;
+static MIXED_ENUM_B: MixedEnum = MixedEnum::B(-10);
+static MIXED_ENUM_C: MixedEnum = MixedEnum::C(254, -254);
+static MIXED_ENUM_D_NONE: MixedEnum = MixedEnum::D(None);
+static MIXED_ENUM_D_SOME: MixedEnum = MixedEnum::D(Some(Struct2 {
+field: 123456,
+inner: Struct1 { field: 123 },
+}));
+
+#[no_mangle]
+pub extern "C" fn _start() -> ! {
+loop {}
+}
+
+#[panic_handler]
+fn my_panic(_info: &core::panic::PanicInfo) -> ! {
+// Option> is tricky type because it's optimized by compiler to hold None as 0 since
+// Some(NonNull) can never be 0
+// This type also cannot be global due to Rust borrow checker rules and Send trait implementation
+// this code is added in order to have compiler emit generic type specialization into symbols
+let non_null = unsafe {
+NonNullHolder {
+inner: NonNull::new(1235 as *mut u64),
+}
+};
+loop {}
+}
Index: lldb/test/API/lang/rust/enum-structs/TestRustEnumStructs.py
===
--- /dev/null
+++ lldb/test/API/lang/rust/enum-structs/TestRustEnumStructs.py
@@ -0,0 +1,187 @@
+"""Test that lldb recognizes enum structs emitted by Rust compiler """
+import logging

[Lldb-commits] [PATCH] D149213: [lldb] Add basic support to Rust enums in TypeSystemClang

[Vladimir Makaev via Phabricator via lldb-commits]

VladimirMakaev added a comment.

I've reached out to Rust wg-debugging for additional feedback 
(https://rust-lang.zulipchat.com/#narrow/stream/317568-t-compiler.2Fwg-debugging/topic/Feedback.20on.20DW_TAG_variant_part.20support.20in.20LLDB)

There a couple of edge cases (when using u128 enums) that not working properly 
so I'm going fix them and add more tests.

Additionally an issue was raised:

> Another comment: Checking the language of the CU might not be reliable 
> because of cross-language inlining. That is, if some Rust code gets inlined 
> into a C++ CU, then the CU will probably still say it is C++ even though it 
> might contain DW_TAG_variant_part DIEs. Just something to be aware of an 
> handle gracefully.

What do you guys think on not gating it behind CU check, maybe there is another 
way? My opinion probably is that there is more risk to CLang users / potential 
conflict with CLang evolution than benefit of having LLDB working for Rust code 
in this niche scenario?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149213/new/

https://reviews.llvm.org/D149213

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149213: [lldb] Add basic support for Rust enums in TypeSystemClang

[Vladimir Makaev via Phabricator via lldb-commits]

VladimirMakaev marked an inline comment as done.
VladimirMakaev added a comment.

In D149213#4520309 , @tom.tromey 
wrote:

>> Apart from just implementing type system itself (which is much bigger scope 
>> than this change) there are other non-trivial issues:
>>
>> 1. There is no "compiler-as-a-service" in Rust so getting expressions to 
>> work is non-trivial. An interpreter of some sort needs to be built with 
>> subset of Rust support
>
> My work also included a parser for some Rust expressions.

Yeah I reviewed your fork and I saw there was a parser. So it could potentially 
be used with some success. With this approach we can use Clang for expression 
evaluations which might be inconvenient for Rust developers but it's decent 
enough. Other concerns with having this tested with CI still stands. Overall I 
agree that having TypeSystemRust will offer a better experience and more tuning 
but this fix is quite cheap and should be a big improvement over current state 
of things.  Do you know by chance if there are other areas apart from enums 
which are currently broken in LLDB and cannot be fixed via synthetic provider?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149213/new/

https://reviews.llvm.org/D149213

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149213: [lldb] Add basic support for Rust enums in TypeSystemClang

[Vladimir Makaev via Phabricator via lldb-commits]

VladimirMakaev added a comment.

I don't have commit access to the repo. Since this was accepted can somebody 
commit this to the repo? CC @clayborg


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149213/new/

https://reviews.llvm.org/D149213

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149213: [lldb] Add basic support for Rust enums in TypeSystemClang

[Vladimir Makaev via Phabricator via lldb-commits]

VladimirMakaev added a comment.

Thanks @DavidSpickett for fixing this. I did not intend to use that type. Any 
number of that size would work.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149213/new/

https://reviews.llvm.org/D149213

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149213: [lldb] Add basic support to Rust enums in TypeSystemClang

[Vladimir Makaev via Phabricator via lldb-commits]

VladimirMakaev created this revision.
VladimirMakaev added reviewers: clayborg, ayermolo, Michael137.
VladimirMakaev added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a reviewer: shafik.
Herald added a project: All.
VladimirMakaev requested review of this revision.
Herald added a subscriber: lldb-commits.

LLDB doesn't yet have a TypeSystemRust implemented however it is used to debug 
Rust applications. Most of the types map well enough to Clang types and there 
are python formatters implemented to display those types reasonably well in a 
debugger. We discussed this with Greg Clayton internally and this approach 
looks like a reasonable thing to implement and fix one of the major pain points 
when debugging Rust with LLDB.

However, Rust enums are completely ignored by LLDB as Clang never emits 
DW_TAG_variant_part inside DW_TAG_structure_type

This diff adds a parser for DW_TAG_variant_part (Rust-only) that creates a 
matching valid Clang declaration that represents a Rust enum. As long as there 
is enough information and all fields have correct offsets synthetic/summary 
providers can be implemented to display it correctly when debugging Rust code

For example given a Rust enum:

  enum EnumOpt2 {
  A(u8),
  B(bool),
  }

The following is a matching Clang declaration:

  struct EnumOpt2 {
  union debugger_test::EnumOpt2$Inner {
  struct A$Variant {
  unsigned char $discr$;
  debugger_test::EnumOpt2::A value : 8;
  };
  debugger_test::EnumOpt2::debugger_test::EnumOpt2$Inner::A$Variant 
$variant$0;
  struct B$Variant {
  unsigned char $discr$;
  debugger_test::EnumOpt2::B value : 8;
  };
  debugger_test::EnumOpt2::debugger_test::EnumOpt2$Inner::B$Variant 
$variant$1;
  };
  struct A {
  unsigned char __0;
  };
  struct B {
  bool __0;
  };
  debugger_test::EnumOpt2::debugger_test::EnumOpt2$Inner inner;
  }

Now this declaration contains all the necessary information to interpret the 
original Rust enum in a Python synthetic formatter and display back on UI as 
Rust enum.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149213

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/test/API/lang/rust/enum-structs/TestEnumStructsGenerated.py
  lldb/test/API/lang/rust/enum-structs/main.rs
  lldb/test/API/lang/rust/enum-structs/main.yaml

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits