[Lldb-commits] [lldb] [lldb] Delete unreachable code and unncesary string conversions (PR #74119)
Felipe de Azevedo Piovezan via lldb-commits
lldb-commits at lists.llvm.org
Fri Dec 1 09:52:38 PST 2023
https://github.com/felipepiovezan created https://github.com/llvm/llvm-project/pull/74119
This PR cleans up OptionArgParser in a couple of ways:
1. We remove unnecessary std::string temporaries
2. Through else-after-return elimination, we prove the existence of unreachable code
>From 6f502ee61bae59a16f90f30357eb72698ad577d0 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at 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 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp b/lldb/source/Interpreter/OptionArgParser.cpp
index ba2d3416e1838a9..9e53261ac885436 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -243,9 +243,9 @@ 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();
+ llvm::StringRef name = matches[1];
+ llvm::StringRef sign = matches[2];
+ llvm::StringRef str_offset = matches[3];
if (!llvm::StringRef(str_offset).getAsInteger(0, offset)) {
Status error;
addr = ToAddress(exe_ctx, name.c_str(), LLDB_INVALID_ADDRESS, &error);
>From f7b4bece1f40ffbb22cffb1ef25d56fb34bfedb2 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at 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 9e53261ac885436..8698f1de224de20 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 25decf3ecd15f3782ffe8e5666423d8a750ff14d Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at 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 8698f1de224de20..2f2c510462dc79a 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 (!llvm::StringRef(str_offset).getAsInteger(0, offset)) {
- Status error;
- addr = ToAddress(exe_ctx, name.c_str(), 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 (!llvm::StringRef(str_offset).getAsInteger(0, offset)) {
+ Status error;
+ addr = ToAddress(exe_ctx, name.c_str(), 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 {};
}
More information about the lldb-commits
mailing list