[lldb-dap] Addressing ubsan enum usage. #133542
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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`.
|
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesRunning tests with ubsan enabled showed that the current Adding Full diff: https://github.com/llvm/llvm-project/pull/133542.diff 4 Files Affected:
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
|
googlewalt
approved these changes
Mar 28, 2025
googlewalt
approved these changes
Mar 28, 2025
|
Sure I’ll convert it to that in a follow up patch |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Running tests with ubsan enabled showed that the current
protocol::AdapterFeatureandprotocol::ClientFeatureenums are incorrectly defined if we want to use them in allvm::DenseSet. The enums are currently untyped and this results in the undefined behavior.Adding
FLAGS_ENUM()wrappers to all the enums in thelldb-dap::protocolnamespace to ensure they're typed so they can be used with types likellvm::DenseSet.