Skip to content

Commit 539ef5e

Browse files
authored
[lldb-dap] Addressing ubsan enum usage. (#133542)
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`.
1 parent 55430f8 commit 539ef5e

File tree

5 files changed

+155
-161
lines changed

5 files changed

+155
-161
lines changed

lldb/tools/lldb-dap/DAP.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -711,12 +711,12 @@ bool DAP::HandleObject(const protocol::Message &M) {
711711
[](const std::string &message) -> llvm::StringRef {
712712
return message;
713713
},
714-
[](const protocol::Response::Message &message)
714+
[](const protocol::ResponseMessage &message)
715715
-> llvm::StringRef {
716716
switch (message) {
717-
case protocol::Response::Message::cancelled:
717+
case protocol::eResponseMessageCancelled:
718718
return "cancelled";
719-
case protocol::Response::Message::notStopped:
719+
case protocol::eResponseMessageNotStopped:
720720
return "notStopped";
721721
}
722722
}),

lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "Protocol/ProtocolBase.h"
10+
#include "lldb/lldb-enumerations.h"
1011
#include "llvm/ADT/StringRef.h"
1112
#include "llvm/ADT/StringSwitch.h"
1213
#include "llvm/Support/ErrorHandling.h"
@@ -31,11 +32,8 @@ static bool mapRaw(const json::Value &Params, StringLiteral Prop,
3132

3233
namespace lldb_dap::protocol {
3334

34-
enum MessageType {
35-
eMessageTypeRequest,
36-
eMessageTypeResponse,
37-
eMessageTypeEvent
38-
};
35+
FLAGS_ENUM(MessageType){eMessageTypeRequest, eMessageTypeResponse,
36+
eMessageTypeEvent};
3937

4038
bool fromJSON(const json::Value &Params, MessageType &M, json::Path P) {
4139
auto rawType = Params.getAsString();
@@ -107,12 +105,12 @@ json::Value toJSON(const Response &R) {
107105

108106
if (R.message) {
109107
assert(!R.success && "message can only be used if success is false");
110-
if (const auto *messageEnum = std::get_if<Response::Message>(&*R.message)) {
108+
if (const auto *messageEnum = std::get_if<ResponseMessage>(&*R.message)) {
111109
switch (*messageEnum) {
112-
case Response::Message::cancelled:
110+
case eResponseMessageCancelled:
113111
Result.insert({"message", "cancelled"});
114112
break;
115-
case Response::Message::notStopped:
113+
case eResponseMessageNotStopped:
116114
Result.insert({"message", "notStopped"});
117115
break;
118116
}
@@ -129,16 +127,16 @@ json::Value toJSON(const Response &R) {
129127
}
130128

131129
bool fromJSON(json::Value const &Params,
132-
std::variant<Response::Message, std::string> &M, json::Path P) {
130+
std::variant<ResponseMessage, std::string> &M, json::Path P) {
133131
auto rawMessage = Params.getAsString();
134132
if (!rawMessage) {
135133
P.report("expected a string");
136134
return false;
137135
}
138-
std::optional<Response::Message> message =
139-
StringSwitch<std::optional<Response::Message>>(*rawMessage)
140-
.Case("cancelled", Response::Message::cancelled)
141-
.Case("notStopped", Response::Message::notStopped)
136+
std::optional<ResponseMessage> message =
137+
StringSwitch<std::optional<ResponseMessage>>(*rawMessage)
138+
.Case("cancelled", eResponseMessageCancelled)
139+
.Case("notStopped", eResponseMessageNotStopped)
142140
.Default(std::nullopt);
143141
if (message)
144142
M = *message;

lldb/tools/lldb-dap/Protocol/ProtocolBase.h

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#ifndef LLDB_TOOLS_LLDB_DAP_PROTOCOL_H
2121
#define LLDB_TOOLS_LLDB_DAP_PROTOCOL_H
2222

23+
#include "lldb/lldb-enumerations.h"
2324
#include "llvm/Support/JSON.h"
2425
#include <cstdint>
2526
#include <optional>
@@ -64,15 +65,15 @@ struct Event {
6465
llvm::json::Value toJSON(const Event &);
6566
bool fromJSON(const llvm::json::Value &, Event &, llvm::json::Path);
6667

67-
/// Response for a request.
68-
struct Response {
69-
enum class Message {
68+
FLAGS_ENUM(ResponseMessage){
7069
/// The request was cancelled
71-
cancelled,
70+
eResponseMessageCancelled,
7271
/// The request may be retried once the adapter is in a 'stopped' state
73-
notStopped,
74-
};
72+
eResponseMessageNotStopped,
73+
};
7574

75+
/// Response for a request.
76+
struct Response {
7677
/// Sequence number of the corresponding request.
7778
int64_t request_seq;
7879

@@ -90,7 +91,7 @@ struct Response {
9091
/// Contains the raw error in short form if `success` is false. This raw error
9192
/// might be interpreted by the client and is not shown in the UI. Some
9293
/// predefined values exist.
93-
std::optional<std::variant<Message, std::string>> message;
94+
std::optional<std::variant<ResponseMessage, std::string>> message;
9495

9596
/// Contains request result if success is true and error details if success is
9697
/// false.

lldb/tools/lldb-dap/Protocol/ProtocolRequests.h

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222

2323
#include "Protocol/ProtocolBase.h"
2424
#include "Protocol/ProtocolTypes.h"
25+
#include "lldb/lldb-enumerations.h"
26+
#include "llvm/ADT/DenseSet.h"
2527
#include "llvm/Support/JSON.h"
2628
#include <cstdint>
2729
#include <optional>
@@ -55,26 +57,26 @@ bool fromJSON(const llvm::json::Value &, DisconnectArguments &,
5557
using DisconnectResponse = VoidResponse;
5658

5759
/// Features supported by DAP clients.
58-
enum ClientFeature {
59-
eClientFeatureVariableType,
60-
eClientFeatureVariablePaging,
61-
eClientFeatureRunInTerminalRequest,
62-
eClientFeatureMemoryReferences,
63-
eClientFeatureProgressReporting,
64-
eClientFeatureInvalidatedEvent,
65-
eClientFeatureMemoryEvent,
66-
/// Client supports the `argsCanBeInterpretedByShell` attribute on the
67-
/// `runInTerminal` request.
68-
eClientFeatureArgsCanBeInterpretedByShell,
69-
eClientFeatureStartDebuggingRequest,
70-
/// The client will interpret ANSI escape sequences in the display of
71-
/// `OutputEvent.output` and `Variable.value` fields when
72-
/// `Capabilities.supportsANSIStyling` is also enabled.
73-
eClientFeatureANSIStyling,
60+
FLAGS_ENUM(ClientFeature){
61+
eClientFeatureVariableType,
62+
eClientFeatureVariablePaging,
63+
eClientFeatureRunInTerminalRequest,
64+
eClientFeatureMemoryReferences,
65+
eClientFeatureProgressReporting,
66+
eClientFeatureInvalidatedEvent,
67+
eClientFeatureMemoryEvent,
68+
/// Client supports the `argsCanBeInterpretedByShell` attribute on the
69+
/// `runInTerminal` request.
70+
eClientFeatureArgsCanBeInterpretedByShell,
71+
eClientFeatureStartDebuggingRequest,
72+
/// The client will interpret ANSI escape sequences in the display of
73+
/// `OutputEvent.output` and `Variable.value` fields when
74+
/// `Capabilities.supportsANSIStyling` is also enabled.
75+
eClientFeatureANSIStyling,
7476
};
7577

7678
/// Format of paths reported by the debug adapter.
77-
enum PathFormat { ePatFormatPath, ePathFormatURI };
79+
FLAGS_ENUM(PathFormat){ePatFormatPath, ePathFormatURI};
7880

7981
/// Arguments for `initialize` request.
8082
struct InitializeRequestArguments {

0 commit comments

Comments
 (0)