https://github.com/felipepiovezan updated
https://github.com/llvm/llvm-project/pull/74119
>From d4143eea927eed4f4f0efaa9e1657de5f452a257 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiove...@apple.com>
Date: Fri, 1 Dec 2023 09:07:11 -0800
Subject: [PATCH 1/3] [lldb][NFC] Remove unnecessary std::string temporaries
The existing code was taking three substrings from a regex match and converting
to std::strings prior to using them. This may have been done to address
null-termination concerns, but this is not the case:
1. `name` was being used to call `c_str()` and then implicitly converted back to
a `StringRef` on the call to `ToAddress`. While the path `const char *` ->
`StringRef` requires null-termination, we can simply use the original StringRef.
2. `str_offset` was being converted back to a StringRef in order to call a
member method. Member methods can't handle non-null termination.
3. `sign` simply had it's 0-th element accessed.
---
lldb/source/Interpreter/OptionArgParser.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp
b/lldb/source/Interpreter/OptionArgParser.cpp
index ba2d3416e1838a9..933bc6514ca2418 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -243,12 +243,12 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
llvm::SmallVector<llvm::StringRef, 4> matches;
if (g_symbol_plus_offset_regex.Execute(sref, &matches)) {
uint64_t offset = 0;
- std::string name = matches[1].str();
- std::string sign = matches[2].str();
- std::string str_offset = matches[3].str();
- if (!llvm::StringRef(str_offset).getAsInteger(0, offset)) {
+ llvm::StringRef name = matches[1];
+ llvm::StringRef sign = matches[2];
+ llvm::StringRef str_offset = matches[3];
+ if (!str_offset.getAsInteger(0, offset)) {
Status error;
- addr = ToAddress(exe_ctx, name.c_str(), LLDB_INVALID_ADDRESS, &error);
+ addr = ToAddress(exe_ctx, name, LLDB_INVALID_ADDRESS, &error);
if (addr != LLDB_INVALID_ADDRESS) {
if (sign[0] == '+')
return addr + offset;
>From 58283e15fd2ac6ecb29d24a77b99e883b3a9831a Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiove...@apple.com>
Date: Fri, 1 Dec 2023 09:44:28 -0800
Subject: [PATCH 2/3] [lldb][NFC] Remove else after return in OptionArgParse
This will enable us to prove that there is unreachable code in this function in
a subsequent commit.
---
lldb/source/Interpreter/OptionArgParser.cpp | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp
b/lldb/source/Interpreter/OptionArgParser.cpp
index 933bc6514ca2418..c61072dfafc19f2 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -168,7 +168,6 @@ lldb::addr_t OptionArgParser::ToAddress(const
ExecutionContext *exe_ctx,
std::optional<lldb::addr_t>
OptionArgParser::DoToAddress(const ExecutionContext *exe_ctx, llvm::StringRef
s,
Status *error_ptr) {
- bool error_set = false;
if (s.empty()) {
if (error_ptr)
error_ptr->SetErrorStringWithFormat("invalid address expression \"%s\"",
@@ -212,6 +211,7 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
target->EvaluateExpression(s, exe_ctx->GetFramePtr(), valobj_sp,
options);
bool success = false;
+ bool error_set = false;
if (expr_result == eExpressionCompleted) {
if (valobj_sp)
valobj_sp = valobj_sp->GetQualifiedRepresentationIfAvailable(
@@ -223,16 +223,14 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
if (error_ptr)
error_ptr->Clear();
return addr;
- } else {
- if (error_ptr) {
- error_set = true;
- error_ptr->SetErrorStringWithFormat(
- "address expression \"%s\" resulted in a value whose type "
- "can't be converted to an address: %s",
- s.str().c_str(), valobj_sp->GetTypeName().GetCString());
- }
}
-
+ if (error_ptr) {
+ error_set = true;
+ error_ptr->SetErrorStringWithFormat(
+ "address expression \"%s\" resulted in a value whose type "
+ "can't be converted to an address: %s",
+ s.str().c_str(), valobj_sp->GetTypeName().GetCString());
+ }
} else {
// Since the compiler can't handle things like "main + 12" we should try to
// do this for now. The compiler doesn't like adding offsets to function
@@ -252,8 +250,7 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
if (addr != LLDB_INVALID_ADDRESS) {
if (sign[0] == '+')
return addr + offset;
- else
- return addr - offset;
+ return addr - offset;
}
}
}
>From d6a4756ca3d65382a532c59ff89ddc6d57fc1804 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiove...@apple.com>
Date: Fri, 1 Dec 2023 09:47:57 -0800
Subject: [PATCH 3/3] [lldb][NFC] Delete unreachable code and dead variable in
OptionArgParser
With the combination of an early return and removing an else-after-return, it
becomes evident that there is unreachable code in the function being changed.
---
lldb/source/Interpreter/OptionArgParser.cpp | 62 +++++++++------------
1 file changed, 26 insertions(+), 36 deletions(-)
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp
b/lldb/source/Interpreter/OptionArgParser.cpp
index c61072dfafc19f2..8a92c7d08c4766c 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -211,7 +211,6 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
target->EvaluateExpression(s, exe_ctx->GetFramePtr(), valobj_sp,
options);
bool success = false;
- bool error_set = false;
if (expr_result == eExpressionCompleted) {
if (valobj_sp)
valobj_sp = valobj_sp->GetQualifiedRepresentationIfAvailable(
@@ -224,48 +223,39 @@ OptionArgParser::DoToAddress(const ExecutionContext
*exe_ctx, llvm::StringRef s,
error_ptr->Clear();
return addr;
}
- if (error_ptr) {
- error_set = true;
+ if (error_ptr)
error_ptr->SetErrorStringWithFormat(
"address expression \"%s\" resulted in a value whose type "
"can't be converted to an address: %s",
s.str().c_str(), valobj_sp->GetTypeName().GetCString());
- }
- } else {
- // Since the compiler can't handle things like "main + 12" we should try to
- // do this for now. The compiler doesn't like adding offsets to function
- // pointer types.
- static RegularExpression g_symbol_plus_offset_regex(
- "^(.*)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*$");
-
- llvm::SmallVector<llvm::StringRef, 4> matches;
- if (g_symbol_plus_offset_regex.Execute(sref, &matches)) {
- uint64_t offset = 0;
- llvm::StringRef name = matches[1];
- llvm::StringRef sign = matches[2];
- llvm::StringRef str_offset = matches[3];
- if (!str_offset.getAsInteger(0, offset)) {
- Status error;
- addr = ToAddress(exe_ctx, name, LLDB_INVALID_ADDRESS, &error);
- if (addr != LLDB_INVALID_ADDRESS) {
- if (sign[0] == '+')
- return addr + offset;
- return addr - offset;
- }
- }
- }
+ return {};
+ }
- if (error_ptr) {
- error_set = true;
- error_ptr->SetErrorStringWithFormat(
- "address expression \"%s\" evaluation failed", s.str().c_str());
+ // Since the compiler can't handle things like "main + 12" we should try to
+ // do this for now. The compiler doesn't like adding offsets to function
+ // pointer types.
+ static RegularExpression g_symbol_plus_offset_regex(
+ "^(.*)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*$");
+
+ llvm::SmallVector<llvm::StringRef, 4> matches;
+ if (g_symbol_plus_offset_regex.Execute(sref, &matches)) {
+ uint64_t offset = 0;
+ llvm::StringRef name = matches[1];
+ llvm::StringRef sign = matches[2];
+ llvm::StringRef str_offset = matches[3];
+ if (!str_offset.getAsInteger(0, offset)) {
+ Status error;
+ addr = ToAddress(exe_ctx, name, LLDB_INVALID_ADDRESS, &error);
+ if (addr != LLDB_INVALID_ADDRESS) {
+ if (sign[0] == '+')
+ return addr + offset;
+ return addr - offset;
+ }
}
}
- if (error_ptr) {
- if (!error_set)
- error_ptr->SetErrorStringWithFormat("invalid address expression \"%s\"",
- s.str().c_str());
- }
+ if (error_ptr)
+ error_ptr->SetErrorStringWithFormat(
+ "address expression \"%s\" evaluation failed", s.str().c_str());
return {};
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
