-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[lldb][RPC] Upstream RPC server interface emitters #138032
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[lldb][RPC] Upstream RPC server interface emitters #138032
Conversation
@llvm/pr-subscribers-lldb Author: Chelsea Cassanova (chelcassanova) ChangesThis commit upstreams the LLDB RPC server interface emitters. These emitters generate the server-side API interfaces for RPC, which communicate directly with liblldb itself out of process using the SB API. https://discourse.llvm.org/t/rfc-upstreaming-lldb-rpc/85804 Patch is 30.96 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/138032.diff 4 Files Affected:
diff --git a/lldb/tools/lldb-rpc/lldb-rpc-gen/server/RPCServerHeaderEmitter.cpp b/lldb/tools/lldb-rpc/lldb-rpc-gen/server/RPCServerHeaderEmitter.cpp
new file mode 100644
index 0000000000000..a2818ada1f323
--- /dev/null
+++ b/lldb/tools/lldb-rpc/lldb-rpc-gen/server/RPCServerHeaderEmitter.cpp
@@ -0,0 +1,73 @@
+//===-- RPCServerHeaderEmitter.cpp ----------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "RPCServerHeaderEmitter.h"
+#include "RPCCommon.h"
+
+#include "clang/AST/AST.h"
+#include "clang/AST/Mangle.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace clang;
+using namespace lldb_rpc_gen;
+
+void RPCServerHeaderEmitter::EmitMethod(const Method &method) {
+ const std::string &MangledName = method.MangledName;
+
+ EmitLine("class " + MangledName +
+ " : public rpc_common::RPCFunctionInstance {");
+ EmitLine("public:");
+ IndentLevel++;
+ EmitConstructor(MangledName);
+ EmitDestructor(MangledName);
+ EmitHandleRPCCall();
+ IndentLevel--;
+ EmitLine("};");
+}
+
+void RPCServerHeaderEmitter::EmitHandleRPCCall() {
+ EmitLine("bool HandleRPCCall(rpc_common::Connection &connection, "
+ "rpc_common::RPCStream &send, rpc_common::RPCStream &response) "
+ "override;");
+}
+
+void RPCServerHeaderEmitter::EmitConstructor(const std::string &MangledName) {
+ EmitLine(MangledName + "() : RPCFunctionInstance(\"" + MangledName +
+ "\") {}");
+}
+
+void RPCServerHeaderEmitter::EmitDestructor(const std::string &MangledName) {
+ EmitLine("~" + MangledName + "() override {}");
+}
+
+std::string RPCServerHeaderEmitter::GetHeaderGuard() {
+ const std::string UpperFilenameNoExt =
+ llvm::sys::path::stem(
+ llvm::sys::path::filename(OutputFile->getFilename()))
+ .upper();
+ return "GENERATED_LLDB_RPC_SERVER_" + UpperFilenameNoExt + "_H";
+}
+
+void RPCServerHeaderEmitter::Begin() {
+ const std::string HeaderGuard = GetHeaderGuard();
+ EmitLine("#ifndef " + HeaderGuard);
+ EmitLine("#define " + HeaderGuard);
+ EmitLine("");
+ EmitLine("#include <lldb-rpc/common/RPCFunction.h>");
+ EmitLine("");
+ EmitLine("namespace rpc_server {");
+}
+
+void RPCServerHeaderEmitter::End() {
+ EmitLine("} // namespace rpc_server");
+ EmitLine("#endif // " + GetHeaderGuard());
+}
diff --git a/lldb/tools/lldb-rpc/lldb-rpc-gen/server/RPCServerHeaderEmitter.h b/lldb/tools/lldb-rpc/lldb-rpc-gen/server/RPCServerHeaderEmitter.h
new file mode 100644
index 0000000000000..cdf201090b7c8
--- /dev/null
+++ b/lldb/tools/lldb-rpc/lldb-rpc-gen/server/RPCServerHeaderEmitter.h
@@ -0,0 +1,46 @@
+//===-- RPCServerHeaderEmitter.h ----------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_RPC_GEN_RPCSERVERHEADEREMITTER_H
+#define LLDB_RPC_GEN_RPCSERVERHEADEREMITTER_H
+
+#include "RPCCommon.h"
+
+#include "clang/AST/AST.h"
+#include "llvm/Support/ToolOutputFile.h"
+
+using namespace clang;
+
+namespace lldb_rpc_gen {
+class RPCServerHeaderEmitter : public FileEmitter {
+public:
+ RPCServerHeaderEmitter(std::unique_ptr<llvm::ToolOutputFile> &&OutputFile)
+ : FileEmitter(std::move(OutputFile)) {
+ Begin();
+ }
+
+ ~RPCServerHeaderEmitter() { End(); }
+
+ void EmitMethod(const Method &method);
+
+private:
+ void EmitHandleRPCCall();
+
+ void EmitConstructor(const std::string &MangledName);
+
+ void EmitDestructor(const std::string &MangledName);
+
+ std::string GetHeaderGuard();
+
+ void Begin();
+
+ void End();
+};
+} // namespace lldb_rpc_gen
+
+#endif // LLDB_RPC_GEN_RPCSERVERHEADEREMITTER_H
diff --git a/lldb/tools/lldb-rpc/lldb-rpc-gen/server/RPCServerSourceEmitter.cpp b/lldb/tools/lldb-rpc/lldb-rpc-gen/server/RPCServerSourceEmitter.cpp
new file mode 100644
index 0000000000000..645ddd11662a7
--- /dev/null
+++ b/lldb/tools/lldb-rpc/lldb-rpc-gen/server/RPCServerSourceEmitter.cpp
@@ -0,0 +1,592 @@
+//===-- RPCServerSourceEmitter.cpp ----------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "RPCServerSourceEmitter.h"
+#include "RPCCommon.h"
+
+#include "clang/AST/AST.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include <map>
+
+using namespace clang;
+using namespace lldb_rpc_gen;
+
+// For methods with pointer return types, it's important that we know how big
+// the type of the pointee is. We must correctly size a buffer (in the form of a
+// Bytes object) before we can actually use it.
+static const std::map<llvm::StringRef, size_t> MethodsWithPointerReturnTypes = {
+ {"_ZN4lldb12SBModuleSpec12GetUUIDBytesEv", 16}, // sizeof(uuid_t) -> 16
+ {"_ZNK4lldb8SBModule12GetUUIDBytesEv", 16}, // sizeof(uuid_t) -> 16
+};
+
+void RPCServerSourceEmitter::EmitMethod(const Method &method) {
+ if (method.ContainsFunctionPointerParameter)
+ EmitCallbackFunction(method);
+
+ EmitCommentHeader(method);
+ EmitFunctionHeader(method);
+ EmitFunctionBody(method);
+ EmitFunctionFooter();
+}
+
+void RPCServerSourceEmitter::EmitCommentHeader(const Method &method) {
+ std::string CommentLine;
+ llvm::raw_string_ostream CommentStream(CommentLine);
+
+ CommentStream << "// " << method.QualifiedName << "("
+ << method.CreateParamListAsString(eServer) << ")";
+ if (method.IsConst)
+ CommentStream << " const";
+
+ EmitLine("//------------------------------------------------------------");
+ EmitLine(CommentLine);
+ EmitLine("//------------------------------------------------------------");
+}
+
+void RPCServerSourceEmitter::EmitFunctionHeader(const Method &method) {
+ std::string FunctionHeader;
+ llvm::raw_string_ostream FunctionHeaderStream(FunctionHeader);
+ FunctionHeaderStream
+ << "bool rpc_server::" << method.MangledName
+ << "::HandleRPCCall(rpc_common::Connection &connection, RPCStream "
+ "&send, RPCStream &response) {";
+ EmitLine(FunctionHeader);
+ IndentLevel++;
+}
+
+void RPCServerSourceEmitter::EmitFunctionBody(const Method &method) {
+ EmitLine("// 1) Make local storage for incoming function arguments");
+ EmitStorageForParameters(method);
+ EmitLine("// 2) Decode all function arguments");
+ EmitDecodeForParameters(method);
+ EmitLine("// 3) Call the method and encode the return value");
+ EmitMethodCallAndEncode(method);
+}
+
+void RPCServerSourceEmitter::EmitFunctionFooter() {
+ EmitLine("return true;");
+ IndentLevel--;
+ EmitLine("}");
+}
+
+void RPCServerSourceEmitter::EmitStorageForParameters(const Method &method) {
+ // If we have an instance method and it isn't a constructor, we'll need to
+ // emit a "this" pointer.
+ if (method.IsInstance && !method.IsCtor)
+ EmitStorageForOneParameter(method.ThisType, "this_ptr", method.Policy,
+ /* IsFollowedByLen = */ false);
+ for (auto Iter = method.Params.begin(); Iter != method.Params.end(); Iter++) {
+ EmitStorageForOneParameter(Iter->Type, Iter->Name, method.Policy,
+ Iter->IsFollowedByLen);
+ // Skip over the length parameter, we don't emit it.
+ if (!lldb_rpc_gen::TypeIsConstCharPtrPtr(Iter->Type) &&
+ Iter->IsFollowedByLen)
+ Iter++;
+ }
+}
+
+void RPCServerSourceEmitter::EmitStorageForOneParameter(
+ QualType ParamType, const std::string &ParamName,
+ const PrintingPolicy &Policy, bool IsFollowedByLen) {
+ // First, we consider `const char *`, `const char **`. They have special
+ // server-side types.
+ if (TypeIsConstCharPtr(ParamType)) {
+ EmitLine("rpc_common::ConstCharPointer " + ParamName + ";");
+ return;
+ } else if (TypeIsConstCharPtrPtr(ParamType)) {
+ EmitLine("rpc_common::StringList " + ParamName + ";");
+ return;
+ }
+
+ QualType UnderlyingType =
+ lldb_rpc_gen::GetUnqualifiedUnderlyingType(ParamType);
+ const bool IsSBClass = lldb_rpc_gen::TypeIsSBClass(UnderlyingType);
+
+ if (ParamType->isPointerType() && !IsSBClass) {
+ // Void pointer with no length is usually a baton for a callback. We're
+ // going to hold onto the pointer value so we can send it back to the
+ // client-side when we implement callbacks.
+ if (ParamType->isVoidPointerType() && !IsFollowedByLen) {
+ EmitLine("void * " + ParamName + " = nullptr;");
+ return;
+ }
+
+ if (!ParamType->isFunctionPointerType()) {
+ EmitLine("Bytes " + ParamName + ";");
+ return;
+ }
+
+ assert(ParamType->isFunctionPointerType() && "Unhandled pointer type");
+ EmitLine("rpc_common::function_ptr_t " + ParamName + " = nullptr;");
+ return;
+ }
+
+ std::string StorageDeclaration;
+ llvm::raw_string_ostream StorageDeclarationStream(StorageDeclaration);
+
+ UnderlyingType.print(StorageDeclarationStream, Policy);
+ StorageDeclarationStream << " ";
+ if (IsSBClass)
+ StorageDeclarationStream << "*";
+ StorageDeclarationStream << ParamName;
+ if (IsSBClass)
+ StorageDeclarationStream << " = nullptr";
+ else
+ StorageDeclarationStream << " = {}";
+ StorageDeclarationStream << ";";
+ EmitLine(StorageDeclaration);
+}
+
+void RPCServerSourceEmitter::EmitDecodeForParameters(const Method &method) {
+ if (method.IsInstance && !method.IsCtor)
+ EmitDecodeForOneParameter(method.ThisType, "this_ptr", method.Policy);
+ for (auto Iter = method.Params.begin(); Iter != method.Params.end(); Iter++) {
+ EmitDecodeForOneParameter(Iter->Type, Iter->Name, method.Policy);
+ if (!lldb_rpc_gen::TypeIsConstCharPtrPtr(Iter->Type) &&
+ Iter->IsFollowedByLen)
+ Iter++;
+ }
+}
+
+void RPCServerSourceEmitter::EmitDecodeForOneParameter(
+ QualType ParamType, const std::string &ParamName,
+ const PrintingPolicy &Policy) {
+ QualType UnderlyingType =
+ lldb_rpc_gen::GetUnqualifiedUnderlyingType(ParamType);
+
+ if (TypeIsSBClass(UnderlyingType)) {
+ std::string DecodeLine;
+ llvm::raw_string_ostream DecodeLineStream(DecodeLine);
+ DecodeLineStream << ParamName << " = "
+ << "RPCServerObjectDecoder<";
+ UnderlyingType.print(DecodeLineStream, Policy);
+ DecodeLineStream << ">(send, rpc_common::RPCPacket::ValueType::Argument);";
+ EmitLine(DecodeLine);
+ EmitLine("if (!" + ParamName + ")");
+ IndentLevel++;
+ EmitLine("return false;");
+ IndentLevel--;
+ } else {
+ EmitLine("if (!RPCValueDecoder(send, "
+ "rpc_common::RPCPacket::ValueType::Argument, " +
+ ParamName + "))");
+ IndentLevel++;
+ EmitLine("return false;");
+ IndentLevel--;
+ }
+}
+
+std::string RPCServerSourceEmitter::CreateMethodCall(const Method &method) {
+ std::string MethodCall;
+ llvm::raw_string_ostream MethodCallStream(MethodCall);
+ if (method.IsInstance) {
+ if (!method.IsCtor)
+ MethodCallStream << "this_ptr->";
+ MethodCallStream << method.BaseName;
+ } else
+ MethodCallStream << method.QualifiedName;
+
+ std::vector<std::string> Args;
+ std::string FunctionPointerName;
+ for (auto Iter = method.Params.begin(); Iter != method.Params.end(); Iter++) {
+ std::string Arg;
+ // We must check for `const char *` and `const char **` first.
+ if (TypeIsConstCharPtr(Iter->Type)) {
+ // `const char *` is stored server-side as rpc_common::ConstCharPointer
+ Arg = Iter->Name + ".c_str()";
+ } else if (TypeIsConstCharPtrPtr(Iter->Type)) {
+ // `const char **` is stored server-side as rpc_common::StringList
+ Arg = Iter->Name + ".argv()";
+ } else if (lldb_rpc_gen::TypeIsSBClass(Iter->Type)) {
+ Arg = Iter->Name;
+ if (!Iter->Type->isPointerType())
+ Arg = "*" + Iter->Name;
+ } else if (Iter->Type->isPointerType() &&
+ !Iter->Type->isFunctionPointerType() &&
+ (!Iter->Type->isVoidPointerType() || Iter->IsFollowedByLen)) {
+ // We move pointers between the server and client as 'Bytes' objects.
+ // Pointers with length arguments will have their length filled in below.
+ // Pointers with no length arguments are assumed to behave like an array
+ // with length of 1, except for void pointers which are handled
+ // differently.
+ Arg = "(" + Iter->Type.getAsString(method.Policy) + ")" + Iter->Name +
+ ".GetData()";
+ } else if (Iter->Type->isFunctionPointerType()) {
+ // If we have a function pointer, we only want to pass something along if
+ // we got a real pointer.
+ Arg = Iter->Name + " ? " + method.MangledName + "_callback : nullptr";
+ FunctionPointerName = Iter->Name;
+ } else if (Iter->Type->isVoidPointerType() && !Iter->IsFollowedByLen &&
+ method.ContainsFunctionPointerParameter) {
+ // Assumptions:
+ // - This is assumed to be the baton for the function pointer.
+ // - This is assumed to come after the function pointer parameter.
+ // We always produce this regardless of the value of the baton argument.
+ Arg = "new CallbackInfo(" + FunctionPointerName + ", " + Iter->Name +
+ ", connection.GetConnectionID())";
+ } else
+ Arg = Iter->Name;
+
+ if (Iter->Type->isRValueReferenceType())
+ Arg = "std::move(" + Arg + ")";
+ Args.push_back(Arg);
+
+ if (!lldb_rpc_gen::TypeIsConstCharPtrPtr(Iter->Type) &&
+ Iter->IsFollowedByLen) {
+ std::string LengthArg = Iter->Name + ".GetSize()";
+ if (!Iter->Type->isVoidPointerType()) {
+ QualType UUT = lldb_rpc_gen::GetUnqualifiedUnderlyingType(Iter->Type);
+ LengthArg += " / sizeof(" + UUT.getAsString(method.Policy) + ")";
+ }
+ Args.push_back(LengthArg);
+ Iter++;
+ }
+ }
+ MethodCallStream << "(" << llvm::join(Args, ", ") << ")";
+
+ return MethodCall;
+}
+
+std::string RPCServerSourceEmitter::CreateEncodeLine(const std::string &Value,
+ bool IsEncodingSBClass) {
+ std::string EncodeLine;
+ llvm::raw_string_ostream EncodeLineStream(EncodeLine);
+
+ if (IsEncodingSBClass)
+ EncodeLineStream << "RPCServerObjectEncoder(";
+ else
+ EncodeLineStream << "RPCValueEncoder(";
+
+ EncodeLineStream
+ << "response, rpc_common::RPCPacket::ValueType::ReturnValue, ";
+ EncodeLineStream << Value;
+ EncodeLineStream << ");";
+ return EncodeLine;
+}
+
+// There are 4 cases to consider:
+// - const SBClass &: No need to do anything.
+// - const foo &: No need to do anything.
+// - SBClass &: The server and the client hold on to IDs to refer to specific
+// instances, so there's no need to send any information back to the client.
+// - foo &: The client is sending us a value over the wire, but because the type
+// is mutable, we must send the changed value back in case the method call
+// mutated it.
+//
+// Updating a mutable reference is done as a return value from the RPC
+// perspective. These return values need to be emitted after the method's return
+// value, and they are emitted in the order in which they occur in the
+// declaration.
+void RPCServerSourceEmitter::EmitEncodesForMutableParameters(
+ const std::vector<Param> &Params) {
+ for (auto Iter = Params.begin(); Iter != Params.end(); Iter++) {
+ // No need to manually update an SBClass
+ if (lldb_rpc_gen::TypeIsSBClass(Iter->Type))
+ continue;
+
+ if (!Iter->Type->isReferenceType() && !Iter->Type->isPointerType())
+ continue;
+
+ // If we have a void pointer with no length, there's nothing to update. This
+ // is likely a baton for a callback. The same goes for function pointers.
+ if (Iter->Type->isFunctionPointerType() ||
+ (Iter->Type->isVoidPointerType() && !Iter->IsFollowedByLen))
+ continue;
+
+ // No need to update pointers and references to const-qualified data.
+ QualType UnderlyingType = lldb_rpc_gen::GetUnderlyingType(Iter->Type);
+ if (UnderlyingType.isConstQualified())
+ continue;
+
+ const std::string EncodeLine =
+ CreateEncodeLine(Iter->Name, /* IsEncodingSBClass = */ false);
+ EmitLine(EncodeLine);
+ }
+}
+
+// There are 3 possible scenarios that this method can encounter:
+// 1. The method has no return value and is not a constructor.
+// Only the method call itself is emitted.
+// 2. The method is a constructor.
+// The call to the constructor is emitted in the encode line.
+// 3. The method has a return value.
+// The method call is emitted and the return value is captured in a variable.
+// After that, an encode call is emitted with the variable that captured the
+// return value.
+void RPCServerSourceEmitter::EmitMethodCallAndEncode(const Method &method) {
+ // FIXME: The hand-written lldb-rpc-server currently doesn't emit destructors
+ // for LocalObjectRefs... even if the type is an ObjectRef. What should we do
+ // here?
+
+ const std::string MethodCall = CreateMethodCall(method);
+
+ // If this function returns nothing, we just emit the call and update any
+ // mutable references. Note that constructors have return type `void` so we
+ // must explicitly check for that here.
+ if (!method.IsCtor && method.ReturnType->isVoidType()) {
+ EmitLine(MethodCall + ";");
+ EmitEncodesForMutableParameters(method.Params);
+ return;
+ }
+
+ static constexpr llvm::StringLiteral ReturnVariableName("__result");
+
+ // If this isn't a constructor, we'll need to store the result of the method
+ // call in a result variable.
+ if (!method.IsCtor) {
+ // We need to determine what the appropriate return type is. Here is the
+ // strategy:
+ // 1.) `SBFoo` -> `SBFoo &&`
+ // 2.) If the type is a pointer other than `const char *` or `const char **`
+ // or `void *`, the return type will be `Bytes` (e.g. `const uint8_t *`
+ // -> `Bytes`).
+ // 3.) Otherwise, emit the exact same return type.
+ std::string ReturnTypeName;
+ std::string AssignLine;
+ llvm::raw_string_ostream AssignLineStream(AssignLine);
+ if (method.ReturnType->isPointerType() &&
+ !lldb_rpc_gen::TypeIsConstCharPtr(method.ReturnType) &&
+ !lldb_rpc_gen::TypeIsConstCharPtrPtr(method.ReturnType) &&
+ !method.ReturnType->isVoidPointerType()) {
+ llvm::StringRef MangledNameRef(method.MangledName);
+ auto Pos = MethodsWithPointerReturnTypes.find(MangledNameRef);
+ assert(Pos != MethodsWithPointerReturnTypes.end() &&
+ "Unable to determine the size of the return buffer");
+ if (Pos == MethodsWithPointerReturnTypes.end()) {
+ EmitLine(
+ "// Intentionally inserting a compiler error. lldb-rpc-gen "
+ "was unable to determine how large the return buffer should be.");
+ EmitLine("ThisShouldNotCompile");
+ return;
+ }
+ AssignLineStream << "Bytes " << ReturnVariableName << "(" << MethodCall
+ << ", " << Pos->second << ");";
+ } else {
+ if (lldb_rpc_gen::TypeIsSBClass(method.ReturnType)) {
+ // We want to preserve constness, so we don't strip qualifications from
+ // the underlying type
+ QualType UnderlyingReturnType =
+ lldb_rpc_gen::GetUnderlyingType(method.ReturnType);
+ ReturnTypeName =
+ UnderlyingReturnType.getAsString(method.Policy) + " &&";
+ } else
+ ReturnTypeName = method.ReturnType.getAsString(method.Policy);
+
+ AssignLineStream << ReturnTypeName << " " << ReturnVariableName << " = "
+ << MethodCall << ";";
+ }
+ EmitLine(AssignLine);
+ }
+
+ const bool IsEncodingSBClass =
+ lldb_rpc_ge...
[truncated]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks OK in isolation, a bit like looking at a TableGen backend, easier to judge the output than the emitters.
Please look over the FIXMEs and see if anything can be done right now, I highlighted one that is potentially a problem but the rest don't seem important. See if you agree.
Also please remove any that are no longer relevant.
void RPCServerSourceEmitter::EmitMethodCallAndEncode(const Method &method) { | ||
// FIXME: The hand-written lldb-rpc-server currently doesn't emit destructors | ||
// for LocalObjectRefs... even if the type is an ObjectRef. What should we do | ||
// here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the impact of this and, to repeat the comment, what should we do here?
Is this a nice to have or a memory leak waiting to happen if someone uses a LocalObjectRef in the right/wrong way?
(whatever a LocalObjectRef is :) )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this FIXME
can be removed. There should be no impact because a LocalObjectRef's only member is a shared pointer to an ObjectRef (which does have a proper destructor).
Future PRs should have the proper definitions for ObjectRef and LocalObjectRef, but effectively every SB class inherits from one of them. ObjectRef holds onto a connection, a class ID (so you know what kind of object it is), and an object ID (so you know which object it refers to).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And to add on to this, ObjectRef
s are held on to by the server (they're usually used to store larger amounts of information about an RPC object and its connection) while LocalObjectRef
s are used locally for smaller classes (like Lists and Specs if I'm not mistaken).
EmitLine( | ||
"// Intentionally inserting a compiler error. lldb-rpc-gen " | ||
"was unable to determine how large the return buffer should be."); | ||
EmitLine("ThisShouldNotCompile"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#error <something vaguely useful>
would be a bit more standard and provide some help to debug this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still needs doing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this with my latest push.
lldb/tools/lldb-rpc/lldb-rpc-gen/server/RPCServerSourceEmitter.cpp
Outdated
Show resolved
Hide resolved
e037e27
to
8a96a26
Compare
Pushed again to address David's comments and remove most of the FIXMEs. There's one FIXME I've kept in for now:
@bulbazord This is referring to how we emit storage for const SB parameters in callback functions. Currently it looks like it only gets applied to one function that gets generated. Any thoughts here? |
Given it only applies to one function, it's probably okay to keep it around for now. The fact that we need a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of comments to be addressed but otherwise I have no problems with this.
8a96a26
to
a6c3566
Compare
✅ With the latest revision this PR passed the C/C++ code formatter. |
8732082
to
b6edf90
Compare
I added some small shell tests to this patch that check the output of the emitter mainly for sanity checking. @bulbazord I think having basic tests like what I added would be good for the server-side emitter, but if there's anything other thing you can think of test here please let me know! |
This commit upstreams the LLDB RPC server interface emitters. These emitters generate the server-side API interfaces for RPC, which communicate directly with liblldb itself out of process using the SB API. https://discourse.llvm.org/t/rfc-upstreaming-lldb-rpc/85804
b6edf90
to
c209f35
Compare
@DavidSpickett I think I've address any previous comments to this point and that this patch is ready to land (contingent on the rpc-gen tool itself landing first). If possible, could you give this another once over? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with the UNSUPPORTED changed to apply to any system.
@@ -0,0 +1,15 @@ | |||
# Disabling until the lldb-rpc-gen tool lands. | |||
UNSUPPORTED: system-windows, system-linux, system-darwin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UNSUPPORTED: *
should disable it everywhere.
Saves someone on FreeBSD getting an unpleasant surprise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do that for the rest of the tests as well.
This commit upstreams the LLDB RPC server interface emitters. These emitters generate the server-side API interfaces for RPC, which communicate directly with liblldb itself out of process using the SB API.
https://discourse.llvm.org/t/rfc-upstreaming-lldb-rpc/85804