morningman opened a new pull request, #69695:
URL: https://github.com/apache/doris/pull/69695
### What problem does this PR solve?
Issue Number: None
Related PR: #48788
Problem Summary:
**In short.** `load_data_from_csv()` in the BE test utilities dereferences
`std::set::end()` in an assertion. With libc++ (macOS) that reads past the set
on the stack, so under ASAN every BE unit test suite that loads a CSV fixture
aborts while loading its fixtures and takes the rest of the `doris_be_test` run
with it. This removes the assertion; it only repeated the size check on the
line above.
**Background.** #48788 added `load_data_from_csv()`
(`be/test/testutil/test_util.cpp`), which the column, data type and SerDe unit
tests use to load their CSV fixtures. It checks that the SerDes, the columns
and the requested column indexes line up: `serders.size() == columns.size()`,
`serders.size() == idxes.size()`, and then `serders.size() == *idxes.end()`.
**The problem, and what it cost.** Dereferencing the end iterator of a
`std::set` is undefined behavior. In libstdc++ the end iterator points at the
tree header, whose value slot overlaps the element count, so on Linux the last
assertion compares against `idxes.size()` again and passes. In libc++ it reads
outside the set object: ASAN reports a stack-buffer-overflow, and with
`abort_on_error` the whole `doris_be_test` process stops. The suites of all 24
test files that load CSV fixtures (`DataTypeDateTimeV2SerDeTest`,
`DataTypeNumberSerDeTest`, `ColumnStringTest`, ...) cannot run on macOS, and a
test filter that includes one of them ends the run there. CI runs on Linux
only, so it never failed there.
**How this PR fixes it.** Remove the assertion. The size check right above
it already guarantees one index per SerDe. No production code changes. #63474,
open since May, carries the same removal among its macOS build fixes; this PR
takes only this line, so the test fix does not wait for the rest.
**Results.**
| | macOS arm64, ASAN | Linux |
|---|---|---|
| Before | `DataTypeDateTimeV2SerDeTest` aborts in `SetUpTestSuite` with a
stack-buffer-overflow | passes (the assertion repeats the size check) |
| After | the 24 CSV-fixture suites (364 tests) pass | unchanged |
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- macOS arm64 (ASAN): the 24 suites whose test files load CSV
fixtures pass, 364 tests. With the assertion back,
`DataTypeDateTimeV2SerDeTest` fails in `SetUpTestSuite` with an ASAN
stack-buffer-overflow at that assertion. `FunctionCastToDecimalPerfTest`, a
performance test that also loads a CSV file, was not run.
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- Behavior changed:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
