llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: John Harrison (ashgti) <details> <summary>Changes</summary> Running tests with ubsan enabled showed that the current `protocol::AdapterFeature` and `protocol::ClientFeature` enums are incorrectly defined if we want to use them in a `llvm::DenseSet`. The enums are currently untyped and this results in the undefined behavior. Adding `FLAGS_ENUM()` wrappers to all the enums in the `lldb-dap::protocol` namespace to ensure they're typed so they can be used with types like `llvm::DenseSet`. --- Full diff: https://github.com/llvm/llvm-project/pull/133542.diff 4 Files Affected: - (modified) lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp (+11-13) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolBase.h (+8-7) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+19-17) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.h (+114-121) ``````````diff diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp index 86e26f4deb111..0d63e37d3eafb 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "Protocol/ProtocolBase.h" +#include "lldb/lldb-enumerations.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/Support/ErrorHandling.h" @@ -31,11 +32,8 @@ static bool mapRaw(const json::Value &Params, StringLiteral Prop, namespace lldb_dap::protocol { -enum MessageType { - eMessageTypeRequest, - eMessageTypeResponse, - eMessageTypeEvent -}; +FLAGS_ENUM(MessageType){eMessageTypeRequest, eMessageTypeResponse, + eMessageTypeEvent}; bool fromJSON(const json::Value &Params, MessageType &M, json::Path P) { auto rawType = Params.getAsString(); @@ -107,12 +105,12 @@ json::Value toJSON(const Response &R) { if (R.message) { assert(!R.success && "message can only be used if success is false"); - if (const auto *messageEnum = std::get_if<Response::Message>(&*R.message)) { + if (const auto *messageEnum = std::get_if<ResponseMessage>(&*R.message)) { switch (*messageEnum) { - case Response::Message::cancelled: + case eResponseMessageCancelled: Result.insert({"message", "cancelled"}); break; - case Response::Message::notStopped: + case eResponseMessageNotStopped: Result.insert({"message", "notStopped"}); break; } @@ -129,16 +127,16 @@ json::Value toJSON(const Response &R) { } bool fromJSON(json::Value const &Params, - std::variant<Response::Message, std::string> &M, json::Path P) { + std::variant<ResponseMessage, std::string> &M, json::Path P) { auto rawMessage = Params.getAsString(); if (!rawMessage) { P.report("expected a string"); return false; } - std::optional<Response::Message> message = - StringSwitch<std::optional<Response::Message>>(*rawMessage) - .Case("cancelled", Response::Message::cancelled) - .Case("notStopped", Response::Message::notStopped) + std::optional<ResponseMessage> message = + StringSwitch<std::optional<ResponseMessage>>(*rawMessage) + .Case("cancelled", eResponseMessageCancelled) + .Case("notStopped", eResponseMessageNotStopped) .Default(std::nullopt); if (message) M = *message; diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolBase.h b/lldb/tools/lldb-dap/Protocol/ProtocolBase.h index baf5f8f165183..5ac68e38cb9c4 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolBase.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolBase.h @@ -20,6 +20,7 @@ #ifndef LLDB_TOOLS_LLDB_DAP_PROTOCOL_H #define LLDB_TOOLS_LLDB_DAP_PROTOCOL_H +#include "lldb/lldb-enumerations.h" #include "llvm/Support/JSON.h" #include <cstdint> #include <optional> @@ -64,15 +65,15 @@ struct Event { llvm::json::Value toJSON(const Event &); bool fromJSON(const llvm::json::Value &, Event &, llvm::json::Path); -/// Response for a request. -struct Response { - enum class Message { +FLAGS_ENUM(ResponseMessage){ /// The request was cancelled - cancelled, + eResponseMessageCancelled, /// The request may be retried once the adapter is in a 'stopped' state - notStopped, - }; + eResponseMessageNotStopped, +}; +/// Response for a request. +struct Response { /// Sequence number of the corresponding request. int64_t request_seq; @@ -90,7 +91,7 @@ struct Response { /// Contains the raw error in short form if `success` is false. This raw error /// might be interpreted by the client and is not shown in the UI. Some /// predefined values exist. - std::optional<std::variant<Message, std::string>> message; + std::optional<std::variant<ResponseMessage, std::string>> message; /// Contains request result if success is true and error details if success is /// false. diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index c49a13711f8c7..116cf8516c52e 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -22,6 +22,8 @@ #include "Protocol/ProtocolBase.h" #include "Protocol/ProtocolTypes.h" +#include "lldb/lldb-enumerations.h" +#include "llvm/ADT/DenseSet.h" #include "llvm/Support/JSON.h" #include <cstdint> #include <optional> @@ -55,26 +57,26 @@ bool fromJSON(const llvm::json::Value &, DisconnectArguments &, using DisconnectResponse = VoidResponse; /// Features supported by DAP clients. -enum ClientFeature { - eClientFeatureVariableType, - eClientFeatureVariablePaging, - eClientFeatureRunInTerminalRequest, - eClientFeatureMemoryReferences, - eClientFeatureProgressReporting, - eClientFeatureInvalidatedEvent, - eClientFeatureMemoryEvent, - /// Client supports the `argsCanBeInterpretedByShell` attribute on the - /// `runInTerminal` request. - eClientFeatureArgsCanBeInterpretedByShell, - eClientFeatureStartDebuggingRequest, - /// The client will interpret ANSI escape sequences in the display of - /// `OutputEvent.output` and `Variable.value` fields when - /// `Capabilities.supportsANSIStyling` is also enabled. - eClientFeatureANSIStyling, +FLAGS_ENUM(ClientFeature){ + eClientFeatureVariableType, + eClientFeatureVariablePaging, + eClientFeatureRunInTerminalRequest, + eClientFeatureMemoryReferences, + eClientFeatureProgressReporting, + eClientFeatureInvalidatedEvent, + eClientFeatureMemoryEvent, + /// Client supports the `argsCanBeInterpretedByShell` attribute on the + /// `runInTerminal` request. + eClientFeatureArgsCanBeInterpretedByShell, + eClientFeatureStartDebuggingRequest, + /// The client will interpret ANSI escape sequences in the display of + /// `OutputEvent.output` and `Variable.value` fields when + /// `Capabilities.supportsANSIStyling` is also enabled. + eClientFeatureANSIStyling, }; /// Format of paths reported by the debug adapter. -enum PathFormat { ePatFormatPath, ePathFormatURI }; +FLAGS_ENUM(PathFormat){ePatFormatPath, ePathFormatURI}; /// Arguments for `initialize` request. struct InitializeRequestArguments { diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h index 934368aa2435f..463f9dbbaf4ea 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h @@ -20,6 +20,7 @@ #ifndef LLDB_TOOLS_LLDB_DAP_PROTOCOL_PROTOCOL_TYPES_H #define LLDB_TOOLS_LLDB_DAP_PROTOCOL_PROTOCOL_TYPES_H +#include "lldb/lldb-enumerations.h" #include "llvm/ADT/DenseSet.h" #include "llvm/Support/JSON.h" #include <cstdint> @@ -56,12 +57,8 @@ struct ExceptionBreakpointsFilter { }; llvm::json::Value toJSON(const ExceptionBreakpointsFilter &); -enum ColumnType { - eColumnTypeString, - eColumnTypeNumber, - eColumnTypeBoolean, - eColumnTypeTimestamp -}; +FLAGS_ENUM(ColumnType){eColumnTypeString, eColumnTypeNumber, eColumnTypeBoolean, + eColumnTypeTimestamp}; /// A ColumnDescriptor specifies what module attribute to show in a column of /// the modules view, how to format it, and what the column’s label should be. @@ -90,27 +87,23 @@ llvm::json::Value toJSON(const ColumnDescriptor &); /// Names of checksum algorithms that may be supported by a debug adapter. /// Values: ‘MD5’, ‘SHA1’, ‘SHA256’, ‘timestamp’. -enum ChecksumAlgorithm { - eChecksumAlgorithmMD5, - eChecksumAlgorithmSHA1, - eChecksumAlgorithmSHA256, - eChecksumAlgorithmTimestamp -}; +FLAGS_ENUM(ChecksumAlgorithm){eChecksumAlgorithmMD5, eChecksumAlgorithmSHA1, + eChecksumAlgorithmSHA256, + eChecksumAlgorithmTimestamp}; llvm::json::Value toJSON(const ChecksumAlgorithm &); /// Describes one or more type of breakpoint a BreakpointMode applies to. This /// is a non-exhaustive enumeration and may expand as future breakpoint types /// are added. -enum BreakpointModeApplicability { - /// In `SourceBreakpoint`'s. - eBreakpointModeApplicabilitySource, - /// In exception breakpoints applied in the `ExceptionFilterOptions`. - eBreakpointModeApplicabilityException, - /// In data breakpoints requested in the `DataBreakpointInfo` request. - eBreakpointModeApplicabilityData, - /// In `InstructionBreakpoint`'s. - eBreakpointModeApplicabilityInstruction -}; +FLAGS_ENUM(BreakpointModeApplicability){ + /// In `SourceBreakpoint`'s. + eBreakpointModeApplicabilitySource, + /// In exception breakpoints applied in the `ExceptionFilterOptions`. + eBreakpointModeApplicabilityException, + /// In data breakpoints requested in the `DataBreakpointInfo` request. + eBreakpointModeApplicabilityData, + /// In `InstructionBreakpoint`'s. + eBreakpointModeApplicabilityInstruction}; llvm::json::Value toJSON(const BreakpointModeApplicability &); /// A `BreakpointMode` is provided as a option when setting breakpoints on @@ -133,101 +126,101 @@ struct BreakpointMode { llvm::json::Value toJSON(const BreakpointMode &); /// Debug Adapter Features flags supported by lldb-dap. -enum AdapterFeature { - /// The debug adapter supports ANSI escape sequences in styling of - /// `OutputEvent.output` and `Variable.value` fields. - eAdapterFeatureANSIStyling, - /// The debug adapter supports the `breakpointLocations` request. - eAdapterFeatureBreakpointLocationsRequest, - /// The debug adapter supports the `cancel` request. - eAdapterFeatureCancelRequest, - /// The debug adapter supports the `clipboard` context value in the - /// `evaluate` request. - eAdapterFeatureClipboardContext, - /// The debug adapter supports the `completions` request. - eAdapterFeatureCompletionsRequest, - /// The debug adapter supports conditional breakpoints. - eAdapterFeatureConditionalBreakpoints, - /// The debug adapter supports the `configurationDone` request. - eAdapterFeatureConfigurationDoneRequest, - /// The debug adapter supports the `asAddress` and `bytes` fields in the - /// `dataBreakpointInfo` request. - eAdapterFeatureDataBreakpointBytes, - /// The debug adapter supports data breakpoints. - eAdapterFeatureDataBreakpoints, - /// The debug adapter supports the delayed loading of parts of the stack, - /// which requires that both the `startFrame` and `levels` arguments and the - /// `totalFrames` result of the `stackTrace` request are supported. - eAdapterFeatureDelayedStackTraceLoading, - /// The debug adapter supports the `disassemble` request. - eAdapterFeatureDisassembleRequest, - /// The debug adapter supports a (side effect free) `evaluate` request for - /// data hovers. - eAdapterFeatureEvaluateForHovers, - /// The debug adapter supports `filterOptions` as an argument on the - /// `setExceptionBreakpoints` request. - eAdapterFeatureExceptionFilterOptions, - /// The debug adapter supports the `exceptionInfo` request. - eAdapterFeatureExceptionInfoRequest, - /// The debug adapter supports `exceptionOptions` on the - /// `setExceptionBreakpoints` request. - eAdapterFeatureExceptionOptions, - /// The debug adapter supports function breakpoints. - eAdapterFeatureFunctionBreakpoints, - /// The debug adapter supports the `gotoTargets` request. - eAdapterFeatureGotoTargetsRequest, - /// The debug adapter supports breakpoints that break execution after a - /// specified number of hits. - eAdapterFeatureHitConditionalBreakpoints, - /// The debug adapter supports adding breakpoints based on instruction - /// references. - eAdapterFeatureInstructionBreakpoints, - /// The debug adapter supports the `loadedSources` request. - eAdapterFeatureLoadedSourcesRequest, - /// The debug adapter supports log points by interpreting the `logMessage` - /// attribute of the `SourceBreakpoint`. - eAdapterFeatureLogPoints, - /// The debug adapter supports the `modules` request. - eAdapterFeatureModulesRequest, - /// The debug adapter supports the `readMemory` request. - eAdapterFeatureReadMemoryRequest, - /// The debug adapter supports restarting a frame. - eAdapterFeatureRestartFrame, - /// The debug adapter supports the `restart` request. In this case a client - /// should not implement `restart` by terminating and relaunching the - /// adapter but by calling the `restart` request. - eAdapterFeatureRestartRequest, - /// The debug adapter supports the `setExpression` request. - eAdapterFeatureSetExpression, - /// The debug adapter supports setting a variable to a value. - eAdapterFeatureSetVariable, - /// The debug adapter supports the `singleThread` property on the execution - /// requests (`continue`, `next`, `stepIn`, `stepOut`, `reverseContinue`, - /// `stepBack`). - eAdapterFeatureSingleThreadExecutionRequests, - /// The debug adapter supports stepping back via the `stepBack` and - /// `reverseContinue` requests. - eAdapterFeatureStepBack, - /// The debug adapter supports the `stepInTargets` request. - eAdapterFeatureStepInTargetsRequest, - /// The debug adapter supports stepping granularities (argument - /// `granularity`) for the stepping requests. - eAdapterFeatureSteppingGranularity, - /// The debug adapter supports the `terminate` request. - eAdapterFeatureTerminateRequest, - /// The debug adapter supports the `terminateThreads` request. - eAdapterFeatureTerminateThreadsRequest, - /// The debug adapter supports the `suspendDebuggee` attribute on the - /// `disconnect` request. - eAdapterFeatureSuspendDebuggee, - /// The debug adapter supports a `format` attribute on the `stackTrace`, - /// `variables`, and `evaluate` requests. - eAdapterFeatureValueFormattingOptions, - /// The debug adapter supports the `writeMemory` request. - eAdapterFeatureWriteMemoryRequest, - /// The debug adapter supports the `terminateDebuggee` attribute on the - /// `disconnect` request. - eAdapterFeatureTerminateDebuggee, +FLAGS_ENUM(AdapterFeature){ + /// The debug adapter supports ANSI escape sequences in styling of + /// `OutputEvent.output` and `Variable.value` fields. + eAdapterFeatureANSIStyling, + /// The debug adapter supports the `breakpointLocations` request. + eAdapterFeatureBreakpointLocationsRequest, + /// The debug adapter supports the `cancel` request. + eAdapterFeatureCancelRequest, + /// The debug adapter supports the `clipboard` context value in the + /// `evaluate` request. + eAdapterFeatureClipboardContext, + /// The debug adapter supports the `completions` request. + eAdapterFeatureCompletionsRequest, + /// The debug adapter supports conditional breakpoints. + eAdapterFeatureConditionalBreakpoints, + /// The debug adapter supports the `configurationDone` request. + eAdapterFeatureConfigurationDoneRequest, + /// The debug adapter supports the `asAddress` and `bytes` fields in the + /// `dataBreakpointInfo` request. + eAdapterFeatureDataBreakpointBytes, + /// The debug adapter supports data breakpoints. + eAdapterFeatureDataBreakpoints, + /// The debug adapter supports the delayed loading of parts of the stack, + /// which requires that both the `startFrame` and `levels` arguments and the + /// `totalFrames` result of the `stackTrace` request are supported. + eAdapterFeatureDelayedStackTraceLoading, + /// The debug adapter supports the `disassemble` request. + eAdapterFeatureDisassembleRequest, + /// The debug adapter supports a (side effect free) `evaluate` request for + /// data hovers. + eAdapterFeatureEvaluateForHovers, + /// The debug adapter supports `filterOptions` as an argument on the + /// `setExceptionBreakpoints` request. + eAdapterFeatureExceptionFilterOptions, + /// The debug adapter supports the `exceptionInfo` request. + eAdapterFeatureExceptionInfoRequest, + /// The debug adapter supports `exceptionOptions` on the + /// `setExceptionBreakpoints` request. + eAdapterFeatureExceptionOptions, + /// The debug adapter supports function breakpoints. + eAdapterFeatureFunctionBreakpoints, + /// The debug adapter supports the `gotoTargets` request. + eAdapterFeatureGotoTargetsRequest, + /// The debug adapter supports breakpoints that break execution after a + /// specified number of hits. + eAdapterFeatureHitConditionalBreakpoints, + /// The debug adapter supports adding breakpoints based on instruction + /// references. + eAdapterFeatureInstructionBreakpoints, + /// The debug adapter supports the `loadedSources` request. + eAdapterFeatureLoadedSourcesRequest, + /// The debug adapter supports log points by interpreting the `logMessage` + /// attribute of the `SourceBreakpoint`. + eAdapterFeatureLogPoints, + /// The debug adapter supports the `modules` request. + eAdapterFeatureModulesRequest, + /// The debug adapter supports the `readMemory` request. + eAdapterFeatureReadMemoryRequest, + /// The debug adapter supports restarting a frame. + eAdapterFeatureRestartFrame, + /// The debug adapter supports the `restart` request. In this case a client + /// should not implement `restart` by terminating and relaunching the + /// adapter but by calling the `restart` request. + eAdapterFeatureRestartRequest, + /// The debug adapter supports the `setExpression` request. + eAdapterFeatureSetExpression, + /// The debug adapter supports setting a variable to a value. + eAdapterFeatureSetVariable, + /// The debug adapter supports the `singleThread` property on the execution + /// requests (`continue`, `next`, `stepIn`, `stepOut`, `reverseContinue`, + /// `stepBack`). + eAdapterFeatureSingleThreadExecutionRequests, + /// The debug adapter supports stepping back via the `stepBack` and + /// `reverseContinue` requests. + eAdapterFeatureStepBack, + /// The debug adapter supports the `stepInTargets` request. + eAdapterFeatureStepInTargetsRequest, + /// The debug adapter supports stepping granularities (argument + /// `granularity`) for the stepping requests. + eAdapterFeatureSteppingGranularity, + /// The debug adapter supports the `terminate` request. + eAdapterFeatureTerminateRequest, + /// The debug adapter supports the `terminateThreads` request. + eAdapterFeatureTerminateThreadsRequest, + /// The debug adapter supports the `suspendDebuggee` attribute on the + /// `disconnect` request. + eAdapterFeatureSuspendDebuggee, + /// The debug adapter supports a `format` attribute on the `stackTrace`, + /// `variables`, and `evaluate` requests. + eAdapterFeatureValueFormattingOptions, + /// The debug adapter supports the `writeMemory` request. + eAdapterFeatureWriteMemoryRequest, + /// The debug adapter supports the `terminateDebuggee` attribute on the + /// `disconnect` request. + eAdapterFeatureTerminateDebuggee, }; /// Information about the capabilities of a debug adapter. @@ -268,10 +261,10 @@ struct Capabilities { }; llvm::json::Value toJSON(const Capabilities &); -enum PresentationHint { - ePresentationHintNormal, - ePresentationHintEmphasize, - ePresentationHintDeemphasize, +FLAGS_ENUM(PresentationHint){ + ePresentationHintNormal, + ePresentationHintEmphasize, + ePresentationHintDeemphasize, }; /// A `Source` is a descriptor for source code. It is returned from the debug `````````` </details> https://github.com/llvm/llvm-project/pull/133542 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits