Skip to content

Fix the insecure comparison function #586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
footka wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix the insecure comparison function #586

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e9f00d8
fix rpc error
Apr 16, 2026
95a3a7a
Fix the insecure comparison function
Apr 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 23 additions & 49 deletions src/sql/engine/sort/ob_sort_compare_vec_op.ipp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,34 +24,24 @@ bool GeneralCompare<Store_Row, has_addon>::operator()(const Store_Row *l, const
{
bool less = false;
int &ret = ret_;
if (OB_UNLIKELY(OB_SUCCESS != ret)) {
// Use pointer comparison as a consistent fallback to maintain strict weak ordering.
// Returning false for all pairs would cause introsort's right-scan loop
// (while !comp(pivot, *i)) to advance past array bounds, triggering abort().
less = l < r;
} else if (OB_FAIL(fast_check_status())) {
SQL_ENG_LOG(WARN, "fast check failed", K(ret));
less = l < r;
if (CompareBase::ENABLE == encode_sk_state_) {
ObLength l_len = 0;
ObLength r_len = 0;
const char *l_data = nullptr;
const char *r_data = nullptr;
l->get_cell_payload(*sk_row_meta_, 0, l_data, l_len);
r->get_cell_payload(*sk_row_meta_, 0, r_data, r_len);
int cmp = 0;
cmp = MEMCMP(l_data, r_data, min(l_len, r_len));
less = cmp != 0 ? (cmp < 0) : (l_len - r_len) < 0;
} else if (CompareBase::FALLBACK_TO_DISABLE == encode_sk_state_ && has_addon) {
const Store_Row *l_real_cmp_row = l->get_addon_ptr(*sk_row_meta_);
const Store_Row *r_real_cmp_row = r->get_addon_ptr(*sk_row_meta_);
less = (compare(l_real_cmp_row, r_real_cmp_row, addon_row_meta_) > 0);
} else {
if (CompareBase::ENABLE == encode_sk_state_) {
ObLength l_len = 0;
ObLength r_len = 0;
const char *l_data = nullptr;
const char *r_data = nullptr;
l->get_cell_payload(*sk_row_meta_, 0, l_data, l_len);
r->get_cell_payload(*sk_row_meta_, 0, r_data, r_len);
int cmp = 0;
cmp = MEMCMP(l_data, r_data, min(l_len, r_len));
less = cmp != 0 ? (cmp < 0) : (l_len - r_len) < 0;
} else if (CompareBase::FALLBACK_TO_DISABLE == encode_sk_state_ && has_addon) {
const Store_Row *l_real_cmp_row = l->get_addon_ptr(*sk_row_meta_);
const Store_Row *r_real_cmp_row = r->get_addon_ptr(*sk_row_meta_);
less = (compare(l_real_cmp_row, r_real_cmp_row, addon_row_meta_) > 0);
} else {
__builtin_prefetch(l, 0 /* read */, 2 /*high temp locality*/);
__builtin_prefetch(r, 0 /* read */, 2 /*high temp locality*/);
less = (compare(l, r, sk_row_meta_) > 0);
}
__builtin_prefetch(l, 0 /* read */, 2 /*high temp locality*/);
__builtin_prefetch(r, 0 /* read */, 2 /*high temp locality*/);
less = (compare(l, r, sk_row_meta_) > 0);
}
return less;
}
Expand Down Expand Up @@ -263,17 +253,9 @@ template <typename Store_Row, bool is_basic_cmp, bool is_topn_sort>
bool SingleColCompare<Store_Row, is_basic_cmp, is_topn_sort>::operator()(const Store_Row *l, const Store_Row *r)
{
bool less = false;
int &ret = ret_;
if (OB_UNLIKELY(OB_SUCCESS != ret)) {
less = l < r;
} else if (OB_FAIL(fast_check_status())) {
SQL_ENG_LOG(WARN, "fast check failed", K(ret));
less = l < r;
} else {
__builtin_prefetch(l, 0 /* read */, 2 /*high temp locality*/);
__builtin_prefetch(r, 0 /* read */, 2 /*high temp locality*/);
less = (compare(l, r, sk_row_meta_) > 0);
}
__builtin_prefetch(l, 0 /* read */, 2 /*high temp locality*/);
__builtin_prefetch(r, 0 /* read */, 2 /*high temp locality*/);
less = (compare(l, r, sk_row_meta_) > 0);
return less;
}

Expand Down Expand Up @@ -497,17 +479,9 @@ template <typename Store_Row, bool has_addon>
bool FixedCompare<Store_Row, has_addon>::operator()(const Store_Row *l, const Store_Row *r)
{
bool less = false;
int &ret = ret_;
if (OB_UNLIKELY(OB_SUCCESS != ret)) {
less = l < r;
} else if (OB_FAIL(fast_check_status())) {
SQL_ENG_LOG(WARN, "fast check failed", K(ret));
less = l < r;
} else {
__builtin_prefetch(l, 0 /* read */, 2 /*high temp locality*/);
__builtin_prefetch(r, 0 /* read */, 2 /*high temp locality*/);
less = (compare(l, r, sk_row_meta_) > 0);
}
__builtin_prefetch(l, 0 /* read */, 2 /*high temp locality*/);
__builtin_prefetch(r, 0 /* read */, 2 /*high temp locality*/);
less = (compare(l, r, sk_row_meta_) > 0);
return less;
}

Expand Down
34 changes: 6 additions & 28 deletions tools/windows/installer/wix_launch_configurator.wxs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,6 @@
failed to link for some reason.
-->
<Fragment>
<!-- Read the configurator exe path persisted to the registry during
install. AppSearch runs early in every session (install, uninstall,
repair) so the property is available when custom actions fire.
This is critical for uninstall: the Directory-table default for
INSTALL_ROOT may revert to the compile-time default, but this
property always reflects the ACTUAL installed location. -->
<Property Id="SEEKDB_CONFIGURATOR_EXE">
<RegistrySearch Id="FindConfiguratorExe" Root="HKLM"
Key="Software\SeekDB" Name="ConfiguratorExe" Type="raw" />
</Property>

<!-- Wire the ExitDialog's Finish button to fire LaunchApplication
when the checkbox is checked. WixUI shows the checkbox but
does NOT include this Publish by default — we must add it.
Expand All @@ -33,34 +22,27 @@
Condition="WIXUI_EXITDIALOGOPTIONALCHECKBOX = 1 and NOT Installed" />
</UI>

<!-- Type 34 custom action: fire-and-forget launch of Configurator (install).
During install [INSTALL_ROOT] is always correctly resolved by the
session, so Type 34 (Directory + ExeCommand) works fine here. -->
<!-- Type 34 custom action: fire-and-forget launch of Configurator (install). -->
<CustomAction Id="LaunchApplication"
Directory="INSTALL_ROOT"
ExeCommand="&quot;[INSTALL_ROOT]bin\seekdbConfigurator.exe&quot;"
Execute="immediate"
Return="asyncNoWait" />

<!-- Type 50 custom action: launch Configurator in remove mode (uninstall).
Uses the SEEKDB_CONFIGURATOR_EXE property (populated from the
registry by AppSearch) so the correct path is resolved even when
the user chose a non-default install directory. Type 34 with
Directory="INSTALL_ROOT" failed in that scenario because the
directory property could revert to its default value during
uninstall, pointing to a non-existent path.
<!-- Type 34 custom action: launch Configurator in remove mode (uninstall).
Runs synchronously so removal completes before MSI deletes files.
Return="ignore" ensures uninstall proceeds even if user cancels. -->
<CustomAction Id="RunConfiguratorRemove"
Property="SEEKDB_CONFIGURATOR_EXE"
ExeCommand="--remove"
Directory="INSTALL_ROOT"
ExeCommand="&quot;[INSTALL_ROOT]bin\seekdbConfigurator.exe&quot; --remove"
Execute="immediate"
Return="ignore" />

<!-- Schedule the removal configurator before files are deleted.
Only runs on full uninstall (not upgrades). -->
<InstallExecuteSequence>
<Custom Action="RunConfiguratorRemove" Before="RemoveFiles"
Condition="(REMOVE=&quot;ALL&quot;) AND (NOT UPGRADINGPRODUCTCODE) AND SEEKDB_CONFIGURATOR_EXE" />
Condition="(REMOVE=&quot;ALL&quot;) AND (NOT UPGRADINGPRODUCTCODE)" />
</InstallExecuteSequence>

<!-- Add bin/ to system PATH (cleanly removed on uninstall). -->
Expand All @@ -70,10 +52,6 @@
Permanent="no" Part="last" Action="set" System="yes" />
<RegistryValue Root="HKLM" Key="Software\SeekDB" Name="PathConfigured"
Type="integer" Value="1" KeyPath="yes" />
<RegistryValue Root="HKLM" Key="Software\SeekDB" Name="InstallRoot"
Type="string" Value="[INSTALL_ROOT]" />
<RegistryValue Root="HKLM" Key="Software\SeekDB" Name="ConfiguratorExe"
Type="string" Value="[INSTALL_ROOT]bin\seekdbConfigurator.exe" />
</Component>
</DirectoryRef>
</Fragment>
Expand Down
Loading