DavidSpickett created this revision.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
When a command option does not have a short version
(e.g. -f for --file), we use an arbitrary value in the
short_option field to mark it as invalid.
(though this value is unqiue to be used later for other
things)
We check that this short option is valid to print using
llvm::isPrint. This implicitly casts our int to char,
meaning we check the last char of any short_option value.
Since the arbitrary value we chose for these options is
some shortened hex version of the name, this returned true
even for invalid values.
Since llvm::isPrint returns true we later call std::islower
and/or std::isupper on the short_option value. (the int)
Calling these functions with something that cannot be validly
converted to unsigned char is undefined. Somehow we got/get
away with this but for me compiling with g++-9 I got a crash
for "help memory read".
The other command that uses this is "target variable" but that
didn't crash for unknown reasons.
Checking that short_option can fit into an unsigned char before
we call llvm::isPrint means we will not attempt to call islower/upper
on these options since we have no reason to print them.
This also fixes bogus short options being shown for "memory read"
and target variable.
For "target variable", before:
-e <filename> ( --file <filename> )
-b <filename> ( --shlib <filename> )
After:
--file <filename>
--shlib <filename>
(note that the bogus short options are just the bottom byte of our
arbitrary short_option value)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D94917
Files:
lldb/include/lldb/Utility/OptionDefinition.h
lldb/test/API/commands/help/TestHelp.py
Index: lldb/test/API/commands/help/TestHelp.py
===================================================================
--- lldb/test/API/commands/help/TestHelp.py
+++ lldb/test/API/commands/help/TestHelp.py
@@ -57,6 +57,11 @@
self.runCmd("help unsigned-integer")
@no_debug_info_test
+ def test_help_memory_read_should_not_crash_lldb(self):
+ """Command 'help memory read' should not crash lldb."""
+ self.runCmd("help memory read", check=False)
+
+ @no_debug_info_test
def test_help_should_not_hang_emacsshell(self):
"""Command 'settings set term-width 0' should not hang the help
command."""
self.expect(
Index: lldb/include/lldb/Utility/OptionDefinition.h
===================================================================
--- lldb/include/lldb/Utility/OptionDefinition.h
+++ lldb/include/lldb/Utility/OptionDefinition.h
@@ -12,6 +12,7 @@
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-private-types.h"
#include "llvm/ADT/StringExtras.h"
+#include <climits>
#include <cstdint>
namespace lldb_private {
@@ -47,7 +48,9 @@
/// Whether this has a short option character.
bool HasShortOption() const {
// See the short_option documentation for more.
- return llvm::isPrint(short_option);
+ // Must not have any bits set outside of unsigned char range
+ return !(short_option & ~((1 << CHAR_BIT) - 1)) &&
+ llvm::isPrint(short_option);
}
};
} // namespace lldb_private
Index: lldb/test/API/commands/help/TestHelp.py
===================================================================
--- lldb/test/API/commands/help/TestHelp.py
+++ lldb/test/API/commands/help/TestHelp.py
@@ -57,6 +57,11 @@
self.runCmd("help unsigned-integer")
@no_debug_info_test
+ def test_help_memory_read_should_not_crash_lldb(self):
+ """Command 'help memory read' should not crash lldb."""
+ self.runCmd("help memory read", check=False)
+
+ @no_debug_info_test
def test_help_should_not_hang_emacsshell(self):
"""Command 'settings set term-width 0' should not hang the help command."""
self.expect(
Index: lldb/include/lldb/Utility/OptionDefinition.h
===================================================================
--- lldb/include/lldb/Utility/OptionDefinition.h
+++ lldb/include/lldb/Utility/OptionDefinition.h
@@ -12,6 +12,7 @@
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-private-types.h"
#include "llvm/ADT/StringExtras.h"
+#include <climits>
#include <cstdint>
namespace lldb_private {
@@ -47,7 +48,9 @@
/// Whether this has a short option character.
bool HasShortOption() const {
// See the short_option documentation for more.
- return llvm::isPrint(short_option);
+ // Must not have any bits set outside of unsigned char range
+ return !(short_option & ~((1 << CHAR_BIT) - 1)) &&
+ llvm::isPrint(short_option);
}
};
} // namespace lldb_private
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
