From a16fffa3f6add51fe1c6ee975ace56aa06a3bea7 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Mon, 23 Mar 2020 16:15:49 +0100 Subject: [PATCH] [Support] Make DataExtractor string functions error-aware Summary: This patch adds an optional Error argument to DataExtractor functions for string extraction, and makes them behave like other DataExtractor functions (set the error if extraction fails, don't do anything if the error is already set). I have merged the StringRef and C string versions of the functions to reduce code duplication. Reviewers: dblaikie, MaskRay Subscribers: hiraditya, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D77307 --- llvm/include/llvm/Support/DataExtractor.h | 32 ++++++++++++++++++++++++---- llvm/lib/Support/DataExtractor.cpp | 19 ++++++----------- llvm/unittests/Support/DataExtractorTest.cpp | 10 ++++++++- 3 files changed, 44 insertions(+), 17 deletions(-) diff --git a/llvm/include/llvm/Support/DataExtractor.h b/llvm/include/llvm/Support/DataExtractor.h index 48c596d..4390988 100644 --- a/llvm/include/llvm/Support/DataExtractor.h +++ b/llvm/include/llvm/Support/DataExtractor.h @@ -112,12 +112,25 @@ public: /// enough bytes to extract this value, the offset will be left /// unmodified. /// + /// @param[in,out] Err + /// A pointer to an Error object. Upon return the Error object is set to + /// indicate the result (success/failure) of the function. If the Error + /// object is already set when calling this function, no extraction is + /// performed. + /// /// @return /// A pointer to the C string value in the data. If the offset /// pointed to by \a offset_ptr is out of bounds, or if the /// offset plus the length of the C string is out of bounds, /// NULL will be returned. - const char *getCStr(uint64_t *offset_ptr) const; + const char *getCStr(uint64_t *OffsetPtr, Error *Err = nullptr) const { + return getCStrRef(OffsetPtr, Err).data(); + } + + /// Extract a C string from the location given by the cursor. In case of an + /// extraction error, or if the cursor is already in an error state, a + /// nullptr is returned. + const char *getCStr(Cursor &C) const { return getCStrRef(C).data(); } /// Extract a C string from \a *offset_ptr. /// @@ -134,12 +147,25 @@ public: /// enough bytes to extract this value, the offset will be left /// unmodified. /// + /// @param[in,out] Err + /// A pointer to an Error object. Upon return the Error object is set to + /// indicate the result (success/failure) of the function. If the Error + /// object is already set when calling this function, no extraction is + /// performed. + /// /// \return /// A StringRef for the C string value in the data. If the offset /// pointed to by \a offset_ptr is out of bounds, or if the /// offset plus the length of the C string is out of bounds, /// a default-initialized StringRef will be returned. - StringRef getCStrRef(uint64_t *offset_ptr) const; + StringRef getCStrRef(uint64_t *OffsetPtr, Error *Err = nullptr) const; + + /// Extract a C string (as a StringRef) from the location given by the cursor. + /// In case of an extraction error, or if the cursor is already in an error + /// state, a default-initialized StringRef is returned. + StringRef getCStrRef(Cursor &C) const { + return getCStrRef(&C.Offset, &C.Err); + } /// Extract a fixed length string from \a *OffsetPtr and consume \a Length /// bytes. @@ -197,8 +223,6 @@ public: /// is out of bounds, a default-initialized StringRef will be returned. StringRef getBytes(uint64_t *OffsetPtr, uint64_t Length) const; - StringRef getCStrRef(Cursor &C) const { return getCStrRef(&C.Offset); } - /// Extract an unsigned integer of size \a byte_size from \a /// *offset_ptr. /// diff --git a/llvm/lib/Support/DataExtractor.cpp b/llvm/lib/Support/DataExtractor.cpp index d6fd9a5..764c3fa 100644 --- a/llvm/lib/Support/DataExtractor.cpp +++ b/llvm/lib/Support/DataExtractor.cpp @@ -152,23 +152,18 @@ DataExtractor::getSigned(uint64_t *offset_ptr, uint32_t byte_size) const { llvm_unreachable("getSigned unhandled case!"); } -const char *DataExtractor::getCStr(uint64_t *offset_ptr) const { - uint64_t offset = *offset_ptr; - StringRef::size_type pos = Data.find('\0', offset); - if (pos != StringRef::npos) { - *offset_ptr = pos + 1; - return Data.data() + offset; - } - return nullptr; -} +StringRef DataExtractor::getCStrRef(uint64_t *OffsetPtr, Error *Err) const { + ErrorAsOutParameter ErrAsOut(Err); + if (isError(Err)) + return StringRef(); -StringRef DataExtractor::getCStrRef(uint64_t *offset_ptr) const { - uint64_t Start = *offset_ptr; + uint64_t Start = *OffsetPtr; StringRef::size_type Pos = Data.find('\0', Start); if (Pos != StringRef::npos) { - *offset_ptr = Pos + 1; + *OffsetPtr = Pos + 1; return StringRef(Data.data() + Start, Pos - Start); } + unexpectedEndReached(Err, Start); return StringRef(); } diff --git a/llvm/unittests/Support/DataExtractorTest.cpp b/llvm/unittests/Support/DataExtractorTest.cpp index 35b25dd..20bc8b0 100644 --- a/llvm/unittests/Support/DataExtractorTest.cpp +++ b/llvm/unittests/Support/DataExtractorTest.cpp @@ -14,7 +14,6 @@ using namespace llvm; namespace { const char numberData[] = "\x80\x90\xFF\xFF\x80\x00\x00\x00"; -const char stringData[] = "hellohello\0hello"; const char leb128data[] = "\xA6\x49"; const char bigleb128data[] = "\xAA\xA9\xFF\xAA\xFF\xAA\xFF\x4A"; @@ -89,6 +88,7 @@ TEST(DataExtractorTest, SignedNumbers) { } TEST(DataExtractorTest, Strings) { + const char stringData[] = "hellohello\0hello"; DataExtractor DE(StringRef(stringData, sizeof(stringData)-1), false, 8); uint64_t offset = 0; @@ -96,6 +96,14 @@ TEST(DataExtractorTest, Strings) { EXPECT_EQ(11U, offset); EXPECT_EQ(nullptr, DE.getCStr(&offset)); EXPECT_EQ(11U, offset); + + DataExtractor::Cursor C(0); + EXPECT_EQ(stringData, DE.getCStr(C)); + EXPECT_EQ(11U, C.tell()); + EXPECT_EQ(nullptr, DE.getCStr(C)); + EXPECT_EQ(11U, C.tell()); + EXPECT_THAT_ERROR(C.takeError(), + FailedWithMessage("unexpected end of data at offset 0xb")); } TEST(DataExtractorTest, LEB128) { -- 2.7.4