[Lldb-commits] [lldb] [lldb-dap] Addressing ubsan enum usage. (PR #133542)
John Harrison via lldb-commits
lldb-commits at lists.llvm.org
Fri Mar 28 16:49:20 PDT 2025
https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/133542
>From 55aa8ef8cc65307948a1cf0803c5c7c240be0efa Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Fri, 28 Mar 2025 16:29:06 -0700
Subject: [PATCH 1/2] [lldb-dap] Addressing ubsan enum usage.
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`.
---
lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp | 24 +-
lldb/tools/lldb-dap/Protocol/ProtocolBase.h | 15 +-
.../lldb-dap/Protocol/ProtocolRequests.h | 36 +--
lldb/tools/lldb-dap/Protocol/ProtocolTypes.h | 235 +++++++++---------
4 files changed, 152 insertions(+), 158 deletions(-)
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
>From 53961f8da4ee73f5e71fa4739f921710f796f355 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Fri, 28 Mar 2025 16:42:03 -0700
Subject: [PATCH 2/2] Missed one usage of an enum that also needed to be
updated.
---
lldb/tools/lldb-dap/DAP.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 23f0400c8bd4d..512cabdf77880 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -711,12 +711,12 @@ bool DAP::HandleObject(const protocol::Message &M) {
[](const std::string &message) -> llvm::StringRef {
return message;
},
- [](const protocol::Response::Message &message)
+ [](const protocol::ResponseMessage &message)
-> llvm::StringRef {
switch (message) {
- case protocol::Response::Message::cancelled:
+ case protocol::eResponseMessageCancelled:
return "cancelled";
- case protocol::Response::Message::notStopped:
+ case protocol::eResponseMessageNotStopped:
return "notStopped";
}
}),
More information about the lldb-commits
mailing list