[I] [Bug] BE SEGV in OrcReader::_seek_to_read_one_line when _row_reader is nullptr [doris]

["heguanhui (via GitHub)" <gi...@apache.org>]

heguanhui opened a new issue, #63921:
URL: https://github.com/apache/doris/issues/63921

   ### Search before asking
   
   - [x] I had searched in the [issues](https://github.com/apache/doris/issues?q=is%3Aissue) and found no similar issues.
   
   ### Version
   
   master (trunk)
   
   ### What's Wrong?
   
   When running `OrcReadLinesTest.test0` BE unit test with ASAN build on x86, AddressSanitizer reports a SEGV crash due to null pointer dereference of `_row_reader` in `OrcReader::_seek_to_read_one_line()`.
   
   **ASAN crash stack:**
   ```
   ==XX==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000030 (pc 0x555558b3c2e0 bp 0x7ffff3c8e400 sp 0x7ffff3c8e3a0 T2)
   ==XX==The signal is caused by a READ memory access.
       #0 0x555558b3c2e0 in orc::RowReaderImpl::seekToRow(unsigned long) be/src/formats/orc/../../thirdparty/installed/include/orc/Reader.hh
       #1 0x5555590a3b5c in doris::vectorized::OrcReader::_seek_to_read_one_line() be/src/format/orc/vorc_reader.h:710
       #2 0x5555590a3b5c in doris::vectorized::OrcReader::_get_next_block_impl(doris::vectorized::Block*, unsigned long*, bool*) be/src/format/orc/vorc_reader.cpp:2350
       #3 0x555558f5c5a0 in doris::vectorized::OrcReader::get_next_block(doris::vectorized::Block*, unsigned long*, bool*) be/src/format/orc/vorc_reader.cpp:2260
       #4 0x555558f5c5a0 in doris::vectorized::GenericReader::read_by_rows(doris::RuntimeState*, doris::vectorized::Block*, unsigned long*, bool*) be/src/format/generic_reader.h:165
       ...
   ```
   
   **Root cause:**
   
   In `OrcReader::_init_orc_row_reader()`, when `createRowReader` throws an exception and `should_stop` is true with error message "stop", the catch block swallows the exception and returns `Status::OK()`, but `_row_reader` remains nullptr. The caller then proceeds to call `_seek_to_read_one_line()` which dereferences the null `_row_reader` via `_row_reader->seekToRow()`, causing SEGV.
   
   This is inconsistent with `_create_file_reader()` which returns `Status::EndOfFile("stop")` in the same `should_stop` scenario.
   
   ### What You Expected?
   
   No SEGV crash. When `_row_reader` is not initialized, the code should either return a proper error status or assert the precondition, not silently continue and dereference a null pointer.
   
   ### How to Reproduce?
   
   1. Build BE with ASAN: `BUILD_TYPE=ASAN ./build.sh --be`
   2. Run: `./run-be-ut.sh --run --filter=OrcReadLinesTest.test0`
   3. Observe ASAN SEGV crash
   
   ### Anything Else?
   
   The x86 vs ARM difference is a typical undefined behavior manifestation - x86 null pointer dereference hits unmapped memory (SIGSEGV), while ARM may happen to access mapped memory and appear to pass.
   
   ### Are you willing to submit PR?
   
   - [x] Yes I am willing to submit a PR!
   
   ### Code of Conduct
   
   - [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)


-- 
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: commits-unsubscribe@doris.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org

Re: [I] [Bug] BE SEGV in OrcReader::_seek_to_read_one_line when _row_reader is nullptr [doris]

["zclllyybb (via GitHub)" <gi...@apache.org>]

zclllyybb commented on issue #63921:
URL: https://github.com/apache/doris/issues/63921#issuecomment-4587215224

   Thanks for the detailed report. I checked the current `apache/doris` upstream `master` code at `487f783334648c60d982c49bdb315b01c29cdcc7`, and this looks like a real ORC reader lifecycle bug rather than ASAN noise.
   
   Confirmed code path:
   
   - `ORCFileInputStream::read()` and `StripeStreamInputStream::read()` throw `orc::ParseError("stop")` when `IOContext::should_stop` is set.
   - `_create_file_reader()` already converts that same stop condition to `Status::EndOfFile("stop")`.
   - `_init_orc_row_reader()` catches exceptions from `_reader->createRowReader(...)`, but for `should_stop && e.what() == "stop"` it suppresses the exception and then returns `Status::OK()` without constructing `_row_reader` or `_batch`.
   - After a successful `init_reader()`, the read-by-row path assumes `_row_reader` exists. `_seek_to_read_one_line()` dereferences it via `_row_reader->seekToRow(...)`, so an OK-with-null-reader state can crash exactly as reported. The same invalid state would also be unsafe for later `nextBatch()` callers.
   
   Judgement:
   
   - I agree with the reported root cause: successful ORC reader initialization must not leave `_row_reader == nullptr`.
   - The x86 versus ARM difference should not be treated as a platform-specific ORC behavior. Once `_row_reader` is dereferenced while null, the code is already in undefined behavior.
   - One nuance from the current code is that `_get_next_block_impl()` has an early `should_stop` check. If the stop flag remains set until read time, it can return EOF before `_seek_to_read_one_line()`. That does not remove the bug: the invalid state is introduced earlier by returning OK from `_init_orc_row_reader()` after row-reader creation failed.
   
   Suggested fix:
   
   1. In `_init_orc_row_reader()`, handle `should_stop && "stop"` consistently with `_create_file_reader()` and return `Status::EndOfFile("stop")` instead of `Status::OK()`.
   2. Add a defensive invariant before returning OK from `_init_orc_row_reader()`, for example returning an internal error if `_row_reader` or `_batch` is still null. The important contract should be: OK means the ORC row reader is usable.
   3. Add or adjust a BE UT that forces the stop path during ORC row-reader creation and verifies no SEGV and no OK-with-null-reader state. `OrcReadLinesTest.test0` is a useful regression surface, but a narrower invariant test around row-reader initialization would make the failure mode clearer.
   
   Missing information that would help the PR but does not block the code fix:
   
   - Exact Doris commit SHA used by the failing ASAN run.
   - Full ASAN log and BE unit-test command output.
   - Confirmation of what set `IOContext::should_stop` during row-reader creation in the failing run, if known.
   
   Breakwater-GitHub-Analysis-Slot: slot_1cd2a3e1dfdc
   


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org