From 53a15b99c2817e42cf5a0c5a0529077c53d88a23 Mon Sep 17 00:00:00 2001 From: Eric Kilmer Date: Tue, 23 May 2023 13:28:41 -0400 Subject: [PATCH 1/4] Run Linux ASAN in CI --- .github/workflows/main.yml | 60 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f7a7a45f8..2ad9d293b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -14,6 +14,66 @@ on: - '*' jobs: + sanitizer: + runs-on: ubuntu-22.04 + strategy: + fail-fast: false + matrix: + release: [stable, HEAD] + env: { CXX: clang++-14 } + steps: + - uses: actions/checkout@v3 + + - name: Setup Git User for Applying Patches + # See this thread for more details https://github.community/t/github-actions-bot-email-address/17204/5 + run: | + git config --global user.email "41898282+github-actions[bot]@users.noreply.github.com" + git config --global user.name "github-actions[bot]" + + - name: Configure the project + run: cmake --preset=ci-sanitize + -Dsleigh_RELEASE_TYPE=${{ matrix.release }} + + - name: Build the project + run: cmake + --build build/sanitize + -j 2 + -v + + - name: Run the example + env: + ASAN_OPTIONS: "strict_string_checks=1:\ + detect_stack_use_after_return=1:\ + check_initialization_order=1:\ + strict_init_order=1:\ + detect_leaks=1" + UBSAN_OPTIONS: print_stacktrace=1 + run: cmake + --build build/sanitize + -j 2 + --target sleigh_example_runner + + - name: Smoketest sleigh lift + env: + ASAN_OPTIONS: "strict_string_checks=1:\ + detect_stack_use_after_return=1:\ + check_initialization_order=1:\ + strict_init_order=1:\ + detect_leaks=1" + UBSAN_OPTIONS: print_stacktrace=1 + run: ./build/sanitize/extra-tools/sleigh-lift/sleigh-lift disassemble x86-64.sla 4881ecc00f0000 + + - name: Run the tests + working-directory: build/sanitize + env: + ASAN_OPTIONS: "strict_string_checks=1:\ + detect_stack_use_after_return=1:\ + check_initialization_order=1:\ + strict_init_order=1:\ + detect_leaks=1" + UBSAN_OPTIONS: print_stacktrace=1 + run: ctest -VV + build: runs-on: ${{ matrix.os }} From c3435dfecf77def2b714e6feaa8cb73694fe329e Mon Sep 17 00:00:00 2001 From: Eric Kilmer Date: Tue, 23 May 2023 13:34:51 -0400 Subject: [PATCH 2/4] Remove documentation building from CMakePresets Only enable explicitly. Speeds up sanitizer build/configure. --- .github/workflows/main.yml | 1 + CMakePresets.json | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 2ad9d293b..5632a453f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -158,6 +158,7 @@ jobs: shell: pwsh run: cmake "--preset=ci-$("${{ matrix.os }}".split("-")[0])" -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} + -Dsleigh_BUILD_DOCUMENTATION=ON -Dsleigh_RELEASE_TYPE=${{ matrix.release }} - name: Build the project diff --git a/CMakePresets.json b/CMakePresets.json index 3c083d2b4..6b80d12ae 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -26,8 +26,7 @@ "hidden": true, "inherits": "cmake-pedantic", "cacheVariables": { - "sleigh_DEVELOPER_MODE": "ON", - "sleigh_BUILD_DOCUMENTATION": "ON" + "sleigh_DEVELOPER_MODE": "ON" } }, { From 4bc7c0de0cb37828567c158e61ceee8dd55d43df Mon Sep 17 00:00:00 2001 From: Eric Kilmer Date: Wed, 24 May 2023 19:33:20 -0400 Subject: [PATCH 3/4] Fix some errors --- .../0001-Fix-UBSAN-errors-in-decompiler.patch | 18 +- ...ead-of-stroul-to-parse-address-offse.patch | 6 +- ...003-Fix-ASAN-initialize-order-fiasco.patch | 67 ++++++ ...004-Fix-memory-leak-after-xml-errors.patch | 195 ++++++++++++++++++ .../0001-Fix-UBSAN-errors-in-decompiler.patch | 16 +- ...ead-of-stroul-to-parse-address-offse.patch | 6 +- ...003-Fix-ASAN-initialize-order-fiasco.patch | 67 ++++++ ...004-Fix-memory-leak-after-xml-errors.patch | 195 ++++++++++++++++++ src/setup-ghidra-source.cmake | 6 +- 9 files changed, 552 insertions(+), 24 deletions(-) create mode 100644 src/patches/HEAD/0003-Fix-ASAN-initialize-order-fiasco.patch create mode 100644 src/patches/HEAD/0004-Fix-memory-leak-after-xml-errors.patch create mode 100644 src/patches/stable/0003-Fix-ASAN-initialize-order-fiasco.patch create mode 100644 src/patches/stable/0004-Fix-memory-leak-after-xml-errors.patch diff --git a/src/patches/HEAD/0001-Fix-UBSAN-errors-in-decompiler.patch b/src/patches/HEAD/0001-Fix-UBSAN-errors-in-decompiler.patch index 50369b3f4..abb6b7fa9 100644 --- a/src/patches/HEAD/0001-Fix-UBSAN-errors-in-decompiler.patch +++ b/src/patches/HEAD/0001-Fix-UBSAN-errors-in-decompiler.patch @@ -1,7 +1,7 @@ -From 098585db4fa8c8eb9184851a40ef6706f1d176ed Mon Sep 17 00:00:00 2001 +From 79a61c7d52db690677e3ac446561b8575a609581 Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Mon, 7 Feb 2022 02:02:03 +1100 -Subject: [PATCH 1/3] Fix UBSAN errors in decompiler +Subject: [PATCH 1/4] Fix UBSAN errors in decompiler --- .../Decompiler/src/decompile/cpp/address.cc | 4 ++-- @@ -42,7 +42,7 @@ index 2eb08b96a..07bf3ba55 100644 mask <<= 1; val &= (~mask); diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc -index 1402bbd80..4b271e055 100644 +index b30e48298..e4395768f 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc @@ -2661,8 +2661,12 @@ void ProtoModelMerged::decode(Decoder &decoder) @@ -159,7 +159,7 @@ index ca9d71ab9..85d4dd281 100644 return res; } diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc -index f11ab8dea..66a0b32fc 100644 +index 4851365d4..d069d1c94 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc @@ -976,7 +976,12 @@ int4 RulePullsubIndirect::applyOp(PcodeOp *op,Funcdata &data) @@ -176,7 +176,7 @@ index f11ab8dea..66a0b32fc 100644 consume = ~consume; if ((consume & indir->getIn(0)->getConsume())!=0) return 0; -@@ -6778,8 +6783,9 @@ int4 RulePtrsubCharConstant::applyOp(PcodeOp *op,Funcdata &data) +@@ -6782,8 +6787,9 @@ int4 RulePtrsubCharConstant::applyOp(PcodeOp *op,Funcdata &data) Varnode *sb = op->getIn(0); Datatype *sbType = sb->getTypeReadFacing(op); if (sbType->getMetatype() != TYPE_PTR) return 0; @@ -188,7 +188,7 @@ index f11ab8dea..66a0b32fc 100644 Varnode *vn1 = op->getIn(1); if (!vn1->isConstant()) return 0; Varnode *outvn = op->getOut(); -@@ -8509,7 +8515,11 @@ int4 RuleSubvarSubpiece::applyOp(PcodeOp *op,Funcdata &data) +@@ -8593,7 +8599,11 @@ int4 RuleSubvarSubpiece::applyOp(PcodeOp *op,Funcdata &data) Varnode *outvn = op->getOut(); int4 flowsize = outvn->getSize(); uintb mask = calc_mask( flowsize ); @@ -262,10 +262,10 @@ index b308e1b71..af2982aee 100644 } diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/type.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/type.cc -index f1c0b6452..68de180fc 100644 +index fa02605a6..74f207063 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/type.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/type.cc -@@ -3296,8 +3296,8 @@ void TypeFactory::recalcPointerSubmeta(Datatype *base,sub_metatype sub) +@@ -3352,8 +3352,8 @@ void TypeFactory::recalcPointerSubmeta(Datatype *base,sub_metatype sub) top.submeta = sub; // Search on the incorrect submeta iter = tree.lower_bound(&top); while(iter != tree.end()) { @@ -289,5 +289,5 @@ index c35bde877..061e53677 100644 uintb true_result = ((uintb)(int32_t)f) & 0xffffffff; uintb encoding = format.getEncoding(f); -- -2.40.0 +2.40.1 diff --git a/src/patches/HEAD/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch b/src/patches/HEAD/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch index 881d6fa5b..e843d31c4 100644 --- a/src/patches/HEAD/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch +++ b/src/patches/HEAD/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch @@ -1,7 +1,7 @@ -From a2d412ce5edb750ee1f768a4a7ebfeaece6f7976 Mon Sep 17 00:00:00 2001 +From af3df62bdede0d6b5bfe18c35b46a97defc3e280 Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Wed, 3 Aug 2022 20:01:18 +1000 -Subject: [PATCH 2/3] Use `stroull` instead of `stroul` to parse address +Subject: [PATCH 2/4] Use `stroull` instead of `stroul` to parse address offsets --- @@ -34,5 +34,5 @@ index bf4e1dc96..594b4583a 100644 enddata = (const char *) tmpdata; if (enddata - s.c_str() == s.size()) { // If no size or offset override -- -2.40.0 +2.40.1 diff --git a/src/patches/HEAD/0003-Fix-ASAN-initialize-order-fiasco.patch b/src/patches/HEAD/0003-Fix-ASAN-initialize-order-fiasco.patch new file mode 100644 index 000000000..72cbc47da --- /dev/null +++ b/src/patches/HEAD/0003-Fix-ASAN-initialize-order-fiasco.patch @@ -0,0 +1,67 @@ +From 0126a87fbebfdcdb0553af325d6880647050c287 Mon Sep 17 00:00:00 2001 +From: Eric Kilmer +Date: Wed, 24 May 2023 19:03:25 -0400 +Subject: [PATCH 3/4] Fix ASAN initialize-order-fiasco + +Use the "Constructo On First Use" idiom from +https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Construct_On_First_Use +--- + Ghidra/Features/Decompiler/src/decompile/cpp/test.cc | 4 +--- + Ghidra/Features/Decompiler/src/decompile/cpp/test.hh | 10 ++++++++-- + 2 files changed, 9 insertions(+), 5 deletions(-) + +diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/test.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/test.cc +index 660f06940..603aaa5bb 100644 +--- a/Ghidra/Features/Decompiler/src/decompile/cpp/test.cc ++++ b/Ghidra/Features/Decompiler/src/decompile/cpp/test.cc +@@ -18,8 +18,6 @@ + + namespace ghidra { + +-vector UnitTest::tests; +- + /// Run all the tests unless a non-empty set of names is passed in. + /// In which case, only the named tests in the set are run. + /// \param testNames is the set of names +@@ -30,7 +28,7 @@ int UnitTest::run(set &testNames) + int total = 0; + int passed = 0; + +- for(auto &t : UnitTest::tests) { ++ for(auto &t : UnitTest::tests()) { + if (testNames.size() > 0 && testNames.find(t->name) == testNames.end()) { + continue; + } +diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/test.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/test.hh +index 27c27bc21..78d7886a3 100644 +--- a/Ghidra/Features/Decompiler/src/decompile/cpp/test.hh ++++ b/Ghidra/Features/Decompiler/src/decompile/cpp/test.hh +@@ -53,7 +53,6 @@ typedef void (*testfunc_t)(); ///< A unit-test function + /// The static run() method calls all the function pointers of all instantiated + /// objects. + struct UnitTest { +- static vector tests; ///< The collection of test objects + string name; ///< Name of the test + testfunc_t func; ///< Call-back function executing the test + +@@ -64,9 +63,16 @@ struct UnitTest { + UnitTest(const string &name,testfunc_t func) : + name(name), func(func) + { +- tests.push_back(this); ++ tests().push_back(this); + } + ++ /// \brief The collection of test objects ++ static vector & tests() { ++ static vector tests; ++ return tests; ++ } ++ ++ + static int run(set &testNames); ///< Run all the instantiated tests + }; + +-- +2.40.1 + diff --git a/src/patches/HEAD/0004-Fix-memory-leak-after-xml-errors.patch b/src/patches/HEAD/0004-Fix-memory-leak-after-xml-errors.patch new file mode 100644 index 000000000..ac8c5cf1a --- /dev/null +++ b/src/patches/HEAD/0004-Fix-memory-leak-after-xml-errors.patch @@ -0,0 +1,195 @@ +From ec635dc18a2ad74639380816e33dbf41ef862de1 Mon Sep 17 00:00:00 2001 +From: Eric Kilmer +Date: Wed, 24 May 2023 19:27:54 -0400 +Subject: [PATCH 4/4] Fix memory leak after xml errors + +Regenerated with bison 3.0.4 on AlmaLinux 8 +--- + .../Decompiler/src/decompile/cpp/xml.cc | 149 ++++++++++++++++-- + .../Decompiler/src/decompile/cpp/xml.y | 3 + + 2 files changed, 143 insertions(+), 9 deletions(-) + +diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/xml.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/xml.cc +index 76bdf4526..6831da5a6 100644 +--- a/Ghidra/Features/Decompiler/src/decompile/cpp/xml.cc ++++ b/Ghidra/Features/Decompiler/src/decompile/cpp/xml.cc +@@ -570,14 +570,14 @@ static const yytype_uint8 yytranslate[] = + /* YYRLINE[YYN] -- Source line where rule number YYN was defined. */ + static const yytype_uint8 yyrline[] = + { +- 0, 138, 138, 139, 140, 141, 142, 143, 144, 145, +- 147, 148, 149, 150, 151, 152, 153, 154, 155, 156, +- 157, 158, 159, 160, 161, 163, 164, 165, 166, 167, +- 168, 169, 171, 172, 173, 174, 175, 176, 177, 179, +- 180, 181, 182, 183, 184, 185, 187, 188, 190, 191, +- 192, 193, 195, 196, 197, 198, 199, 200, 202, 203, +- 204, 205, 206, 207, 208, 210, 211, 213, 214, 215, +- 216 ++ 0, 141, 141, 142, 143, 144, 145, 146, 147, 148, ++ 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, ++ 160, 161, 162, 163, 164, 166, 167, 168, 169, 170, ++ 171, 172, 174, 175, 176, 177, 178, 179, 180, 182, ++ 183, 184, 185, 186, 187, 188, 190, 191, 193, 194, ++ 195, 196, 198, 199, 200, 201, 202, 203, 205, 206, ++ 207, 208, 209, 210, 211, 213, 214, 216, 217, 218, ++ 219 + }; + #endif + +@@ -1207,7 +1207,138 @@ yydestruct (const char *yymsg, int yytype, YYSTYPE *yyvaluep) + YY_SYMBOL_PRINT (yymsg, yytype, yyvaluep, yylocationp); + + YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN +- YYUSE (yytype); ++ switch (yytype) ++ { ++ case 3: /* CHARDATA */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 4: /* CDATA */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 5: /* ATTVALUE */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 6: /* COMMENT */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 7: /* CHARREF */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 8: /* NAME */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 9: /* SNAME */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 10: /* ELEMBRACE */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 11: /* COMMBRACE */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 54: /* attsinglemid */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 55: /* attdoublemid */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 56: /* AttValue */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 61: /* CDSect */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 75: /* STag */ ++ ++ { delete ((*yyvaluep).attr); } ++ ++ break; ++ ++ case 76: /* EmptyElemTag */ ++ ++ { delete ((*yyvaluep).attr); } ++ ++ break; ++ ++ case 77: /* stagstart */ ++ ++ { delete ((*yyvaluep).attr); } ++ ++ break; ++ ++ case 78: /* SAttribute */ ++ ++ { delete ((*yyvaluep).pair); } ++ ++ break; ++ ++ case 80: /* ETag */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 82: /* Reference */ ++ ++ { } ++ ++ break; ++ ++ case 85: /* CharRef */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 86: /* EntityRef */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ ++ default: ++ break; ++ } + YY_IGNORE_MAYBE_UNINITIALIZED_END + } + +diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/xml.y b/Ghidra/Features/Decompiler/src/decompile/cpp/xml.y +index 30ed0acd5..222e9d495 100644 +--- a/Ghidra/Features/Decompiler/src/decompile/cpp/xml.y ++++ b/Ghidra/Features/Decompiler/src/decompile/cpp/xml.y +@@ -133,6 +133,9 @@ static ContentHandler *handler; ///< Global reference to the content handler + %type Reference + %type EmptyElemTag STag stagstart + %type SAttribute ++ ++%destructor { } ++%destructor { delete $$; } <*> + %% + + document: element Misc; +-- +2.40.1 + diff --git a/src/patches/stable/0001-Fix-UBSAN-errors-in-decompiler.patch b/src/patches/stable/0001-Fix-UBSAN-errors-in-decompiler.patch index 50369b3f4..88ab2c6d8 100644 --- a/src/patches/stable/0001-Fix-UBSAN-errors-in-decompiler.patch +++ b/src/patches/stable/0001-Fix-UBSAN-errors-in-decompiler.patch @@ -1,7 +1,7 @@ -From 098585db4fa8c8eb9184851a40ef6706f1d176ed Mon Sep 17 00:00:00 2001 +From 7f760d9be3728818ae63e77e1748894710809144 Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Mon, 7 Feb 2022 02:02:03 +1100 -Subject: [PATCH 1/3] Fix UBSAN errors in decompiler +Subject: [PATCH 1/4] Fix UBSAN errors in decompiler --- .../Decompiler/src/decompile/cpp/address.cc | 4 ++-- @@ -42,7 +42,7 @@ index 2eb08b96a..07bf3ba55 100644 mask <<= 1; val &= (~mask); diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc -index 1402bbd80..4b271e055 100644 +index b30e48298..e4395768f 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc @@ -2661,8 +2661,12 @@ void ProtoModelMerged::decode(Decoder &decoder) @@ -159,7 +159,7 @@ index ca9d71ab9..85d4dd281 100644 return res; } diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc -index f11ab8dea..66a0b32fc 100644 +index e4961f984..cd0bbcf7e 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc @@ -976,7 +976,12 @@ int4 RulePullsubIndirect::applyOp(PcodeOp *op,Funcdata &data) @@ -188,7 +188,7 @@ index f11ab8dea..66a0b32fc 100644 Varnode *vn1 = op->getIn(1); if (!vn1->isConstant()) return 0; Varnode *outvn = op->getOut(); -@@ -8509,7 +8515,11 @@ int4 RuleSubvarSubpiece::applyOp(PcodeOp *op,Funcdata &data) +@@ -8589,7 +8595,11 @@ int4 RuleSubvarSubpiece::applyOp(PcodeOp *op,Funcdata &data) Varnode *outvn = op->getOut(); int4 flowsize = outvn->getSize(); uintb mask = calc_mask( flowsize ); @@ -262,10 +262,10 @@ index b308e1b71..af2982aee 100644 } diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/type.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/type.cc -index f1c0b6452..68de180fc 100644 +index fa02605a6..74f207063 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/type.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/type.cc -@@ -3296,8 +3296,8 @@ void TypeFactory::recalcPointerSubmeta(Datatype *base,sub_metatype sub) +@@ -3352,8 +3352,8 @@ void TypeFactory::recalcPointerSubmeta(Datatype *base,sub_metatype sub) top.submeta = sub; // Search on the incorrect submeta iter = tree.lower_bound(&top); while(iter != tree.end()) { @@ -289,5 +289,5 @@ index c35bde877..061e53677 100644 uintb true_result = ((uintb)(int32_t)f) & 0xffffffff; uintb encoding = format.getEncoding(f); -- -2.40.0 +2.40.1 diff --git a/src/patches/stable/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch b/src/patches/stable/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch index 881d6fa5b..7882c9bce 100644 --- a/src/patches/stable/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch +++ b/src/patches/stable/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch @@ -1,7 +1,7 @@ -From a2d412ce5edb750ee1f768a4a7ebfeaece6f7976 Mon Sep 17 00:00:00 2001 +From e64380cd8fcf473d710180d9007a31dfc08eae6a Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Wed, 3 Aug 2022 20:01:18 +1000 -Subject: [PATCH 2/3] Use `stroull` instead of `stroul` to parse address +Subject: [PATCH 2/4] Use `stroull` instead of `stroul` to parse address offsets --- @@ -34,5 +34,5 @@ index bf4e1dc96..594b4583a 100644 enddata = (const char *) tmpdata; if (enddata - s.c_str() == s.size()) { // If no size or offset override -- -2.40.0 +2.40.1 diff --git a/src/patches/stable/0003-Fix-ASAN-initialize-order-fiasco.patch b/src/patches/stable/0003-Fix-ASAN-initialize-order-fiasco.patch new file mode 100644 index 000000000..fea58c359 --- /dev/null +++ b/src/patches/stable/0003-Fix-ASAN-initialize-order-fiasco.patch @@ -0,0 +1,67 @@ +From 8fbbfed97dec5ddb0c687a783c009ac55f020969 Mon Sep 17 00:00:00 2001 +From: Eric Kilmer +Date: Wed, 24 May 2023 19:03:25 -0400 +Subject: [PATCH 3/4] Fix ASAN initialize-order-fiasco + +Use the "Constructo On First Use" idiom from +https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Construct_On_First_Use +--- + Ghidra/Features/Decompiler/src/decompile/cpp/test.cc | 4 +--- + Ghidra/Features/Decompiler/src/decompile/cpp/test.hh | 10 ++++++++-- + 2 files changed, 9 insertions(+), 5 deletions(-) + +diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/test.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/test.cc +index 660f06940..603aaa5bb 100644 +--- a/Ghidra/Features/Decompiler/src/decompile/cpp/test.cc ++++ b/Ghidra/Features/Decompiler/src/decompile/cpp/test.cc +@@ -18,8 +18,6 @@ + + namespace ghidra { + +-vector UnitTest::tests; +- + /// Run all the tests unless a non-empty set of names is passed in. + /// In which case, only the named tests in the set are run. + /// \param testNames is the set of names +@@ -30,7 +28,7 @@ int UnitTest::run(set &testNames) + int total = 0; + int passed = 0; + +- for(auto &t : UnitTest::tests) { ++ for(auto &t : UnitTest::tests()) { + if (testNames.size() > 0 && testNames.find(t->name) == testNames.end()) { + continue; + } +diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/test.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/test.hh +index 27c27bc21..78d7886a3 100644 +--- a/Ghidra/Features/Decompiler/src/decompile/cpp/test.hh ++++ b/Ghidra/Features/Decompiler/src/decompile/cpp/test.hh +@@ -53,7 +53,6 @@ typedef void (*testfunc_t)(); ///< A unit-test function + /// The static run() method calls all the function pointers of all instantiated + /// objects. + struct UnitTest { +- static vector tests; ///< The collection of test objects + string name; ///< Name of the test + testfunc_t func; ///< Call-back function executing the test + +@@ -64,9 +63,16 @@ struct UnitTest { + UnitTest(const string &name,testfunc_t func) : + name(name), func(func) + { +- tests.push_back(this); ++ tests().push_back(this); + } + ++ /// \brief The collection of test objects ++ static vector & tests() { ++ static vector tests; ++ return tests; ++ } ++ ++ + static int run(set &testNames); ///< Run all the instantiated tests + }; + +-- +2.40.1 + diff --git a/src/patches/stable/0004-Fix-memory-leak-after-xml-errors.patch b/src/patches/stable/0004-Fix-memory-leak-after-xml-errors.patch new file mode 100644 index 000000000..22227bcf3 --- /dev/null +++ b/src/patches/stable/0004-Fix-memory-leak-after-xml-errors.patch @@ -0,0 +1,195 @@ +From aa60017f1d68af44ae198d51c318c7e911b56448 Mon Sep 17 00:00:00 2001 +From: Eric Kilmer +Date: Wed, 24 May 2023 19:27:54 -0400 +Subject: [PATCH 4/4] Fix memory leak after xml errors + +Regenerated with bison 3.0.4 on AlmaLinux 8 +--- + .../Decompiler/src/decompile/cpp/xml.cc | 149 ++++++++++++++++-- + .../Decompiler/src/decompile/cpp/xml.y | 3 + + 2 files changed, 143 insertions(+), 9 deletions(-) + +diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/xml.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/xml.cc +index 76bdf4526..6831da5a6 100644 +--- a/Ghidra/Features/Decompiler/src/decompile/cpp/xml.cc ++++ b/Ghidra/Features/Decompiler/src/decompile/cpp/xml.cc +@@ -570,14 +570,14 @@ static const yytype_uint8 yytranslate[] = + /* YYRLINE[YYN] -- Source line where rule number YYN was defined. */ + static const yytype_uint8 yyrline[] = + { +- 0, 138, 138, 139, 140, 141, 142, 143, 144, 145, +- 147, 148, 149, 150, 151, 152, 153, 154, 155, 156, +- 157, 158, 159, 160, 161, 163, 164, 165, 166, 167, +- 168, 169, 171, 172, 173, 174, 175, 176, 177, 179, +- 180, 181, 182, 183, 184, 185, 187, 188, 190, 191, +- 192, 193, 195, 196, 197, 198, 199, 200, 202, 203, +- 204, 205, 206, 207, 208, 210, 211, 213, 214, 215, +- 216 ++ 0, 141, 141, 142, 143, 144, 145, 146, 147, 148, ++ 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, ++ 160, 161, 162, 163, 164, 166, 167, 168, 169, 170, ++ 171, 172, 174, 175, 176, 177, 178, 179, 180, 182, ++ 183, 184, 185, 186, 187, 188, 190, 191, 193, 194, ++ 195, 196, 198, 199, 200, 201, 202, 203, 205, 206, ++ 207, 208, 209, 210, 211, 213, 214, 216, 217, 218, ++ 219 + }; + #endif + +@@ -1207,7 +1207,138 @@ yydestruct (const char *yymsg, int yytype, YYSTYPE *yyvaluep) + YY_SYMBOL_PRINT (yymsg, yytype, yyvaluep, yylocationp); + + YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN +- YYUSE (yytype); ++ switch (yytype) ++ { ++ case 3: /* CHARDATA */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 4: /* CDATA */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 5: /* ATTVALUE */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 6: /* COMMENT */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 7: /* CHARREF */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 8: /* NAME */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 9: /* SNAME */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 10: /* ELEMBRACE */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 11: /* COMMBRACE */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 54: /* attsinglemid */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 55: /* attdoublemid */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 56: /* AttValue */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 61: /* CDSect */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 75: /* STag */ ++ ++ { delete ((*yyvaluep).attr); } ++ ++ break; ++ ++ case 76: /* EmptyElemTag */ ++ ++ { delete ((*yyvaluep).attr); } ++ ++ break; ++ ++ case 77: /* stagstart */ ++ ++ { delete ((*yyvaluep).attr); } ++ ++ break; ++ ++ case 78: /* SAttribute */ ++ ++ { delete ((*yyvaluep).pair); } ++ ++ break; ++ ++ case 80: /* ETag */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 82: /* Reference */ ++ ++ { } ++ ++ break; ++ ++ case 85: /* CharRef */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ case 86: /* EntityRef */ ++ ++ { delete ((*yyvaluep).str); } ++ ++ break; ++ ++ ++ default: ++ break; ++ } + YY_IGNORE_MAYBE_UNINITIALIZED_END + } + +diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/xml.y b/Ghidra/Features/Decompiler/src/decompile/cpp/xml.y +index 30ed0acd5..222e9d495 100644 +--- a/Ghidra/Features/Decompiler/src/decompile/cpp/xml.y ++++ b/Ghidra/Features/Decompiler/src/decompile/cpp/xml.y +@@ -133,6 +133,9 @@ static ContentHandler *handler; ///< Global reference to the content handler + %type Reference + %type EmptyElemTag STag stagstart + %type SAttribute ++ ++%destructor { } ++%destructor { delete $$; } <*> + %% + + document: element Misc; +-- +2.40.1 + diff --git a/src/setup-ghidra-source.cmake b/src/setup-ghidra-source.cmake index 89c6a8f04..b6b193ace 100644 --- a/src/setup-ghidra-source.cmake +++ b/src/setup-ghidra-source.cmake @@ -40,6 +40,8 @@ set(ghidra_patches "${GIT_EXECUTABLE}" am --ignore-space-change --ignore-whitespace --no-gpg-sign "${CMAKE_CURRENT_LIST_DIR}/patches/stable/0001-Fix-UBSAN-errors-in-decompiler.patch" "${CMAKE_CURRENT_LIST_DIR}/patches/stable/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch" + "${CMAKE_CURRENT_LIST_DIR}/patches/stable/0003-Fix-ASAN-initialize-order-fiasco.patch" + "${CMAKE_CURRENT_LIST_DIR}/patches/stable/0004-Fix-memory-leak-after-xml-errors.patch" ) # Ghidra pinned commits used for pinning last known working HEAD commit @@ -48,7 +50,7 @@ if("${sleigh_RELEASE_TYPE}" STREQUAL "HEAD") # TODO: CMake only likes numeric characters in the version string.... set(ghidra_head_version "10.4") set(ghidra_version "${ghidra_head_version}") - set(ghidra_head_git_tag "894bd3cfc2db78686727bbf2a1d369de92878033") + set(ghidra_head_git_tag "2daddb7d7c2ffda98a4efa8ea49ba5c6c0a98fc6") set(ghidra_git_tag "${ghidra_head_git_tag}") set(ghidra_shallow FALSE) set(ghidra_patches @@ -57,6 +59,8 @@ if("${sleigh_RELEASE_TYPE}" STREQUAL "HEAD") "${GIT_EXECUTABLE}" am --ignore-space-change --ignore-whitespace --no-gpg-sign "${CMAKE_CURRENT_LIST_DIR}/patches/HEAD/0001-Fix-UBSAN-errors-in-decompiler.patch" "${CMAKE_CURRENT_LIST_DIR}/patches/HEAD/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch" + "${CMAKE_CURRENT_LIST_DIR}/patches/HEAD/0003-Fix-ASAN-initialize-order-fiasco.patch" + "${CMAKE_CURRENT_LIST_DIR}/patches/HEAD/0004-Fix-memory-leak-after-xml-errors.patch" ) string(SUBSTRING "${ghidra_git_tag}" 0 7 ghidra_short_commit) else() From faf1367c8a57ebdb61cfa8146dce58efab39c052 Mon Sep 17 00:00:00 2001 From: Eric Kilmer Date: Thu, 25 May 2023 17:33:45 -0400 Subject: [PATCH 4/4] Update patches with fix for large bitshift UBSAN While this is likely the correct handling of the large bitshift, we now have a failing test, "Modulo #28". See note in patch for more info. --- .../0001-Fix-UBSAN-errors-in-decompiler.patch | 4 +- ...ead-of-stroul-to-parse-address-offse.patch | 4 +- ...003-Fix-ASAN-initialize-order-fiasco.patch | 4 +- ...004-Fix-memory-leak-after-xml-errors.patch | 4 +- ...0005-Fix-UBSAN-large-bitshift-errors.patch | 71 +++++++++++++++++++ .../0001-Fix-UBSAN-errors-in-decompiler.patch | 4 +- ...ead-of-stroul-to-parse-address-offse.patch | 4 +- ...003-Fix-ASAN-initialize-order-fiasco.patch | 4 +- ...004-Fix-memory-leak-after-xml-errors.patch | 4 +- ...0005-Fix-UBSAN-large-bitshift-errors.patch | 71 +++++++++++++++++++ src/setup-ghidra-source.cmake | 2 + 11 files changed, 160 insertions(+), 16 deletions(-) create mode 100644 src/patches/HEAD/0005-Fix-UBSAN-large-bitshift-errors.patch create mode 100644 src/patches/stable/0005-Fix-UBSAN-large-bitshift-errors.patch diff --git a/src/patches/HEAD/0001-Fix-UBSAN-errors-in-decompiler.patch b/src/patches/HEAD/0001-Fix-UBSAN-errors-in-decompiler.patch index abb6b7fa9..daa1ee60f 100644 --- a/src/patches/HEAD/0001-Fix-UBSAN-errors-in-decompiler.patch +++ b/src/patches/HEAD/0001-Fix-UBSAN-errors-in-decompiler.patch @@ -1,7 +1,7 @@ -From 79a61c7d52db690677e3ac446561b8575a609581 Mon Sep 17 00:00:00 2001 +From 1b5bc985f5abcbd47acd8864983863bd7800c123 Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Mon, 7 Feb 2022 02:02:03 +1100 -Subject: [PATCH 1/4] Fix UBSAN errors in decompiler +Subject: [PATCH 1/5] Fix UBSAN errors in decompiler --- .../Decompiler/src/decompile/cpp/address.cc | 4 ++-- diff --git a/src/patches/HEAD/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch b/src/patches/HEAD/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch index e843d31c4..7d94a2b68 100644 --- a/src/patches/HEAD/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch +++ b/src/patches/HEAD/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch @@ -1,7 +1,7 @@ -From af3df62bdede0d6b5bfe18c35b46a97defc3e280 Mon Sep 17 00:00:00 2001 +From aa6352010bf7e61f6bd2ebbe59bd3f280e6c3c12 Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Wed, 3 Aug 2022 20:01:18 +1000 -Subject: [PATCH 2/4] Use `stroull` instead of `stroul` to parse address +Subject: [PATCH 2/5] Use `stroull` instead of `stroul` to parse address offsets --- diff --git a/src/patches/HEAD/0003-Fix-ASAN-initialize-order-fiasco.patch b/src/patches/HEAD/0003-Fix-ASAN-initialize-order-fiasco.patch index 72cbc47da..0a1898ab3 100644 --- a/src/patches/HEAD/0003-Fix-ASAN-initialize-order-fiasco.patch +++ b/src/patches/HEAD/0003-Fix-ASAN-initialize-order-fiasco.patch @@ -1,7 +1,7 @@ -From 0126a87fbebfdcdb0553af325d6880647050c287 Mon Sep 17 00:00:00 2001 +From 24e39c23c92a337cf51310698bfa4ca9e78d9523 Mon Sep 17 00:00:00 2001 From: Eric Kilmer Date: Wed, 24 May 2023 19:03:25 -0400 -Subject: [PATCH 3/4] Fix ASAN initialize-order-fiasco +Subject: [PATCH 3/5] Fix ASAN initialize-order-fiasco Use the "Constructo On First Use" idiom from https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Construct_On_First_Use diff --git a/src/patches/HEAD/0004-Fix-memory-leak-after-xml-errors.patch b/src/patches/HEAD/0004-Fix-memory-leak-after-xml-errors.patch index ac8c5cf1a..9d434a489 100644 --- a/src/patches/HEAD/0004-Fix-memory-leak-after-xml-errors.patch +++ b/src/patches/HEAD/0004-Fix-memory-leak-after-xml-errors.patch @@ -1,7 +1,7 @@ -From ec635dc18a2ad74639380816e33dbf41ef862de1 Mon Sep 17 00:00:00 2001 +From d808aa710d74414d3d62bed2a90d55b001febc87 Mon Sep 17 00:00:00 2001 From: Eric Kilmer Date: Wed, 24 May 2023 19:27:54 -0400 -Subject: [PATCH 4/4] Fix memory leak after xml errors +Subject: [PATCH 4/5] Fix memory leak after xml errors Regenerated with bison 3.0.4 on AlmaLinux 8 --- diff --git a/src/patches/HEAD/0005-Fix-UBSAN-large-bitshift-errors.patch b/src/patches/HEAD/0005-Fix-UBSAN-large-bitshift-errors.patch new file mode 100644 index 000000000..801a3c770 --- /dev/null +++ b/src/patches/HEAD/0005-Fix-UBSAN-large-bitshift-errors.patch @@ -0,0 +1,71 @@ +From 886f1a74b0469d391cf7c1013d1755540d03fd89 Mon Sep 17 00:00:00 2001 +From: Eric Kilmer +Date: Thu, 25 May 2023 16:17:43 -0400 +Subject: [PATCH 5/5] Fix UBSAN large bitshift errors + +While I believe this is technically the correct fix, the "Modulo #28" +test now fails. + +Debugging can be performed by placing a breakpoint in +`runTests` function in `testfunction.cc` to view the `result` variable +with the output when running the datatests as follows: + +``` +sleigh_ghidra_test datatests modulo.xml +``` + +This output is compared against the `stringmatch` fields in modulo.xml +in the `datatests` directory. Specifically, `stringmatch` with name +"Modulo #28" fails to match. +--- + .../Decompiler/src/decompile/cpp/jumptable.cc | 11 ++++++++--- + .../Decompiler/src/decompile/cpp/ruleaction.cc | 11 ++++++++--- + 2 files changed, 16 insertions(+), 6 deletions(-) + +diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/jumptable.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/jumptable.cc +index ee449456c..3b84952cf 100644 +--- a/Ghidra/Features/Decompiler/src/decompile/cpp/jumptable.cc ++++ b/Ghidra/Features/Decompiler/src/decompile/cpp/jumptable.cc +@@ -726,9 +726,14 @@ Varnode *GuardRecord::quasiCopy(Varnode *vn,int4 &bitsPreserved) + { + bitsPreserved = mostsigbit_set(vn->getNZMask()) + 1; + if (bitsPreserved == 0) return vn; +- uintb mask = 1; +- mask <<= bitsPreserved; +- mask -= 1; ++ uintb mask; ++ if (bitsPreserved != sizeof(mask) * 8) { ++ mask = 1; ++ mask <<= bitsPreserved; ++ mask -= 1; ++ } else { ++ mask = ~(uintb)0; ++ } + PcodeOp *op = vn->getDef(); + Varnode *constVn; + while(op != (PcodeOp *)0) { +diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc +index d069d1c94..4a5995464 100644 +--- a/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc ++++ b/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc +@@ -7680,9 +7680,14 @@ uintb RuleDivOpt::calcDivisor(uintb n,uint8 y,int4 xsize) + // The optimization of division to multiplication + // by the reciprocal holds true, if the maximum value + // of x times the remainder is less than 2^n +- uint8 maxx = 1; +- maxx <<= xsize; +- maxx -= 1; // Maximum possible x value ++ uint8 maxx; // Maximum possible x value ++ if (xsize != sizeof(maxx) * 8) { ++ maxx = 1; ++ maxx <<= xsize; ++ maxx -= 1; ++ } else { ++ maxx = ~(uint8)0; ++ } + uint8 tmp; + if (n < 64) + tmp = power / (d-r); // r < d => divisor is non-zero +-- +2.40.1 + diff --git a/src/patches/stable/0001-Fix-UBSAN-errors-in-decompiler.patch b/src/patches/stable/0001-Fix-UBSAN-errors-in-decompiler.patch index 88ab2c6d8..4440c460b 100644 --- a/src/patches/stable/0001-Fix-UBSAN-errors-in-decompiler.patch +++ b/src/patches/stable/0001-Fix-UBSAN-errors-in-decompiler.patch @@ -1,7 +1,7 @@ -From 7f760d9be3728818ae63e77e1748894710809144 Mon Sep 17 00:00:00 2001 +From 913326003d1b871669b68b20c16f2e602894d37a Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Mon, 7 Feb 2022 02:02:03 +1100 -Subject: [PATCH 1/4] Fix UBSAN errors in decompiler +Subject: [PATCH 1/5] Fix UBSAN errors in decompiler --- .../Decompiler/src/decompile/cpp/address.cc | 4 ++-- diff --git a/src/patches/stable/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch b/src/patches/stable/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch index 7882c9bce..1fea69ec3 100644 --- a/src/patches/stable/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch +++ b/src/patches/stable/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch @@ -1,7 +1,7 @@ -From e64380cd8fcf473d710180d9007a31dfc08eae6a Mon Sep 17 00:00:00 2001 +From b344b27d128f30aabbe8f7cd648d4be025fa4834 Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Wed, 3 Aug 2022 20:01:18 +1000 -Subject: [PATCH 2/4] Use `stroull` instead of `stroul` to parse address +Subject: [PATCH 2/5] Use `stroull` instead of `stroul` to parse address offsets --- diff --git a/src/patches/stable/0003-Fix-ASAN-initialize-order-fiasco.patch b/src/patches/stable/0003-Fix-ASAN-initialize-order-fiasco.patch index fea58c359..564310a3e 100644 --- a/src/patches/stable/0003-Fix-ASAN-initialize-order-fiasco.patch +++ b/src/patches/stable/0003-Fix-ASAN-initialize-order-fiasco.patch @@ -1,7 +1,7 @@ -From 8fbbfed97dec5ddb0c687a783c009ac55f020969 Mon Sep 17 00:00:00 2001 +From 3019f9a4d415cf775f05ca4d3accd584792a3f28 Mon Sep 17 00:00:00 2001 From: Eric Kilmer Date: Wed, 24 May 2023 19:03:25 -0400 -Subject: [PATCH 3/4] Fix ASAN initialize-order-fiasco +Subject: [PATCH 3/5] Fix ASAN initialize-order-fiasco Use the "Constructo On First Use" idiom from https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Construct_On_First_Use diff --git a/src/patches/stable/0004-Fix-memory-leak-after-xml-errors.patch b/src/patches/stable/0004-Fix-memory-leak-after-xml-errors.patch index 22227bcf3..988d7a147 100644 --- a/src/patches/stable/0004-Fix-memory-leak-after-xml-errors.patch +++ b/src/patches/stable/0004-Fix-memory-leak-after-xml-errors.patch @@ -1,7 +1,7 @@ -From aa60017f1d68af44ae198d51c318c7e911b56448 Mon Sep 17 00:00:00 2001 +From 81d96008df1ea0c0f4e648f85c473990ea2e8e37 Mon Sep 17 00:00:00 2001 From: Eric Kilmer Date: Wed, 24 May 2023 19:27:54 -0400 -Subject: [PATCH 4/4] Fix memory leak after xml errors +Subject: [PATCH 4/5] Fix memory leak after xml errors Regenerated with bison 3.0.4 on AlmaLinux 8 --- diff --git a/src/patches/stable/0005-Fix-UBSAN-large-bitshift-errors.patch b/src/patches/stable/0005-Fix-UBSAN-large-bitshift-errors.patch new file mode 100644 index 000000000..1ba6edc74 --- /dev/null +++ b/src/patches/stable/0005-Fix-UBSAN-large-bitshift-errors.patch @@ -0,0 +1,71 @@ +From 4978c2139b1de4d672dadaefb45e1f3fca17e0f6 Mon Sep 17 00:00:00 2001 +From: Eric Kilmer +Date: Thu, 25 May 2023 16:17:43 -0400 +Subject: [PATCH 5/5] Fix UBSAN large bitshift errors + +While I believe this is technically the correct fix, the "Modulo #28" +test now fails. + +Debugging can be performed by placing a breakpoint in +`runTests` function in `testfunction.cc` to view the `result` variable +with the output when running the datatests as follows: + +``` +sleigh_ghidra_test datatests modulo.xml +``` + +This output is compared against the `stringmatch` fields in modulo.xml +in the `datatests` directory. Specifically, `stringmatch` with name +"Modulo #28" fails to match. +--- + .../Decompiler/src/decompile/cpp/jumptable.cc | 11 ++++++++--- + .../Decompiler/src/decompile/cpp/ruleaction.cc | 11 ++++++++--- + 2 files changed, 16 insertions(+), 6 deletions(-) + +diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/jumptable.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/jumptable.cc +index ee449456c..3b84952cf 100644 +--- a/Ghidra/Features/Decompiler/src/decompile/cpp/jumptable.cc ++++ b/Ghidra/Features/Decompiler/src/decompile/cpp/jumptable.cc +@@ -726,9 +726,14 @@ Varnode *GuardRecord::quasiCopy(Varnode *vn,int4 &bitsPreserved) + { + bitsPreserved = mostsigbit_set(vn->getNZMask()) + 1; + if (bitsPreserved == 0) return vn; +- uintb mask = 1; +- mask <<= bitsPreserved; +- mask -= 1; ++ uintb mask; ++ if (bitsPreserved != sizeof(mask) * 8) { ++ mask = 1; ++ mask <<= bitsPreserved; ++ mask -= 1; ++ } else { ++ mask = ~(uintb)0; ++ } + PcodeOp *op = vn->getDef(); + Varnode *constVn; + while(op != (PcodeOp *)0) { +diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc +index cd0bbcf7e..35e25c7aa 100644 +--- a/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc ++++ b/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc +@@ -7676,9 +7676,14 @@ uintb RuleDivOpt::calcDivisor(uintb n,uint8 y,int4 xsize) + // The optimization of division to multiplication + // by the reciprocal holds true, if the maximum value + // of x times the remainder is less than 2^n +- uint8 maxx = 1; +- maxx <<= xsize; +- maxx -= 1; // Maximum possible x value ++ uint8 maxx; // Maximum possible x value ++ if (xsize != sizeof(maxx) * 8) { ++ maxx = 1; ++ maxx <<= xsize; ++ maxx -= 1; ++ } else { ++ maxx = ~(uint8)0; ++ } + uint8 tmp; + if (n < 64) + tmp = power / (d-r); // r < d => divisor is non-zero +-- +2.40.1 + diff --git a/src/setup-ghidra-source.cmake b/src/setup-ghidra-source.cmake index b6b193ace..113437609 100644 --- a/src/setup-ghidra-source.cmake +++ b/src/setup-ghidra-source.cmake @@ -42,6 +42,7 @@ set(ghidra_patches "${CMAKE_CURRENT_LIST_DIR}/patches/stable/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch" "${CMAKE_CURRENT_LIST_DIR}/patches/stable/0003-Fix-ASAN-initialize-order-fiasco.patch" "${CMAKE_CURRENT_LIST_DIR}/patches/stable/0004-Fix-memory-leak-after-xml-errors.patch" + "${CMAKE_CURRENT_LIST_DIR}/patches/stable/0005-Fix-UBSAN-large-bitshift-errors.patch" ) # Ghidra pinned commits used for pinning last known working HEAD commit @@ -61,6 +62,7 @@ if("${sleigh_RELEASE_TYPE}" STREQUAL "HEAD") "${CMAKE_CURRENT_LIST_DIR}/patches/HEAD/0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch" "${CMAKE_CURRENT_LIST_DIR}/patches/HEAD/0003-Fix-ASAN-initialize-order-fiasco.patch" "${CMAKE_CURRENT_LIST_DIR}/patches/HEAD/0004-Fix-memory-leak-after-xml-errors.patch" + "${CMAKE_CURRENT_LIST_DIR}/patches/HEAD/0005-Fix-UBSAN-large-bitshift-errors.patch" ) string(SUBSTRING "${ghidra_git_tag}" 0 7 ghidra_short_commit) else()